From 15bc5981717a5bd18af93a6a789733bf1ebe70d7 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Tue, 3 May 2022 16:40:26 -0400 Subject: [PATCH] Add issuer usage restrictions bitset This allows issuers to have usage restrictions, limiting whether they can be used to issue certificates or if they can generate CRLs. This allows certain issuers to not generate a CRL (if the global config is with the CRL enabled) or allows the issuer to not issue new certificates (but potentially letting the CRL generation continue). Setting both fields to false effectively forms a soft delete capability. Signed-off-by: Alexander Scheel --- builtin/logical/pki/backend_test.go | 2 +- builtin/logical/pki/cert_util.go | 6 +- builtin/logical/pki/crl_util.go | 13 +++- builtin/logical/pki/path_fetch.go | 2 +- builtin/logical/pki/path_fetch_issuers.go | 25 +++++++ builtin/logical/pki/path_issue_sign.go | 2 +- builtin/logical/pki/path_root.go | 4 +- builtin/logical/pki/storage.go | 88 +++++++++++++++++++++++ builtin/logical/pki/storage_migrations.go | 8 ++- 9 files changed, 142 insertions(+), 8 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index bb95880d8238..1fc8ea38233a 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2660,7 +2660,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { t.Fatal(err) } - signingBundle, err := fetchCAInfo(context.Background(), b, &logical.Request{Storage: storage}, defaultRef) + signingBundle, err := fetchCAInfo(context.Background(), b, &logical.Request{Storage: storage}, defaultRef, ReadOnlyUsage) if err != nil { t.Fatal(err) } diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index c5bc87020404..46d2e579000b 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -80,7 +80,7 @@ func getFormat(data *framework.FieldData) string { } // fetchCAInfo will fetch the CA info, will return an error if no ca info exists. -func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRef string) (*certutil.CAInfoBundle, error) { +func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRef string, usage issuerUsage) (*certutil.CAInfoBundle, error) { entry, bundle, err := fetchCertBundle(ctx, b, req.Storage, issuerRef) if err != nil { switch err.(type) { @@ -93,6 +93,10 @@ func fetchCAInfo(ctx context.Context, b *backend, req *logical.Request, issuerRe } } + if err := entry.EnsureUsage(usage); err != nil { + return nil, errutil.InternalError{Err: fmt.Sprintf("error while attempting to use issuer %v: %v", issuerRef, err)} + } + if bundle == nil { return nil, errutil.UserError{Err: "no CA information is present"} } diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index e043e605a96a..5ff47fb15efb 100644 --- a/builtin/logical/pki/crl_util.go +++ b/builtin/logical/pki/crl_util.go @@ -100,7 +100,7 @@ func revokeCert(ctx context.Context, b *backend, req *logical.Request, serial st return nil, nil } - signingBundle, caErr := fetchCAInfo(ctx, b, req, defaultRef) + signingBundle, caErr := fetchCAInfo(ctx, b, req, defaultRef, ReadOnlyUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: @@ -270,6 +270,11 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b continue } + // Skip entries which aren't enabled for CRL signing. + if err := thisEntry.EnsureUsage(CRLSigningUsage); err != nil { + continue + } + issuerIDEntryMap[issuer] = thisEntry thisCert, err := thisEntry.GetCertificate() @@ -365,6 +370,12 @@ func buildCRLs(ctx context.Context, b *backend, req *logical.Request, forceNew b // remove them, remembering their CRLs IDs. If we've completely removed // all issuers pointing to that CRL number, we can remove it from the // number map and from storage. + // + // Note that we persist the last generated CRL for a specified issuer + // if it is later disabled for CRL generation. This mirrors the old + // root deletion behavior, but using soft issuer deletes. If there is an + // alternate, equivalent issuer however, we'll keep updating the shared + // CRL; all equivalent issuers must have their CRLs disabled. for mapIssuerId := range crlConfig.IssuerIDCRLMap { stillHaveIssuer := false for _, listedIssuerId := range issuers { diff --git a/builtin/logical/pki/path_fetch.go b/builtin/logical/pki/path_fetch.go index dd474cc06e05..f4a2a6632c0f 100644 --- a/builtin/logical/pki/path_fetch.go +++ b/builtin/logical/pki/path_fetch.go @@ -206,7 +206,7 @@ func (b *backend) pathFetchRead(ctx context.Context, req *logical.Request, data // Prefer fetchCAInfo to fetchCertBySerial for CA certificates. if serial == "ca_chain" || serial == "ca" { - caInfo, err := fetchCAInfo(ctx, b, req, defaultRef) + caInfo, err := fetchCAInfo(ctx, b, req, defaultRef, ReadOnlyUsage) if err != nil { switch err.(type) { case errutil.UserError: diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 42d0055897b7..0bfd362750be 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/builtin/logical/pki/path_fetch_issuers.go @@ -91,6 +91,14 @@ the entire validity period. It is suggested to use "truncate" for intermediate CAs and "permit" only for root CAs.`, Default: "err", } + fields["usage"] = &framework.FieldSchema{ + Type: framework.TypeCommaStringSlice, + Description: `Comma-separated list (or string slice) of usages for +this issuer; valid values are "read-only", "issuing-certificates", and +"crl-signing". Multiple values may be specified. Read-only is implicit +and always set.`, + Default: []string{"read-only", "issuing-certificates", "crl-signing"}, + } return &framework.Path{ // Returns a JSON entry. @@ -162,6 +170,7 @@ func (b *backend) pathGetIssuer(ctx context.Context, req *logical.Request, data "manual_chain": respManualChain, "ca_chain": issuer.CAChain, "leaf_not_after_behavior": issuer.LeafNotAfterBehavior, + "usage": issuer.Usage.Names(), }, }, nil } @@ -202,6 +211,10 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da // errs should still be surfaced, however. return logical.ErrorResponse(err.Error()), nil } + if err == errIssuerNameInUse && issuer.Name != newName { + // When the new name is in use but isn't this name, throw an error. + return logical.ErrorResponse(err.Error()), nil + } newPath := data.Get("manual_chain").([]string) rawLeafBehavior := data.Get("leaf_not_after_behavior").(string) @@ -217,6 +230,12 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da return logical.ErrorResponse("Unknown value for field `leaf_not_after_behavior`. Possible values are `err`, `truncate`, and `permit`."), nil } + rawUsage := data.Get("usage").([]string) + newUsage, err := NewIssuerUsageFromNames(rawUsage) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("Unable to parse specified usages: %v - valid values are %v", rawUsage, AllIssuerUsages.Names())), nil + } + modified := false if newName != issuer.Name { @@ -229,6 +248,11 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da modified = true } + if newUsage != issuer.Usage { + issuer.Usage = newUsage + modified = true + } + var updateChain bool var constructedChain []issuerID for index, newPathRef := range newPath { @@ -289,6 +313,7 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da "manual_chain": respManualChain, "ca_chain": issuer.CAChain, "leaf_not_after_behavior": issuer.LeafNotAfterBehavior, + "usage": issuer.Usage.Names(), }, }, nil } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 093c1181cc1e..041fd596ce28 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -262,7 +262,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d } var caErr error - signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName) + signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName, IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index aa46e65b73f0..f20ef96959fa 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -276,7 +276,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R } var caErr error - signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName) + signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName, IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: @@ -417,7 +417,7 @@ func (b *backend) pathIssuerSignSelfIssued(ctx context.Context, req *logical.Req } var caErr error - signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName) + signingBundle, caErr := fetchCAInfo(ctx, b, req, issuerName, IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 0c6725291a30..7237b96b3d4d 100644 --- a/builtin/logical/pki/storage.go +++ b/builtin/logical/pki/storage.go @@ -67,6 +67,68 @@ func (e keyEntry) isManagedPrivateKey() bool { return e.PrivateKeyType == certutil.ManagedPrivateKey } +type issuerUsage uint + +const ( + ReadOnlyUsage issuerUsage = iota + IssuanceUsage issuerUsage = 1 << iota + CRLSigningUsage issuerUsage = 1 << iota + + // When adding a new usage in the future, we'll need to create a usage + // mask field on the IssuerEntry and handle migrations to a newer mask, + // inferring a value for the new bits. + AllIssuerUsages issuerUsage = ReadOnlyUsage | IssuanceUsage | CRLSigningUsage +) + +var namedIssuerUsages = map[string]issuerUsage{ + "read-only": ReadOnlyUsage, + "issuing-certificates": IssuanceUsage, + "crl-signing": CRLSigningUsage, +} + +func (i *issuerUsage) ToggleUsage(usages ...issuerUsage) { + for _, usage := range usages { + *i ^= usage + } +} + +func (i issuerUsage) HasUsage(usage issuerUsage) bool { + return (i & usage) == usage +} + +func (i issuerUsage) Names() string { + var names []string + var builtUsage issuerUsage + + for name, usage := range namedIssuerUsages { + if i.HasUsage(usage) { + names = append(names, name) + builtUsage.ToggleUsage(usage) + } + } + + if i != builtUsage { + // Found some unknown usage, we should indicate this in the names. + names = append(names, fmt.Sprintf("unknown:%v", i^builtUsage)) + } + + return strings.Join(names, ",") +} + +func NewIssuerUsageFromNames(names []string) (issuerUsage, error) { + var result issuerUsage + for index, name := range names { + usage, ok := namedIssuerUsages[name] + if !ok { + return ReadOnlyUsage, fmt.Errorf("unknown name for usage at index %v: %v", index, name) + } + + result.ToggleUsage(usage) + } + + return result, nil +} + type issuerEntry struct { ID issuerID `json:"id" structs:"id" mapstructure:"id"` Name string `json:"name" structs:"name" mapstructure:"name"` @@ -76,6 +138,7 @@ type issuerEntry struct { ManualChain []issuerID `json:"manual_chain" structs:"manual_chain" mapstructure:"manual_chain"` SerialNumber string `json:"serial_number" structs:"serial_number" mapstructure:"serial_number"` LeafNotAfterBehavior certutil.NotAfterBehavior `json:"not_after_behavior" structs:"not_after_behavior" mapstructure:"not_after_behavior"` + Usage issuerUsage `json:"usage" structs:"usage" mapstructure:"usage"` } type localCRLConfigEntry struct { @@ -297,6 +360,30 @@ func (i issuerEntry) GetCertificate() (*x509.Certificate, error) { return x509.ParseCertificate(block.Bytes) } +func (i issuerEntry) EnsureUsage(usage issuerUsage) error { + // We want to spit out a nice error message about missing usages. + if i.Usage.HasUsage(usage) { + return nil + } + + issuerRef := fmt.Sprintf("id:%v", i.ID) + if len(i.Name) > 0 { + issuerRef = fmt.Sprintf("%v / name:%v", issuerRef, i.Name) + } + + // These usages differ at some point in time. We've gotta find the first + // usage that differs and return a logical-sounding error message around + // that difference. + for name, candidate := range namedIssuerUsages { + if usage.HasUsage(candidate) && !i.Usage.HasUsage(candidate) { + return fmt.Errorf("requested usage %v for issuer [%v] but only had usage %v", name, issuerRef, i.Usage.Names()) + } + } + + // Maybe we have an unnamed usage that's requested. + return fmt.Errorf("unknown delta between usages: %v -> %v / for issuer [%v]", usage.Names(), i.Usage.Names(), issuerRef) +} + func listIssuers(ctx context.Context, s logical.Storage) ([]issuerID, error) { strList, err := s.List(ctx, issuerPrefix) if err != nil { @@ -464,6 +551,7 @@ func importIssuer(ctx managedKeyContext, s logical.Storage, certValue string, is result.Name = issuerName result.Certificate = certValue result.LeafNotAfterBehavior = certutil.ErrNotAfterBehavior + result.Usage.ToggleUsage(IssuanceUsage, CRLSigningUsage) // We shouldn't add CSRs or multiple certificates in this countCertificates := strings.Count(result.Certificate, "-BEGIN ") diff --git a/builtin/logical/pki/storage_migrations.go b/builtin/logical/pki/storage_migrations.go index 3b3f75267465..18c17ecea3e8 100644 --- a/builtin/logical/pki/storage_migrations.go +++ b/builtin/logical/pki/storage_migrations.go @@ -13,7 +13,10 @@ import ( // This allows us to record the version of the migration code within the log entry // in case we find out in the future that something was horribly wrong with the migration, // and we need to perform it again... -const latestMigrationVersion = 1 +const ( + latestMigrationVersion = 1 + legacyBundleShimID = issuerID("legacy-entry-shim-id") +) type legacyBundleMigrationLog struct { Hash string `json:"hash" structs:"hash" mapstructure:"hash"` @@ -173,8 +176,11 @@ func getLegacyCertBundle(ctx context.Context, s logical.Storage) (*issuerEntry, // Fake a storage entry with backwards compatibility in mind. We only need // the fields in the CAInfoBundle; everything else doesn't matter. issuer := &issuerEntry{ + ID: legacyBundleShimID, + Name: "legacy-entry-shim", LeafNotAfterBehavior: certutil.ErrNotAfterBehavior, } + issuer.Usage.ToggleUsage(IssuanceUsage, CRLSigningUsage) return issuer, cb, nil }