diff --git a/authorize/evaluator/functions.go b/authorize/evaluator/functions.go index 0a4e37311..10ab2f863 100644 --- a/authorize/evaluator/functions.go +++ b/authorize/evaluator/functions.go @@ -175,38 +175,29 @@ func validateClientCertificateChain( return err } - // 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 + // Consult CRLs only for the first CA in the chain, to match Envoy's + // behavior when the only_verify_leaf_cert_crl option is set (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 len(chain) < 2 { + return nil + } + cert, issuer := chain[0], chain[1] + crl := crls[string(issuer.RawSubject)] + if crl == nil { + return nil } - if anyIssuerHasCRL && lastIssuerWithoutCRL != nil { - return fmt.Errorf("no CRL provided for issuer %q", lastIssuerWithoutCRL.Subject) + // 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) + } } return nil diff --git a/authorize/evaluator/functions_test.go b/authorize/evaluator/functions_test.go index ffb0cb781..05517c91b 100644 --- a/authorize/evaluator/functions_test.go +++ b/authorize/evaluator/functions_test.go @@ -238,16 +238,6 @@ func Test_isValidClientCertificate(t *testing.T) { assert.NoError(t, err, "should not return an error") assert.True(t, valid, "should return true") }) - t.Run("missing CRL", func(t *testing.T) { - // If a CRL is provided for any CA, it must be provided for all CAs. - valid, err := isValidClientCertificate(testCA, testCRL, ClientCertificateInfo{ - Presented: true, - Leaf: testValidIntermediateCert, - Intermediates: testIntermediateCA, - }, noConstraints) - assert.NoError(t, err, "should not return an error") - assert.False(t, valid, "should return false") - }) t.Run("chain too deep", func(t *testing.T) { valid, err := isValidClientCertificate(testCA, "", ClientCertificateInfo{ Presented: true, diff --git a/config/envoyconfig/listeners.go b/config/envoyconfig/listeners.go index 0e8f5e9f6..2e807340d 100644 --- a/config/envoyconfig/listeners.go +++ b/config/envoyconfig/listeners.go @@ -553,6 +553,7 @@ func (b *Builder) buildDownstreamValidationContext( TrustedCa: b.filemgr.BytesDataSource("client-ca.pem", clientCA), MatchTypedSubjectAltNames: make([]*envoy_extensions_transport_sockets_tls_v3.SubjectAltNameMatcher, 0, len(cfg.Options.DownstreamMTLS.MatchSubjectAltNames)), + OnlyVerifyLeafCertCrl: true, } for i := range cfg.Options.DownstreamMTLS.MatchSubjectAltNames { vc.MatchTypedSubjectAltNames = append(vc.MatchTypedSubjectAltNames, diff --git a/config/envoyconfig/listeners_test.go b/config/envoyconfig/listeners_test.go index 17de2815d..f12b736d1 100644 --- a/config/envoyconfig/listeners_test.go +++ b/config/envoyconfig/listeners_test.go @@ -119,6 +119,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { "alpnProtocols": ["h2", "http/1.1"], "validationContext": { "maxVerifyDepth": 1, + "onlyVerifyLeafCertCrl": true, "trustChainVerification": "ACCEPT_UNTRUSTED", "trustedCa": { "filename": "`+clientCAFileName+`" @@ -151,6 +152,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { "alpnProtocols": ["h2", "http/1.1"], "validationContext": { "maxVerifyDepth": 1, + "onlyVerifyLeafCertCrl": true, "trustedCa": { "filename": "`+clientCAFileName+`" } @@ -186,6 +188,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { "alpnProtocols": ["h2", "http/1.1"], "validationContext": { "maxVerifyDepth": 1, + "onlyVerifyLeafCertCrl": true, "trustChainVerification": "ACCEPT_UNTRUSTED", "trustedCa": { "filename": "`+clientCAFileName+`" @@ -209,6 +212,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { require.NoError(t, err) testutil.AssertProtoJSONEqual(t, `{ "maxVerifyDepth": 10, + "onlyVerifyLeafCertCrl": true, "trustChainVerification": "ACCEPT_UNTRUSTED", "trustedCa": { "filename": "`+clientCAFileName+`" @@ -220,6 +224,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { b.buildDownstreamTLSContextMulti(context.Background(), config, nil) require.NoError(t, err) testutil.AssertProtoJSONEqual(t, `{ + "onlyVerifyLeafCertCrl": true, "trustChainVerification": "ACCEPT_UNTRUSTED", "trustedCa": { "filename": "`+clientCAFileName+`" @@ -281,6 +286,7 @@ func Test_buildDownstreamTLSContext(t *testing.T) { "sanType": "URI" } ], + "onlyVerifyLeafCertCrl": true, "trustChainVerification": "ACCEPT_UNTRUSTED", "trustedCa": { "filename": "`+clientCAFileName+`"