Merge pull request from GHSA-99qr-h2rp-65rc

DebugRequestMiddleware: unconditionally scrub data field

api/server/middleware/debug.go

@@ -41,7 +41,7 @@ func DebugRequestMiddleware(handler func(ctx context.Context, w http.ResponseWri
 
 		var postForm map[string]interface{}
 		if err := json.Unmarshal(b, &postForm); err == nil {
-			maskSecretKeys(postForm, r.RequestURI)
+			maskSecretKeys(postForm)
 			formStr, errMarshal := json.Marshal(postForm)
 			if errMarshal == nil {
 				logrus.Debugf("form data: %s", string(formStr))
@@ -54,41 +54,37 @@ func DebugRequestMiddleware(handler func(ctx context.Context, w http.ResponseWri
 	}
 }
 
-func maskSecretKeys(inp interface{}, path string) {
-	// Remove any query string from the path
-	idx := strings.Index(path, "?")
-	if idx != -1 {
-		path = path[:idx]
-	}
-	// Remove trailing / characters
-	path = strings.TrimRight(path, "/")
-
+func maskSecretKeys(inp interface{}) {
 	if arr, ok := inp.([]interface{}); ok {
 		for _, f := range arr {
-			maskSecretKeys(f, path)
+			maskSecretKeys(f)
 		}
 		return
 	}
 
 	if form, ok := inp.(map[string]interface{}); ok {
+		scrub := []string{
+			// Note: The Data field contains the base64-encoded secret in 'secret'
+			// and 'config' create and update requests. Currently, no other POST
+			// API endpoints use a data field, so we scrub this field unconditionally.
+			// Change this handling to be conditional if a new endpoint is added
+			// in future where this field should not be scrubbed.
+			"data",
+			"jointoken",
+			"password",
+			"secret",
+			"signingcakey",
+			"unlockkey",
+		}
 	loop0:
 		for k, v := range form {
-			for _, m := range []string{"password", "secret", "jointoken", "unlockkey", "signingcakey"} {
+			for _, m := range scrub {
 				if strings.EqualFold(m, k) {
 					form[k] = "*****"
 					continue loop0
 				}
 			}
-			maskSecretKeys(v, path)
-		}
-
-		// Route-specific redactions
-		if strings.HasSuffix(path, "/secrets/create") {
-			for k := range form {
-				if k == "Data" {
-					form[k] = "*****"
-				}
-			}
+			maskSecretKeys(v)
 		}
 	}
 }

api/server/middleware/debug_test.go

@@ -9,31 +9,25 @@ import (
 
 func TestMaskSecretKeys(t *testing.T) {
 	tests := []struct {
-		path     string
+		doc      string
 		input    map[string]interface{}
 		expected map[string]interface{}
 	}{
 		{
-			path:     "/v1.30/secrets/create",
+			doc:      "secret/config create and update requests",
 			input:    map[string]interface{}{"Data": "foo", "Name": "name", "Labels": map[string]interface{}{}},
 			expected: map[string]interface{}{"Data": "*****", "Name": "name", "Labels": map[string]interface{}{}},
 		},
 		{
-			path:     "/v1.30/secrets/create//",
-			input:    map[string]interface{}{"Data": "foo", "Name": "name", "Labels": map[string]interface{}{}},
-			expected: map[string]interface{}{"Data": "*****", "Name": "name", "Labels": map[string]interface{}{}},
-		},
-
-		{
-			path:     "/secrets/create?key=val",
-			input:    map[string]interface{}{"Data": "foo", "Name": "name", "Labels": map[string]interface{}{}},
-			expected: map[string]interface{}{"Data": "*****", "Name": "name", "Labels": map[string]interface{}{}},
-		},
-		{
-			path: "/v1.30/some/other/path",
+			doc: "masking other fields (recursively)",
 			input: map[string]interface{}{
-				"password": "pass",
+				"password":     "pass",
+				"secret":       "secret",
+				"jointoken":    "jointoken",
+				"unlockkey":    "unlockkey",
+				"signingcakey": "signingcakey",
 				"other": map[string]interface{}{
+					"password":     "pass",
 					"secret":       "secret",
 					"jointoken":    "jointoken",
 					"unlockkey":    "unlockkey",
@@ -41,8 +35,13 @@ func TestMaskSecretKeys(t *testing.T) {
 				},
 			},
 			expected: map[string]interface{}{
-				"password": "*****",
+				"password":     "*****",
+				"secret":       "*****",
+				"jointoken":    "*****",
+				"unlockkey":    "*****",
+				"signingcakey": "*****",
 				"other": map[string]interface{}{
+					"password":     "*****",
 					"secret":       "*****",
 					"jointoken":    "*****",
 					"unlockkey":    "*****",
@@ -50,10 +49,27 @@ func TestMaskSecretKeys(t *testing.T) {
 				},
 			},
 		},
+		{
+			doc: "case insensitive field matching",
+			input: map[string]interface{}{
+				"PASSWORD": "pass",
+				"other": map[string]interface{}{
+					"PASSWORD": "pass",
+				},
+			},
+			expected: map[string]interface{}{
+				"PASSWORD": "*****",
+				"other": map[string]interface{}{
+					"PASSWORD": "*****",
+				},
+			},
+		},
 	}
 
 	for _, testcase := range tests {
-		maskSecretKeys(testcase.input, testcase.path)
-		assert.Check(t, is.DeepEqual(testcase.expected, testcase.input))
+		t.Run(testcase.doc, func(t *testing.T) {
+			maskSecretKeys(testcase.input)
+			assert.Check(t, is.DeepEqual(testcase.expected, testcase.input))
+		})
 	}
 }