From 7f38b0440ee0ef303d7cd84599a3fcce1191ee05 Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Fri, 16 Sep 2022 15:44:30 -0600 Subject: [PATCH] Fetch CRLs from a user defined URL (#17136) * Fetch CRLs from a user defined CDP (PoC) * Handle no param sent * Move CRL fetch to a periodFunc. Use configured CA certs + system root as trusted certs for CRL fetch * comments * changelog * Just use root trust * cdp->url in api * Store CRL and populate it initially in cdlWrite * Update docs * Update builtin/credential/cert/path_crls.go Co-authored-by: Steven Clark * Handle pre-verification of a CRL url better * just in case * Fix crl write locking * Add a CRL fetch unit test * Remove unnecessary validity clear * Better func name * Don't exit early updating CRLs * lock in updateCRLs * gofumpt * err- Co-authored-by: Steven Clark --- builtin/credential/cert/backend.go | 49 +++++- builtin/credential/cert/path_crls.go | 123 +++++++++++---- builtin/credential/cert/path_crls_test.go | 184 ++++++++++++++++++++++ builtin/credential/cert/path_login.go | 2 + changelog/17136.txt | 5 + website/content/docs/auth/cert.mdx | 6 +- 6 files changed, 326 insertions(+), 43 deletions(-) create mode 100644 builtin/credential/cert/path_crls_test.go create mode 100644 changelog/17136.txt diff --git a/builtin/credential/cert/backend.go b/builtin/credential/cert/backend.go index 121f7bea9..89625fbb4 100644 --- a/builtin/credential/cert/backend.go +++ b/builtin/credential/cert/backend.go @@ -2,9 +2,15 @@ package cert import ( "context" + "crypto/x509" + "fmt" + "io" + "net/http" "strings" "sync" + "time" + "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -14,7 +20,7 @@ func Factory(ctx context.Context, conf *logical.BackendConfig) (logical.Backend, if err := b.Setup(ctx, conf); err != nil { return nil, err } - if err := b.populateCRLs(ctx, conf.StorageView); err != nil { + if err := b.lockThenpopulateCRLs(ctx, conf.StorageView); err != nil { return nil, err } return b, nil @@ -36,9 +42,10 @@ func Backend() *backend { pathCerts(&b), pathCRLs(&b), }, - AuthRenew: b.pathLoginRenew, - Invalidate: b.invalidate, - BackendType: logical.TypeCredential, + AuthRenew: b.pathLoginRenew, + Invalidate: b.invalidate, + BackendType: logical.TypeCredential, + PeriodicFunc: b.updateCRLs, } b.crlUpdateMutex = &sync.RWMutex{} @@ -63,6 +70,40 @@ func (b *backend) invalidate(_ context.Context, key string) { } } +func (b *backend) fetchCRL(ctx context.Context, storage logical.Storage, name string, crl *CRLInfo) error { + response, err := http.Get(crl.CDP.Url) + if err != nil { + return err + } + if response.StatusCode == http.StatusOK { + body, err := io.ReadAll(response.Body) + if err != nil { + return err + } + certList, err := x509.ParseCRL(body) + if err != nil { + return err + } + crl.CDP.ValidUntil = certList.TBSCertList.NextUpdate + return b.setCRL(ctx, storage, certList, name, crl.CDP) + } + return fmt.Errorf("unexpected response code %d fetching CRL from %s", response.StatusCode, crl.CDP.Url) +} + +func (b *backend) updateCRLs(ctx context.Context, req *logical.Request) error { + b.crlUpdateMutex.Lock() + defer b.crlUpdateMutex.Unlock() + var errs *multierror.Error + for name, crl := range b.crls { + if crl.CDP != nil && time.Now().After(crl.CDP.ValidUntil) { + if err := b.fetchCRL(ctx, req.Storage, name, &crl); err != nil { + errs = multierror.Append(errs, err) + } + } + } + return errs.ErrorOrNil() +} + const backendHelp = ` The "cert" credential provider allows authentication using TLS client certificates. A client connects to Vault and uses diff --git a/builtin/credential/cert/path_crls.go b/builtin/credential/cert/path_crls.go index e031768a5..fc87671d8 100644 --- a/builtin/credential/cert/path_crls.go +++ b/builtin/credential/cert/path_crls.go @@ -3,9 +3,12 @@ package cert import ( "context" "crypto/x509" + "crypto/x509/pkix" "fmt" "math/big" + url2 "net/url" "strings" + "time" "github.com/fatih/structs" "github.com/hashicorp/vault/sdk/framework" @@ -24,11 +27,15 @@ func pathCRLs(b *backend) *framework.Path { "crl": { Type: framework.TypeString, - Description: `The public certificate that should be trusted. + Description: `The public CRL that should be trusted to attest to certificates' validity statuses. May be DER or PEM encoded. Note: the expiration time is ignored; if the CRL is no longer valid, delete it using the same name as specified here.`, }, + "url": { + Type: framework.TypeString, + Description: `The URL of a CRL distribution point. Only one of 'crl' or 'url' parameters should be specified.`, + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ @@ -42,10 +49,13 @@ using the same name as specified here.`, } } -func (b *backend) populateCRLs(ctx context.Context, storage logical.Storage) error { +func (b *backend) lockThenpopulateCRLs(ctx context.Context, storage logical.Storage) error { b.crlUpdateMutex.Lock() defer b.crlUpdateMutex.Unlock() + return b.populateCRLs(ctx, storage) +} +func (b *backend) populateCRLs(ctx context.Context, storage logical.Storage) error { if b.crls != nil { return nil } @@ -129,7 +139,7 @@ func (b *backend) pathCRLDelete(ctx context.Context, req *logical.Request, d *fr return logical.ErrorResponse(`"name" parameter cannot be empty`), nil } - if err := b.populateCRLs(ctx, req.Storage); err != nil { + if err := b.lockThenpopulateCRLs(ctx, req.Storage); err != nil { return nil, err } @@ -160,7 +170,7 @@ func (b *backend) pathCRLRead(ctx context.Context, req *logical.Request, d *fram return logical.ErrorResponse(`"name" parameter must be set`), nil } - if err := b.populateCRLs(ctx, req.Storage); err != nil { + if err := b.lockThenpopulateCRLs(ctx, req.Storage); err != nil { return nil, err } @@ -188,44 +198,86 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra if name == "" { return logical.ErrorResponse(`"name" parameter cannot be empty`), nil } - crl := d.Get("crl").(string) + if crlRaw, ok := d.GetOk("crl"); ok { + crl := crlRaw.(string) + certList, err := x509.ParseCRL([]byte(crl)) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("failed to parse CRL: %v", err)), nil + } + if certList == nil { + return logical.ErrorResponse("parsed CRL is nil"), nil + } - certList, err := x509.ParseCRL([]byte(crl)) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("failed to parse CRL: %v", err)), nil - } - if certList == nil { - return logical.ErrorResponse("parsed CRL is nil"), nil - } + b.crlUpdateMutex.Lock() + defer b.crlUpdateMutex.Unlock() + err = b.setCRL(ctx, req.Storage, certList, name, nil) + if err != nil { + return nil, err + } + } else if urlRaw, ok := d.GetOk("url"); ok { + url := urlRaw.(string) + if url == "" { + return logical.ErrorResponse("empty CRL url"), nil + } + _, err := url2.Parse(url) + if err != nil { + return logical.ErrorResponse("invalid CRL url: %v", err), nil + } - if err := b.populateCRLs(ctx, req.Storage); err != nil { - return nil, err - } + b.crlUpdateMutex.Lock() + defer b.crlUpdateMutex.Unlock() - b.crlUpdateMutex.Lock() - defer b.crlUpdateMutex.Unlock() - - crlInfo := CRLInfo{ - Serials: map[string]RevokedSerialInfo{}, + cdpInfo := &CDPInfo{ + Url: url, + } + err = b.fetchCRL(ctx, req.Storage, name, &CRLInfo{ + CDP: cdpInfo, + }) + if err != nil { + return nil, err + } + } else { + return logical.ErrorResponse("one of 'crl' or 'url' must be provided"), nil } - for _, revokedCert := range certList.TBSCertList.RevokedCertificates { - crlInfo.Serials[revokedCert.SerialNumber.String()] = RevokedSerialInfo{} - } - - entry, err := logical.StorageEntryJSON("crls/"+name, crlInfo) - if err != nil { - return nil, err - } - if err = req.Storage.Put(ctx, entry); err != nil { - return nil, err - } - - b.crls[name] = crlInfo return nil, nil } +func (b *backend) setCRL(ctx context.Context, storage logical.Storage, certList *pkix.CertificateList, name string, cdp *CDPInfo) error { + if err := b.populateCRLs(ctx, storage); err != nil { + return err + } + + crlInfo := CRLInfo{ + CDP: cdp, + Serials: map[string]RevokedSerialInfo{}, + } + + if certList != nil { + for _, revokedCert := range certList.TBSCertList.RevokedCertificates { + crlInfo.Serials[revokedCert.SerialNumber.String()] = RevokedSerialInfo{} + } + } + + entry, err := logical.StorageEntryJSON("crls/"+name, crlInfo) + if err != nil { + return err + } + if err = storage.Put(ctx, entry); err != nil { + return err + } + + b.crls[name] = crlInfo + return err +} + +type CDPInfo struct { + Url string `json:"url" structs:"url" mapstructure:"url"` + ValidUntil time.Time `json:"valid_until" structs:"valid_until" mapstructure:"valid_until"` +} + type CRLInfo struct { + CDP *CDPInfo `json:"cdp" structs:"cdp" mapstructure:"cdp"` Serials map[string]RevokedSerialInfo `json:"serials" structs:"serials" mapstructure:"serials"` } @@ -237,10 +289,11 @@ Manage Certificate Revocation Lists checked during authentication. const pathCRLsHelpDesc = ` This endpoint allows you to create, read, update, and delete the Certificate -Revocation Lists checked during authentication. +Revocation Lists checked during authentication, and/or CRL Distribution Point +URLs. When any CRLs are in effect, any login will check the trust chains sent by a -client against the submitted CRLs. Any chain containing a serial number revoked +client against the submitted or retrieved CRLs. Any chain containing a serial number revoked by one or more of the CRLs causes that chain to be marked as invalid for the authentication attempt. Conversely, *any* valid chain -- that is, a chain in which none of the serials are revoked by any CRL -- allows authentication. diff --git a/builtin/credential/cert/path_crls_test.go b/builtin/credential/cert/path_crls_test.go new file mode 100644 index 000000000..4b7955af7 --- /dev/null +++ b/builtin/credential/cert/path_crls_test.go @@ -0,0 +1,184 @@ +package cert + +import ( + "context" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "io/ioutil" + "math/big" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + "github.com/hashicorp/vault/sdk/framework" + "github.com/hashicorp/vault/sdk/helper/certutil" + "github.com/hashicorp/vault/sdk/logical" + "github.com/stretchr/testify/require" +) + +func TestCRLFetch(t *testing.T) { + storage := &logical.InmemStorage{} + + lb, err := Factory(context.Background(), &logical.BackendConfig{ + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: 300 * time.Second, + MaxLeaseTTLVal: 1800 * time.Second, + }, + StorageView: storage, + }) + + require.NoError(t, err) + b := lb.(*backend) + closeChan := make(chan bool) + go func() { + t := time.NewTicker(50 * time.Millisecond) + for { + select { + case <-t.C: + b.PeriodicFunc(context.Background(), &logical.Request{Storage: storage}) + case <-closeChan: + break + } + } + }() + defer close(closeChan) + + if err != nil { + t.Fatalf("error: %s", err) + } + connState, err := testConnState("test-fixtures/keys/cert.pem", + "test-fixtures/keys/key.pem", "test-fixtures/root/rootcacert.pem") + require.NoError(t, err) + caPEM, err := ioutil.ReadFile("test-fixtures/root/rootcacert.pem") + require.NoError(t, err) + caKeyPEM, err := ioutil.ReadFile("test-fixtures/keys/key.pem") + require.NoError(t, err) + certPEM, err := ioutil.ReadFile("test-fixtures/keys/cert.pem") + + caBundle, err := certutil.ParsePEMBundle(string(caPEM)) + require.NoError(t, err) + bundle, err := certutil.ParsePEMBundle(string(certPEM) + "\n" + string(caKeyPEM)) + require.NoError(t, err) + // Entry with one cert first + + revocationListTemplate := &x509.RevocationList{ + RevokedCertificates: []pkix.RevokedCertificate{ + { + SerialNumber: big.NewInt(1), + RevocationTime: time.Now(), + }, + }, + Number: big.NewInt(1), + ThisUpdate: time.Now(), + NextUpdate: time.Now().Add(50 * time.Millisecond), + SignatureAlgorithm: x509.SHA1WithRSA, + } + + crlBytes, err := x509.CreateRevocationList(rand.Reader, revocationListTemplate, caBundle.Certificate, bundle.PrivateKey) + require.NoError(t, err) + + var serverURL *url.URL + crlServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Host == serverURL.Host { + w.Write(crlBytes) + } else { + w.WriteHeader(http.StatusNotFound) + } + })) + serverURL, _ = url.Parse(crlServer.URL) + + req := &logical.Request{ + Connection: &logical.Connection{ + ConnState: &connState, + }, + Storage: storage, + Auth: &logical.Auth{}, + } + + fd := &framework.FieldData{ + Raw: map[string]interface{}{ + "name": "test", + "certificate": string(caPEM), + "policies": "foo,bar", + }, + Schema: pathCerts(b).Fields, + } + + resp, err := b.pathCertWrite(context.Background(), req, fd) + if err != nil { + t.Fatal(err) + } + + empty_login_fd := &framework.FieldData{ + Raw: map[string]interface{}{}, + Schema: pathLogin(b).Fields, + } + resp, err = b.pathLogin(context.Background(), req, empty_login_fd) + if err != nil { + t.Fatal(err) + } + if resp.IsError() { + t.Fatalf("got error: %#v", *resp) + } + + // Set a bad CRL + fd = &framework.FieldData{ + Raw: map[string]interface{}{ + "name": "testcrl", + "url": "http://wrongserver.com", + }, + Schema: pathCRLs(b).Fields, + } + resp, err = b.pathCRLWrite(context.Background(), req, fd) + if err == nil { + t.Fatal(err) + } + if resp.IsError() { + t.Fatalf("got error: %#v", *resp) + } + + // Set good CRL + fd = &framework.FieldData{ + Raw: map[string]interface{}{ + "name": "testcrl", + "url": crlServer.URL, + }, + Schema: pathCRLs(b).Fields, + } + resp, err = b.pathCRLWrite(context.Background(), req, fd) + if err != nil { + t.Fatal(err) + } + if resp.IsError() { + t.Fatalf("got error: %#v", *resp) + } + + if len(b.crls["testcrl"].Serials) != 1 { + t.Fatalf("wrong number of certs in CRL") + } + + // Add a cert to the CRL, then wait to see if it gets automatically picked up + revocationListTemplate.RevokedCertificates = []pkix.RevokedCertificate{ + { + SerialNumber: big.NewInt(1), + RevocationTime: revocationListTemplate.RevokedCertificates[0].RevocationTime, + }, + { + SerialNumber: big.NewInt(2), + RevocationTime: time.Now(), + }, + } + revocationListTemplate.ThisUpdate = time.Now() + revocationListTemplate.NextUpdate = time.Now().Add(1 * time.Minute) + revocationListTemplate.Number = big.NewInt(2) + + crlBytes, err = x509.CreateRevocationList(rand.Reader, revocationListTemplate, caBundle.Certificate, bundle.PrivateKey) + require.NoError(t, err) + time.Sleep(60 * time.Millisecond) + if len(b.crls["testcrl"].Serials) != 2 { + t.Fatalf("wrong number of certs in CRL") + } +} diff --git a/builtin/credential/cert/path_login.go b/builtin/credential/cert/path_login.go index 4df73387e..11e63d75e 100644 --- a/builtin/credential/cert/path_login.go +++ b/builtin/credential/cert/path_login.go @@ -83,6 +83,7 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, data *fra } if b.crls == nil { + // Probably invalidated due to replication, but we need these to proceed if err := b.populateCRLs(ctx, req.Storage); err != nil { return nil, err } @@ -552,6 +553,7 @@ func (b *backend) checkForChainInCRLs(chain []*x509.Certificate) bool { badChain = true break } + } return badChain } diff --git a/changelog/17136.txt b/changelog/17136.txt new file mode 100644 index 000000000..e5e9744c5 --- /dev/null +++ b/changelog/17136.txt @@ -0,0 +1,5 @@ +```release-note:improvement +auth/cert: Operators can now specify a CRL distribution point URL, in which +case the cert auth engine will fetch and use the CRL from that location +rather than needing to push CRLs directly to auth/cert. +``` \ No newline at end of file diff --git a/website/content/docs/auth/cert.mdx b/website/content/docs/auth/cert.mdx index 06c35d14a..4b5439bbb 100644 --- a/website/content/docs/auth/cert.mdx +++ b/website/content/docs/auth/cert.mdx @@ -28,10 +28,8 @@ configuration. This is because the certificates are sent through TLS communicati Since Vault 0.4, the method supports revocation checking. An authorized user can submit PEM-formatted CRLs identified by a given name; -these can be updated or deleted at will. (Note: Vault **does not** fetch CRLs; -the CRLs themselves and any updates must be pushed into Vault when desired, -such as via a `cron` job that fetches them from the source and pushes them into -Vault.) +these can be updated or deleted at will. They may also set the URL of a +trusted CRL distribution point, and have Vault fetch the CRL as needed. When there are CRLs present, at the time of client authentication: