Merge pull request #24073 from a-robinson/backport1.1-23851

cherrypick-1.1: server: Use remote_debugging setting to restrict all access to keys

pkg/server/admin.go

@@ -151,6 +151,7 @@ func (s *adminServer) isNotFoundError(err error) bool {
 func (s *adminServer) NewContextAndSessionForRPC(
 	ctx context.Context, args sql.SessionArgs,
 ) (context.Context, *sql.Session) {
+	ctx = propagateGatewayMetadata(ctx)
 	ctx = s.server.AnnotateCtx(ctx)
 	session := sql.NewSession(ctx, args, s.server.sqlExecutor, nil, s.memMetrics)
 	session.StartMonitor(&s.memMonitor, mon.BoundAccount{})
@@ -741,6 +742,8 @@ func (s *adminServer) RangeLog(
 		limit = defaultAPIEventLimit
 	}
 
+	includeRawKeys := GatewayRemoteAllowed(ctx, s.server.ClusterSettings())
+
 	// Execute the query.
 	q := makeSQLQuery()
 	q.Append(`SELECT timestamp, "rangeID", "storeID", "eventType", "otherRangeID", info `)
@@ -814,9 +817,17 @@ func (s *adminServer) RangeLog(
 				return nil, errors.Wrap(err, fmt.Sprintf("info didn't parse correctly: %s", info))
 			}
 			if event.Info.NewDesc != nil {
+				if !includeRawKeys {
+					event.Info.NewDesc.StartKey = nil
+					event.Info.NewDesc.EndKey = nil
+				}
 				prettyInfo.NewDesc = event.Info.NewDesc.String()
 			}
 			if event.Info.UpdatedDesc != nil {
+				if !includeRawKeys {
+					event.Info.UpdatedDesc.StartKey = nil
+					event.Info.UpdatedDesc.EndKey = nil
+				}
 				prettyInfo.UpdatedDesc = event.Info.UpdatedDesc.String()
 			}
 			if event.Info.AddedReplica != nil {

pkg/server/debug.go

@@ -16,14 +16,18 @@ package server
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"strings"
 
 	// Register the net/trace endpoint with http.DefaultServeMux.
 
+	"golang.org/x/net/context"
 	"golang.org/x/net/trace"
+	"google.golang.org/grpc/metadata"
 
 	"github.com/cockroachdb/cockroach/pkg/settings"
+	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/pkg/errors"
 	"github.com/rcrowley/go-metrics"
@@ -91,26 +95,62 @@ func handleDebug(w http.ResponseWriter, r *http.Request) {
 	handler.ServeHTTP(w, r)
 }
 
-// traceAuthRequest is the original trace.AuthRequest, populated in init().
-var traceAuthRequest func(*http.Request) (bool, bool)
-
 // authRequest restricts access to /debug/*.
 func authRequest(r *http.Request) (allow, sensitive bool) {
-	allow, sensitive = traceAuthRequest(r)
-	switch DebugRemoteMode(strings.ToLower(debugRemote.Get(settings.TODO()))) {
+	allow = authRequestImpl(r.RemoteAddr, settings.TODO())
+	sensitive = allow
+	return allow, sensitive
+}
+
+// authRequestImpl restricts access according to the debugRemote setting.
+func authRequestImpl(remoteAddr string, st *settings.Values) bool {
+	switch DebugRemoteMode(strings.ToLower(debugRemote.Get(st))) {
 	case DebugRemoteAny:
-		allow = true
+		return true
 	case DebugRemoteLocal:
-		break
+		return isLocalhost(remoteAddr)
 	default:
-		allow = false
+		return false
 	}
-	return allow, sensitive
 }
 
-func init() {
-	traceAuthRequest = trace.AuthRequest
+// isLocalhost returns true if the remoteAddr represents a client talking to
+// us via localhost.
+func isLocalhost(remoteAddr string) bool {
+	// RemoteAddr is commonly in the form "IP" or "IP:port".
+	// If it is in the form "IP:port", split off the port.
+	host, _, err := net.SplitHostPort(remoteAddr)
+	if err != nil {
+		host = remoteAddr
+	}
+	switch host {
+	case "localhost", "127.0.0.1", "::1":
+		return true
+	default:
+		return false
+	}
+}
+
+// GatewayRemoteAllowed returns whether a request that has been passed through
+// the grpc gateway should be allowed accessed to privileged debugging
+// information. Because this function assumes the presence of a context field
+// populated by the grpc gateway, it's not applicable for other uses.
+func GatewayRemoteAllowed(ctx context.Context, st *cluster.Settings) bool {
+	md, ok := metadata.FromIncomingContext(ctx)
+	if !ok {
+		// This should only happen for direct grpc connections, which are allowed.
+		return true
+	}
+	peerAddr, ok := md["x-forwarded-for"]
+	if !ok || len(peerAddr) == 0 {
+		// This should only happen for direct grpc connections, which are allowed.
+		return true
+	}
+
+	return authRequestImpl(peerAddr[0], &st.SV)
+}
 
+func init() {
 	// Tweak the authentication logic for the tracing endpoint. By default it's
 	// open for localhost only, but we want it to behave according to our
 	// settings.

pkg/server/server.go

@@ -383,6 +383,7 @@ func NewServer(cfg Config, stopper *stop.Stopper) (*Server, error) {
 	s.admin = newAdminServer(s)
 	s.status = newStatusServer(
 		s.cfg.AmbientCtx,
+		st,
 		s.cfg.Config,
 		s.admin,
 		s.db,