From 7a717a1cc420d81b6fbf77764d51546e9dc15105 Mon Sep 17 00:00:00 2001 From: Luiz Carvalho Date: Wed, 1 Nov 2023 13:41:22 -0400 Subject: [PATCH] Ignore malformed secrets when creating keychain This changes the behavior of the NewFromPullSecrets function to ignore secrets that contain malformed data. This allows processing of the other secrets which may be sufficient for authentication. Resolves #1833 Signed-off-by: Luiz Carvalho --- pkg/authn/kubernetes/keychain.go | 20 ++++++-- pkg/authn/kubernetes/keychain_test.go | 68 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pkg/authn/kubernetes/keychain.go b/pkg/authn/kubernetes/keychain.go index 368d829a2..e18062617 100644 --- a/pkg/authn/kubernetes/keychain.go +++ b/pkg/authn/kubernetes/keychain.go @@ -17,7 +17,7 @@ package kubernetes import ( "context" "encoding/json" - "fmt" + "errors" "net" "net/url" "path/filepath" @@ -132,7 +132,6 @@ func New(ctx context.Context, client kubernetes.Interface, opt Options) (authn.K } } } - return NewFromPullSecrets(ctx, pullSecrets) } @@ -165,17 +164,22 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key } var cfg dockerConfigJSON + // hasValid tracks if at least one of the given secrets provided is valid and contains at least + // one valid auth. + var hasValid bool // From: https://github.com/kubernetes/kubernetes/blob/0dcafb1f37ee522be3c045753623138e5b907001/pkg/credentialprovider/keyring.go for _, secret := range secrets { if b, exists := secret.Data[corev1.DockerConfigJsonKey]; secret.Type == corev1.SecretTypeDockerConfigJson && exists && len(b) > 0 { if err := json.Unmarshal(b, &cfg); err != nil { - return nil, err + logs.Warn.Printf("cannot decode secret %s/%s; ignoring", secret.Namespace, secret.Name) + continue } } if b, exists := secret.Data[corev1.DockerConfigKey]; secret.Type == corev1.SecretTypeDockercfg && exists && len(b) > 0 { if err := json.Unmarshal(b, &cfg.Auths); err != nil { - return nil, err + logs.Warn.Printf("cannot decode secret %s/%s; ignoring", secret.Namespace, secret.Name) + continue } } @@ -186,9 +190,11 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key } parsed, err := url.Parse(value) if err != nil { - return nil, fmt.Errorf("Entry %q in dockercfg invalid (%w)", value, err) + logs.Warn.Printf("entry %q in dockercfg secret %s/%s invalid (%s); ignoring", secret.Namespace, secret.Name, value, err) + continue } + hasValid = true // The docker client allows exact matches: // foo.bar.com/namespace // Or hostname matches: @@ -218,6 +224,10 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key // when matching for creds sort.Sort(sort.Reverse(sort.StringSlice(keyring.index))) } + if !hasValid && len(secrets) > 0 { + return nil, errors.New("could not find a valid secret") + } + return keyring, nil } diff --git a/pkg/authn/kubernetes/keychain_test.go b/pkg/authn/kubernetes/keychain_test.go index e01577115..101ff7346 100644 --- a/pkg/authn/kubernetes/keychain_test.go +++ b/pkg/authn/kubernetes/keychain_test.go @@ -132,6 +132,49 @@ func TestServiceAccountNotFound(t *testing.T) { testResolve(t, kc, registry(t, "fake.registry.io"), authn.Anonymous) } +func TestMalformedSecretsIgnored(t *testing.T) { + secrets := []corev1.Secret{ + // Malformed .dockerconfigjson is ignored + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-dockerconfigjson", + Namespace: "ns", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte("bad-base64"), + }, + }, + // Malformed .dockercfg is ignored + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-dockercfg", + Namespace: "ns", + }, + Type: corev1.SecretTypeDockercfg, + Data: map[string][]byte{ + corev1.DockerConfigKey: []byte("bad-base64"), + }, + }, + // Malformed registry URL is ignored + *dockerCfgSecretType.Create(t, "ns", "secret", `\tfake.registry.io`, authn.AuthConfig{ + Username: "ignored", Password: "ignored", + }), + // Correct secret is used + *dockerCfgSecretType.Create(t, "ns", "secret", "fake.registry.io", authn.AuthConfig{ + Username: "user", Password: "pass", + }), + } + + kc, err := NewFromPullSecrets(context.Background(), secrets) + if err != nil { + t.Fatalf("NewFromPullSecrets() = %v", err) + } + + expectedAuth := &authn.Basic{Username: "user", Password: "pass"} + testResolve(t, kc, registry(t, "fake.registry.io"), expectedAuth) +} + func TestImagePullSecretAttachedServiceAccount(t *testing.T) { username, password := "foo", "bar" client := fakeclient.NewSimpleClientset(&corev1.ServiceAccount{ @@ -413,6 +456,31 @@ func TestAuthWithGlobs(t *testing.T) { testResolve(t, kc, repo(t, "blah.registry.io/repo"), authn.FromConfig(auth)) } +func TestNoValidAuthsFound(t *testing.T) { + secrets := []corev1.Secret{ + // Malformed .dockerconfigjson is ignored + { + ObjectMeta: metav1.ObjectMeta{ + Name: "bad-dockerconfigjson", + Namespace: "ns", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte("bad-base64"), + }, + }, + } + + kc, err := NewFromPullSecrets(context.Background(), secrets) + if kc != nil { + t.Fatal("NewFromPullSecrets() = expected nil keychain") + } + expectedErr := "could not find a valid secret" + if err == nil || fmt.Sprintf("%s", err) != expectedErr { + t.Fatalf("NewFromPullSecrets() = want error %q, got %q", expectedErr, err) + } +} + func testResolve(t *testing.T, kc authn.Keychain, target authn.Resource, expectedAuth authn.Authenticator) { t.Helper()