Move X.509 client certificate validation out of pkg/grpc

Right now we only perform client certificate validation within the
context of gRPC request authentication. Within Bonanza I'm interested in
also using X.509 client certificates outside of TLS, namely to perform
ECDH to encrypt traffic between workers and clients.

Let's decompose our bb_grpc.TLSClientCertificateAuthenticator, moving
all certificate validation logic into bb_x509.ClientCertificateVerifier.
This is similar to how JWT validation lives in pkg/jwt.

Author: EdSchouten

pkg/grpc/BUILD.bazel

@@ -42,6 +42,7 @@ go_library(
         "//pkg/proto/auth",
         "//pkg/proto/configuration/grpc",
         "//pkg/util",
+        "//pkg/x509",
         "@bazel_remote_apis//build/bazel/remote/execution/v2:remote_execution_go_proto",
         "@com_github_grpc_ecosystem_go_grpc_middleware//:go-grpc-middleware",
         "@com_github_grpc_ecosystem_go_grpc_prometheus//:go-grpc-prometheus",
@@ -103,7 +104,6 @@ go_test(
         "proto_trace_attributes_extractor_test.go",
         "request_headers_authenticator_test.go",
         "request_metadata_tracing_interceptor_test.go",
-        "tls_client_certificate_authenticator_test.go",
     ] + select({
         "@rules_go//go/platform:android": [
             "peer_transport_credentials_test.go",
@@ -138,7 +138,6 @@ go_test(
         "@io_opentelemetry_go_proto_otlp//common/v1:common",
         "@org_golang_google_grpc//:grpc",
         "@org_golang_google_grpc//codes",
-        "@org_golang_google_grpc//credentials",
         "@org_golang_google_grpc//metadata",
         "@org_golang_google_grpc//peer",
         "@org_golang_google_grpc//status",

pkg/grpc/authenticator.go

@@ -2,7 +2,6 @@ package grpc
 
 import (
 	"context"
-	"crypto/x509"
 
 	"github.com/buildbarn/bb-storage/pkg/auth"
 	"github.com/buildbarn/bb-storage/pkg/clock"
@@ -11,6 +10,7 @@ import (
 	"github.com/buildbarn/bb-storage/pkg/program"
 	configuration "github.com/buildbarn/bb-storage/pkg/proto/configuration/grpc"
 	"github.com/buildbarn/bb-storage/pkg/util"
+	"github.com/buildbarn/bb-storage/pkg/x509"
 	"github.com/jmespath/go-jmespath"
 
 	"google.golang.org/grpc/codes"
@@ -68,24 +68,11 @@ func NewAuthenticatorFromConfiguration(policy *configuration.AuthenticationPolic
 	case *configuration.AuthenticationPolicy_Deny:
 		return NewDenyAuthenticator(policyKind.Deny), false, false, nil
 	case *configuration.AuthenticationPolicy_TlsClientCertificate:
-		clientCAs := x509.NewCertPool()
-		if !clientCAs.AppendCertsFromPEM([]byte(policyKind.TlsClientCertificate.ClientCertificateAuthorities)) {
-			return nil, false, false, status.Error(codes.InvalidArgument, "Failed to parse client certificate authorities")
-		}
-		validator, err := jmespath.Compile(policyKind.TlsClientCertificate.ValidationJmespathExpression)
-		if err != nil {
-			return nil, false, false, util.StatusWrap(err, "Failed to compile validation JMESPath expression")
-		}
-		metadataExtractor, err := jmespath.Compile(policyKind.TlsClientCertificate.MetadataExtractionJmespathExpression)
+		clientCertificateVerifier, err := x509.NewClientCertificateVerifierFromConfiguration(policyKind.TlsClientCertificate)
 		if err != nil {
-			return nil, false, false, util.StatusWrap(err, "Failed to compile metadata extraction JMESPath expression")
+			return nil, false, false, err
 		}
-		return NewTLSClientCertificateAuthenticator(
-			clientCAs,
-			clock.SystemClock,
-			validator,
-			metadataExtractor,
-		), false, true, nil
+		return NewTLSClientCertificateAuthenticator(clientCertificateVerifier), false, true, nil
 	case *configuration.AuthenticationPolicy_Jwt:
 		authorizationHeaderParser, err := jwt.NewAuthorizationHeaderParserFromConfiguration(policyKind.Jwt, group)
 		if err != nil {

pkg/grpc/tls_client_certificate_authenticator.go

@@ -2,12 +2,9 @@ package grpc
 
 import (
 	"context"
-	"crypto/x509"
 
 	"github.com/buildbarn/bb-storage/pkg/auth"
-	"github.com/buildbarn/bb-storage/pkg/clock"
-	"github.com/buildbarn/bb-storage/pkg/util"
-	"github.com/jmespath/go-jmespath"
+	"github.com/buildbarn/bb-storage/pkg/x509"
 
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/credentials"
@@ -16,22 +13,16 @@ import (
 )
 
 type tlsClientCertificateAuthenticator struct {
-	clientCAs         *x509.CertPool
-	clock             clock.Clock
-	validator         *jmespath.JMESPath
-	metadataExtractor *jmespath.JMESPath
+	verifier *x509.ClientCertificateVerifier
 }
 
 // NewTLSClientCertificateAuthenticator creates an Authenticator that
 // only grants access in case the client connected to the gRPC server
 // using a TLS client certificate that can be validated against the
 // chain of CAs used by the server.
-func NewTLSClientCertificateAuthenticator(clientCAs *x509.CertPool, clock clock.Clock, validator, metadataExtractor *jmespath.JMESPath) Authenticator {
+func NewTLSClientCertificateAuthenticator(verifier *x509.ClientCertificateVerifier) Authenticator {
 	return &tlsClientCertificateAuthenticator{
-		clientCAs:         clientCAs,
-		clock:             clock,
-		validator:         validator,
-		metadataExtractor: metadataExtractor,
+		verifier: verifier,
 	}
 }
 
@@ -45,71 +36,5 @@ func (a *tlsClientCertificateAuthenticator) Authenticate(ctx context.Context) (*
 	if !ok {
 		return nil, status.Error(codes.Unauthenticated, "Connection was not established using TLS")
 	}
-	certs := tlsInfo.State.PeerCertificates
-	if len(certs) == 0 {
-		return nil, status.Error(codes.Unauthenticated, "Client provided no TLS client certificate")
-	}
-
-	// Perform certificate verification.
-	// TODO: Should this be memoized?
-	opts := x509.VerifyOptions{
-		Roots:         a.clientCAs,
-		CurrentTime:   a.clock.Now(),
-		Intermediates: x509.NewCertPool(),
-		KeyUsages:     []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
-	}
-	for _, cert := range certs[1:] {
-		opts.Intermediates.AddCert(cert)
-	}
-	if _, err := certs[0].Verify(opts); err != nil {
-		return nil, util.StatusWrapWithCode(err, codes.Unauthenticated, "Cannot validate TLS client certificate")
-	}
-
-	searchContext := getClientCertificateJMESPathSearchContext(certs[0])
-
-	// Validate the client cert matches our expectations.
-	validationResult, err := a.validator.Search(searchContext)
-	if err != nil {
-		return nil, util.StatusWrapWithCode(err, codes.Unauthenticated, "Cannot validate TLS client certificate claims")
-	}
-	if validationResult != true {
-		return nil, status.Error(codes.Unauthenticated, "Rejected TLS client certificate claims")
-	}
-
-	// Extract metadata from the client cert.
-	metadataRaw, err := a.metadataExtractor.Search(searchContext)
-	if err != nil {
-		return nil, util.StatusWrapWithCode(err, codes.Unauthenticated, "Cannot extract metadata from TLS client certificate")
-	}
-
-	return auth.NewAuthenticationMetadataFromRaw(metadataRaw)
-}
-
-func getClientCertificateJMESPathSearchContext(cert *x509.Certificate) map[string]any {
-	// We have to go through this copying and json dance in order to
-	// ensure that we don't replace [] with null, and that we have the proper
-	// types needed for JMESPath to search over without typing failures.
-
-	dnsNames := make([]any, 0, len(cert.DNSNames))
-	for _, d := range cert.DNSNames {
-		dnsNames = append(dnsNames, d)
-	}
-	emailAddresses := make([]any, 0, len(cert.EmailAddresses))
-	for _, e := range cert.EmailAddresses {
-		emailAddresses = append(emailAddresses, e)
-	}
-
-	uris := make([]any, 0, len(cert.URIs))
-	for _, e := range cert.URIs {
-		uris = append(uris, e.String())
-	}
-
-	// The data structure that users can search over
-	searchContext := map[string]any{
-		"dnsNames":       dnsNames,
-		"emailAddresses": emailAddresses,
-		"uris":           uris,
-	}
-
-	return searchContext
+	return a.verifier.VerifyClientCertificate(tlsInfo.State.PeerCertificates)
 }

pkg/proto/configuration/grpc/BUILD.bazel

@@ -11,6 +11,7 @@ proto_library(
         "//pkg/proto/configuration/eviction:eviction_proto",
         "//pkg/proto/configuration/jwt:jwt_proto",
         "//pkg/proto/configuration/tls:tls_proto",
+        "//pkg/proto/configuration/x509:x509_proto",
         "@protobuf//:duration_proto",
         "@protobuf//:empty_proto",
         "@protobuf//:struct_proto",
@@ -27,6 +28,7 @@ go_proto_library(
         "//pkg/proto/configuration/eviction",
         "//pkg/proto/configuration/jwt",
         "//pkg/proto/configuration/tls",
+        "//pkg/proto/configuration/x509",
     ],
 )