Skip to content

Commit 2748423

Browse files
author
OpenShift Bot
authored
Merge pull request #11308 from liggitt/x509-intermediate-1.3.1
Merged by openshift-bot
2 parents 6827f5b + 5ec3a07 commit 2748423

24 files changed

+381
-39
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBpTCCAUugAwIBAgIUPV4LAC5KK8YWY1FegyTuhkGUr3EwCgYIKoZIzj0EAwIw
3+
GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMB4XDTkwMTIzMTIzNTkwMFoXDTkw
4+
MTIzMTIzNTkwMFowFDESMBAGA1UEAxMJTXkgQ2xpZW50MFkwEwYHKoZIzj0CAQYI
5+
KoZIzj0DAQcDQgAEyYUnseNUN87rfHgekrfZu5sj4wlt5LYr3JYZZkfSbsb+BW3/
6+
RzX02ifjp+8w7mI4qUGg6y6J7oXHGFT3uj9kj6N1MHMwDgYDVR0PAQH/BAQDAgWg
7+
MBMGA1UdJQQMMAoGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFKsX
8+
EnXwDg8j2LIEM1QzmFrE6537MB8GA1UdIwQYMBaAFF+p0JcY31pz+mjNZnjv0Gum
9+
92vZMAoGCCqGSM49BAMCA0gAMEUCIG4FBcb57oqOCoaFiJ+Yx6S0zkaash7bTv3V
10+
CIy9JvFdAiEAy8bf2S9EkvZyURZ6ycgEMnekll57Ebze6rjlPx8+B1Y=
11+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
3+
GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
4+
MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
5+
BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
6+
KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
7+
BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
8+
K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
9+
a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
10+
MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
11+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
{
2+
"signing": {
3+
"profiles": {
4+
"valid": {
5+
"expiry": "876000h",
6+
"usages": [
7+
"signing",
8+
"key encipherment",
9+
"client auth"
10+
]
11+
},
12+
"expired": {
13+
"expiry": "1h",
14+
"not_before": "1990-12-31T23:59:00Z",
15+
"not_after": "1990-12-31T23:59:00Z",
16+
"usages": [
17+
"signing",
18+
"key encipherment",
19+
"client auth"
20+
]
21+
}
22+
}
23+
}
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"CN": "My Client"
3+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#!/usr/bin/env bash
2+
3+
cfssl gencert -initca root.csr.json | cfssljson -bare root
4+
5+
cfssl gencert -initca intermediate.csr.json | cfssljson -bare intermediate
6+
cfssl sign -ca root.pem -ca-key root-key.pem -config intermediate.config.json intermediate.csr | cfssljson -bare intermediate
7+
8+
cfssl gencert -ca intermediate.pem -ca-key intermediate-key.pem -config client.config.json --profile=valid client.csr.json | cfssljson -bare client-valid
9+
cfssl gencert -ca intermediate.pem -ca-key intermediate-key.pem -config client.config.json --profile=expired client.csr.json | cfssljson -bare client-expired
10+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"signing": {
3+
"default": {
4+
"usages": [
5+
"digital signature",
6+
"cert sign",
7+
"crl sign",
8+
"signing",
9+
"key encipherment",
10+
"client auth"
11+
],
12+
"expiry": "876000h",
13+
"ca_constraint": {
14+
"is_ca": true
15+
}
16+
}
17+
}
18+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"CN": "Intermediate-CA",
3+
"ca": {
4+
"expiry": "876000h"
5+
}
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBqDCCAU6gAwIBAgIUfqZtjoFgczZ+oQZbEC/BDSS2J6wwCgYIKoZIzj0EAwIw
3+
EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1
4+
MDYwMFowGjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMFkwEwYHKoZIzj0CAQYI
5+
KoZIzj0DAQcDQgAEyWHEMMCctJg8Xa5YWLqaCPbk3MjB+uvXac42JM9pj4k9jedD
6+
kpUJRkWIPzgJI8Zk/3cSzluUTixP6JBSDKtwwaN4MHYwDgYDVR0PAQH/BAQDAgGm
7+
MBMGA1UdJQQMMAoGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYE
8+
FF+p0JcY31pz+mjNZnjv0Gum92vZMB8GA1UdIwQYMBaAFB7P6+i4/pfNjqZgJv/b
9+
dgA7Fe4tMAoGCCqGSM49BAMCA0gAMEUCIQCTT1YWQZaAqfQ2oBxzOkJE2BqLFxhz
10+
3smQlrZ5gCHddwIgcvT7puhYOzAgcvMn9+SZ1JOyZ7edODjshCVCRnuHK2c=
11+
-----END CERTIFICATE-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"CN": "Root-CA",
3+
"ca": {
4+
"expiry": "876000h"
5+
}
6+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIBizCCATGgAwIBAgIUH4plk9qwD61FVXgiOTngFU5FeSkwCgYIKoZIzj0EAwIw
3+
EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1
4+
MDYwMFowEjEQMA4GA1UEAxMHUm9vdC1DQTBZMBMGByqGSM49AgEGCCqGSM49AwEH
5+
A0IABI2CsrAnMGT8P2VGU2MLo5pv86Z74kcV9hgkLJUkSaeNyc1s89w7X5V2wvwu
6+
iWEJRGm5RoZJausmyZLZEoKEVXejYzBhMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
7+
Af8EBTADAQH/MB0GA1UdDgQWBBQez+vouP6XzY6mYCb/23YAOxXuLTAfBgNVHSME
8+
GDAWgBQez+vouP6XzY6mYCb/23YAOxXuLTAKBggqhkjOPQQDAgNIADBFAiBGclts
9+
vJRM+QMVoV/1L9b+hvhgLIp/OupUFsSOReefIwIhALY06hBklyh8eFwuBtyX2VcE
10+
8xlVn4/5idUvc3Xv2h9s
11+
-----END CERTIFICATE-----

pkg/auth/authenticator/request/x509request/x509.go

+35-26
Original file line numberDiff line numberDiff line change
@@ -40,28 +40,34 @@ func New(opts x509.VerifyOptions, user UserConversion) *Authenticator {
4040

4141
// AuthenticateRequest authenticates the request using presented client certificates
4242
func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
43-
if req.TLS == nil {
43+
if req.TLS == nil || len(req.TLS.PeerCertificates) == 0 {
4444
return nil, false, nil
4545
}
4646

47+
// Use intermediates, if provided
48+
optsCopy := a.opts
49+
if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
50+
optsCopy.Intermediates = x509.NewCertPool()
51+
for _, intermediate := range req.TLS.PeerCertificates[1:] {
52+
optsCopy.Intermediates.AddCert(intermediate)
53+
}
54+
}
55+
56+
chains, err := req.TLS.PeerCertificates[0].Verify(optsCopy)
57+
if err != nil {
58+
return nil, false, err
59+
}
60+
4761
var errlist []error
48-
for _, cert := range req.TLS.PeerCertificates {
49-
chains, err := cert.Verify(a.opts)
62+
for _, chain := range chains {
63+
user, ok, err := a.user.User(chain)
5064
if err != nil {
5165
errlist = append(errlist, err)
5266
continue
5367
}
5468

55-
for _, chain := range chains {
56-
user, ok, err := a.user.User(chain)
57-
if err != nil {
58-
errlist = append(errlist, err)
59-
continue
60-
}
61-
62-
if ok {
63-
return user, ok, err
64-
}
69+
if ok {
70+
return user, ok, err
6571
}
6672
}
6773
return nil, false, kerrors.NewAggregate(errlist)
@@ -81,25 +87,28 @@ func NewVerifier(opts x509.VerifyOptions, auth authenticator.Request, allowedCom
8187
return &Verifier{opts, auth, allowedCommonNames}
8288
}
8389

84-
// AuthenticateRequest verifies the presented client certificates, then delegates to the wrapped auth
90+
// AuthenticateRequest verifies the presented client certificate, then delegates to the wrapped auth
8591
func (a *Verifier) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
86-
if req.TLS == nil {
92+
if req.TLS == nil || len(req.TLS.PeerCertificates) == 0 {
8793
return nil, false, nil
8894
}
8995

90-
var errlist []error
91-
for _, cert := range req.TLS.PeerCertificates {
92-
if _, err := cert.Verify(a.opts); err != nil {
93-
errlist = append(errlist, err)
94-
continue
95-
}
96-
if err := a.verifySubject(cert.Subject); err != nil {
97-
errlist = append(errlist, err)
98-
continue
96+
// Use intermediates, if provided
97+
optsCopy := a.opts
98+
if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
99+
optsCopy.Intermediates = x509.NewCertPool()
100+
for _, intermediate := range req.TLS.PeerCertificates[1:] {
101+
optsCopy.Intermediates.AddCert(intermediate)
99102
}
100-
return a.auth.AuthenticateRequest(req)
101103
}
102-
return nil, false, kerrors.NewAggregate(errlist)
104+
105+
if _, err := req.TLS.PeerCertificates[0].Verify(optsCopy); err != nil {
106+
return nil, false, err
107+
}
108+
if err := a.verifySubject(req.TLS.PeerCertificates[0].Subject); err != nil {
109+
return nil, false, err
110+
}
111+
return a.auth.AuthenticateRequest(req)
103112
}
104113

105114
func (a *Verifier) verifySubject(subject pkix.Name) error {

pkg/auth/authenticator/request/x509request/x509_test.go

+55
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"crypto/x509"
66
"encoding/pem"
77
"errors"
8+
"io/ioutil"
89
"net/http"
910
"testing"
1011
"time"
@@ -357,6 +358,10 @@ mFlG6tStAWz3TmydciZNdiEbeqHw5uaIYWj1zC5AdvFXBFue0ojIrJ5JtbTWccH9
357358
)
358359

359360
func TestX509(t *testing.T) {
361+
multilevelOpts := DefaultVerifyOptions()
362+
multilevelOpts.Roots = x509.NewCertPool()
363+
multilevelOpts.Roots.AddCert(getCertsFromFile(t, "root")[0])
364+
360365
testCases := map[string]struct {
361366
Insecure bool
362367
Certs []*x509.Certificate
@@ -495,6 +500,24 @@ func TestX509(t *testing.T) {
495500
ExpectOK: false,
496501
ExpectErr: true,
497502
},
503+
504+
"multi-level, valid": {
505+
Opts: multilevelOpts,
506+
Certs: getCertsFromFile(t, "client-valid", "intermediate"),
507+
User: CommonNameUserConversion,
508+
509+
ExpectUserName: "My Client",
510+
ExpectOK: true,
511+
ExpectErr: false,
512+
},
513+
"multi-level, expired": {
514+
Opts: multilevelOpts,
515+
Certs: getCertsFromFile(t, "client-expired", "intermediate"),
516+
User: CommonNameUserConversion,
517+
518+
ExpectOK: false,
519+
ExpectErr: true,
520+
},
498521
}
499522

500523
for k, testCase := range testCases {
@@ -531,6 +554,10 @@ func TestX509(t *testing.T) {
531554
}
532555

533556
func TestX509Verifier(t *testing.T) {
557+
multilevelOpts := DefaultVerifyOptions()
558+
multilevelOpts.Roots = x509.NewCertPool()
559+
multilevelOpts.Roots.AddCert(getCertsFromFile(t, "root")[0])
560+
534561
testCases := map[string]struct {
535562
Insecure bool
536563
Certs []*x509.Certificate
@@ -619,6 +646,21 @@ func TestX509Verifier(t *testing.T) {
619646
ExpectOK: false,
620647
ExpectErr: true,
621648
},
649+
650+
"multi-level, valid": {
651+
Opts: multilevelOpts,
652+
Certs: getCertsFromFile(t, "client-valid", "intermediate"),
653+
654+
ExpectOK: true,
655+
ExpectErr: false,
656+
},
657+
"multi-level, expired": {
658+
Opts: multilevelOpts,
659+
Certs: getCertsFromFile(t, "client-expired", "intermediate"),
660+
661+
ExpectOK: false,
662+
ExpectErr: true,
663+
},
622664
}
623665

624666
for k, testCase := range testCases {
@@ -681,6 +723,19 @@ func getRootCertPool(t *testing.T) *x509.CertPool {
681723
return pool
682724
}
683725

726+
func getCertsFromFile(t *testing.T, names ...string) []*x509.Certificate {
727+
certs := []*x509.Certificate{}
728+
for _, name := range names {
729+
filename := "testdata/" + name + ".pem"
730+
data, err := ioutil.ReadFile(filename)
731+
if err != nil {
732+
t.Fatalf("error reading %s: %v", filename, err)
733+
}
734+
certs = append(certs, getCert(t, string(data)))
735+
}
736+
return certs
737+
}
738+
684739
func getCert(t *testing.T, pemData string) *x509.Certificate {
685740
pemBlock, _ := pem.Decode([]byte(pemData))
686741
cert, err := x509.ParseCertificate(pemBlock.Bytes)

vendor/k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/x509/testdata/client-expired.pem

+11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/x509/testdata/client-valid.pem

+11
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/x509/testdata/client.config.json

+24
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/x509/testdata/client.csr.json

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/k8s.io/kubernetes/plugin/pkg/auth/authenticator/request/x509/testdata/generate.sh

+24
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)