Merge pull request #3762 from hashicorp/ban-nonprint

Wraps HTTP mux to ban all non-printable characters from paths.

CHANGELOG.md

@@ -1,12 +1,16 @@
 ## 1.0.3 (UNRELEASED)
 
+BREAKING CHANGES:
+
+agent: Updated Consul's HTTP server to ban all URLs containing non-printable characters (a bad request status will be returned for these cases). This affects some user-facing areas like key/value entry key names which are carried in URLs. [[GH-3762](https://github.com/hashicorp/consul/issues/3762)]
+
 FEATURES:
 
 IMPROVEMENTS:
 
 BUG FIXES:
 
-* ui: Added a URI escape around key/value keys so that it's not possible to create unexpected partial key names when entering characters like `?` inside a key. [GH-3760]
+* ui: Added a URI escape around key/value keys so that it's not possible to create unexpected partial key names when entering characters like `?` inside a key. [[GH-3760](https://github.com/hashicorp/consul/issues/3760)]
 
 ## 1.0.2 (December 15, 2017)
 

agent/http.go

@@ -15,6 +15,7 @@ import (
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/structs"
+	"github.com/hashicorp/go-cleanhttp"
 	"github.com/mitchellh/mapstructure"
 )
 
@@ -62,6 +63,17 @@ func registerEndpoint(pattern string, fn unboundEndpoint) {
 	endpoints[pattern] = fn
 }
 
+// wrappedMux hangs on to the underlying mux for unit tests.
+type wrappedMux struct {
+	mux     *http.ServeMux
+	handler http.Handler
+}
+
+// ServeHTTP implements the http.Handler interface.
+func (w *wrappedMux) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+	w.handler.ServeHTTP(resp, req)
+}
+
 // handler is used to attach our handlers to the mux
 func (s *HTTPServer) handler(enableDebug bool) http.Handler {
 	mux := http.NewServeMux()
@@ -118,7 +130,13 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler {
 	} else if s.agent.config.EnableUI {
 		mux.Handle("/ui/", http.StripPrefix("/ui/", http.FileServer(assetFS())))
 	}
-	return mux
+
+	// Wrap the whole mux with a handler that bans URLs with non-printable
+	// characters.
+	return &wrappedMux{
+		mux:     mux,
+		handler: cleanhttp.PrintablePathCheckHandler(mux, nil),
+	}
 }
 
 // aclEndpointRE is used to find old ACL endpoints that take tokens in the URL

agent/http_test.go

@@ -165,11 +165,11 @@ func TestHTTPServer_H2(t *testing.T) {
 		resp.WriteHeader(http.StatusOK)
 		fmt.Fprint(resp, req.Proto)
 	}
-	mux, ok := a.srv.Handler.(*http.ServeMux)
+	w, ok := a.srv.Handler.(*wrappedMux)
 	if !ok {
 		t.Fatalf("handler is not expected type")
 	}
-	mux.HandleFunc("/echo", handler)
+	w.mux.HandleFunc("/echo", handler)
 
 	// Call it and make sure we see HTTP/2.
 	url := fmt.Sprintf("https://%s/echo", a.srv.ln.Addr().String())
@@ -315,6 +315,18 @@ func TestHTTPAPI_BlockEndpoints(t *testing.T) {
 	}
 }
 
+func TestHTTPAPI_Ban_Nonprintable_Characters(t *testing.T) {
+	a := NewTestAgent(t.Name(), "")
+	defer a.Shutdown()
+
+	req, _ := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
+	resp := httptest.NewRecorder()
+	a.srv.Handler.ServeHTTP(resp, req)
+	if got, want := resp.Code, http.StatusBadRequest; got != want {
+		t.Fatalf("bad response code got %d want %d", got, want)
+	}
+}
+
 func TestHTTPAPI_TranslateAddrHeader(t *testing.T) {
 	t.Parallel()
 	// Header should not be present if address translation is off.

vendor/github.com/hashicorp/go-cleanhttp/cleanhttp.go

@@ -26,6 +26,7 @@ func DefaultPooledTransport() *http.Transport {
 		DialContext: (&net.Dialer{
 			Timeout:   30 * time.Second,
 			KeepAlive: 30 * time.Second,
+			DualStack: true,
 		}).DialContext,
 		MaxIdleConns:          100,
 		IdleConnTimeout:       90 * time.Second,

vendor/github.com/hashicorp/go-cleanhttp/handlers.go

@@ -0,0 +1,43 @@
+package cleanhttp
+
+import (
+	"net/http"
+	"strings"
+	"unicode"
+)
+
+// HandlerInput provides input options to cleanhttp's handlers
+type HandlerInput struct {
+	ErrStatus int
+}
+
+// PrintablePathCheckHandler is a middleware that ensures the request path
+// contains only printable runes.
+func PrintablePathCheckHandler(next http.Handler, input *HandlerInput) http.Handler {
+	// Nil-check on input to make it optional
+	if input == nil {
+		input = &HandlerInput{
+			ErrStatus: http.StatusBadRequest,
+		}
+	}
+
+	// Default to http.StatusBadRequest on error
+	if input.ErrStatus == 0 {
+		input.ErrStatus = http.StatusBadRequest
+	}
+
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Check URL path for non-printable characters
+		idx := strings.IndexFunc(r.URL.Path, func(c rune) bool {
+			return !unicode.IsPrint(c)
+		})
+
+		if idx != -1 {
+			w.WriteHeader(input.ErrStatus)
+			return
+		}
+
+		next.ServeHTTP(w, r)
+		return
+	})
+}

vendor/vendor.json

@@ -24,7 +24,7 @@
 		{"path":"github.com/google/gofuzz","checksumSHA1":"PFtXkXPO7pwRtykVUUXtc07wc7U=","revision":"24818f796faf91cd76ec7bddd72458fbced7a6c1","revisionTime":"2017-06-12T17:47:53Z"},
 		{"path":"github.com/hashicorp/errwrap","checksumSHA1":"cdOCt0Yb+hdErz8NAQqayxPmRsY=","revision":"7554cd9344cec97297fa6649b055a8c98c2a1e55","revisionTime":"2014-10-28T05:47:10Z"},
 		{"path":"github.com/hashicorp/go-checkpoint","checksumSHA1":"D267IUMW2rcb+vNe3QU+xhfSrgY=","revision":"1545e56e46dec3bba264e41fde2c1e2aa65b5dd4","revisionTime":"2017-10-09T17:35:28Z"},
-		{"path":"github.com/hashicorp/go-cleanhttp","checksumSHA1":"b8F628srIitj5p7Y130xc9k0QWs=","revision":"3573b8b52aa7b37b9358d966a898feb387f62437","revisionTime":"2017-02-11T01:34:15Z"},
+		{"path":"github.com/hashicorp/go-cleanhttp","checksumSHA1":"YAq1rqZIp+M74Q+jMBQkkMKm3VM=","revision":"d5fe4b57a186c716b0e00b8c301cbd9b4182694d","revisionTime":"2017-12-18T14:54:08Z"},
 		{"path":"github.com/hashicorp/go-discover","checksumSHA1":"Ks9Bo8kevhaI5xTRdw1lULNMoOA=","revision":"c98e36ab72ce62b7d8fbc7f7e76f9a60e163cb45","revisionTime":"2017-10-30T10:26:55Z"},
 		{"path":"github.com/hashicorp/go-discover/provider/aliyun","checksumSHA1":"ZmU/47XUGUQpFP6E8T6Tl8QKszI=","revision":"c98e36ab72ce62b7d8fbc7f7e76f9a60e163cb45","revisionTime":"2017-10-30T10:26:55Z","tree":true},
 		{"path":"github.com/hashicorp/go-discover/provider/aws","checksumSHA1":"lyPRg8aZKgGiNkMILk/VKwOqMy4=","revision":"c98e36ab72ce62b7d8fbc7f7e76f9a60e163cb45","revisionTime":"2017-10-30T10:26:55Z","tree":true},