diff --git a/pkg/controller/certificates/BUILD b/pkg/controller/certificates/BUILD index d6075fcb7a8..ce63218fd09 100644 --- a/pkg/controller/certificates/BUILD +++ b/pkg/controller/certificates/BUILD @@ -13,6 +13,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/controller/certificates", visibility = ["//visibility:public"], deps = [ + "//pkg/apis/certificates/v1beta1:go_default_library", "//pkg/controller:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/pkg/controller/certificates/approver/sarapprove.go b/pkg/controller/certificates/approver/sarapprove.go index 3af1eee76ff..1273ee0ddc2 100644 --- a/pkg/controller/certificates/approver/sarapprove.go +++ b/pkg/controller/certificates/approver/sarapprove.go @@ -21,14 +21,13 @@ import ( "context" "crypto/x509" "fmt" - "reflect" - "strings" authorization "k8s.io/api/authorization/v1" capi "k8s.io/api/certificates/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1" clientset "k8s.io/client-go/kubernetes" + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/controller/certificates" ) @@ -146,45 +145,12 @@ func appendApprovalCondition(csr *capi.CertificateSigningRequest, message string }) } -func hasExactUsages(csr *capi.CertificateSigningRequest, usages []capi.KeyUsage) bool { - if len(usages) != len(csr.Spec.Usages) { - return false - } - - usageMap := map[capi.KeyUsage]struct{}{} - for _, u := range usages { - usageMap[u] = struct{}{} - } - - for _, u := range csr.Spec.Usages { - if _, ok := usageMap[u]; !ok { - return false - } - } - - return true -} - -var kubeletClientUsages = []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - capi.UsageClientAuth, -} - func isNodeClientCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool { - if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) { + isClientCSR := capihelper.IsKubeletClientCSR(x509cr, csr.Spec.Usages) + if !isClientCSR { return false } - if len(x509cr.DNSNames) > 0 || len(x509cr.EmailAddresses) > 0 || len(x509cr.IPAddresses) > 0 || len(x509cr.URIs) > 0 { - return false - } - if !hasExactUsages(csr, kubeletClientUsages) { - return false - } - if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") { - return false - } - return true + return *csr.Spec.SignerName == capi.KubeAPIServerClientKubeletSignerName } func isSelfNodeClientCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool { diff --git a/pkg/controller/certificates/approver/sarapprove_test.go b/pkg/controller/certificates/approver/sarapprove_test.go index e20f26ba9b0..ee0afa11ee7 100644 --- a/pkg/controller/certificates/approver/sarapprove_test.go +++ b/pkg/controller/certificates/approver/sarapprove_test.go @@ -36,54 +36,6 @@ import ( k8s_certificates_v1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" ) -func TestHasKubeletUsages(t *testing.T) { - cases := []struct { - usages []capi.KeyUsage - expected bool - }{ - { - usages: nil, - expected: false, - }, - { - usages: []capi.KeyUsage{}, - expected: false, - }, - { - usages: []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - }, - expected: false, - }, - { - usages: []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - capi.UsageServerAuth, - }, - expected: false, - }, - { - usages: []capi.KeyUsage{ - capi.UsageKeyEncipherment, - capi.UsageDigitalSignature, - capi.UsageClientAuth, - }, - expected: true, - }, - } - for _, c := range cases { - if hasExactUsages(&capi.CertificateSigningRequest{ - Spec: capi.CertificateSigningRequestSpec{ - Usages: c.usages, - }, - }, kubeletClientUsages) != c.expected { - t.Errorf("unexpected result of hasKubeletUsages(%v), expecting: %v", c.usages, c.expected) - } - } -} - func TestHandle(t *testing.T) { cases := []struct { allowed bool @@ -208,6 +160,12 @@ func TestRecognizers(t *testing.T) { func(b *csrBuilder) { b.usages = append(b.usages, capi.UsageServerAuth) }, + func(b *csrBuilder) { + b.signerName = "example.com/not-correct" + }, + func(b *csrBuilder) { + b.signerName = capi.KubeletServingSignerName + }, } testRecognizer(t, badCases, isNodeClientCert, false) @@ -230,9 +188,10 @@ func TestRecognizers(t *testing.T) { func testRecognizer(t *testing.T, cases []func(b *csrBuilder), recognizeFunc func(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool, shouldRecognize bool) { for _, c := range cases { b := csrBuilder{ - cn: "system:node:foo", - orgs: []string{"system:nodes"}, - requestor: "system:node:foo", + signerName: capi.KubeAPIServerClientKubeletSignerName, + cn: "system:node:foo", + orgs: []string{"system:nodes"}, + requestor: "system:node:foo", usages: []capi.KeyUsage{ capi.UsageKeyEncipherment, capi.UsageDigitalSignature, @@ -262,13 +221,14 @@ func makeTestCsr() *capi.CertificateSigningRequest { } type csrBuilder struct { - cn string - orgs []string - requestor string - usages []capi.KeyUsage - dns []string - emails []string - ips []net.IP + cn string + orgs []string + requestor string + usages []capi.KeyUsage + dns []string + emails []string + ips []net.IP + signerName string } func makeFancyTestCsr(b csrBuilder) *capi.CertificateSigningRequest { @@ -290,9 +250,10 @@ func makeFancyTestCsr(b csrBuilder) *capi.CertificateSigningRequest { } return &capi.CertificateSigningRequest{ Spec: capi.CertificateSigningRequestSpec{ - Username: b.requestor, - Usages: b.usages, - Request: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrb}), + Username: b.requestor, + Usages: b.usages, + SignerName: &b.signerName, + Request: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrb}), }, } } diff --git a/pkg/controller/certificates/certificate_controller.go b/pkg/controller/certificates/certificate_controller.go index f1d2f8ac08d..11e5488683a 100644 --- a/pkg/controller/certificates/certificate_controller.go +++ b/pkg/controller/certificates/certificate_controller.go @@ -36,6 +36,7 @@ import ( "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" "k8s.io/klog" + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" "k8s.io/kubernetes/pkg/controller" ) @@ -192,7 +193,15 @@ func (cc *CertificateController) syncFunc(key string) error { // need to operate on a copy so we don't mutate the csr in the shared cache csr = csr.DeepCopy() - + // If the `signerName` field is not set, we are talking to a pre-1.18 apiserver. + // As per the KEP document for the certificates API, this will be defaulted here + // in the controller to maintain backwards compatibility. + // This should be removed after a deprecation window has passed. + // Default here to allow handlers to assume the field is set. + if csr.Spec.SignerName == nil { + signerName := capihelper.DefaultSignerNameFromSpec(&csr.Spec) + csr.Spec.SignerName = &signerName + } return cc.handler(csr) } diff --git a/pkg/controller/certificates/signer/BUILD b/pkg/controller/certificates/signer/BUILD index d86feab5ebd..c66c4024264 100644 --- a/pkg/controller/certificates/signer/BUILD +++ b/pkg/controller/certificates/signer/BUILD @@ -16,9 +16,12 @@ go_test( ], embed = [":go_default_library"], deps = [ + "//pkg/apis/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", + "//staging/src/k8s.io/client-go/testing:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", "//vendor/github.com/google/go-cmp/cmp:go_default_library", ], diff --git a/pkg/controller/certificates/signer/signer.go b/pkg/controller/certificates/signer/signer.go index da9e8e35760..836119bd8ea 100644 --- a/pkg/controller/certificates/signer/signer.go +++ b/pkg/controller/certificates/signer/signer.go @@ -19,8 +19,10 @@ package signer import ( "context" + "crypto/x509" "encoding/pem" "fmt" + "strings" "time" capi "k8s.io/api/certificates/v1beta1" @@ -89,13 +91,31 @@ func newSigner(caFile, caKeyFile string, client clientset.Interface, certificate } func (s *signer) handle(csr *capi.CertificateSigningRequest) error { + // Ignore unapproved requests if !certificates.IsCertificateRequestApproved(csr) { return nil } - csr, err := s.sign(csr) + + // Fast-path to avoid any additional processing if the CSRs signerName does + // not have a 'kubernetes.io/' prefix. + if !strings.HasPrefix(*csr.Spec.SignerName, "kubernetes.io/") { + return nil + } + + x509cr, err := capihelper.ParseCSR(csr.Spec.Request) + if err != nil { + return fmt.Errorf("unable to parse csr %q: %v", csr.Name, err) + } + if !requestValidForSignerName(x509cr, csr.Spec.Usages, *csr.Spec.SignerName) { + // TODO: mark the CertificateRequest as being in a terminal state and + // communicate to the user why the request has been refused. + return nil + } + cert, err := s.sign(x509cr, csr.Spec.Usages) if err != nil { return fmt.Errorf("error auto signing csr: %v", err) } + csr.Status.Certificate = cert _, err = s.client.CertificatesV1beta1().CertificateSigningRequests().UpdateStatus(context.TODO(), csr, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("error updating signature for csr: %v", err) @@ -103,23 +123,50 @@ func (s *signer) handle(csr *capi.CertificateSigningRequest) error { return nil } -func (s *signer) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSigningRequest, error) { - x509cr, err := capihelper.ParseCSR(csr.Spec.Request) - if err != nil { - return nil, fmt.Errorf("unable to parse csr %q: %v", csr.Name, err) - } - +func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage) ([]byte, error) { currCA, err := s.caProvider.currentCA() if err != nil { return nil, err } der, err := currCA.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{ TTL: s.certTTL, - Usages: csr.Spec.Usages, + Usages: usages, }) if err != nil { return nil, err } - csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) - return csr, nil + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), nil +} + +func requestValidForSignerName(req *x509.CertificateRequest, usages []capi.KeyUsage, signerName string) bool { + // Only handle CSRs with the specific known signerNames. + switch signerName { + case capi.KubeletServingSignerName: + return capihelper.IsKubeletServingCSR(req, usages) + case capi.KubeAPIServerClientKubeletSignerName: + return capihelper.IsKubeletClientCSR(req, usages) + case capi.KubeAPIServerClientSignerName: + return validAPIServerClientUsages(usages) + case capi.LegacyUnknownSignerName: + // No restrictions are applied to the legacy-unknown signerName to + // maintain backward compatibility in v1beta1. + return true + default: + return false + } +} + +func validAPIServerClientUsages(usages []capi.KeyUsage) bool { + hasClientAuth := false + for _, u := range usages { + switch u { + // these usages are optional + case capi.UsageDigitalSignature, capi.UsageKeyEncipherment: + case capi.UsageClientAuth: + hasClientAuth = true + default: + return false + } + } + return hasClientAuth } diff --git a/pkg/controller/certificates/signer/signer_test.go b/pkg/controller/certificates/signer/signer_test.go index 1a91f22f9d7..802e7ade4c6 100644 --- a/pkg/controller/certificates/signer/signer_test.go +++ b/pkg/controller/certificates/signer/signer_test.go @@ -17,9 +17,13 @@ limitations under the License. package signer import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/x509" "crypto/x509/pkix" + "encoding/pem" "io/ioutil" + "math/rand" "testing" "time" @@ -28,7 +32,11 @@ import ( capi "k8s.io/api/certificates/v1beta1" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/client-go/kubernetes/fake" + testclient "k8s.io/client-go/testing" "k8s.io/client-go/util/cert" + + capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1" ) func TestSigner(t *testing.T) { @@ -50,24 +58,20 @@ func TestSigner(t *testing.T) { if err != nil { t.Fatalf("failed to read CSR: %v", err) } - - csr := &capi.CertificateSigningRequest{ - Spec: capi.CertificateSigningRequestSpec{ - Request: []byte(csrb), - Usages: []capi.KeyUsage{ - capi.UsageSigning, - capi.UsageKeyEncipherment, - capi.UsageServerAuth, - capi.UsageClientAuth, - }, - }, + x509cr, err := capihelper.ParseCSR(csrb) + if err != nil { + t.Fatalf("failed to parse CSR: %v", err) } - csr, err = s.sign(csr) + certData, err := s.sign(x509cr, []capi.KeyUsage{ + capi.UsageSigning, + capi.UsageKeyEncipherment, + capi.UsageServerAuth, + capi.UsageClientAuth, + }) if err != nil { t.Fatalf("failed to sign CSR: %v", err) } - certData := csr.Status.Certificate if len(certData) == 0 { t.Fatalf("expected a certificate after signing") } @@ -99,3 +103,207 @@ func TestSigner(t *testing.T) { t.Errorf("unexpected diff: %v", cmp.Diff(certs[0], want, diff.IgnoreUnset())) } } + +func TestHandle(t *testing.T) { + cases := []struct { + name string + // parameters to be set on the generated CSR + commonName string + dnsNames []string + org []string + usages []capi.KeyUsage + // whether the generated CSR should be marked as approved + approved bool + // the signerName to be set on the generated CSR + signerName string + // if true, expect an error to be returned + err bool + // additional verification function + verify func(*testing.T, []testclient.Action) + }{ + { + name: "should sign if signerName is kubernetes.io/kube-apiserver-client", + signerName: "kubernetes.io/kube-apiserver-client", + commonName: "hello-world", + org: []string{"some-org"}, + usages: []capi.KeyUsage{capi.UsageClientAuth, capi.UsageDigitalSignature, capi.UsageKeyEncipherment}, + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 1 { + t.Errorf("expected one Update action but got %d", len(as)) + return + } + csr := as[0].(testclient.UpdateAction).GetObject().(*capi.CertificateSigningRequest) + if len(csr.Status.Certificate) == 0 { + t.Errorf("expected certificate to be issued but it was not") + } + }, + }, + { + name: "should refuse to sign if signerName is kubernetes.io/kube-apiserver-client and contains an unexpected usage", + signerName: "kubernetes.io/kube-apiserver-client", + commonName: "hello-world", + org: []string{"some-org"}, + usages: []capi.KeyUsage{capi.UsageServerAuth, capi.UsageClientAuth, capi.UsageDigitalSignature, capi.UsageKeyEncipherment}, + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no Update action but got %d", len(as)) + return + } + }, + }, + { + name: "should sign if signerName is kubernetes.io/kube-apiserver-client-kubelet", + signerName: "kubernetes.io/kube-apiserver-client-kubelet", + commonName: "system:node:hello-world", + org: []string{"system:nodes"}, + usages: []capi.KeyUsage{capi.UsageClientAuth, capi.UsageDigitalSignature, capi.UsageKeyEncipherment}, + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 1 { + t.Errorf("expected one Update action but got %d", len(as)) + return + } + csr := as[0].(testclient.UpdateAction).GetObject().(*capi.CertificateSigningRequest) + if len(csr.Status.Certificate) == 0 { + t.Errorf("expected certificate to be issued but it was not") + } + }, + }, + { + name: "should sign if signerName is kubernetes.io/legacy-unknown", + signerName: "kubernetes.io/legacy-unknown", + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 1 { + t.Errorf("expected one Update action but got %d", len(as)) + return + } + csr := as[0].(testclient.UpdateAction).GetObject().(*capi.CertificateSigningRequest) + if len(csr.Status.Certificate) == 0 { + t.Errorf("expected certificate to be issued but it was not") + } + }, + }, + { + name: "should sign if signerName is kubernetes.io/kubelet-serving", + signerName: "kubernetes.io/kubelet-serving", + commonName: "system:node:testnode", + org: []string{"system:nodes"}, + usages: []capi.KeyUsage{capi.UsageServerAuth, capi.UsageDigitalSignature, capi.UsageKeyEncipherment}, + dnsNames: []string{"example.com"}, + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 1 { + t.Errorf("expected one Update action but got %d", len(as)) + return + } + csr := as[0].(testclient.UpdateAction).GetObject().(*capi.CertificateSigningRequest) + if len(csr.Status.Certificate) == 0 { + t.Errorf("expected certificate to be issued but it was not") + } + }, + }, + { + name: "should do nothing if an unrecognised signerName is used", + signerName: "kubernetes.io/not-recognised", + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + { + name: "should do nothing if not approved", + signerName: "kubernetes.io/kubelet-serving", + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + { + name: "should do nothing if signerName does not start with kubernetes.io", + signerName: "example.com/sample-name", + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + { + name: "should do nothing if signerName starts with kubernetes.io but is unrecognised", + signerName: "kubernetes.io/not-a-real-signer", + approved: true, + verify: func(t *testing.T, as []testclient.Action) { + if len(as) != 0 { + t.Errorf("expected no action to be taken") + } + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + client := &fake.Clientset{} + s, err := newSigner("./testdata/ca.crt", "./testdata/ca.key", client, 1*time.Hour) + if err != nil { + t.Fatalf("failed to create signer: %v", err) + } + + csr := makeTestCSR(csrBuilder{cn: c.commonName, signerName: c.signerName, approved: c.approved, usages: c.usages, org: c.org, dnsNames: c.dnsNames}) + if err := s.handle(csr); err != nil && !c.err { + t.Errorf("unexpected err: %v", err) + } + c.verify(t, client.Actions()) + }) + } +} + +// noncryptographic for faster testing +// DO NOT COPY THIS CODE +var insecureRand = rand.New(rand.NewSource(0)) + +type csrBuilder struct { + cn string + dnsNames []string + org []string + signerName string + approved bool + usages []capi.KeyUsage +} + +func makeTestCSR(b csrBuilder) *capi.CertificateSigningRequest { + pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand) + if err != nil { + panic(err) + } + csrb, err := x509.CreateCertificateRequest(insecureRand, &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: b.cn, + Organization: b.org, + }, + DNSNames: b.dnsNames, + }, pk) + if err != nil { + panic(err) + } + csr := &capi.CertificateSigningRequest{ + Spec: capi.CertificateSigningRequestSpec{ + Request: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrb}), + Usages: b.usages, + }, + } + if b.signerName != "" { + csr.Spec.SignerName = &b.signerName + } + if b.approved { + csr.Status.Conditions = append(csr.Status.Conditions, capi.CertificateSigningRequestCondition{ + Type: capi.CertificateApproved, + }) + } + return csr +} diff --git a/pkg/kubelet/certificate/bootstrap/bootstrap.go b/pkg/kubelet/certificate/bootstrap/bootstrap.go index 1d0bc9cc0cd..dda60479d95 100644 --- a/pkg/kubelet/certificate/bootstrap/bootstrap.go +++ b/pkg/kubelet/certificate/bootstrap/bootstrap.go @@ -344,7 +344,7 @@ func requestNodeCertificate(client certificatesv1beta1.CertificateSigningRequest return nil, err } - req, err := csr.RequestCertificate(client, csrData, name, usages, privateKey) + req, err := csr.RequestCertificate(client, csrData, name, certificates.KubeAPIServerClientKubeletSignerName, usages, privateKey) if err != nil { return nil, err } diff --git a/pkg/kubelet/certificate/kubelet.go b/pkg/kubelet/certificate/kubelet.go index 667285774fd..8c3b26523cd 100644 --- a/pkg/kubelet/certificate/kubelet.go +++ b/pkg/kubelet/certificate/kubelet.go @@ -107,6 +107,7 @@ func NewKubeletServerCertificateManager(kubeClient clientset.Interface, kubeCfg return certSigningRequestClient, nil }, GetTemplate: getTemplate, + SignerName: certificates.KubeletServingSignerName, Usages: []certificates.KeyUsage{ // https://tools.ietf.org/html/rfc5280#section-4.2.1.3 // @@ -238,6 +239,7 @@ func NewKubeletClientCertificateManager( Organization: []string{"system:nodes"}, }, }, + SignerName: certificates.KubeAPIServerClientKubeletSignerName, Usages: []certificates.KeyUsage{ // https://tools.ietf.org/html/rfc5280#section-4.2.1.3 // diff --git a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go index 1af46abaa4e..48eb7b3ccb0 100644 --- a/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go +++ b/staging/src/k8s.io/client-go/util/certificate/certificate_manager.go @@ -85,6 +85,9 @@ type Config struct { // If no template is available, nil may be returned, and no certificate will be requested. // If specified, takes precedence over Template. GetTemplate func() *x509.CertificateRequest + // SignerName is the name of the certificate signer that should sign certificates + // generated by the manager. + SignerName string // Usages is the types of usages that certificates generated by the manager // can be used for. Usages []certificates.KeyUsage @@ -174,6 +177,7 @@ type manager struct { lastRequest *x509.CertificateRequest dynamicTemplate bool + signerName string usages []certificates.KeyUsage forceRotation bool @@ -219,6 +223,7 @@ func NewManager(config *Config) (Manager, error) { clientFn: config.ClientFn, getTemplate: getTemplate, dynamicTemplate: config.GetTemplate != nil, + signerName: config.SignerName, usages: config.Usages, certStore: config.CertificateStore, cert: cert, @@ -424,7 +429,7 @@ func (m *manager) rotateCerts() (bool, error) { // Call the Certificate Signing Request API to get a certificate for the // new private key. - req, err := csr.RequestCertificate(client, csrPEM, "", m.usages, privateKey) + req, err := csr.RequestCertificate(client, csrPEM, "", m.signerName, m.usages, privateKey) if err != nil { utilruntime.HandleError(fmt.Errorf("Failed while requesting a signed certificate from the master: %v", err)) if m.certificateRenewFailure != nil { diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go index 19f42238fe5..c5766d2421b 100644 --- a/staging/src/k8s.io/client-go/util/certificate/csr/csr.go +++ b/staging/src/k8s.io/client-go/util/certificate/csr/csr.go @@ -46,7 +46,7 @@ import ( // status, once approved by API server, it will return the API server's issued // certificate (pem-encoded). If there is any errors, or the watch timeouts, it // will return an error. -func RequestCertificate(client certificatesclient.CertificateSigningRequestInterface, csrData []byte, name string, usages []certificates.KeyUsage, privateKey interface{}) (req *certificates.CertificateSigningRequest, err error) { +func RequestCertificate(client certificatesclient.CertificateSigningRequestInterface, csrData []byte, name string, signerName string, usages []certificates.KeyUsage, privateKey interface{}) (req *certificates.CertificateSigningRequest, err error) { csr := &certificates.CertificateSigningRequest{ // Username, UID, Groups will be injected by API server. TypeMeta: metav1.TypeMeta{Kind: "CertificateSigningRequest"}, @@ -54,8 +54,9 @@ func RequestCertificate(client certificatesclient.CertificateSigningRequestInter Name: name, }, Spec: certificates.CertificateSigningRequestSpec{ - Request: csrData, - Usages: usages, + Request: csrData, + Usages: usages, + SignerName: &signerName, }, } if len(csr.Name) == 0 { diff --git a/test/integration/certificates/BUILD b/test/integration/certificates/BUILD index 3af1dccc9db..edcc9dc8680 100644 --- a/test/integration/certificates/BUILD +++ b/test/integration/certificates/BUILD @@ -7,6 +7,7 @@ go_test( "admission_sign_test.go", "admission_subjectrestriction_test.go", "admission_test.go", + "controller_approval_test.go", "defaulting_test.go", "field_selector_test.go", "main_test.go", @@ -14,12 +15,15 @@ go_test( tags = ["integration"], deps = [ "//cmd/kube-apiserver/app/testing:go_default_library", + "//pkg/controller/certificates:go_default_library", + "//pkg/controller/certificates/approver:go_default_library", "//staging/src/k8s.io/api/authorization/v1:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/api/rbac/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/authorization/v1:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/certificates/v1beta1:go_default_library", diff --git a/test/integration/certificates/controller_approval_test.go b/test/integration/certificates/controller_approval_test.go new file mode 100644 index 00000000000..bb52a05a415 --- /dev/null +++ b/test/integration/certificates/controller_approval_test.go @@ -0,0 +1,220 @@ +/* +Copyright 2020 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 certificates + +import ( + "context" + "crypto/x509" + "crypto/x509/pkix" + "fmt" + "testing" + "time" + + certv1beta1 "k8s.io/api/certificates/v1beta1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" + clientset "k8s.io/client-go/kubernetes" + restclient "k8s.io/client-go/rest" + + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + "k8s.io/kubernetes/pkg/controller/certificates" + "k8s.io/kubernetes/pkg/controller/certificates/approver" + "k8s.io/kubernetes/test/integration/framework" +) + +// Integration tests that verify the behaviour of the CSR auto-approving controller. +func TestController_AutoApproval(t *testing.T) { + validKubeAPIServerClientKubeletUsername := "system:node:abc" + validKubeAPIServerClientKubeletCSR := pemWithTemplate(&x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: validKubeAPIServerClientKubeletUsername, + Organization: []string{"system:nodes"}, + }, + }) + validKubeAPIServerClientKubeletUsages := []certv1beta1.KeyUsage{ + certv1beta1.UsageDigitalSignature, + certv1beta1.UsageKeyEncipherment, + certv1beta1.UsageClientAuth, + } + tests := map[string]struct { + signerName string + request []byte + usages []certv1beta1.KeyUsage + username string + autoApproved bool + grantNodeClient bool + grantSelfNodeClient bool + }{ + "should auto-approve CSR that has kube-apiserver-client-kubelet signerName and matches requirements": { + signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, + request: validKubeAPIServerClientKubeletCSR, + usages: validKubeAPIServerClientKubeletUsages, + username: validKubeAPIServerClientKubeletUsername, + grantSelfNodeClient: true, + autoApproved: true, + }, + "should auto-approve CSR that has kube-apiserver-client-kubelet signerName and matches requirements despite missing username if nodeclient permissions are granted": { + signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, + request: validKubeAPIServerClientKubeletCSR, + usages: validKubeAPIServerClientKubeletUsages, + username: "does-not-match-cn", + grantNodeClient: true, + autoApproved: true, + }, + "should not auto-approve CSR that has kube-apiserver-client-kubelet signerName that does not match requirements": { + signerName: certv1beta1.KubeAPIServerClientKubeletSignerName, + request: pemWithGroup("system:notnodes"), + autoApproved: false, + }, + "should not auto-approve CSR that has kube-apiserver-client signerName that DOES match kubelet CSR requirements": { + signerName: certv1beta1.KubeAPIServerClientSignerName, + request: validKubeAPIServerClientKubeletCSR, + usages: validKubeAPIServerClientKubeletUsages, + username: validKubeAPIServerClientKubeletUsername, + autoApproved: false, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // Run an apiserver with the default configuration options. + s := kubeapiservertesting.StartTestServerOrDie(t, kubeapiservertesting.NewDefaultTestServerOptions(), []string{""}, framework.SharedEtcd()) + defer s.TearDownFn() + client := clientset.NewForConfigOrDie(s.ClientConfig) + informers := informers.NewSharedInformerFactory(clientset.NewForConfigOrDie(restclient.AddUserAgent(s.ClientConfig, "certificatesigningrequest-informers")), time.Second) + + // Register the controller + c := approver.NewCSRApprovingController(client, informers.Certificates().V1beta1().CertificateSigningRequests()) + // Start the controller & informers + stopCh := make(chan struct{}) + defer close(stopCh) + informers.Start(stopCh) + go c.Run(1, stopCh) + + // Configure appropriate permissions + if test.grantNodeClient { + grantUserNodeClientPermissions(t, client, test.username, false) + } + if test.grantSelfNodeClient { + grantUserNodeClientPermissions(t, client, test.username, true) + } + + // Use a client that impersonates the test case 'username' to ensure the `spec.username` + // field on the CSR is set correctly. + impersonationConfig := restclient.CopyConfig(s.ClientConfig) + impersonationConfig.Impersonate.UserName = test.username + impersonationClient, err := clientset.NewForConfig(impersonationConfig) + if err != nil { + t.Fatalf("Error in create clientset: %v", err) + } + csr := &certv1beta1.CertificateSigningRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: "csr", + }, + Spec: certv1beta1.CertificateSigningRequestSpec{ + Request: test.request, + Usages: test.usages, + SignerName: &test.signerName, + }, + } + _, err = impersonationClient.CertificatesV1beta1().CertificateSigningRequests().Create(context.TODO(), csr, metav1.CreateOptions{}) + if err != nil { + t.Fatalf("failed to create testing CSR: %v", err) + } + + if test.autoApproved { + if err := waitForCertificateRequestApproved(client, csr.Name); err != nil { + t.Errorf("failed to wait for CSR to be auto-approved: %v", err) + } + } else { + if err := ensureCertificateRequestNotApproved(client, csr.Name); err != nil { + t.Errorf("failed to ensure that CSR was not auto-approved: %v", err) + } + } + }) + } +} + +const ( + interval = 100 * time.Millisecond + timeout = 5 * time.Second +) + +func waitForCertificateRequestApproved(client kubernetes.Interface, name string) error { + if err := wait.Poll(interval, timeout, func() (bool, error) { + csr, err := client.CertificatesV1beta1().CertificateSigningRequests().Get(context.TODO(), name, metav1.GetOptions{}) + if err != nil { + return false, err + } + if certificates.IsCertificateRequestApproved(csr) { + return true, nil + } + return false, nil + }); err != nil { + return err + } + return nil +} + +func ensureCertificateRequestNotApproved(client kubernetes.Interface, name string) error { + // If waiting for the CSR to be approved times out, we class this as 'not auto approved'. + // There is currently no way to explicitly check if the CSR has been rejected for auto-approval. + err := waitForCertificateRequestApproved(client, name) + switch { + case err == wait.ErrWaitTimeout: + return nil + case err == nil: + return fmt.Errorf("CertificateSigningRequest was auto-approved") + default: + return err + } +} + +func grantUserNodeClientPermissions(t *testing.T, client clientset.Interface, username string, selfNodeClient bool) { + resourceType := "certificatesigningrequests/nodeclient" + if selfNodeClient { + resourceType = "certificatesigningrequests/selfnodeclient" + } + cr := buildNodeClientRoleForUser("role", resourceType) + crb := buildClusterRoleBindingForUser("rolebinding", username, cr.Name) + if _, err := client.RbacV1().ClusterRoles().Create(context.TODO(), cr, metav1.CreateOptions{}); err != nil { + t.Fatalf("failed to create test fixtures: %v", err) + } + if _, err := client.RbacV1().ClusterRoleBindings().Create(context.TODO(), crb, metav1.CreateOptions{}); err != nil { + t.Fatalf("failed to create test fixtures: %v", err) + } + rule := cr.Rules[0] + waitForNamedAuthorizationUpdate(t, client.AuthorizationV1(), username, "", rule.Verbs[0], "", schema.GroupResource{Group: rule.APIGroups[0], Resource: rule.Resources[0]}, true) +} + +func buildNodeClientRoleForUser(name string, resourceType string) *rbacv1.ClusterRole { + return &rbacv1.ClusterRole{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Rules: []rbacv1.PolicyRule{ + { + Verbs: []string{"create"}, + APIGroups: []string{certv1beta1.SchemeGroupVersion.Group}, + Resources: []string{resourceType}, + }, + }, + } +}