diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index c844a051c24..3f4338128e5 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -337,6 +337,13 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po } } } + for _, envFrom := range container.EnvFrom { + if envFrom.SecretRef != nil { + if !mountableSecrets.Has(envFrom.SecretRef.Name) { + return fmt.Errorf("init container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) + } + } + } } for _, container := range pod.Spec.Containers { @@ -347,6 +354,13 @@ func (s *Plugin) limitSecretReferences(serviceAccount *corev1.ServiceAccount, po } } } + for _, envFrom := range container.EnvFrom { + if envFrom.SecretRef != nil { + if !mountableSecrets.Has(envFrom.SecretRef.Name) { + return fmt.Errorf("container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) + } + } + } } // limit pull secret references as well @@ -388,6 +402,13 @@ func (s *Plugin) limitEphemeralContainerSecretReferences(pod *api.Pod, a admissi } } } + for _, envFrom := range container.EnvFrom { + if envFrom.SecretRef != nil { + if !mountableSecrets.Has(envFrom.SecretRef.Name) { + return fmt.Errorf("ephemeral container %s with envFrom referencing secret.secretName=\"%s\" is not allowed because service account %s does not reference that secret", container.Name, envFrom.SecretRef.Name, serviceAccount.Name) + } + } + } } return nil } diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index b5155f5cd33..01b08da455f 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -520,6 +520,25 @@ func TestAllowsReferencedSecret(t *testing.T) { t.Errorf("Unexpected error: %v", err) } + pod2 = &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "container-1", + EnvFrom: []api.EnvFromSource{ + { + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { + t.Errorf("Unexpected error: %v", err) + } + pod2 = &api.Pod{ Spec: api.PodSpec{ InitContainers: []api.Container{ @@ -544,6 +563,25 @@ func TestAllowsReferencedSecret(t *testing.T) { t.Errorf("Unexpected error: %v", err) } + pod2 = &api.Pod{ + Spec: api.PodSpec{ + InitContainers: []api.Container{ + { + Name: "container-1", + EnvFrom: []api.EnvFromSource{ + { + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { + t.Errorf("Unexpected error: %v", err) + } + pod2 = &api.Pod{ Spec: api.PodSpec{ ServiceAccountName: DefaultServiceAccountName, @@ -571,6 +609,28 @@ func TestAllowsReferencedSecret(t *testing.T) { if err := admit.Validate(context.TODO(), attrs, nil); err != nil { t.Errorf("Unexpected error: %v", err) } + + pod2 = &api.Pod{ + Spec: api.PodSpec{ + ServiceAccountName: DefaultServiceAccountName, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "container-2", + EnvFrom: []api.EnvFromSource{{ + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, + }, + }, + }, + }, + } + // validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers" + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) + if err := admit.Validate(context.TODO(), attrs, nil); err != nil { + t.Errorf("Unexpected error: %v", err) + } } func TestRejectsUnreferencedSecretVolumes(t *testing.T) { @@ -627,25 +687,20 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { pod2 = &api.Pod{ Spec: api.PodSpec{ - InitContainers: []api.Container{ + Containers: []api.Container{ { Name: "container-1", - Env: []api.EnvVar{ + EnvFrom: []api.EnvFromSource{ { - Name: "env-1", - ValueFrom: &api.EnvVarSource{ - SecretKeyRef: &api.SecretKeySelector{ - LocalObjectReference: api.LocalObjectReference{Name: "foo"}, - }, - }, - }, - }, + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, }, }, }, } attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) - if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { t.Errorf("Unexpected error: %v", err) } @@ -678,6 +733,30 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) } + pod2 = &api.Pod{ + Spec: api.PodSpec{ + ServiceAccountName: DefaultServiceAccountName, + InitContainers: []api.Container{ + { + Name: "container-1", + EnvFrom: []api.EnvFromSource{ + { + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Update, &metav1.UpdateOptions{}, false, nil) + if err := admissiontesting.WithReinvocationTesting(t, admit).Admit(context.TODO(), attrs, nil); err != nil { + t.Errorf("admit only enforces restrictions on secret mounts when operation==create. Unexpected error: %v", err) + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "", admission.Create, &metav1.CreateOptions{}, false, nil) + if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { + t.Errorf("validate only enforces restrictions on secret mounts when operation==create and subresource==''. Unexpected error: %v", err) + } + pod2 = &api.Pod{ Spec: api.PodSpec{ ServiceAccountName: DefaultServiceAccountName, @@ -708,6 +787,27 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envVar") { t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err) } + + pod2 = &api.Pod{ + Spec: api.PodSpec{ + ServiceAccountName: DefaultServiceAccountName, + EphemeralContainers: []api.EphemeralContainer{ + { + EphemeralContainerCommon: api.EphemeralContainerCommon{ + Name: "container-2", + EnvFrom: []api.EnvFromSource{{ + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "foo"}}}}, + }, + }, + }, + }, + } + attrs = admission.NewAttributesRecord(pod2, nil, api.Kind("Pod").WithVersion("version"), ns, "myname", api.Resource("pods").WithVersion("version"), "ephemeralcontainers", admission.Update, &metav1.UpdateOptions{}, false, nil) + if err := admit.Validate(context.TODO(), attrs, nil); err == nil || !strings.Contains(err.Error(), "with envFrom") { + t.Errorf("validate enforces restrictions on secret mounts when operation==update and subresource==ephemeralcontainers. Unexpected error: %v", err) + } } func TestAllowUnreferencedSecretVolumesForPermissiveSAs(t *testing.T) {