Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test x509 intermediates correctly #34524

Merged
merged 1 commit into from
Oct 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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-----
Original file line number Diff line number Diff line change
@@ -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-----
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"CN": "My Client"
}
24 changes: 24 additions & 0 deletions plugin/pkg/auth/authenticator/request/x509/testdata/generate.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/usr/bin/env bash

# Copyright 2016 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.

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

Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"CN": "Intermediate-CA",
"ca": {
"expiry": "876000h"
}
}
Original file line number Diff line number Diff line change
@@ -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-----
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"CN": "Root-CA",
"ca": {
"expiry": "876000h"
}
}
11 changes: 11 additions & 0 deletions plugin/pkg/auth/authenticator/request/x509/testdata/root.pem
Original file line number Diff line number Diff line change
@@ -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-----
32 changes: 19 additions & 13 deletions plugin/pkg/auth/authenticator/request/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see https://golang.org/src/crypto/tls/handshake_server.go, processCertsFromClient for how the stdlib verifies the cert chain

only the leaf cert is verified, the remaining certs are client-supplied intermediates

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, utilerrors.NewAggregate(errlist)
Expand Down
36 changes: 36 additions & 0 deletions plugin/pkg/auth/authenticator/request/x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"crypto/x509"
"encoding/pem"
"errors"
"io/ioutil"
"net/http"
"reflect"
"sort"
Expand Down Expand Up @@ -507,6 +508,10 @@ PKJQCs0CM0zkesktuLi/gFpuB0nEwyOgLg==
)

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
Expand Down Expand Up @@ -659,6 +664,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 {
Expand Down Expand Up @@ -718,6 +741,19 @@ func getRootCertPoolFor(t *testing.T, certs ...string) *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)
Expand Down