Proceed with cert auth login attempts if ocsp_fail_open is true and servers are unreachable (#25982)

This commit is contained in:
Steven Clark
2024-03-19 10:39:37 -04:00
committed by GitHub
parent 846476e857
commit 6fca34eace
7 changed files with 481 additions and 6 deletions

View File

@@ -26,8 +26,9 @@ const (
operationPrefixCert = "cert" operationPrefixCert = "cert"
trustedCertPath = "cert/" trustedCertPath = "cert/"
defaultRoleCacheSize = 200 defaultRoleCacheSize = 200
maxRoleCacheSize = 10000 defaultOcspMaxRetries = 4
maxRoleCacheSize = 10000
) )
func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) { func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, error) {

View File

@@ -5,6 +5,7 @@ package cert
import ( import (
"context" "context"
"crypto"
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/elliptic"
"crypto/rand" "crypto/rand"
@@ -20,6 +21,7 @@ import (
mathrand "math/rand" mathrand "math/rand"
"net" "net"
"net/http" "net/http"
"net/http/httptest"
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
@@ -28,8 +30,8 @@ import (
"time" "time"
"github.com/go-test/deep" "github.com/go-test/deep"
cleanhttp "github.com/hashicorp/go-cleanhttp" "github.com/hashicorp/go-cleanhttp"
rootcerts "github.com/hashicorp/go-rootcerts" "github.com/hashicorp/go-rootcerts"
"github.com/hashicorp/go-sockaddr" "github.com/hashicorp/go-sockaddr"
"github.com/hashicorp/vault/api" "github.com/hashicorp/vault/api"
"github.com/hashicorp/vault/builtin/logical/pki" "github.com/hashicorp/vault/builtin/logical/pki"
@@ -41,6 +43,8 @@ import (
"github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault" "github.com/hashicorp/vault/vault"
"github.com/mitchellh/mapstructure" "github.com/mitchellh/mapstructure"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ocsp"
"golang.org/x/net/http2" "golang.org/x/net/http2"
) )
@@ -2057,6 +2061,11 @@ func testConnState(certPath, keyPath, rootCertPath string) (tls.ConnectionState,
if err != nil { if err != nil {
return tls.ConnectionState{}, err return tls.ConnectionState{}, err
} }
return testConnStateWithCert(cert, rootCAs)
}
func testConnStateWithCert(cert tls.Certificate, rootCAs *x509.CertPool) (tls.ConnectionState, error) {
listenConf := &tls.Config{ listenConf := &tls.Config{
Certificates: []tls.Certificate{cert}, Certificates: []tls.Certificate{cert},
ClientAuth: tls.RequestClientCert, ClientAuth: tls.RequestClientCert,
@@ -2336,3 +2345,409 @@ func TestBackend_CertUpgrade(t *testing.T) {
t.Fatal(diff) t.Fatal(diff)
} }
} }
// TestOCSPWithMixedValidResponses validates the expected behavior of multiple OCSP servers configured,
// with and without ocsp_query_all_servers enabled or disabled.
func TestOCSPWithMixedValidResponses(t *testing.T) {
caFile := "test-fixtures/root/rootcacert.pem"
pemCa, err := os.ReadFile(caFile)
require.NoError(t, err, "failed reading in file %s", caFile)
caTLS := loadCerts(t, caFile, "test-fixtures/root/rootcakey.pem")
leafTLS := loadCerts(t, "test-fixtures/keys/cert.pem", "test-fixtures/keys/key.pem")
rootConfig := &rootcerts.Config{
CAFile: caFile,
}
rootCAs, err := rootcerts.LoadCACerts(rootConfig)
connState, err := testConnStateWithCert(leafTLS, rootCAs)
require.NoError(t, err, "error testing connection state: %v", err)
// Setup an OCSP handler
ocspHandler := func(status int) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
now := time.Now()
ocspRes := ocsp.Response{
SerialNumber: leafTLS.Leaf.SerialNumber,
ThisUpdate: now.Add(-1 * time.Hour),
NextUpdate: now.Add(30 * time.Minute),
Status: status,
}
response, err := ocsp.CreateResponse(caTLS.Leaf, caTLS.Leaf, ocspRes, caTLS.PrivateKey.(crypto.Signer))
if err != nil {
t.Fatalf("failed generating OCSP response: %v", err)
}
_, _ = w.Write(response)
})
}
goodTs := httptest.NewServer(ocspHandler(ocsp.Good))
revokeTs := httptest.NewServer(ocspHandler(ocsp.Revoked))
defer goodTs.Close()
defer revokeTs.Close()
steps := []logicaltest.TestStep{
// step 1/2: This should pass as we will query the first server and get a valid good response, not testing
// the second configured server
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{goodTs.URL, revokeTs.URL},
"ocsp_query_all_servers": false,
}),
testAccStepLogin(t, connState),
// step 3/4: This should fail as we will query the revoking OCSP server first and get a revoke response
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{revokeTs.URL, goodTs.URL},
"ocsp_query_all_servers": false,
}),
testAccStepLoginInvalid(t, connState),
}
// Setup a new factory everytime to avoid OCSP caching from influencing the test
for i := 0; i < len(steps); i += 2 {
setup := i
execute := i + 1
t.Run(fmt.Sprintf("steps-%d-%d", setup+1, execute+1), func(t *testing.T) {
logicaltest.Test(t, logicaltest.TestCase{
CredentialBackend: testFactory(t),
Steps: []logicaltest.TestStep{steps[setup], steps[execute]},
})
})
}
}
// TestOCSPFailOpenWithGoodResponse validates the expected behavior with multiple OCSP servers configured
// one that returns a Good response the other is not available, along with the ocsp_fail_open in multiple modes
func TestOCSPFailOpenWithGoodResponse(t *testing.T) {
caFile := "test-fixtures/root/rootcacert.pem"
pemCa, err := os.ReadFile(caFile)
require.NoError(t, err, "failed reading in file %s", caFile)
caTLS := loadCerts(t, caFile, "test-fixtures/root/rootcakey.pem")
leafTLS := loadCerts(t, "test-fixtures/keys/cert.pem", "test-fixtures/keys/key.pem")
rootConfig := &rootcerts.Config{
CAFile: caFile,
}
rootCAs, err := rootcerts.LoadCACerts(rootConfig)
connState, err := testConnStateWithCert(leafTLS, rootCAs)
require.NoError(t, err, "error testing connection state: %v", err)
// Setup an OCSP handler
ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
now := time.Now()
ocspRes := ocsp.Response{
SerialNumber: leafTLS.Leaf.SerialNumber,
ThisUpdate: now.Add(-1 * time.Hour),
NextUpdate: now.Add(30 * time.Minute),
Status: ocsp.Good,
}
response, err := ocsp.CreateResponse(caTLS.Leaf, caTLS.Leaf, ocspRes, caTLS.PrivateKey.(crypto.Signer))
if err != nil {
t.Fatalf("failed generating OCSP response: %v", err)
}
_, _ = w.Write(response)
})
ts := httptest.NewServer(ocspHandler)
defer ts.Close()
steps := []logicaltest.TestStep{
// Step 1/2 With no proper responses from any OCSP server and fail_open to true, we should pass validation
// as fail_open is true
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{"http://127.0.0.1:30000", "http://127.0.0.1:30001"},
"ocsp_fail_open": true,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLogin(t, connState),
// Step 3/4 With no proper responses from any OCSP server and fail_open to false we should fail validation
// as fail_open is false
testAccStepCertWithExtraParams(t, "web", pemCa, "foo",
allowed{names: "cert.example.com"}, false, map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{"http://127.0.0.1:30000", "http://127.0.0.1:30001"},
"ocsp_fail_open": false,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLoginInvalid(t, connState),
// Step 5/6 With a single positive response, query all servers set to false and fail open true, pass validation
// as query all servers is false
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": true,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLogin(t, connState),
// Step 7/8 With a single positive response, query all servers set to false and fail open false, pass validation
// as query all servers is false
testAccStepCertWithExtraParams(t, "web", pemCa, "foo",
allowed{names: "cert.example.com"}, false, map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": false,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLogin(t, connState),
}
// Setup a new factory everytime to avoid OCSP caching from influencing the test
for i := 0; i < len(steps); i += 2 {
setup := i
execute := i + 1
t.Run(fmt.Sprintf("steps-%d-%d", setup+1, execute+1), func(t *testing.T) {
logicaltest.Test(t, logicaltest.TestCase{
CredentialBackend: testFactory(t),
Steps: []logicaltest.TestStep{steps[setup], steps[execute]},
})
})
}
}
// TestOCSPFailOpenWithRevokeResponse validates the expected behavior with multiple OCSP servers configured
// one that returns a Revoke response the other is not available, along with the ocsp_fail_open in multiple modes
func TestOCSPFailOpenWithRevokeResponse(t *testing.T) {
caFile := "test-fixtures/root/rootcacert.pem"
pemCa, err := os.ReadFile(caFile)
require.NoError(t, err, "failed reading in file %s", caFile)
caTLS := loadCerts(t, caFile, "test-fixtures/root/rootcakey.pem")
leafTLS := loadCerts(t, "test-fixtures/keys/cert.pem", "test-fixtures/keys/key.pem")
rootConfig := &rootcerts.Config{
CAFile: caFile,
}
rootCAs, err := rootcerts.LoadCACerts(rootConfig)
connState, err := testConnStateWithCert(leafTLS, rootCAs)
require.NoError(t, err, "error testing connection state: %v", err)
// Setup an OCSP handler
ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
now := time.Now()
ocspRes := ocsp.Response{
SerialNumber: leafTLS.Leaf.SerialNumber,
ThisUpdate: now.Add(-1 * time.Hour),
NextUpdate: now.Add(30 * time.Minute),
Status: ocsp.Revoked,
}
response, err := ocsp.CreateResponse(caTLS.Leaf, caTLS.Leaf, ocspRes, caTLS.PrivateKey.(crypto.Signer))
if err != nil {
t.Fatalf("failed generating OCSP response: %v", err)
}
_, _ = w.Write(response)
})
ts := httptest.NewServer(ocspHandler)
defer ts.Close()
// With no OCSP servers available, make sure that we behave as we expect
steps := []logicaltest.TestStep{
// Step 1/2 With a single revoke response, query all servers set to false and fail open true, fail validation
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": true,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLoginInvalid(t, connState),
// Step 3/4 With a single revoke response, query all servers set to false and fail open false, fail validation
testAccStepCertWithExtraParams(t, "web", pemCa, "foo",
allowed{names: "cert.example.com"}, false, map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": false,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLoginInvalid(t, connState),
}
// Setup a new factory everytime to avoid OCSP caching from influencing the test
for i := 0; i < len(steps); i += 2 {
setup := i
execute := i + 1
t.Run(fmt.Sprintf("steps-%d-%d", setup+1, execute+1), func(t *testing.T) {
logicaltest.Test(t, logicaltest.TestCase{
CredentialBackend: testFactory(t),
Steps: []logicaltest.TestStep{steps[setup], steps[execute]},
})
})
}
}
// TestOCSPFailOpenWithUnknownResponse validates the expected behavior with multiple OCSP servers configured
// one that returns an Unknown response the other is not available, along with the ocsp_fail_open in multiple modes
func TestOCSPFailOpenWithUnknownResponse(t *testing.T) {
caFile := "test-fixtures/root/rootcacert.pem"
pemCa, err := os.ReadFile(caFile)
require.NoError(t, err, "failed reading in file %s", caFile)
caTLS := loadCerts(t, caFile, "test-fixtures/root/rootcakey.pem")
leafTLS := loadCerts(t, "test-fixtures/keys/cert.pem", "test-fixtures/keys/key.pem")
rootConfig := &rootcerts.Config{
CAFile: caFile,
}
rootCAs, err := rootcerts.LoadCACerts(rootConfig)
connState, err := testConnStateWithCert(leafTLS, rootCAs)
require.NoError(t, err, "error testing connection state: %v", err)
// Setup an OCSP handler
ocspHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
now := time.Now()
ocspRes := ocsp.Response{
SerialNumber: leafTLS.Leaf.SerialNumber,
ThisUpdate: now.Add(-1 * time.Hour),
NextUpdate: now.Add(30 * time.Minute),
Status: ocsp.Unknown,
}
response, err := ocsp.CreateResponse(caTLS.Leaf, caTLS.Leaf, ocspRes, caTLS.PrivateKey.(crypto.Signer))
if err != nil {
t.Fatalf("failed generating OCSP response: %v", err)
}
_, _ = w.Write(response)
})
ts := httptest.NewServer(ocspHandler)
defer ts.Close()
// With no OCSP servers available, make sure that we behave as we expect
steps := []logicaltest.TestStep{
// Step 1/2 With a single unknown response, query all servers set to false and fail open true, pass validation
testAccStepCertWithExtraParams(t, "web", pemCa, "foo", allowed{names: "cert.example.com"}, false,
map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": true,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLogin(t, connState),
// Step 3/4 With a single unknown response, query all servers set to false and fail open false, fail validation
testAccStepCertWithExtraParams(t, "web", pemCa, "foo",
allowed{names: "cert.example.com"}, false, map[string]interface{}{
"ocsp_enabled": true,
"ocsp_servers_override": []string{ts.URL, "http://127.0.0.1:30001"},
"ocsp_fail_open": false,
"ocsp_query_all_servers": false,
"ocsp_max_retries": 0,
}),
testAccStepLoginInvalid(t, connState),
}
// Setup a new factory everytime to avoid OCSP caching from influencing the test
for i := 0; i < len(steps); i += 2 {
setup := i
execute := i + 1
t.Run(fmt.Sprintf("steps-%d-%d", setup+1, execute+1), func(t *testing.T) {
logicaltest.Test(t, logicaltest.TestCase{
CredentialBackend: testFactory(t),
Steps: []logicaltest.TestStep{steps[setup], steps[execute]},
})
})
}
}
// TestOcspMaxRetriesUpdate verifies that the ocsp_max_retries field is properly initialized
// with our default value of 4, legacy roles have it initialized automatically to 4 and we
// can properly store and retrieve updates to the field.
func TestOcspMaxRetriesUpdate(t *testing.T) {
storage := &logical.InmemStorage{}
ctx := context.Background()
lb, err := Factory(context.Background(), &logical.BackendConfig{
System: &logical.StaticSystemView{
DefaultLeaseTTLVal: 300 * time.Second,
MaxLeaseTTLVal: 1800 * time.Second,
},
StorageView: storage,
})
require.NoError(t, err, "failed creating backend")
caFile := "test-fixtures/root/rootcacert.pem"
pemCa, err := os.ReadFile(caFile)
require.NoError(t, err, "failed reading in file %s", caFile)
data := map[string]interface{}{
"certificate": string(pemCa),
"display_name": "test",
}
// Test initial creation of role sets ocsp_max_retries to a default of 4
_, err = lb.HandleRequest(ctx, &logical.Request{
Operation: logical.UpdateOperation,
Path: "certs/test",
Data: data,
Storage: storage,
})
require.NoError(t, err, "failed initial role creation request")
resp, err := lb.HandleRequest(ctx, &logical.Request{
Operation: logical.ReadOperation,
Path: "certs/test",
Storage: storage,
})
require.NoError(t, err, "failed reading role request")
require.NotNil(t, resp)
require.Equal(t, 4, resp.Data["ocsp_max_retries"], "ocsp config didn't match expectations")
// Test we can update the field and read it back
data["ocsp_max_retries"] = 1
_, err = lb.HandleRequest(ctx, &logical.Request{
Operation: logical.UpdateOperation,
Path: "certs/test",
Data: data,
Storage: storage,
})
require.NoError(t, err, "failed updating role request")
resp, err = lb.HandleRequest(ctx, &logical.Request{
Operation: logical.ReadOperation,
Path: "certs/test",
Storage: storage,
})
require.NoError(t, err, "failed reading role request")
require.NotNil(t, resp)
require.Equal(t, 1, resp.Data["ocsp_max_retries"], "ocsp config didn't match expectations on update")
// Verify existing storage entries get updated with a value of 4
entry := &logical.StorageEntry{
Key: "cert/legacy",
Value: []byte(`{"token_bound_cidrs":null,"token_explicit_max_ttl":0,"token_max_ttl":0,
"token_no_default_policy":false,"token_num_uses":0,"token_period":0,
"token_policies":null,"token_type":0,"token_ttl":0,"Name":"test",
"Certificate":"-----BEGIN CERTIFICATE-----\nMIIDPDCCAiSgAwIBAgIUb5id+GcaMeMnYBv3MvdTGWigyJ0wDQYJKoZIhvcNAQEL\nBQAwFjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wHhcNMTYwMjI5MDIyNzI5WhcNMjYw\nMjI2MDIyNzU5WjAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTCCASIwDQYJKoZIhvcN\nAQEBBQADggEPADCCAQoCggEBAOxTMvhTuIRc2YhxZpmPwegP86cgnqfT1mXxi1A7\nQ7qax24Nqbf00I3oDMQtAJlj2RB3hvRSCb0/lkF7i1Bub+TGxuM7NtZqp2F8FgG0\nz2md+W6adwW26rlxbQKjmRvMn66G9YPTkoJmPmxt2Tccb9+apmwW7lslL5j8H48x\nAHJTMb+PMP9kbOHV5Abr3PT4jXUPUr/mWBvBiKiHG0Xd/HEmlyOEPeAThxK+I5tb\n6m+eB+7cL9BsvQpy135+2bRAxUphvFi5NhryJ2vlAvoJ8UqigsNK3E28ut60FAoH\nSWRfFUFFYtfPgTDS1yOKU/z/XMU2giQv2HrleWt0mp4jqBUCAwEAAaOBgTB/MA4G\nA1UdDwEB/wQEAwIBBjAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBSdxLNP/ocx\n7HK6JT3/sSAe76iTmzAfBgNVHSMEGDAWgBSdxLNP/ocx7HK6JT3/sSAe76iTmzAc\nBgNVHREEFTATggtleGFtcGxlLmNvbYcEfwAAATANBgkqhkiG9w0BAQsFAAOCAQEA\nwHThDRsXJunKbAapxmQ6bDxSvTvkLA6m97TXlsFgL+Q3Jrg9HoJCNowJ0pUTwhP2\nU946dCnSCkZck0fqkwVi4vJ5EQnkvyEbfN4W5qVsQKOFaFVzep6Qid4rZT6owWPa\ncNNzNcXAee3/j6hgr6OQ/i3J6fYR4YouYxYkjojYyg+CMdn6q8BoV0BTsHdnw1/N\nScbnBHQIvIZMBDAmQueQZolgJcdOuBLYHe/kRy167z8nGg+PUFKIYOL8NaOU1+CJ\nt2YaEibVq5MRqCbRgnd9a2vG0jr5a3Mn4CUUYv+5qIjP3hUusYenW1/EWtn1s/gk\nzehNe5dFTjFpylg1o6b8Ow==\n-----END CERTIFICATE-----\n",
"DisplayName":"test","Policies":null,"TTL":0,"MaxTTL":0,"Period":0,
"AllowedNames":null,"AllowedCommonNames":null,"AllowedDNSSANs":null,
"AllowedEmailSANs":null,"AllowedURISANs":null,"AllowedOrganizationalUnits":null,
"RequiredExtensions":null,"AllowedMetadataExtensions":null,"BoundCIDRs":null,
"OcspCaCertificates":"","OcspEnabled":false,"OcspServersOverride":null,
"OcspFailOpen":false,"OcspQueryAllServers":false}`),
}
err = storage.Put(ctx, entry)
require.NoError(t, err, "failed putting legacy storage entry")
resp, err = lb.HandleRequest(ctx, &logical.Request{
Operation: logical.ReadOperation,
Path: "certs/legacy",
Storage: storage,
})
require.NoError(t, err, "failed reading role request")
require.NotNil(t, resp)
require.Equal(t, 4, resp.Data["ocsp_max_retries"], "ocsp config didn't match expectations on legacy entry")
}
func loadCerts(t *testing.T, certFile, certKey string) tls.Certificate {
caTLS, err := tls.LoadX509KeyPair(certFile, certKey)
require.NoError(t, err, "failed reading ca/key files")
caTLS.Leaf, err = x509.ParseCertificate(caTLS.Certificate[0])
require.NoError(t, err, "failed parsing certificate from file %s", certFile)
return caTLS
}

View File

@@ -96,6 +96,11 @@ from the AuthorityInformationAccess extension on the certificate being inspected
Default: 0, Default: 0,
Description: "If greater than 0, specifies the maximum age of an OCSP thisUpdate field to avoid accepting old responses without a nextUpdate field.", Description: "If greater than 0, specifies the maximum age of an OCSP thisUpdate field to avoid accepting old responses without a nextUpdate field.",
}, },
"ocsp_max_retries": {
Type: framework.TypeInt,
Default: 4,
Description: "The number of retries the OCSP client should attempt per query.",
},
"allowed_names": { "allowed_names": {
Type: framework.TypeCommaStringSlice, Type: framework.TypeCommaStringSlice,
Description: `A comma-separated list of names. Description: `A comma-separated list of names.
@@ -248,7 +253,7 @@ func (b *backend) Cert(ctx context.Context, s logical.Storage, n string) (*CertE
return nil, nil return nil, nil
} }
var result CertEntry result := CertEntry{OcspMaxRetries: defaultOcspMaxRetries} // Specify our defaults if the key is missing
if err := entry.DecodeJSON(&result); err != nil { if err := entry.DecodeJSON(&result); err != nil {
return nil, err return nil, err
} }
@@ -315,6 +320,7 @@ func (b *backend) pathCertRead(ctx context.Context, req *logical.Request, d *fra
"ocsp_fail_open": cert.OcspFailOpen, "ocsp_fail_open": cert.OcspFailOpen,
"ocsp_query_all_servers": cert.OcspQueryAllServers, "ocsp_query_all_servers": cert.OcspQueryAllServers,
"ocsp_this_update_max_age": int64(cert.OcspThisUpdateMaxAge.Seconds()), "ocsp_this_update_max_age": int64(cert.OcspThisUpdateMaxAge.Seconds()),
"ocsp_max_retries": cert.OcspMaxRetries,
} }
cert.PopulateTokenData(data) cert.PopulateTokenData(data)
@@ -350,7 +356,8 @@ func (b *backend) pathCertWrite(ctx context.Context, req *logical.Request, d *fr
if cert == nil { if cert == nil {
cert = &CertEntry{ cert = &CertEntry{
Name: name, Name: name,
OcspMaxRetries: defaultOcspMaxRetries,
} }
} }
@@ -380,6 +387,12 @@ func (b *backend) pathCertWrite(ctx context.Context, req *logical.Request, d *fr
} }
cert.OcspThisUpdateMaxAge = maxAgeDuration cert.OcspThisUpdateMaxAge = maxAgeDuration
} }
if ocspMaxRetries, ok := d.GetOk("ocsp_max_retries"); ok {
cert.OcspMaxRetries = ocspMaxRetries.(int)
if cert.OcspMaxRetries < 0 {
return nil, fmt.Errorf("ocsp_max_retries can not be a negative number")
}
}
if displayNameRaw, ok := d.GetOk("display_name"); ok { if displayNameRaw, ok := d.GetOk("display_name"); ok {
cert.DisplayName = displayNameRaw.(string) cert.DisplayName = displayNameRaw.(string)
} }
@@ -531,6 +544,7 @@ type CertEntry struct {
OcspFailOpen bool OcspFailOpen bool
OcspQueryAllServers bool OcspQueryAllServers bool
OcspThisUpdateMaxAge time.Duration OcspThisUpdateMaxAge time.Duration
OcspMaxRetries int
} }
const pathCertHelpSyn = ` const pathCertHelpSyn = `

View File

@@ -14,6 +14,7 @@ import (
"encoding/pem" "encoding/pem"
"errors" "errors"
"fmt" "fmt"
"net/url"
"strings" "strings"
"github.com/hashicorp/errwrap" "github.com/hashicorp/errwrap"
@@ -663,6 +664,7 @@ func (b *backend) loadTrustedCerts(ctx context.Context, storage logical.Storage,
} }
conf.QueryAllServers = conf.QueryAllServers || entry.OcspQueryAllServers conf.QueryAllServers = conf.QueryAllServers || entry.OcspQueryAllServers
conf.OcspThisUpdateMaxAge = entry.OcspThisUpdateMaxAge conf.OcspThisUpdateMaxAge = entry.OcspThisUpdateMaxAge
conf.OcspMaxRetries = entry.OcspMaxRetries
} }
} }
@@ -685,6 +687,12 @@ func (b *backend) checkForCertInOCSP(ctx context.Context, clientCert *x509.Certi
defer b.ocspClientMutex.RUnlock() defer b.ocspClientMutex.RUnlock()
err := b.ocspClient.VerifyLeafCertificate(ctx, clientCert, chain[1], conf) err := b.ocspClient.VerifyLeafCertificate(ctx, clientCert, chain[1], conf)
if err != nil { if err != nil {
if conf.OcspFailureMode == ocsp.FailOpenTrue {
onlyNetworkErrors := b.handleOcspErrorInFailOpen(err)
if onlyNetworkErrors {
return true, nil
}
}
// We want to preserve error messages when they have additional, // We want to preserve error messages when they have additional,
// potentially useful information. Just having a revoked cert // potentially useful information. Just having a revoked cert
// isn't additionally useful. // isn't additionally useful.
@@ -696,6 +704,28 @@ func (b *backend) checkForCertInOCSP(ctx context.Context, clientCert *x509.Certi
return true, nil return true, nil
} }
func (b *backend) handleOcspErrorInFailOpen(err error) bool {
urlError := &url.Error{}
allNetworkErrors := true
if multiError, ok := err.(*multierror.Error); ok {
for _, myErr := range multiError.Errors {
if !errors.As(myErr, &urlError) {
allNetworkErrors = false
}
}
} else if !errors.As(err, &urlError) {
allNetworkErrors = false
}
if allNetworkErrors {
b.Logger().Warn("OCSP is set to fail-open, and could not retrieve "+
"OCSP based revocation but proceeding.", "detail", err)
return true
}
return false
}
func (b *backend) checkForChainInCRLs(chain []*x509.Certificate) bool { func (b *backend) checkForChainInCRLs(chain []*x509.Certificate) bool {
badChain := false badChain := false
for _, cert := range chain { for _, cert := range chain {

3
changelog/25982.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
auth/cert: Allow cert auth login attempts if ocsp_fail_open is true and OCSP servers are unreachable
```

View File

@@ -516,6 +516,7 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509.
timeout := defaultOCSPResponderTimeout timeout := defaultOCSPResponderTimeout
ocspClient := retryablehttp.NewClient() ocspClient := retryablehttp.NewClient()
ocspClient.RetryMax = conf.OcspMaxRetries
ocspClient.HTTPClient.Timeout = timeout ocspClient.HTTPClient.Timeout = timeout
ocspClient.HTTPClient.Transport = newInsecureOcspTransport(conf.ExtraCas) ocspClient.HTTPClient.Transport = newInsecureOcspTransport(conf.ExtraCas)
@@ -594,6 +595,14 @@ func (c *Client) GetRevocationStatus(ctx context.Context, subject, issuer *x509.
} }
} }
// If querying all servers is enabled, and we have an error from a host, we can't trust
// a good status from the other as we can't confirm the other server would have returned the
// same response, we do allow revoke responses through
if conf.QueryAllServers && firstError != nil && (ret != nil && ret.code == ocspStatusGood) {
return nil, fmt.Errorf("encountered an error on a server, "+
"ignoring good response status as ocsp_query_all_servers is set to true: %w", firstError)
}
// If no server reported the cert revoked, but we did have an error, report it // If no server reported the cert revoked, but we did have an error, report it
if (ret == nil || ret.code == ocspStatusUnknown) && firstError != nil { if (ret == nil || ret.code == ocspStatusUnknown) && firstError != nil {
return nil, firstError return nil, firstError
@@ -639,6 +648,7 @@ type VerifyConfig struct {
OcspFailureMode FailOpenMode OcspFailureMode FailOpenMode
QueryAllServers bool QueryAllServers bool
OcspThisUpdateMaxAge time.Duration OcspThisUpdateMaxAge time.Duration
OcspMaxRetries int
} }
// VerifyLeafCertificate verifies just the subject against it's direct issuer // VerifyLeafCertificate verifies just the subject against it's direct issuer

View File

@@ -80,6 +80,8 @@ Sets a CA cert and associated parameters in a role name.
- `ocsp_this_update_max_age` `(integer:0 or string: "")` - If greater than 0, specifies - `ocsp_this_update_max_age` `(integer:0 or string: "")` - If greater than 0, specifies
the maximum age of an OCSP thisUpdate field. This avoids accepting old responses the maximum age of an OCSP thisUpdate field. This avoids accepting old responses
without a nextUpdate field. without a nextUpdate field.
- `ocsp_max_retries` `(integer: 4)` - The number of retries attempted before giving
up on an OCSP request. 0 will disable retries
- `ocsp_query_all_servers` `(bool: false)` - If set to true, rather than accepting - `ocsp_query_all_servers` `(bool: false)` - If set to true, rather than accepting
the first successful OCSP response, query all servers and consider the certificate the first successful OCSP response, query all servers and consider the certificate
valid only if all servers agree. valid only if all servers agree.