diff --git a/plugin/pkg/admission/certificates/ctbattest/admission.go b/plugin/pkg/admission/certificates/ctbattest/admission.go index 6b100ee328e..8b2dfea7661 100644 --- a/plugin/pkg/admission/certificates/ctbattest/admission.go +++ b/plugin/pkg/admission/certificates/ctbattest/admission.go @@ -101,22 +101,7 @@ func (p *Plugin) Validate(ctx context.Context, a admission.Attributes, _ admissi } // Unlike CSRs, it's OK to validate against the *new* object, because - // updates to signer name will be rejected during validation. For defense - // in depth, reject attempts to change signer at this layer as well. - // - // We want to use the new object because we also need to perform the signer - // name permission check on *create*. - - if a.GetOperation() == admission.Update { - oldBundle, ok := a.GetOldObject().(*api.ClusterTrustBundle) - if !ok { - return admission.NewForbidden(a, fmt.Errorf("expected type ClusterTrustBundle, got: %T", a.GetOldObject())) - } - - if oldBundle.Spec.SignerName != newBundle.Spec.SignerName { - return admission.NewForbidden(a, fmt.Errorf("changing signerName is forbidden")) - } - } + // updates to signer name will be rejected during validation. // If signer name isn't specified, we don't need to perform the // attest check. diff --git a/test/integration/clustertrustbundles/admission_establishtrust_test.go b/test/integration/clustertrustbundles/admission_establishtrust_test.go new file mode 100644 index 00000000000..f97444829bc --- /dev/null +++ b/test/integration/clustertrustbundles/admission_establishtrust_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2022 The Kubernetes 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 clustertrustbundles + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + + certsv1alpha1 "k8s.io/api/certificates/v1alpha1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/authutil" + "k8s.io/kubernetes/test/integration/framework" +) + +// Verifies that the ClusterTrustBundle attest admission plugin correctly +// enforces that a user has "attest" on the affected signer name. +func TestCTBAttestPlugin(t *testing.T) { + testCases := []struct { + description string + trustBundleName string + allowedSignerName string + targetSignerName string + wantError string + }{ + { + description: "should admit if the clustertrustbundle doesn't target a signer", + trustBundleName: "foo", + allowedSignerName: "foo.com/bar", + }, + { + description: "should admit if the user has attest for the exact signer name", + trustBundleName: "foo.com:bar:abc", + allowedSignerName: "foo.com/bar", + targetSignerName: "foo.com/bar", + }, + { + description: "should admit if the user has attest for the wildcard-suffixed signer name", + trustBundleName: "foo.com:bar:abc", + allowedSignerName: "foo.com/*", + targetSignerName: "foo.com/bar", + }, + { + description: "should deny if the user does not have permission for the signer name", + trustBundleName: "foo.com:bar:abc", + allowedSignerName: "abc.com/def", + targetSignerName: "foo.com/bar", + wantError: "clustertrustbundles.certificates.k8s.io \"foo.com:bar:abc\" is forbidden: user not permitted to attest for signerName \"foo.com/bar\"", + }, + } + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + ctx := context.Background() + + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--authorization-mode=RBAC", "--feature-gates=ClusterTrustBundle=true"}, framework.SharedEtcd()) + defer server.TearDownFn() + + client := kubernetes.NewForConfigOrDie(server.ClientConfig) + + if tc.allowedSignerName != "" { + grantUserPermissionToAttestFor(ctx, t, client, "test-user", tc.allowedSignerName) + } + + // Create a second client that impersonates test-user. + testUserConfig := rest.CopyConfig(server.ClientConfig) + testUserConfig.Impersonate = rest.ImpersonationConfig{UserName: "test-user"} + testUserClient := kubernetes.NewForConfigOrDie(testUserConfig) + + bundle := &certsv1alpha1.ClusterTrustBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.trustBundleName, + }, + Spec: certsv1alpha1.ClusterTrustBundleSpec{ + SignerName: tc.targetSignerName, + TrustBundle: mustMakePEMBlock("CERTIFICATE", nil, mustMakeCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "root1", + }, + IsCA: true, + BasicConstraintsValid: true, + })), + }, + } + _, err := testUserClient.CertificatesV1alpha1().ClusterTrustBundles().Create(ctx, bundle, metav1.CreateOptions{}) + if err != nil && err.Error() != tc.wantError { + t.Fatalf("Bad error while creating ClusterTrustBundle; got %q want %q", err.Error(), tc.wantError) + } else if err == nil && tc.wantError != "" { + t.Fatalf("Bad error while creating ClusterTrustBundle; got nil want %q", tc.wantError) + } + }) + } +} + +func grantUserPermissionToAttestFor(ctx context.Context, t *testing.T, client kubernetes.Interface, username string, signerNames ...string) { + resourceName := "signername-" + username + cr := buildApprovalClusterRoleForSigners(resourceName, signerNames...) + crb := buildClusterRoleBindingForUser(resourceName, username, cr.Name) + if _, err := client.RbacV1().ClusterRoles().Create(ctx, cr, metav1.CreateOptions{}); err != nil { + t.Fatalf("unable to create test fixture RBAC rules: %v", err) + } + if _, err := client.RbacV1().ClusterRoleBindings().Create(ctx, crb, metav1.CreateOptions{}); err != nil { + t.Fatalf("unable to create test fixture RBAC rules: %v", err) + } + attestRule := cr.Rules[0] + createRule := cr.Rules[1] + authutil.WaitForNamedAuthorizationUpdate(t, ctx, client.AuthorizationV1(), username, "", attestRule.Verbs[0], attestRule.ResourceNames[0], schema.GroupResource{Group: attestRule.APIGroups[0], Resource: attestRule.Resources[0]}, true) + authutil.WaitForNamedAuthorizationUpdate(t, ctx, client.AuthorizationV1(), username, "", createRule.Verbs[0], "", schema.GroupResource{Group: createRule.APIGroups[0], Resource: createRule.Resources[0]}, true) +} + +func buildApprovalClusterRoleForSigners(name string, signerNames ...string) *rbacv1.ClusterRole { + return &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"attest"}, + APIGroups: []string{"certificates.k8s.io"}, + Resources: []string{"signers"}, + ResourceNames: signerNames, + }, + { + Verbs: []string{"create"}, + APIGroups: []string{"certificates.k8s.io"}, + Resources: []string{"clustertrustbundles"}, + }, + }, + } +} + +func buildClusterRoleBindingForUser(name, username, clusterRoleName string) *rbacv1.ClusterRoleBinding { + return &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Subjects: []rbacv1.Subject{ + { + Kind: rbacv1.UserKind, + Name: username, + }, + }, + RoleRef: rbacv1.RoleRef{ + APIGroup: rbacv1.SchemeGroupVersion.Group, + Kind: "ClusterRole", + Name: clusterRoleName, + }, + } +} diff --git a/test/integration/clustertrustbundles/field_selector_test.go b/test/integration/clustertrustbundles/field_selector_test.go new file mode 100644 index 00000000000..53db389f859 --- /dev/null +++ b/test/integration/clustertrustbundles/field_selector_test.go @@ -0,0 +1,135 @@ +/* +Copyright 2022 The Kubernetes 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 clustertrustbundles + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "testing" + + certsv1alpha1 "k8s.io/api/certificates/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/framework" +) + +func TestCTBSignerNameFieldSelector(t *testing.T) { + ctx := context.Background() + + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--feature-gates=ClusterTrustBundle=true"}, framework.SharedEtcd()) + defer server.TearDownFn() + + client := kubernetes.NewForConfigOrDie(server.ClientConfig) + + bundle1 := &certsv1alpha1.ClusterTrustBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.com:bar:v1", + }, + Spec: certsv1alpha1.ClusterTrustBundleSpec{ + SignerName: "foo.com/bar", + TrustBundle: mustMakePEMBlock("CERTIFICATE", nil, mustMakeCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "root1", + }, + IsCA: true, + BasicConstraintsValid: true, + })), + }, + } + if _, err := client.CertificatesV1alpha1().ClusterTrustBundles().Create(ctx, bundle1, metav1.CreateOptions{}); err != nil { + t.Fatalf("Error while creating bundle1: %v", err) + } + + bundle2 := &certsv1alpha1.ClusterTrustBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.com:bar:v2", + }, + Spec: certsv1alpha1.ClusterTrustBundleSpec{ + SignerName: "foo.com/bar", + TrustBundle: mustMakePEMBlock("CERTIFICATE", nil, mustMakeCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "root2", + }, + IsCA: true, + BasicConstraintsValid: true, + })), + }, + } + if _, err := client.CertificatesV1alpha1().ClusterTrustBundles().Create(ctx, bundle2, metav1.CreateOptions{}); err != nil { + t.Fatalf("Error while creating bundle2: %v", err) + } + + bundle3 := &certsv1alpha1.ClusterTrustBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: "baz.com:bar:v1", + }, + Spec: certsv1alpha1.ClusterTrustBundleSpec{ + SignerName: "baz.com/bar", + TrustBundle: mustMakePEMBlock("CERTIFICATE", nil, mustMakeCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "root3", + }, + IsCA: true, + BasicConstraintsValid: true, + })), + }, + } + if _, err := client.CertificatesV1alpha1().ClusterTrustBundles().Create(ctx, bundle3, metav1.CreateOptions{}); err != nil { + t.Fatalf("Error while creating bundle3: %v", err) + } + + fooList, err := client.CertificatesV1alpha1().ClusterTrustBundles().List(ctx, metav1.ListOptions{FieldSelector: "spec.signerName=foo.com/bar"}) + if err != nil { + t.Fatalf("Unable to list ClusterTrustBundles with spec.signerName=foo.com/bar") + } + if len(fooList.Items) != 2 { + t.Errorf("Wrong number of items in list for foo.com/bar; got %d, want 2", len(fooList.Items)) + } + found1 := false + found2 := false + for _, ctb := range fooList.Items { + if ctb.ObjectMeta.Name == "foo.com:bar:v1" { + found1 = true + } + if ctb.ObjectMeta.Name == "foo.com:bar:v2" { + found2 = true + } + } + if !found1 { + t.Errorf("Didn't find foo.com:bar:v1 in the list when listing for foo.com/bar") + } + if !found2 { + t.Errorf("Didn't find foo.com:bar:v2 in the list when listing for foo.com/bar") + } + + bazList, err := client.CertificatesV1alpha1().ClusterTrustBundles().List(ctx, metav1.ListOptions{FieldSelector: "spec.signerName=baz.com/bar"}) + if err != nil { + t.Fatalf("Unable to list ClusterTrustBundles with spec.signerName=baz.com/bar") + } + if len(bazList.Items) != 1 { + t.Fatalf("Wrong number of items in list for baz.com/bar; got %d, want 1", len(bazList.Items)) + } + if bazList.Items[0].ObjectMeta.Name != "baz.com:bar:v1" { + t.Errorf("Didn't find baz.com:bar:v1 in the list when listing for baz.com/bar") + } +} diff --git a/test/integration/clustertrustbundles/main_test.go b/test/integration/clustertrustbundles/main_test.go new file mode 100644 index 00000000000..95fc32ea611 --- /dev/null +++ b/test/integration/clustertrustbundles/main_test.go @@ -0,0 +1,55 @@ +/* +Copyright 2022 The Kubernetes 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 clustertrustbundles + +import ( + "crypto/ed25519" + "crypto/x509" + "encoding/pem" + mathrand "math/rand" + "testing" + + "k8s.io/kubernetes/test/integration/framework" +) + +func TestMain(m *testing.M) { + framework.EtcdMain(m.Run) +} + +func mustMakeCertificate(t *testing.T, template *x509.Certificate) []byte { + gen := mathrand.New(mathrand.NewSource(12345)) + + pub, priv, err := ed25519.GenerateKey(gen) + if err != nil { + t.Fatalf("Error while generating key: %v", err) + } + + cert, err := x509.CreateCertificate(gen, template, template, pub, priv) + if err != nil { + t.Fatalf("Error while making certificate: %v", err) + } + + return cert +} + +func mustMakePEMBlock(blockType string, headers map[string]string, data []byte) string { + return string(pem.EncodeToMemory(&pem.Block{ + Type: blockType, + Headers: headers, + Bytes: data, + })) +} diff --git a/test/integration/clustertrustbundles/signer_name_change_forbidden_test.go b/test/integration/clustertrustbundles/signer_name_change_forbidden_test.go new file mode 100644 index 00000000000..d8f22df392f --- /dev/null +++ b/test/integration/clustertrustbundles/signer_name_change_forbidden_test.go @@ -0,0 +1,101 @@ +/* +Copyright 2022 The Kubernetes 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 clustertrustbundles + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "math/big" + "testing" + + certsv1alpha1 "k8s.io/api/certificates/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/test/integration/framework" +) + +func TestCTBSignerNameChangeForbidden(t *testing.T) { + testCases := []struct { + objectName string + signer1 string + signer2 string + }{ + { + objectName: "foo", + signer1: "", + signer2: "foo.com/bar", + }, + { + objectName: "foo.com:bar:abc", + signer1: "foo.com/bar", + signer2: "", + }, + { + objectName: "foo.com:bar:abc", + signer1: "foo.com/bar", + signer2: "foo.com/bar2", + }, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s -> %s", tc.signer1, tc.signer2), func(t *testing.T) { + + ctx := context.Background() + + server := kubeapiservertesting.StartTestServerOrDie(t, nil, []string{"--feature-gates=ClusterTrustBundle=true"}, framework.SharedEtcd()) + defer server.TearDownFn() + + client := kubernetes.NewForConfigOrDie(server.ClientConfig) + + bundle1 := &certsv1alpha1.ClusterTrustBundle{ + ObjectMeta: metav1.ObjectMeta{ + Name: tc.objectName, + }, + Spec: certsv1alpha1.ClusterTrustBundleSpec{ + SignerName: tc.signer1, + TrustBundle: mustMakePEMBlock("CERTIFICATE", nil, mustMakeCertificate(t, &x509.Certificate{ + SerialNumber: big.NewInt(0), + Subject: pkix.Name{ + CommonName: "root1", + }, + IsCA: true, + BasicConstraintsValid: true, + })), + }, + } + bundle1, err := client.CertificatesV1alpha1().ClusterTrustBundles().Create(ctx, bundle1, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("Error while creating bundle1: %v", err) + } + + // Pick a new signer name that is still compatible with the admission + // restrictions on object name. That way the admission plugin won't get in + // the way by forbidding the update due to an incompatible name on the + // cluster trust bundle. + bundle1.Spec.SignerName = tc.signer2 + + _, err = client.CertificatesV1alpha1().ClusterTrustBundles().Update(ctx, bundle1, metav1.UpdateOptions{}) + if err == nil { + t.Fatalf("Got nil error from updating bundle foo-com--bar from signerName=foo.com/bar to signerName=foo.com/bar2, but wanted an error") + } + }) + } + +} diff --git a/test/integration/etcd/data.go b/test/integration/etcd/data.go index 45479db7cac..7f39d402d4d 100644 --- a/test/integration/etcd/data.go +++ b/test/integration/etcd/data.go @@ -182,6 +182,13 @@ func GetEtcdStorageDataForNamespace(namespace string) map[schema.GroupVersionRes }, // -- + // k8s.io/kubernetes/pkg/apis/certificates/v1alpha1 + gvr("certificates.k8s.io", "v1alpha1", "clustertrustbundles"): { + Stub: `{"metadata": {"name": "example.com:signer:abc"}, "spec": {"signerName":"example.com/signer", "trustBundle": "-----BEGIN CERTIFICATE-----\nMIIBBDCBt6ADAgECAgEAMAUGAytlcDAQMQ4wDAYDVQQDEwVyb290MTAiGA8wMDAx\nMDEwMTAwMDAwMFoYDzAwMDEwMTAxMDAwMDAwWjAQMQ4wDAYDVQQDEwVyb290MTAq\nMAUGAytlcAMhAF2MoFeGa97gK2NGT1h6p1/a1GlMXAXbcjI/OShyIobPozIwMDAP\nBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTWDdK2CNQiHqRjPaAWYPPtIykQgjAF\nBgMrZXADQQCtom9WGl7m2SAa4tXM9Soo/mbInBsRhn187BMoqTAHInHchKup5/3y\nl1tYJSZZsEXnXrCvw2qLCBNif6+2YYgE\n-----END CERTIFICATE-----\n"}}`, + ExpectedEtcdPath: "/registry/clustertrustbundles/example.com:signer:abc", + }, + // -- + // k8s.io/kubernetes/pkg/apis/coordination/v1 gvr("coordination.k8s.io", "v1", "leases"): { Stub: `{"metadata": {"name": "leasev1"}, "spec": {"holderIdentity": "holder", "leaseDurationSeconds": 5}}`,