diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go index 7b1ee7fee..6b0054ffe 100644 --- a/security/advancedtls/advancedtls_test.go +++ b/security/advancedtls/advancedtls_test.go @@ -427,15 +427,15 @@ func (s) TestClientServerHandshake(t *testing.T) { return &GetRootCAsResults{TrustCerts: cs.ServerTrust3}, nil } - makeStaticCRLRevocationOptions := func(crlPath string, allowUndetermined bool) *RevocationOptions { + makeStaticCRLRevocationOptions := func(crlPath string, denyUndetermined bool) *RevocationOptions { rawCRL, err := os.ReadFile(crlPath) if err != nil { t.Fatalf("readFile(%v) failed err = %v", crlPath, err) } cRLProvider := NewStaticCRLProvider([][]byte{rawCRL}) return &RevocationOptions{ - AllowUndetermined: allowUndetermined, - CRLProvider: cRLProvider, + DenyUndetermined: denyUndetermined, + CRLProvider: cRLProvider, } } @@ -758,18 +758,18 @@ func (s) TestClientServerHandshake(t *testing.T) { clientVerifyFunc: clientVerifyFuncGood, clientVerificationType: CertVerification, clientRevocationOptions: &RevocationOptions{ - RootDir: testdata.Path("crl"), - AllowUndetermined: true, - Cache: cache, + RootDir: testdata.Path("crl"), + DenyUndetermined: false, + Cache: cache, }, serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCert1}, serverGetRoot: getRootCAsForServer, serverVerificationType: CertVerification, serverRevocationOptions: &RevocationOptions{ - RootDir: testdata.Path("crl"), - AllowUndetermined: true, - Cache: cache, + RootDir: testdata.Path("crl"), + DenyUndetermined: false, + Cache: cache, }, }, // Client: set valid credentials with the revocation config @@ -813,7 +813,7 @@ func (s) TestClientServerHandshake(t *testing.T) { clientGetRoot: getRootCAsForClientCRL, clientVerifyFunc: clientVerifyFuncGood, clientVerificationType: CertVerification, - clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), false), + clientRevocationOptions: makeStaticCRLRevocationOptions(testdata.Path("crl/provider_malicious_crl_empty.pem"), true), serverMutualTLS: true, serverCert: []tls.Certificate{cs.ServerCertForCRL}, serverGetRoot: getRootCAsForServerCRL, diff --git a/security/advancedtls/crl.go b/security/advancedtls/crl.go index fc01fa9e6..bf26160c7 100644 --- a/security/advancedtls/crl.go +++ b/security/advancedtls/crl.go @@ -61,8 +61,13 @@ type RevocationOptions struct { // Directory format must match OpenSSL X509_LOOKUP_hash_dir(3). // Deprecated: use CRLProvider instead. RootDir string + // DenyUndetermined controls if certificate chains with RevocationUndetermined + // revocation status are allowed to complete. + DenyUndetermined bool // AllowUndetermined controls if certificate chains with RevocationUndetermined // revocation status are allowed to complete. + // + // Deprecated: use DenyUndetermined instead AllowUndetermined bool // Cache will store CRL files if not nil, otherwise files are reloaded for every lookup. // Only used for caching CRLs when using the RootDir setting. @@ -222,11 +227,16 @@ func checkChainRevocation(verifiedChains [][]*x509.Certificate, cfg RevocationOp count[RevocationRevoked]++ continue case RevocationUndetermined: - if cfg.AllowUndetermined { - return nil - } count[RevocationUndetermined]++ - continue + // TODO(gtcooke94) Remove when deprecated AllowUndetermined is removed + // For now, if the deprecated value is explicitly set, use it + if cfg.AllowUndetermined { + cfg.DenyUndetermined = !cfg.AllowUndetermined + } + if cfg.DenyUndetermined { + continue + } + return nil } } return fmt.Errorf("no unrevoked chains found: %v", count) diff --git a/security/advancedtls/crl_test.go b/security/advancedtls/crl_test.go index 7ed985e51..9cb53d83f 100644 --- a/security/advancedtls/crl_test.go +++ b/security/advancedtls/crl_test.go @@ -546,10 +546,10 @@ func TestRevokedCert(t *testing.T) { } var revocationTests = []struct { - desc string - in tls.ConnectionState - revoked bool - allowUndetermined bool + desc string + in tls.ConnectionState + revoked bool + denyUndetermined bool }{ { desc: "Single unrevoked", @@ -586,24 +586,24 @@ func TestRevokedCert(t *testing.T) { in: tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{ {&x509.Certificate{CRLDistributionPoints: []string{"test"}}}, }}, - revoked: true, + revoked: true, + denyUndetermined: true, }, { desc: "Undetermined allowed", in: tls.ConnectionState{VerifiedChains: [][]*x509.Certificate{ {&x509.Certificate{CRLDistributionPoints: []string{"test"}}}, }}, - revoked: false, - allowUndetermined: true, + revoked: false, }, } for _, tt := range revocationTests { t.Run(fmt.Sprintf("%v with x509 crl hash dir", tt.desc), func(t *testing.T) { err := checkRevocation(tt.in, RevocationOptions{ - RootDir: testdata.Path("crl"), - AllowUndetermined: tt.allowUndetermined, - Cache: cache, + RootDir: testdata.Path("crl"), + DenyUndetermined: tt.denyUndetermined, + Cache: cache, }) t.Logf("checkRevocation err = %v", err) if tt.revoked && err == nil { @@ -614,8 +614,8 @@ func TestRevokedCert(t *testing.T) { }) t.Run(fmt.Sprintf("%v with static provider", tt.desc), func(t *testing.T) { err := checkRevocation(tt.in, RevocationOptions{ - AllowUndetermined: tt.allowUndetermined, - CRLProvider: cRLProvider, + DenyUndetermined: tt.denyUndetermined, + CRLProvider: cRLProvider, }) t.Logf("checkRevocation err = %v", err) if tt.revoked && err == nil {