advancedTLS: Swap to DenyUndetermined from AllowUndetermined in revocation settings (#7179)
* swap to `DenyUndetermined` from `AllowUndetermined`
diff --git a/security/advancedtls/advancedtls_test.go b/security/advancedtls/advancedtls_test.go
index 7b1ee7f..6b0054f 100644
--- a/security/advancedtls/advancedtls_test.go
+++ b/security/advancedtls/advancedtls_test.go
@@ -427,15 +427,15 @@
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 @@
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 @@
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 fc01fa9..bf26160 100644
--- a/security/advancedtls/crl.go
+++ b/security/advancedtls/crl.go
@@ -61,8 +61,13 @@
// 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 @@
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 7ed985e..9cb53d8 100644
--- a/security/advancedtls/crl_test.go
+++ b/security/advancedtls/crl_test.go
@@ -546,10 +546,10 @@
}
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 @@
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 @@
})
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 {
