Ooops that was meant to be to a branch no master... EMORECOFFEE

Revert "Add config option to disable HTTP printable char path check"

This reverts commit eebe45a47b.

agent/config/builder.go

@@ -690,7 +690,6 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		DisableAnonymousSignature:               b.boolVal(c.DisableAnonymousSignature),
 		DisableCoordinates:                      b.boolVal(c.DisableCoordinates),
 		DisableHostNodeID:                       b.boolVal(c.DisableHostNodeID),
-		DisableHTTPUnprintableCharFilter:        b.boolVal(c.DisableHTTPUnprintableCharFilter),
 		DisableKeyringFile:                      b.boolVal(c.DisableKeyringFile),
 		DisableRemoteExec:                       b.boolVal(c.DisableRemoteExec),
 		DisableUpdateCheck:                      b.boolVal(c.DisableUpdateCheck),

agent/config/config.go

@@ -170,7 +170,6 @@ type Config struct {
 	DisableAnonymousSignature   *bool                    `json:"disable_anonymous_signature,omitempty" hcl:"disable_anonymous_signature" mapstructure:"disable_anonymous_signature"`
 	DisableCoordinates          *bool                    `json:"disable_coordinates,omitempty" hcl:"disable_coordinates" mapstructure:"disable_coordinates"`
 	DisableHostNodeID           *bool                    `json:"disable_host_node_id,omitempty" hcl:"disable_host_node_id" mapstructure:"disable_host_node_id"`
-	DisableHTTPUnprintableCharFilter *bool                    `json:"disable_http_unprintable_char_filter,omitempty" hcl:"disable_http_unprintable_char_filter" mapstructure:"disable_http_unprintable_char_filter"`
 	DisableKeyringFile          *bool                    `json:"disable_keyring_file,omitempty" hcl:"disable_keyring_file" mapstructure:"disable_keyring_file"`
 	DisableRemoteExec           *bool                    `json:"disable_remote_exec,omitempty" hcl:"disable_remote_exec" mapstructure:"disable_remote_exec"`
 	DisableUpdateCheck          *bool                    `json:"disable_update_check,omitempty" hcl:"disable_update_check" mapstructure:"disable_update_check"`

agent/config/runtime.go

@@ -564,16 +564,6 @@ type RuntimeConfig struct {
 	// flag: -disable-host-node-id
 	DisableHostNodeID bool
 
-	// DisableHTTPUnprintableCharFilter will bypass the filter preventing HTTP
-	// URLs from containing unprintable chars. This filter was added in 1.0.3 as a
-	// response to a vulnerability report. Disabling this is never recommended in
-	// general however some users who have keys written in older versions of
-	// Consul may use this to temporarily disable the filter such that they can
-	// delete those keys again! We do not recommend leaving it disabled long term.
-	//
-	// hcl: disable_http_unprintable_char_filter
-	DisableHTTPUnprintableCharFilter bool
-
 	// DisableKeyringFile disables writing the keyring to a file.
 	//
 	// hcl: disable_keyring_file = (true|false)

agent/config/runtime_test.go

@@ -2621,7 +2621,6 @@ func TestFullConfig(t *testing.T) {
 			"disable_anonymous_signature": true,
 			"disable_coordinates": true,
 			"disable_host_node_id": true,
-			"disable_http_unprintable_char_filter": true,
 			"disable_keyring_file": true,
 			"disable_remote_exec": true,
 			"disable_update_check": true,
@@ -3085,7 +3084,6 @@ func TestFullConfig(t *testing.T) {
 			disable_anonymous_signature = true
 			disable_coordinates = true
 			disable_host_node_id = true
-			disable_http_unprintable_char_filter = true
 			disable_keyring_file = true
 			disable_remote_exec = true
 			disable_update_check = true
@@ -3701,7 +3699,6 @@ func TestFullConfig(t *testing.T) {
 		DisableAnonymousSignature: true,
 		DisableCoordinates:        true,
 		DisableHostNodeID:         true,
-		DisableHTTPUnprintableCharFilter: true,
 		DisableKeyringFile:        true,
 		DisableRemoteExec:         true,
 		DisableUpdateCheck:        true,
@@ -4406,7 +4403,6 @@ func TestSanitize(t *testing.T) {
     "DevMode": false,
     "DisableAnonymousSignature": false,
     "DisableCoordinates": false,
-    "DisableHTTPUnprintableCharFilter": false,
     "DisableHostNodeID": false,
     "DisableKeyringFile": false,
     "DisableRemoteExec": false,

agent/http.go

@@ -187,15 +187,10 @@ func (s *HTTPServer) handler(enableDebug bool) http.Handler {
 	}
 
 	// Wrap the whole mux with a handler that bans URLs with non-printable
-	// characters, unless disabled explicitly to deal with old keys that fail this
-	// check.
-	h := cleanhttp.PrintablePathCheckHandler(mux, nil)
-	if s.agent.config.DisableHTTPUnprintableCharFilter {
-		h = mux
-	}
+	// characters.
 	return &wrappedMux{
 		mux:     mux,
-		handler: h,
+		handler: cleanhttp.PrintablePathCheckHandler(mux, nil),
 	}
 }
 

agent/http_test.go

@@ -327,19 +327,6 @@ func TestHTTPAPI_Ban_Nonprintable_Characters(t *testing.T) {
 	}
 }
 
-func TestHTTPAPI_Allow_Nonprintable_Characters_With_Flag(t *testing.T) {
-	a := NewTestAgent(t.Name(), "disable_http_unprintable_char_filter = true")
-	defer a.Shutdown()
-
-	req, _ := http.NewRequest("GET", "/v1/kv/bad\x00ness", nil)
-	resp := httptest.NewRecorder()
-	a.srv.Handler.ServeHTTP(resp, req)
-	// Key doesn't actually exist so we should get 404
-	if got, want := resp.Code, http.StatusNotFound; 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.

website/source/docs/agent/options.html.md

@@ -749,17 +749,6 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass
 * <a name="disable_host_node_id"></a><a href="#disable_host_node_id">`disable_host_node_id`</a>
   Equivalent to the [`-disable-host-node-id` command-line flag](#_disable_host_node_id).
 
-* <a name="disable_http_unprintable_char_filter"></a><a href="#disable_http_unprintable_char_filter">`disable_http_unprintable_char_filter`</a>
-  Defaults to false. Consul 1.0.3 fixed a potential security vulnerability where
-  malicious users could craft KV keys with unprintable chars that would confuse
-  operators using the CLI or UI into taking wrong actions. Users who had data
-  written in older versions of Consul that did not have this restriction will be
-  unable to delete those values by default in 1.0.3 or later. This setting
-  enables those users to _temporarily_ disable the filter such that delete
-  operations can work on those keys again to get back to a healthy state. It is
-  strongly recommended that this filter is not disabled permanently as it
-  exposes the original security vulnerability.
-
 * <a name="disable_remote_exec"></a><a href="#disable_remote_exec">`disable_remote_exec`</a>
   Disables support for remote execution. When set to true, the agent will ignore any incoming
   remote exec requests. In versions of Consul prior to 0.8, this defaulted to false. In Consul