authorize: allow client certificate intermediates (#4451)

Update the isValidClientCertificate() method to consider any
client-supplied intermediate certificates. Previously, in order to trust
client certificates issued by an intermediate CA, users would need to
include that intermediate CA's certificate directly in the client_ca
setting. After this change, only the trusted root CA needs to be set: as
long as the client can supply a set of certificates that chain back to
this trusted root, the client's certificate will validate successfully.

Rework the previous CRL checking logic to now consider CRLs for all
issuers in the verified chains.
This commit is contained in:
Kenneth Jenkins 2023-08-10 09:33:29 -07:00 committed by GitHub
parent ac475f4c5d
commit 0fcc3f16de
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 181 additions and 94 deletions

View file

@ -12,7 +12,7 @@ import (
"github.com/pomerium/pomerium/internal/log"
)
var isValidClientCertificateCache, _ = lru.New2Q[[3]string, bool](100)
var isValidClientCertificateCache, _ = lru.New2Q[[4]string, bool](100)
func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (bool, error) {
// when ca is the empty string, client certificates are not required
@ -21,22 +21,24 @@ func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (b
}
cert := certInfo.Leaf
intermediates := certInfo.Intermediates
if cert == "" {
return false, nil
}
cacheKey := [3]string{ca, crl, cert}
cacheKey := [4]string{ca, crl, cert, intermediates}
value, ok := isValidClientCertificateCache.Get(cacheKey)
if ok {
return value, nil
}
roots, err := parseCertificates([]byte(ca))
if err != nil {
return false, err
}
roots := x509.NewCertPool()
roots.AppendCertsFromPEM([]byte(ca))
intermediatesPool := x509.NewCertPool()
intermediatesPool.AppendCertsFromPEM([]byte(intermediates))
xcert, err := parseCertificate(cert)
if err != nil {
@ -48,7 +50,7 @@ func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (b
return false, err
}
verifyErr := verifyClientCertificate(xcert, roots, crls)
verifyErr := verifyClientCertificate(xcert, roots, intermediatesPool, crls)
valid := verifyErr == nil
if verifyErr != nil {
@ -60,52 +62,73 @@ func isValidClientCertificate(ca, crl string, certInfo ClientCertificateInfo) (b
return valid, nil
}
var errCertificateRevoked = errors.New("certificate revoked")
func verifyClientCertificate(
cert *x509.Certificate,
roots map[string]*x509.Certificate,
roots *x509.CertPool,
intermediates *x509.CertPool,
crls map[string]*x509.RevocationList,
) error {
rootPool := x509.NewCertPool()
for _, root := range roots {
rootPool.AddCert(root)
}
if _, err := cert.Verify(x509.VerifyOptions{
Roots: rootPool,
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
}); err != nil {
chains, err := cert.Verify(x509.VerifyOptions{
Roots: roots,
Intermediates: intermediates,
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
})
if err != nil {
return err
}
// Consult any CRL for the presented certificate's Issuer.
issuer := string(cert.RawIssuer)
crl := crls[issuer]
if crl == nil {
return nil
}
// Do we have a corresponding trusted CA certificate?
root, ok := roots[issuer]
if !ok {
return fmt.Errorf("could not check CRL: no matching trusted CA for issuer %s",
cert.Issuer)
}
// Is the CRL signature itself valid?
if err := crl.CheckSignatureFrom(root); err != nil {
return fmt.Errorf("could not check CRL for issuer %s: signature verification "+
"error: %w", cert.Issuer, err)
}
// Is the client certificate listed as revoked in this CRL?
for i := range crl.RevokedCertificates {
if cert.SerialNumber.Cmp(crl.RevokedCertificates[i].SerialNumber) == 0 {
return errCertificateRevoked
// At least one of the verified chains must also pass revocation checking.
err = errors.New("internal error: no verified chains")
for _, chain := range chains {
err = validateClientCertificateChain(chain, crls)
if err == nil {
return nil
}
}
// Return an error from one of the chains that did not validate.
// (In the common case there will be at most one verified chain.)
return err
}
func validateClientCertificateChain(
chain []*x509.Certificate,
crls map[string]*x509.RevocationList,
) error {
// Consult CRLs for all CAs in the chain (that is, all certificates except
// for the first one). To match Envoy's behavior, if a CRL is provided for
// any CA in the chain, CRLs must be provided for all CAs in the chain (see
// https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/transport_sockets/tls/v3/common.proto).
var anyIssuerHasCRL bool
var lastIssuerWithoutCRL *x509.Certificate
for i := 0; i < len(chain)-1; i++ {
cert, issuer := chain[i], chain[i+1]
crl := crls[string(issuer.RawSubject)]
if crl == nil {
lastIssuerWithoutCRL = issuer
continue
}
anyIssuerHasCRL = true
// Is the CRL signature itself valid?
if err := crl.CheckSignatureFrom(issuer); err != nil {
return fmt.Errorf("CRL signature verification failed for issuer %q: %w",
issuer.Subject, err)
}
// Is the certificate listed as revoked in the CRL?
for i := range crl.RevokedCertificates {
if cert.SerialNumber.Cmp(crl.RevokedCertificates[i].SerialNumber) == 0 {
return fmt.Errorf("certificate %q was revoked", cert.Subject)
}
}
}
if anyIssuerHasCRL && lastIssuerWithoutCRL != nil {
return fmt.Errorf("no CRL provided for issuer %q", lastIssuerWithoutCRL.Subject)
}
return nil
}
@ -120,25 +143,6 @@ func parseCertificate(pemStr string) (*x509.Certificate, error) {
return x509.ParseCertificate(block.Bytes)
}
func parseCertificates(certs []byte) (map[string]*x509.Certificate, error) {
m := make(map[string]*x509.Certificate)
for {
var block *pem.Block
block, certs = pem.Decode(certs)
if block == nil {
return m, nil
}
if block.Type != "CERTIFICATE" {
continue
}
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return nil, err
}
m[string(cert.RawSubject)] = cert
}
}
func parseCRLs(crl []byte) (map[string]*x509.RevocationList, error) {
m := make(map[string]*x509.RevocationList)
for {