Merge pull request #11308 from liggitt/x509-intermediate-1.3.1

Merged by openshift-bot

Author: OpenShift Bot

pkg/auth/authenticator/request/x509request/testdata/client-expired.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBpTCCAUugAwIBAgIUPV4LAC5KK8YWY1FegyTuhkGUr3EwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMB4XDTkwMTIzMTIzNTkwMFoXDTkw
+MTIzMTIzNTkwMFowFDESMBAGA1UEAxMJTXkgQ2xpZW50MFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEyYUnseNUN87rfHgekrfZu5sj4wlt5LYr3JYZZkfSbsb+BW3/
+RzX02ifjp+8w7mI4qUGg6y6J7oXHGFT3uj9kj6N1MHMwDgYDVR0PAQH/BAQDAgWg
+MBMGA1UdJQQMMAoGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFKsX
+EnXwDg8j2LIEM1QzmFrE6537MB8GA1UdIwQYMBaAFF+p0JcY31pz+mjNZnjv0Gum
+92vZMAoGCCqGSM49BAMCA0gAMEUCIG4FBcb57oqOCoaFiJ+Yx6S0zkaash7bTv3V
+CIy9JvFdAiEAy8bf2S9EkvZyURZ6ycgEMnekll57Ebze6rjlPx8+B1Y=
+-----END CERTIFICATE-----

pkg/auth/authenticator/request/x509request/testdata/client-valid.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU2gAwIBAgIUfbqeieihh/oERbfvRm38XvS/xHAwCgYIKoZIzj0EAwIw
+GjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMCAXDTE2MTAxMTA1MDYwMFoYDzIx
+MTYwOTE3MDUwNjAwWjAUMRIwEAYDVQQDEwlNeSBDbGllbnQwWTATBgcqhkjOPQIB
+BggqhkjOPQMBBwNCAARv6N4R/sjMR65iMFGNLN1GC/vd7WhDW6J4X/iAjkRLLnNb
+KbRG/AtOUZ+7upJ3BWIRKYbOabbQGQe2BbKFiap4o3UwczAOBgNVHQ8BAf8EBAMC
+BaAwEwYDVR0lBAwwCgYIKwYBBQUHAwIwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU
+K/pZOWpNcYai6eHFpmJEeFpeQlEwHwYDVR0jBBgwFoAUX6nQlxjfWnP6aM1meO/Q
+a6b3a9kwCgYIKoZIzj0EAwIDSQAwRgIhAIWTKw/sjJITqeuNzJDAKU4xo1zL+xJ5
+MnVCuBwfwDXCAiEAw/1TA+CjPq9JC5ek1ifR0FybTURjeQqYkKpve1dveps=
+-----END CERTIFICATE-----

pkg/auth/authenticator/request/x509request/testdata/client.config.json

@@ -0,0 +1,24 @@
+{
+    "signing": {
+        "profiles": {
+            "valid": {
+                "expiry": "876000h",
+                "usages": [
+                    "signing",
+                    "key encipherment",
+                    "client auth"
+                ]
+            },
+            "expired": {
+                "expiry": "1h",
+                "not_before": "1990-12-31T23:59:00Z",
+                "not_after": "1990-12-31T23:59:00Z",
+                "usages": [
+                    "signing",
+                    "key encipherment",
+                    "client auth"
+                ]
+            }
+        }
+    }
+}

pkg/auth/authenticator/request/x509request/testdata/client.csr.json

@@ -0,0 +1,3 @@
+{
+    "CN": "My Client"
+}

pkg/auth/authenticator/request/x509request/testdata/generate.sh

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+cfssl gencert -initca root.csr.json | cfssljson -bare root
+
+cfssl gencert -initca intermediate.csr.json | cfssljson -bare intermediate
+cfssl sign -ca root.pem -ca-key root-key.pem -config intermediate.config.json intermediate.csr | cfssljson -bare intermediate
+
+cfssl gencert -ca intermediate.pem -ca-key intermediate-key.pem -config client.config.json --profile=valid   client.csr.json | cfssljson -bare client-valid
+cfssl gencert -ca intermediate.pem -ca-key intermediate-key.pem -config client.config.json --profile=expired client.csr.json | cfssljson -bare client-expired
+ 

pkg/auth/authenticator/request/x509request/testdata/intermediate.config.json

@@ -0,0 +1,18 @@
+{
+    "signing": {
+        "default": {
+            "usages": [
+                "digital signature",
+                "cert sign",
+                "crl sign",
+                "signing",
+                "key encipherment",
+                "client auth"
+            ],
+            "expiry": "876000h",
+            "ca_constraint": {
+                "is_ca": true
+            }
+        }
+    }
+}

pkg/auth/authenticator/request/x509request/testdata/intermediate.csr.json

@@ -0,0 +1,6 @@
+{
+    "CN": "Intermediate-CA",
+    "ca": {
+        "expiry": "876000h"
+    }
+}

pkg/auth/authenticator/request/x509request/testdata/intermediate.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBqDCCAU6gAwIBAgIUfqZtjoFgczZ+oQZbEC/BDSS2J6wwCgYIKoZIzj0EAwIw
+EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1
+MDYwMFowGjEYMBYGA1UEAxMPSW50ZXJtZWRpYXRlLUNBMFkwEwYHKoZIzj0CAQYI
+KoZIzj0DAQcDQgAEyWHEMMCctJg8Xa5YWLqaCPbk3MjB+uvXac42JM9pj4k9jedD
+kpUJRkWIPzgJI8Zk/3cSzluUTixP6JBSDKtwwaN4MHYwDgYDVR0PAQH/BAQDAgGm
+MBMGA1UdJQQMMAoGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYE
+FF+p0JcY31pz+mjNZnjv0Gum92vZMB8GA1UdIwQYMBaAFB7P6+i4/pfNjqZgJv/b
+dgA7Fe4tMAoGCCqGSM49BAMCA0gAMEUCIQCTT1YWQZaAqfQ2oBxzOkJE2BqLFxhz
+3smQlrZ5gCHddwIgcvT7puhYOzAgcvMn9+SZ1JOyZ7edODjshCVCRnuHK2c=
+-----END CERTIFICATE-----

pkg/auth/authenticator/request/x509request/testdata/root.csr.json

@@ -0,0 +1,6 @@
+{
+    "CN": "Root-CA",
+    "ca": {
+        "expiry": "876000h"
+    }
+}

pkg/auth/authenticator/request/x509request/testdata/root.pem

@@ -0,0 +1,11 @@
+-----BEGIN CERTIFICATE-----
+MIIBizCCATGgAwIBAgIUH4plk9qwD61FVXgiOTngFU5FeSkwCgYIKoZIzj0EAwIw
+EjEQMA4GA1UEAxMHUm9vdC1DQTAgFw0xNjEwMTEwNTA2MDBaGA8yMTE2MDkxNzA1
+MDYwMFowEjEQMA4GA1UEAxMHUm9vdC1DQTBZMBMGByqGSM49AgEGCCqGSM49AwEH
+A0IABI2CsrAnMGT8P2VGU2MLo5pv86Z74kcV9hgkLJUkSaeNyc1s89w7X5V2wvwu
+iWEJRGm5RoZJausmyZLZEoKEVXejYzBhMA4GA1UdDwEB/wQEAwIBBjAPBgNVHRMB
+Af8EBTADAQH/MB0GA1UdDgQWBBQez+vouP6XzY6mYCb/23YAOxXuLTAfBgNVHSME
+GDAWgBQez+vouP6XzY6mYCb/23YAOxXuLTAKBggqhkjOPQQDAgNIADBFAiBGclts
+vJRM+QMVoV/1L9b+hvhgLIp/OupUFsSOReefIwIhALY06hBklyh8eFwuBtyX2VcE
+8xlVn4/5idUvc3Xv2h9s
+-----END CERTIFICATE-----

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

@@ -40,28 +40,34 @@ func New(opts x509.VerifyOptions, user UserConversion) *Authenticator {
 
 // AuthenticateRequest authenticates the request using presented client certificates
 func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
-	if req.TLS == nil {
+	if req.TLS == nil || len(req.TLS.PeerCertificates) == 0 {
 		return nil, false, nil
 	}
 
+	// Use intermediates, if provided
+	optsCopy := a.opts
+	if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
+		optsCopy.Intermediates = x509.NewCertPool()
+		for _, intermediate := range req.TLS.PeerCertificates[1:] {
+			optsCopy.Intermediates.AddCert(intermediate)
+		}
+	}
+
+	chains, err := req.TLS.PeerCertificates[0].Verify(optsCopy)
+	if err != nil {
+		return nil, false, err
+	}
+
 	var errlist []error
-	for _, cert := range req.TLS.PeerCertificates {
-		chains, err := cert.Verify(a.opts)
+	for _, chain := range chains {
+		user, ok, err := a.user.User(chain)
 		if err != nil {
 			errlist = append(errlist, err)
 			continue
 		}
 
-		for _, chain := range chains {
-			user, ok, err := a.user.User(chain)
-			if err != nil {
-				errlist = append(errlist, err)
-				continue
-			}
-
-			if ok {
-				return user, ok, err
-			}
+		if ok {
+			return user, ok, err
 		}
 	}
 	return nil, false, kerrors.NewAggregate(errlist)
@@ -81,25 +87,28 @@ func NewVerifier(opts x509.VerifyOptions, auth authenticator.Request, allowedCom
 	return &Verifier{opts, auth, allowedCommonNames}
 }
 
-// AuthenticateRequest verifies the presented client certificates, then delegates to the wrapped auth
+// AuthenticateRequest verifies the presented client certificate, then delegates to the wrapped auth
 func (a *Verifier) AuthenticateRequest(req *http.Request) (user.Info, bool, error) {
-	if req.TLS == nil {
+	if req.TLS == nil || len(req.TLS.PeerCertificates) == 0 {
 		return nil, false, nil
 	}
 
-	var errlist []error
-	for _, cert := range req.TLS.PeerCertificates {
-		if _, err := cert.Verify(a.opts); err != nil {
-			errlist = append(errlist, err)
-			continue
-		}
-		if err := a.verifySubject(cert.Subject); err != nil {
-			errlist = append(errlist, err)
-			continue
+	// Use intermediates, if provided
+	optsCopy := a.opts
+	if optsCopy.Intermediates == nil && len(req.TLS.PeerCertificates) > 1 {
+		optsCopy.Intermediates = x509.NewCertPool()
+		for _, intermediate := range req.TLS.PeerCertificates[1:] {
+			optsCopy.Intermediates.AddCert(intermediate)
 		}
-		return a.auth.AuthenticateRequest(req)
 	}
-	return nil, false, kerrors.NewAggregate(errlist)
+
+	if _, err := req.TLS.PeerCertificates[0].Verify(optsCopy); err != nil {
+		return nil, false, err
+	}
+	if err := a.verifySubject(req.TLS.PeerCertificates[0].Subject); err != nil {
+		return nil, false, err
+	}
+	return a.auth.AuthenticateRequest(req)
 }
 
 func (a *Verifier) verifySubject(subject pkix.Name) error {

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

@@ -5,6 +5,7 @@ import (
 	"crypto/x509"
 	"encoding/pem"
 	"errors"
+	"io/ioutil"
 	"net/http"
 	"testing"
 	"time"
@@ -357,6 +358,10 @@ mFlG6tStAWz3TmydciZNdiEbeqHw5uaIYWj1zC5AdvFXBFue0ojIrJ5JtbTWccH9
 )
 
 func TestX509(t *testing.T) {
+	multilevelOpts := DefaultVerifyOptions()
+	multilevelOpts.Roots = x509.NewCertPool()
+	multilevelOpts.Roots.AddCert(getCertsFromFile(t, "root")[0])
+
 	testCases := map[string]struct {
 		Insecure bool
 		Certs    []*x509.Certificate
@@ -495,6 +500,24 @@ func TestX509(t *testing.T) {
 			ExpectOK:  false,
 			ExpectErr: true,
 		},
+
+		"multi-level, valid": {
+			Opts:  multilevelOpts,
+			Certs: getCertsFromFile(t, "client-valid", "intermediate"),
+			User:  CommonNameUserConversion,
+
+			ExpectUserName: "My Client",
+			ExpectOK:       true,
+			ExpectErr:      false,
+		},
+		"multi-level, expired": {
+			Opts:  multilevelOpts,
+			Certs: getCertsFromFile(t, "client-expired", "intermediate"),
+			User:  CommonNameUserConversion,
+
+			ExpectOK:  false,
+			ExpectErr: true,
+		},
 	}
 
 	for k, testCase := range testCases {
@@ -531,6 +554,10 @@ func TestX509(t *testing.T) {
 }
 
 func TestX509Verifier(t *testing.T) {
+	multilevelOpts := DefaultVerifyOptions()
+	multilevelOpts.Roots = x509.NewCertPool()
+	multilevelOpts.Roots.AddCert(getCertsFromFile(t, "root")[0])
+
 	testCases := map[string]struct {
 		Insecure bool
 		Certs    []*x509.Certificate
@@ -619,6 +646,21 @@ func TestX509Verifier(t *testing.T) {
 			ExpectOK:  false,
 			ExpectErr: true,
 		},
+
+		"multi-level, valid": {
+			Opts:  multilevelOpts,
+			Certs: getCertsFromFile(t, "client-valid", "intermediate"),
+
+			ExpectOK:  true,
+			ExpectErr: false,
+		},
+		"multi-level, expired": {
+			Opts:  multilevelOpts,
+			Certs: getCertsFromFile(t, "client-expired", "intermediate"),
+
+			ExpectOK:  false,
+			ExpectErr: true,
+		},
 	}
 
 	for k, testCase := range testCases {
@@ -681,6 +723,19 @@ func getRootCertPool(t *testing.T) *x509.CertPool {
 	return pool
 }
 
+func getCertsFromFile(t *testing.T, names ...string) []*x509.Certificate {
+	certs := []*x509.Certificate{}
+	for _, name := range names {
+		filename := "testdata/" + name + ".pem"
+		data, err := ioutil.ReadFile(filename)
+		if err != nil {
+			t.Fatalf("error reading %s: %v", filename, err)
+		}
+		certs = append(certs, getCert(t, string(data)))
+	}
+	return certs
+}
+
 func getCert(t *testing.T, pemData string) *x509.Certificate {
 	pemBlock, _ := pem.Decode([]byte(pemData))
 	cert, err := x509.ParseCertificate(pemBlock.Bytes)