diff --git a/pkg/apis/certificates/v1beta1/defaults_test.go b/pkg/apis/certificates/v1beta1/defaults_test.go index 41062082a57..f0bdee3a538 100644 --- a/pkg/apis/certificates/v1beta1/defaults_test.go +++ b/pkg/apis/certificates/v1beta1/defaults_test.go @@ -30,6 +30,141 @@ import ( capi "k8s.io/api/certificates/v1beta1" ) +func TestIsKubeletServingCSR(t *testing.T) { + newCSR := func(base pemOptions, overlays ...pemOptions) *x509.CertificateRequest { + b := csrWithOpts(base, overlays...) + csr, err := ParseCSR(b) + if err != nil { + t.Fatal(err) + } + return csr + } + tests := map[string]struct { + req *x509.CertificateRequest + usages []capi.KeyUsage + exp bool + }{ + "defaults for kubelet-serving": { + req: newCSR(kubeletServerPEMOptions), + usages: kubeletServerUsages, + exp: true, + }, + "does not default to kube-apiserver-client-kubelet if org is not 'system:nodes'": { + req: newCSR(kubeletServerPEMOptions, pemOptions{org: "not-system:nodes"}), + usages: kubeletServerUsages, + exp: false, + }, + "does not default to kubelet-serving if CN does not have system:node: prefix": { + req: newCSR(kubeletServerPEMOptions, pemOptions{cn: "notprefixed"}), + usages: kubeletServerUsages, + exp: false, + }, + "does not default to kubelet-serving if it has an unexpected usage": { + req: newCSR(kubeletServerPEMOptions), + usages: append(kubeletServerUsages, capi.UsageClientAuth), + exp: false, + }, + "does not default to kubelet-serving if it is missing an expected usage": { + req: newCSR(kubeletServerPEMOptions), + usages: kubeletServerUsages[1:], + exp: false, + }, + "does not default to kubelet-serving if it does not specify any dnsNames or ipAddresses": { + req: newCSR(kubeletServerPEMOptions, pemOptions{ipAddresses: []net.IP{}, dnsNames: []string{}}), + usages: kubeletServerUsages[1:], + exp: false, + }, + "does not default to kubelet-serving if it specifies a URI SAN": { + req: newCSR(kubeletServerPEMOptions, pemOptions{uris: []string{"http://something"}}), + usages: kubeletServerUsages, + exp: false, + }, + "does not default to kubelet-serving if it specifies an emailAddress SAN": { + req: newCSR(kubeletServerPEMOptions, pemOptions{emailAddresses: []string{"something"}}), + usages: kubeletServerUsages, + exp: false, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := IsKubeletServingCSR(test.req, test.usages) + if test.exp != got { + t.Errorf("unexpected IsKubeletClientCSR output: exp=%v, got=%v", test.exp, got) + } + }) + } +} + +func TestIsKubeletClientCSR(t *testing.T) { + newCSR := func(base pemOptions, overlays ...pemOptions) *x509.CertificateRequest { + b := csrWithOpts(base, overlays...) + csr, err := ParseCSR(b) + if err != nil { + t.Fatal(err) + } + return csr + } + tests := map[string]struct { + req *x509.CertificateRequest + usages []capi.KeyUsage + exp bool + }{ + "defaults for kube-apiserver-client-kubelet": { + req: newCSR(kubeletClientPEMOptions), + usages: kubeletClientUsages, + exp: true, + }, + "does not default to kube-apiserver-client-kubelet if org is not 'system:nodes'": { + req: newCSR(kubeletClientPEMOptions, pemOptions{org: "not-system:nodes"}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if a dnsName is set": { + req: newCSR(kubeletClientPEMOptions, pemOptions{dnsNames: []string{"something"}}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if an emailAddress is set": { + req: newCSR(kubeletClientPEMOptions, pemOptions{emailAddresses: []string{"something"}}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if a uri SAN is set": { + req: newCSR(kubeletClientPEMOptions, pemOptions{uris: []string{"http://something"}}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if an ipAddress is set": { + req: newCSR(kubeletClientPEMOptions, pemOptions{ipAddresses: []net.IP{{0, 0, 0, 0}}}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if CN does not have 'system:node:' prefix": { + req: newCSR(kubeletClientPEMOptions, pemOptions{cn: "not-prefixed"}), + usages: kubeletClientUsages, + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if it has an unexpected usage": { + req: newCSR(kubeletClientPEMOptions), + usages: append(kubeletClientUsages, capi.UsageServerAuth), + exp: false, + }, + "does not default to kube-apiserver-client-kubelet if it is missing an expected usage": { + req: newCSR(kubeletClientPEMOptions), + usages: kubeletClientUsages[1:], + exp: false, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + got := IsKubeletClientCSR(test.req, test.usages) + if test.exp != got { + t.Errorf("unexpected IsKubeletClientCSR output: exp=%v, got=%v", test.exp, got) + } + }) + } +} + var ( kubeletClientUsages = []capi.KeyUsage{ capi.UsageDigitalSignature, diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/BUILD b/staging/src/k8s.io/client-go/util/certificate/csr/BUILD index eb28433e05b..17fa901ace1 100644 --- a/staging/src/k8s.io/client-go/util/certificate/csr/BUILD +++ b/staging/src/k8s.io/client-go/util/certificate/csr/BUILD @@ -1,6 +1,6 @@ package(default_visibility = ["//visibility:public"]) -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -35,3 +35,13 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["csr_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", + ], +) 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 c5766d2421b..dd41772498e 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 @@ -150,6 +150,9 @@ func ensureCompatible(new, orig *certificates.CertificateSigningRequest, private if !reflect.DeepEqual(newCSR.Subject, origCSR.Subject) { return fmt.Errorf("csr subjects differ: new: %#v, orig: %#v", newCSR.Subject, origCSR.Subject) } + if new.Spec.SignerName != nil && orig.Spec.SignerName != nil && *new.Spec.SignerName != *orig.Spec.SignerName { + return fmt.Errorf("csr signerNames differ: new %q, orig: %q", *new.Spec.SignerName, *orig.Spec.SignerName) + } signer, ok := privateKey.(crypto.Signer) if !ok { return fmt.Errorf("privateKey is not a signer") diff --git a/staging/src/k8s.io/client-go/util/certificate/csr/csr_test.go b/staging/src/k8s.io/client-go/util/certificate/csr/csr_test.go new file mode 100644 index 00000000000..63c2e82764e --- /dev/null +++ b/staging/src/k8s.io/client-go/util/certificate/csr/csr_test.go @@ -0,0 +1,146 @@ +/* +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 csr + +import ( + "crypto" + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "testing" + + certificates "k8s.io/api/certificates/v1beta1" + "k8s.io/utils/pointer" +) + +func TestEnsureCompatible(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + req := pemWithPrivateKey(privateKey) + + tests := map[string]struct { + new, orig *certificates.CertificateSigningRequest + privateKey interface{} + err string + }{ + "nil signerName on 'new' matches any signerName on 'orig'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + privateKey: privateKey, + }, + "nil signerName on 'orig' matches any signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + }, + }, + privateKey: privateKey, + }, + "signerName on 'orig' matches signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + privateKey: privateKey, + }, + "signerName on 'orig' does not match signerName on 'new'": { + new: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/test"), + }, + }, + orig: &certificates.CertificateSigningRequest{ + Spec: certificates.CertificateSigningRequestSpec{ + Request: req, + SignerName: pointer.StringPtr("example.com/not-test"), + }, + }, + privateKey: privateKey, + err: `csr signerNames differ: new "example.com/test", orig: "example.com/not-test"`, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + err := ensureCompatible(test.new, test.orig, test.privateKey) + if err != nil && test.err == "" { + t.Errorf("expected no error, but got: %v", err) + } else if err != nil && test.err != err.Error() { + t.Errorf("error did not match as expected, got=%v, exp=%s", err, test.err) + } + if err == nil && test.err != "" { + t.Errorf("expected to get an error but got none") + } + }) + } +} + +func pemWithPrivateKey(pk crypto.PrivateKey) []byte { + template := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "something", + Organization: []string{"test"}, + }, + } + return pemWithTemplate(template, pk) +} + +func pemWithTemplate(template *x509.CertificateRequest, key crypto.PrivateKey) []byte { + csrDER, err := x509.CreateCertificateRequest(rand.Reader, template, key) + if err != nil { + panic(err) + } + + csrPemBlock := &pem.Block{ + Type: "CERTIFICATE REQUEST", + Bytes: csrDER, + } + + p := pem.EncodeToMemory(csrPemBlock) + if p == nil { + panic("invalid pem block") + } + + return p +}