Merge pull request #94712 from droslean/cred-leak

[credentialprovider] avoid potential secret leaking while reading .dockercfg

pkg/credentialprovider/config.go

@@ -117,10 +117,14 @@ func ReadDockercfgFile(searchPaths []string) (cfg DockerConfig, err error) {
 			continue
 		}
 		cfg, err := readDockerConfigFileFromBytes(contents)
-		if err == nil {
+		if err != nil {
-			klog.V(4).Infof("found .dockercfg at %s", absDockerConfigFileLocation)
+			klog.V(4).Infof("couldn't get the config from %q contents: %v", absDockerConfigFileLocation, err)
-			return cfg, nil
+			continue
 		}
+
+		klog.V(4).Infof("found .dockercfg at %s", absDockerConfigFileLocation)
+		return cfg, nil
+
 	}
 	return nil, fmt.Errorf("couldn't find valid .dockercfg after checking in %v", searchPaths)
 }
@@ -230,8 +234,7 @@ func ReadDockerConfigFileFromURL(url string, client *http.Client, header *http.H
 
 func readDockerConfigFileFromBytes(contents []byte) (cfg DockerConfig, err error) {
 	if err = json.Unmarshal(contents, &cfg); err != nil {
-		klog.Errorf("while trying to parse blob %q: %v", contents, err)
+		return nil, errors.New("error occurred while trying to unmarshal json")
-		return nil, err
 	}
 	return
 }
@@ -239,8 +242,7 @@ func readDockerConfigFileFromBytes(contents []byte) (cfg DockerConfig, err error
 func readDockerConfigJSONFileFromBytes(contents []byte) (cfg DockerConfig, err error) {
 	var cfgJSON DockerConfigJSON
 	if err = json.Unmarshal(contents, &cfgJSON); err != nil {
-		klog.Errorf("while trying to parse blob %q: %v", contents, err)
+		return nil, errors.New("error occurred while trying to unmarshal json")
-		return nil, err
 	}
 	cfg = cfgJSON.Auths
 	return

pkg/credentialprovider/config_test.go

@@ -309,3 +309,96 @@ func TestDockerConfigEntryJSONCompatibleEncode(t *testing.T) {
 		}
 	}
 }
+
+func TestReadDockerConfigFileFromBytes(t *testing.T) {
+	testCases := []struct {
+		id               string
+		input            []byte
+		expectedCfg      DockerConfig
+		errorExpected    bool
+		expectedErrorMsg string
+	}{
+		{
+			id:    "valid input, no error expected",
+			input: []byte(`{"http://foo.example.com":{"username": "foo", "password": "bar", "email": "foo@example.com"}}`),
+			expectedCfg: DockerConfig(map[string]DockerConfigEntry{
+				"http://foo.example.com": {
+					Username: "foo",
+					Password: "bar",
+					Email:    "foo@example.com",
+				},
+			}),
+		},
+		{
+			id:               "invalid input, error expected",
+			input:            []byte(`{"http://foo.example.com":{"username": "foo", "password": "bar", "email": "foo@example.com"`),
+			errorExpected:    true,
+			expectedErrorMsg: "error occurred while trying to unmarshal json",
+		},
+	}
+
+	for _, tc := range testCases {
+		cfg, err := readDockerConfigFileFromBytes(tc.input)
+		if err != nil && !tc.errorExpected {
+			t.Fatalf("Error was not expected: %v", err)
+		}
+		if err != nil && tc.errorExpected {
+			if !reflect.DeepEqual(err.Error(), tc.expectedErrorMsg) {
+				t.Fatalf("Expected error message: `%s` got `%s`", tc.expectedErrorMsg, err.Error())
+			}
+		} else {
+			if !reflect.DeepEqual(cfg, tc.expectedCfg) {
+				t.Fatalf("expected: %v got %v", tc.expectedCfg, cfg)
+			}
+		}
+	}
+}
+
+func TestReadDockerConfigJSONFileFromBytes(t *testing.T) {
+	testCases := []struct {
+		id               string
+		input            []byte
+		expectedCfg      DockerConfig
+		errorExpected    bool
+		expectedErrorMsg string
+	}{
+		{
+			id:    "valid input, no error expected",
+			input: []byte(`{"auths": {"http://foo.example.com":{"username": "foo", "password": "bar", "email": "foo@example.com"}, "http://bar.example.com":{"username": "bar", "password": "baz", "email": "bar@example.com"}}}`),
+			expectedCfg: DockerConfig(map[string]DockerConfigEntry{
+				"http://foo.example.com": {
+					Username: "foo",
+					Password: "bar",
+					Email:    "foo@example.com",
+				},
+				"http://bar.example.com": {
+					Username: "bar",
+					Password: "baz",
+					Email:    "bar@example.com",
+				},
+			}),
+		},
+		{
+			id:               "invalid input, error expected",
+			input:            []byte(`{"auths": {"http://foo.example.com":{"username": "foo", "password": "bar", "email": "foo@example.com"}, "http://bar.example.com":{"username": "bar", "password": "baz", "email": "bar@example.com"`),
+			errorExpected:    true,
+			expectedErrorMsg: "error occurred while trying to unmarshal json",
+		},
+	}
+
+	for _, tc := range testCases {
+		cfg, err := readDockerConfigJSONFileFromBytes(tc.input)
+		if err != nil && !tc.errorExpected {
+			t.Fatalf("Error was not expected: %v", err)
+		}
+		if err != nil && tc.errorExpected {
+			if !reflect.DeepEqual(err.Error(), tc.expectedErrorMsg) {
+				t.Fatalf("Expected error message: `%s` got `%s`", tc.expectedErrorMsg, err.Error())
+			}
+		} else {
+			if !reflect.DeepEqual(cfg, tc.expectedCfg) {
+				t.Fatalf("expected: %v got %v", tc.expectedCfg, cfg)
+			}
+		}
+	}
+}