Skip to content

Commit d5dae04

Browse files
author
James Munnelly
committed
certificates: update controllers to understand signerName field
Signed-off-by: James Munnelly <[email protected]>
1 parent d7e10f9 commit d5dae04

File tree

13 files changed

+555
-128
lines changed

13 files changed

+555
-128
lines changed

pkg/controller/certificates/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
importpath = "k8s.io/kubernetes/pkg/controller/certificates",
1414
visibility = ["//visibility:public"],
1515
deps = [
16+
"//pkg/apis/certificates/v1beta1:go_default_library",
1617
"//pkg/controller:go_default_library",
1718
"//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
1819
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",

pkg/controller/certificates/approver/sarapprove.go

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@ import (
2121
"context"
2222
"crypto/x509"
2323
"fmt"
24-
"reflect"
25-
"strings"
2624

2725
authorization "k8s.io/api/authorization/v1"
2826
capi "k8s.io/api/certificates/v1beta1"
2927
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3028
certificatesinformers "k8s.io/client-go/informers/certificates/v1beta1"
3129
clientset "k8s.io/client-go/kubernetes"
30+
3231
capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
3332
"k8s.io/kubernetes/pkg/controller/certificates"
3433
)
@@ -146,45 +145,12 @@ func appendApprovalCondition(csr *capi.CertificateSigningRequest, message string
146145
})
147146
}
148147

149-
func hasExactUsages(csr *capi.CertificateSigningRequest, usages []capi.KeyUsage) bool {
150-
if len(usages) != len(csr.Spec.Usages) {
151-
return false
152-
}
153-
154-
usageMap := map[capi.KeyUsage]struct{}{}
155-
for _, u := range usages {
156-
usageMap[u] = struct{}{}
157-
}
158-
159-
for _, u := range csr.Spec.Usages {
160-
if _, ok := usageMap[u]; !ok {
161-
return false
162-
}
163-
}
164-
165-
return true
166-
}
167-
168-
var kubeletClientUsages = []capi.KeyUsage{
169-
capi.UsageKeyEncipherment,
170-
capi.UsageDigitalSignature,
171-
capi.UsageClientAuth,
172-
}
173-
174148
func isNodeClientCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool {
175-
if !reflect.DeepEqual([]string{"system:nodes"}, x509cr.Subject.Organization) {
176-
return false
177-
}
178-
if len(x509cr.DNSNames) > 0 || len(x509cr.EmailAddresses) > 0 || len(x509cr.IPAddresses) > 0 || len(x509cr.URIs) > 0 {
149+
isClientCSR := capihelper.IsKubeletClientCSR(x509cr, csr.Spec.Usages)
150+
if !isClientCSR {
179151
return false
180152
}
181-
if !hasExactUsages(csr, kubeletClientUsages) {
182-
return false
183-
}
184-
if !strings.HasPrefix(x509cr.Subject.CommonName, "system:node:") {
185-
return false
186-
}
187-
return true
153+
return *csr.Spec.SignerName == capi.KubeAPIServerClientKubeletSignerName
188154
}
189155

190156
func isSelfNodeClientCert(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool {

pkg/controller/certificates/approver/sarapprove_test.go

Lines changed: 22 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -36,54 +36,6 @@ import (
3636
k8s_certificates_v1beta1 "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
3737
)
3838

39-
func TestHasKubeletUsages(t *testing.T) {
40-
cases := []struct {
41-
usages []capi.KeyUsage
42-
expected bool
43-
}{
44-
{
45-
usages: nil,
46-
expected: false,
47-
},
48-
{
49-
usages: []capi.KeyUsage{},
50-
expected: false,
51-
},
52-
{
53-
usages: []capi.KeyUsage{
54-
capi.UsageKeyEncipherment,
55-
capi.UsageDigitalSignature,
56-
},
57-
expected: false,
58-
},
59-
{
60-
usages: []capi.KeyUsage{
61-
capi.UsageKeyEncipherment,
62-
capi.UsageDigitalSignature,
63-
capi.UsageServerAuth,
64-
},
65-
expected: false,
66-
},
67-
{
68-
usages: []capi.KeyUsage{
69-
capi.UsageKeyEncipherment,
70-
capi.UsageDigitalSignature,
71-
capi.UsageClientAuth,
72-
},
73-
expected: true,
74-
},
75-
}
76-
for _, c := range cases {
77-
if hasExactUsages(&capi.CertificateSigningRequest{
78-
Spec: capi.CertificateSigningRequestSpec{
79-
Usages: c.usages,
80-
},
81-
}, kubeletClientUsages) != c.expected {
82-
t.Errorf("unexpected result of hasKubeletUsages(%v), expecting: %v", c.usages, c.expected)
83-
}
84-
}
85-
}
86-
8739
func TestHandle(t *testing.T) {
8840
cases := []struct {
8941
allowed bool
@@ -208,6 +160,12 @@ func TestRecognizers(t *testing.T) {
208160
func(b *csrBuilder) {
209161
b.usages = append(b.usages, capi.UsageServerAuth)
210162
},
163+
func(b *csrBuilder) {
164+
b.signerName = "example.com/not-correct"
165+
},
166+
func(b *csrBuilder) {
167+
b.signerName = capi.KubeletServingSignerName
168+
},
211169
}
212170

213171
testRecognizer(t, badCases, isNodeClientCert, false)
@@ -230,9 +188,10 @@ func TestRecognizers(t *testing.T) {
230188
func testRecognizer(t *testing.T, cases []func(b *csrBuilder), recognizeFunc func(csr *capi.CertificateSigningRequest, x509cr *x509.CertificateRequest) bool, shouldRecognize bool) {
231189
for _, c := range cases {
232190
b := csrBuilder{
233-
cn: "system:node:foo",
234-
orgs: []string{"system:nodes"},
235-
requestor: "system:node:foo",
191+
signerName: capi.KubeAPIServerClientKubeletSignerName,
192+
cn: "system:node:foo",
193+
orgs: []string{"system:nodes"},
194+
requestor: "system:node:foo",
236195
usages: []capi.KeyUsage{
237196
capi.UsageKeyEncipherment,
238197
capi.UsageDigitalSignature,
@@ -262,13 +221,14 @@ func makeTestCsr() *capi.CertificateSigningRequest {
262221
}
263222

264223
type csrBuilder struct {
265-
cn string
266-
orgs []string
267-
requestor string
268-
usages []capi.KeyUsage
269-
dns []string
270-
emails []string
271-
ips []net.IP
224+
cn string
225+
orgs []string
226+
requestor string
227+
usages []capi.KeyUsage
228+
dns []string
229+
emails []string
230+
ips []net.IP
231+
signerName string
272232
}
273233

274234
func makeFancyTestCsr(b csrBuilder) *capi.CertificateSigningRequest {
@@ -290,9 +250,10 @@ func makeFancyTestCsr(b csrBuilder) *capi.CertificateSigningRequest {
290250
}
291251
return &capi.CertificateSigningRequest{
292252
Spec: capi.CertificateSigningRequestSpec{
293-
Username: b.requestor,
294-
Usages: b.usages,
295-
Request: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrb}),
253+
Username: b.requestor,
254+
Usages: b.usages,
255+
SignerName: &b.signerName,
256+
Request: pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrb}),
296257
},
297258
}
298259
}

pkg/controller/certificates/certificate_controller.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"k8s.io/client-go/tools/record"
3737
"k8s.io/client-go/util/workqueue"
3838
"k8s.io/klog"
39+
capihelper "k8s.io/kubernetes/pkg/apis/certificates/v1beta1"
3940
"k8s.io/kubernetes/pkg/controller"
4041
)
4142

@@ -192,7 +193,15 @@ func (cc *CertificateController) syncFunc(key string) error {
192193

193194
// need to operate on a copy so we don't mutate the csr in the shared cache
194195
csr = csr.DeepCopy()
195-
196+
// If the `signerName` field is not set, we are talking to a pre-1.18 apiserver.
197+
// As per the KEP document for the certificates API, this will be defaulted here
198+
// in the controller to maintain backwards compatibility.
199+
// This should be removed after a deprecation window has passed.
200+
// Default here to allow handlers to assume the field is set.
201+
if csr.Spec.SignerName == nil {
202+
signerName := capihelper.DefaultSignerNameFromSpec(&csr.Spec)
203+
csr.Spec.SignerName = &signerName
204+
}
196205
return cc.handler(csr)
197206
}
198207

pkg/controller/certificates/signer/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@ go_test(
1616
],
1717
embed = [":go_default_library"],
1818
deps = [
19+
"//pkg/apis/certificates/v1beta1:go_default_library",
1920
"//staging/src/k8s.io/api/certificates/v1beta1:go_default_library",
2021
"//staging/src/k8s.io/apimachinery/pkg/util/clock:go_default_library",
2122
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
23+
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
24+
"//staging/src/k8s.io/client-go/testing:go_default_library",
2225
"//staging/src/k8s.io/client-go/util/cert:go_default_library",
2326
"//vendor/github.com/google/go-cmp/cmp:go_default_library",
2427
],

pkg/controller/certificates/signer/signer.go

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ package signer
1919

2020
import (
2121
"context"
22+
"crypto/x509"
2223
"encoding/pem"
2324
"fmt"
25+
"strings"
2426
"time"
2527

2628
capi "k8s.io/api/certificates/v1beta1"
@@ -89,37 +91,82 @@ func newSigner(caFile, caKeyFile string, client clientset.Interface, certificate
8991
}
9092

9193
func (s *signer) handle(csr *capi.CertificateSigningRequest) error {
94+
// Ignore unapproved requests
9295
if !certificates.IsCertificateRequestApproved(csr) {
9396
return nil
9497
}
95-
csr, err := s.sign(csr)
98+
99+
// Fast-path to avoid any additional processing if the CSRs signerName does
100+
// not have a 'kubernetes.io/' prefix.
101+
if !strings.HasPrefix(*csr.Spec.SignerName, "kubernetes.io/") {
102+
return nil
103+
}
104+
105+
x509cr, err := capihelper.ParseCSR(csr.Spec.Request)
106+
if err != nil {
107+
return fmt.Errorf("unable to parse csr %q: %v", csr.Name, err)
108+
}
109+
if !requestValidForSignerName(x509cr, csr.Spec.Usages, *csr.Spec.SignerName) {
110+
// TODO: mark the CertificateRequest as being in a terminal state and
111+
// communicate to the user why the request has been refused.
112+
return nil
113+
}
114+
cert, err := s.sign(x509cr, csr.Spec.Usages)
96115
if err != nil {
97116
return fmt.Errorf("error auto signing csr: %v", err)
98117
}
118+
csr.Status.Certificate = cert
99119
_, err = s.client.CertificatesV1beta1().CertificateSigningRequests().UpdateStatus(context.TODO(), csr, metav1.UpdateOptions{})
100120
if err != nil {
101121
return fmt.Errorf("error updating signature for csr: %v", err)
102122
}
103123
return nil
104124
}
105125

106-
func (s *signer) sign(csr *capi.CertificateSigningRequest) (*capi.CertificateSigningRequest, error) {
107-
x509cr, err := capihelper.ParseCSR(csr.Spec.Request)
108-
if err != nil {
109-
return nil, fmt.Errorf("unable to parse csr %q: %v", csr.Name, err)
110-
}
111-
126+
func (s *signer) sign(x509cr *x509.CertificateRequest, usages []capi.KeyUsage) ([]byte, error) {
112127
currCA, err := s.caProvider.currentCA()
113128
if err != nil {
114129
return nil, err
115130
}
116131
der, err := currCA.Sign(x509cr.Raw, authority.PermissiveSigningPolicy{
117132
TTL: s.certTTL,
118-
Usages: csr.Spec.Usages,
133+
Usages: usages,
119134
})
120135
if err != nil {
121136
return nil, err
122137
}
123-
csr.Status.Certificate = pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
124-
return csr, nil
138+
return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), nil
139+
}
140+
141+
func requestValidForSignerName(req *x509.CertificateRequest, usages []capi.KeyUsage, signerName string) bool {
142+
// Only handle CSRs with the specific known signerNames.
143+
switch signerName {
144+
case capi.KubeletServingSignerName:
145+
return capihelper.IsKubeletServingCSR(req, usages)
146+
case capi.KubeAPIServerClientKubeletSignerName:
147+
return capihelper.IsKubeletClientCSR(req, usages)
148+
case capi.KubeAPIServerClientSignerName:
149+
return validAPIServerClientUsages(usages)
150+
case capi.LegacyUnknownSignerName:
151+
// No restrictions are applied to the legacy-unknown signerName to
152+
// maintain backward compatibility in v1beta1.
153+
return true
154+
default:
155+
return false
156+
}
157+
}
158+
159+
func validAPIServerClientUsages(usages []capi.KeyUsage) bool {
160+
hasClientAuth := false
161+
for _, u := range usages {
162+
switch u {
163+
// these usages are optional
164+
case capi.UsageDigitalSignature, capi.UsageKeyEncipherment:
165+
case capi.UsageClientAuth:
166+
hasClientAuth = true
167+
default:
168+
return false
169+
}
170+
}
171+
return hasClientAuth
125172
}

0 commit comments

Comments
 (0)