From f79fd034fd1ed63287b0d55fb9f352ecab32c52e Mon Sep 17 00:00:00 2001 From: Sunny Date: Tue, 31 May 2022 18:28:56 +0530 Subject: [PATCH] registry: repo URL and dockerconfig URL mismatch Registry login option should verify that the obtained dockerconfig credentials are for the same host. When the helmrepo URL and the URL in docker auth config don't match, the docker config store returns an empty auth config, instead of failing. This results in accepting empty username and password. The HelmRepo would appear to be ready in such situation because the creds are empty, no login is attempted. But when a HelmChart tries to use the login options, it'd fail. Signed-off-by: Sunny --- internal/helm/registry/auth.go | 25 ++++++ internal/helm/registry/auth_test.go | 131 ++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 internal/helm/registry/auth_test.go diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index a37e4c65..75667f1d 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package registry import ( @@ -6,6 +22,7 @@ import ( "net/url" "github.com/docker/cli/cli/config" + "github.com/docker/cli/cli/config/credentials" "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" ) @@ -30,6 +47,14 @@ func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.L if err != nil { return nil, fmt.Errorf("unable to get authentication data from Secret '%s': %w", secret.Name, err) } + + // Make sure that the obtained auth config is for the requested host. + // When the docker config does not contain the credentials for a host, + // the credential store returns an empty auth config. + // Refer: https://github.com/docker/cli/blob/v20.10.16/cli/config/credentials/file_store.go#L44 + if credentials.ConvertToHostname(authConfig.ServerAddress) != parsedURL.Host { + return nil, fmt.Errorf("no auth config for '%s' in the docker-registry Secret '%s'", parsedURL.Host, secret.Name) + } username = authConfig.Username password = authConfig.Password } else { diff --git a/internal/helm/registry/auth_test.go b/internal/helm/registry/auth_test.go new file mode 100644 index 00000000..921ecbf1 --- /dev/null +++ b/internal/helm/registry/auth_test.go @@ -0,0 +1,131 @@ +/* +Copyright 2022 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package registry + +import ( + "testing" + + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" +) + +func TestLoginOptionFromSecret(t *testing.T) { + testURL := "oci://registry.example.com/foo/bar" + testUser := "flux" + testPassword := "somepassword" + testDockerconfigjson := `{"auths":{"registry.example.com":{"username":"flux","password":"somepassword","auth":"Zmx1eDpzb21lcGFzc3dvcmQ="}}}` + testDockerconfigjsonHTTPS := `{"auths":{"https://registry.example.com":{"username":"flux","password":"somepassword","auth":"Zmx1eDpzb21lcGFzc3dvcmQ="}}}` + dockerconfigjsonKey := ".dockerconfigjson" + + tests := []struct { + name string + url string + secretType corev1.SecretType + secretData map[string][]byte + wantErr bool + }{ + { + name: "generic secret", + url: testURL, + secretType: corev1.SecretTypeOpaque, + secretData: map[string][]byte{ + "username": []byte(testUser), + "password": []byte(testPassword), + }, + }, + { + name: "generic secret without username", + url: testURL, + secretType: corev1.SecretTypeOpaque, + secretData: map[string][]byte{ + "password": []byte(testPassword), + }, + wantErr: true, + }, + { + name: "generic secret without password", + url: testURL, + secretType: corev1.SecretTypeOpaque, + secretData: map[string][]byte{ + "username": []byte(testUser), + }, + wantErr: true, + }, + { + name: "generic secret without username and password", + url: testURL, + secretType: corev1.SecretTypeOpaque, + }, + { + name: "docker-registry secret", + url: testURL, + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + dockerconfigjsonKey: []byte(testDockerconfigjson), + }, + }, + { + name: "docker-registry secret host mismatch", + url: "oci://registry.gitlab.com", + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + dockerconfigjsonKey: []byte(testDockerconfigjson), + }, + wantErr: true, + }, + { + name: "docker-registry secret invalid host", + url: "oci://registry .gitlab.com", + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + dockerconfigjsonKey: []byte(testDockerconfigjson), + }, + wantErr: true, + }, + { + name: "docker-registry secret invalid docker config", + url: testURL, + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + dockerconfigjsonKey: []byte("foo"), + }, + wantErr: true, + }, + { + name: "docker-registry secret with URL scheme", + url: testURL, + secretType: corev1.SecretTypeDockerConfigJson, + secretData: map[string][]byte{ + dockerconfigjsonKey: []byte(testDockerconfigjsonHTTPS), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + secret := corev1.Secret{} + secret.Name = "test-secret" + secret.Data = tt.secretData + secret.Type = tt.secretType + + _, err := LoginOptionFromSecret(tt.url, secret) + g.Expect(err != nil).To(Equal(tt.wantErr)) + }) + } +}