Merge pull request #8443 from liggitt/requestheader-cn

Merged by openshift-bot

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

@@ -3,11 +3,14 @@ package x509request
 import (
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"fmt"
 	"net/http"
 
+	"github.com/golang/glog"
 	"github.com/openshift/origin/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/user"
 	kerrors "k8s.io/kubernetes/pkg/util/errors"
+	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 // UserConversion defines an interface for extracting user info from a client certificate chain
@@ -68,10 +71,14 @@ func (a *Authenticator) AuthenticateRequest(req *http.Request) (user.Info, bool,
 type Verifier struct {
 	opts x509.VerifyOptions
 	auth authenticator.Request
+
+	// allowedCommonNames contains the common names which a verified certificate is allowed to have.
+	// If empty, all verified certificates are allowed.
+	allowedCommonNames sets.String
 }
 
-func NewVerifier(opts x509.VerifyOptions, auth authenticator.Request) authenticator.Request {
-	return &Verifier{opts, auth}
+func NewVerifier(opts x509.VerifyOptions, auth authenticator.Request, allowedCommonNames sets.String) authenticator.Request {
+	return &Verifier{opts, auth, allowedCommonNames}
 }
 
 // AuthenticateRequest verifies the presented client certificates, then delegates to the wrapped auth
@@ -82,8 +89,11 @@ func (a *Verifier) AuthenticateRequest(req *http.Request) (user.Info, bool, erro
 
 	var errlist []error
 	for _, cert := range req.TLS.PeerCertificates {
-		_, err := cert.Verify(a.opts)
-		if err != nil {
+		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
 		}
@@ -92,6 +102,19 @@ func (a *Verifier) AuthenticateRequest(req *http.Request) (user.Info, bool, erro
 	return nil, false, kerrors.NewAggregate(errlist)
 }
 
+func (a *Verifier) verifySubject(subject pkix.Name) error {
+	// No CN restrictions
+	if len(a.allowedCommonNames) == 0 {
+		return nil
+	}
+	// Enforce CN restrictions
+	if a.allowedCommonNames.Has(subject.CommonName) {
+		return nil
+	}
+	glog.Warningf("x509: subject with cn=%s is not in the allowed list: %v", subject.CommonName, a.allowedCommonNames.List())
+	return fmt.Errorf("x509: subject with cn=%s is not allowed", subject.CommonName)
+}
+
 // DefaultVerifyOptions returns VerifyOptions that use the system root certificates, current time,
 // and requires certificates to be valid for client auth (x509.ExtKeyUsageClientAuth)
 func DefaultVerifyOptions() x509.VerifyOptions {

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

@@ -11,6 +11,7 @@ import (
 
 	"github.com/openshift/origin/pkg/auth/authenticator"
 	"k8s.io/kubernetes/pkg/auth/user"
+	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 const (
@@ -536,6 +537,8 @@ func TestX509Verifier(t *testing.T) {
 
 		Opts x509.VerifyOptions
 
+		AllowedCNs sets.String
+
 		ExpectOK  bool
 		ExpectErr bool
 	}{
@@ -579,6 +582,22 @@ func TestX509Verifier(t *testing.T) {
 			ExpectOK:  true,
 			ExpectErr: false,
 		},
+		"valid client cert with wrong CN": {
+			Opts:       getDefaultVerifyOptions(t),
+			AllowedCNs: sets.NewString("foo", "bar"),
+			Certs:      getCerts(t, clientCNCert),
+
+			ExpectOK:  false,
+			ExpectErr: true,
+		},
+		"valid client cert with right CN": {
+			Opts:       getDefaultVerifyOptions(t),
+			AllowedCNs: sets.NewString("client_cn"),
+			Certs:      getCerts(t, clientCNCert),
+
+			ExpectOK:  true,
+			ExpectErr: false,
+		},
 
 		"future cert": {
 			Opts: x509.VerifyOptions{
@@ -614,7 +633,7 @@ func TestX509Verifier(t *testing.T) {
 			return &user.DefaultInfo{Name: "innerauth"}, true, nil
 		})
 
-		a := NewVerifier(testCase.Opts, auth)
+		a := NewVerifier(testCase.Opts, auth, testCase.AllowedCNs)
 
 		user, ok, err := a.AuthenticateRequest(req)
 

pkg/cmd/server/api/types.go

@@ -742,6 +742,9 @@ type RequestHeaderIdentityProvider struct {
 
 	// ClientCA is a file with the trusted signer certs.  If empty, no request verification is done, and any direct request to the OAuth server can impersonate any identity from this provider, merely by setting a request header.
 	ClientCA string
+	// ClientCommonNames is an optional list of common names to require a match from. If empty, any client certificate validated against the clientCA bundle is considered authoritative.
+	ClientCommonNames []string
+
 	// Headers is the set of headers to check for identity information
 	Headers []string
 	// PreferredUsernameHeaders is the set of headers to check for the preferred username

pkg/cmd/server/api/v1/swagger_doc.go

@@ -617,6 +617,7 @@ var map_RequestHeaderIdentityProvider = map[string]string{
 	"loginURL":                 "LoginURL is a URL to redirect unauthenticated /authorize requests to Unauthenticated requests from OAuth clients which expect interactive logins will be redirected here ${url} is replaced with the current URL, escaped to be safe in a query parameter\n  https://www.example.com/sso-login?then=${url}\n${query} is replaced with the current query string\n  https://www.example.com/auth-proxy/oauth/authorize?${query}",
 	"challengeURL":             "ChallengeURL is a URL to redirect unauthenticated /authorize requests to Unauthenticated requests from OAuth clients which expect WWW-Authenticate challenges will be redirected here ${url} is replaced with the current URL, escaped to be safe in a query parameter\n  https://www.example.com/sso-login?then=${url}\n${query} is replaced with the current query string\n  https://www.example.com/auth-proxy/oauth/authorize?${query}",
 	"clientCA":                 "ClientCA is a file with the trusted signer certs.  If empty, no request verification is done, and any direct request to the OAuth server can impersonate any identity from this provider, merely by setting a request header.",
+	"clientCommonNames":        "ClientCommonNames is an optional list of common names to require a match from. If empty, any client certificate validated against the clientCA bundle is considered authoritative.",
 	"headers":                  "Headers is the set of headers to check for identity information",
 	"preferredUsernameHeaders": "PreferredUsernameHeaders is the set of headers to check for the preferred username",
 	"nameHeaders":              "NameHeaders is the set of headers to check for the display name",

pkg/cmd/server/api/v1/types.go

@@ -732,6 +732,9 @@ type RequestHeaderIdentityProvider struct {
 
 	// ClientCA is a file with the trusted signer certs.  If empty, no request verification is done, and any direct request to the OAuth server can impersonate any identity from this provider, merely by setting a request header.
 	ClientCA string `json:"clientCA"`
+	// ClientCommonNames is an optional list of common names to require a match from. If empty, any client certificate validated against the clientCA bundle is considered authoritative.
+	ClientCommonNames []string `json:"clientCommonNames"`
+
 	// Headers is the set of headers to check for identity information
 	Headers []string `json:"headers"`
 	// PreferredUsernameHeaders is the set of headers to check for the preferred username

pkg/cmd/server/api/v1/types_test.go

@@ -272,6 +272,7 @@ oauthConfig:
       apiVersion: v1
       challengeURL: ""
       clientCA: ""
+      clientCommonNames: null
       emailHeaders: null
       headers: null
       kind: RequestHeaderIdentityProvider

pkg/cmd/server/api/validation/oauth.go

@@ -235,7 +235,10 @@ func ValidateRequestHeaderIdentityProvider(provider *api.RequestHeaderIdentityPr
 
 	if len(provider.ClientCA) > 0 {
 		validationResults.AddErrors(ValidateFile(provider.ClientCA, fieldPath.Child("provider", "clientCA"))...)
+	} else if len(provider.ClientCommonNames) > 0 {
+		validationResults.AddErrors(field.Invalid(fieldPath.Child("provider", "clientCommonNames"), provider.ClientCommonNames, "clientCA must be specified in order to use clientCommonNames"))
 	}
+
 	if len(provider.Headers) == 0 {
 		validationResults.AddErrors(field.Required(fieldPath.Child("provider", "headers"), ""))
 	}

pkg/cmd/server/origin/auth.go

@@ -696,7 +696,7 @@ func (c *AuthConfig) getAuthenticationRequestHandler() (authenticator.Request, e
 						return nil, fmt.Errorf("Error loading certs from %s: %v", provider.ClientCA, err)
 					}
 
-					authRequestHandler = x509request.NewVerifier(opts, authRequestHandler)
+					authRequestHandler = x509request.NewVerifier(opts, authRequestHandler, sets.NewString(provider.ClientCommonNames...))
 				}
 				authRequestHandlers = append(authRequestHandlers, authRequestHandler)