diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index a86e65e878..7f039711ed 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -88,7 +88,7 @@ func (sc *storageContext) fetchCAInfo(issuerRef string, usage issuing.IssuerUsag return bundle, err } -func (sc *storageContext) fetchCAInfoWithIssuer(issuerRef string, usage issuing.IssuerUsage) (*certutil.CAInfoBundle, issuing.IssuerID, error) { +func (sc *storageContext) fetchCAInfoWithIssuer(issuerRef string, usage issuing.IssuerUsage) (*certutil.CAInfoBundle, *issuing.IssuerEntry, error) { var issuerId issuing.IssuerID if sc.UseLegacyBundleCaStorage() { @@ -100,21 +100,21 @@ func (sc *storageContext) fetchCAInfoWithIssuer(issuerRef string, usage issuing. issuerId, err = sc.resolveIssuerReference(issuerRef) if err != nil { // Usually a bad label from the user or mis-configured default. - return nil, issuing.IssuerRefNotFound, errutil.UserError{Err: err.Error()} + return nil, nil, errutil.UserError{Err: err.Error()} } } - bundle, err := sc.fetchCAInfoByIssuerId(issuerId, usage) + bundle, issuer, err := sc.fetchCAInfoByIssuerId(issuerId, usage) if err != nil { - return nil, issuing.IssuerRefNotFound, err + return nil, nil, err } - return bundle, issuerId, nil + return bundle, issuer, nil } // fetchCAInfoByIssuerId will fetch the CA info, will return an error if no ca info exists for the given issuerId. // This does support the loading using the legacyBundleShimID -func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuing.IssuerID, usage issuing.IssuerUsage) (*certutil.CAInfoBundle, error) { +func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuing.IssuerID, usage issuing.IssuerUsage) (*certutil.CAInfoBundle, *issuing.IssuerEntry, error) { return issuing.FetchCAInfoByIssuerId(sc.Context, sc.Storage, sc.GetPkiManagedView(), issuerId, usage) } diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index 2426953c3e..b0a751deb4 100644 --- a/builtin/logical/pki/crl_util.go +++ b/builtin/logical/pki/crl_util.go @@ -2142,7 +2142,7 @@ func buildCRL(sc *storageContext, crlInfo *pki_backend.CrlConfig, forceNew bool, revokedCerts = revoked WRITE: - signingBundle, caErr := sc.fetchCAInfoByIssuerId(thisIssuerId, issuing.CRLSigningUsage) + signingBundle, _, caErr := sc.fetchCAInfoByIssuerId(thisIssuerId, issuing.CRLSigningUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: diff --git a/builtin/logical/pki/issuing/cert_verify.go b/builtin/logical/pki/issuing/cert_verify.go index 6f2af5ad90..42e174f684 100644 --- a/builtin/logical/pki/issuing/cert_verify.go +++ b/builtin/logical/pki/issuing/cert_verify.go @@ -4,14 +4,12 @@ package issuing import ( - "context" "fmt" "os" "strconv" ctx509 "github.com/google/certificate-transparency-go/x509" "github.com/hashicorp/vault/sdk/helper/certutil" - "github.com/hashicorp/vault/sdk/logical" ) // disableVerifyCertificateEnvVar is an environment variable that can be used to disable the @@ -34,7 +32,7 @@ func isCertificateVerificationDisabled() (bool, error) { return disable, nil } -func VerifyCertificate(ctx context.Context, storage logical.Storage, issuerId IssuerID, parsedBundle *certutil.ParsedCertBundle) error { +func VerifyCertificate(issuer *IssuerEntry, parsedBundle *certutil.ParsedCertBundle) error { if verificationDisabled, err := isCertificateVerificationDisabled(); err != nil { return err } else if verificationDisabled { @@ -53,7 +51,7 @@ func VerifyCertificate(ctx context.Context, storage logical.Storage, issuerId Is DisablePathLenChecks: false, DisableNameConstraintChecks: false, } - if err := entSetCertVerifyOptions(ctx, storage, issuerId, &options); err != nil { + if err := entSetCertVerifyOptions(issuer, &options); err != nil { return err } diff --git a/builtin/logical/pki/issuing/issuers.go b/builtin/logical/pki/issuing/issuers.go index cc497f14ec..14f2a414e9 100644 --- a/builtin/logical/pki/issuing/issuers.go +++ b/builtin/logical/pki/issuing/issuers.go @@ -428,33 +428,33 @@ func upgradeIssuerIfRequired(issuer *IssuerEntry) *IssuerEntry { // FetchCAInfoByIssuerId will fetch the CA info, will return an error if no ca info exists for the given issuerId. // This does support the loading using the legacyBundleShimID -func FetchCAInfoByIssuerId(ctx context.Context, s logical.Storage, mkv managed_key.PkiManagedKeyView, issuerId IssuerID, usage IssuerUsage) (*certutil.CAInfoBundle, error) { +func FetchCAInfoByIssuerId(ctx context.Context, s logical.Storage, mkv managed_key.PkiManagedKeyView, issuerId IssuerID, usage IssuerUsage) (*certutil.CAInfoBundle, *IssuerEntry, error) { entry, bundle, err := FetchCertBundleByIssuerId(ctx, s, issuerId, true) if err != nil { switch err.(type) { case errutil.UserError: - return nil, err + return nil, nil, err case errutil.InternalError: - return nil, err + return nil, nil, err default: - return nil, errutil.InternalError{Err: fmt.Sprintf("error fetching CA info: %v", err)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("error fetching CA info: %v", err)} } } if err = entry.EnsureUsage(usage); err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("error while attempting to use issuer %v: %v", issuerId, err)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("error while attempting to use issuer %v: %v", issuerId, err)} } parsedBundle, err := ParseCABundle(ctx, mkv, bundle) if err != nil { - return nil, errutil.InternalError{Err: err.Error()} + return nil, nil, errutil.InternalError{Err: err.Error()} } if parsedBundle.Certificate == nil { - return nil, errutil.InternalError{Err: "stored CA information not able to be parsed"} + return nil, nil, errutil.InternalError{Err: "stored CA information not able to be parsed"} } if parsedBundle.PrivateKey == nil { - return nil, errutil.UserError{Err: fmt.Sprintf("unable to fetch corresponding key for issuer %v; unable to use this issuer for signing", issuerId)} + return nil, nil, errutil.UserError{Err: fmt.Sprintf("unable to fetch corresponding key for issuer %v; unable to use this issuer for signing", issuerId)} } caInfo := &certutil.CAInfoBundle{ @@ -466,11 +466,11 @@ func FetchCAInfoByIssuerId(ctx context.Context, s logical.Storage, mkv managed_k entries, err := GetAIAURLs(ctx, s, entry) if err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch AIA URL information: %v", err)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch AIA URL information: %v", err)} } caInfo.URLs = entries - return caInfo, nil + return caInfo, entry, nil } func ParseCABundle(ctx context.Context, mkv managed_key.PkiManagedKeyView, bundle *certutil.CertBundle) (*certutil.ParsedCertBundle, error) { diff --git a/builtin/logical/pki/issuing/issuing_stubs_oss.go b/builtin/logical/pki/issuing/issuing_stubs_oss.go index 365786099b..a7a9f09861 100644 --- a/builtin/logical/pki/issuing/issuing_stubs_oss.go +++ b/builtin/logical/pki/issuing/issuing_stubs_oss.go @@ -6,8 +6,6 @@ package issuing import ( - "context" - ctx509 "github.com/google/certificate-transparency-go/x509" "github.com/hashicorp/vault/sdk/helper/certutil" "github.com/hashicorp/vault/sdk/logical" @@ -15,7 +13,7 @@ import ( //go:generate go run github.com/hashicorp/vault/tools/stubmaker -func entSetCertVerifyOptions(ctx context.Context, storage logical.Storage, issuerId IssuerID, options *ctx509.VerifyOptions) error { +func entSetCertVerifyOptions(issuer *IssuerEntry, options *ctx509.VerifyOptions) error { return nil } diff --git a/builtin/logical/pki/path_acme_order.go b/builtin/logical/pki/path_acme_order.go index cf0ed9dc14..99d2f2c844 100644 --- a/builtin/logical/pki/path_acme_order.go +++ b/builtin/logical/pki/path_acme_order.go @@ -573,7 +573,7 @@ func issueCertFromCsr(b *backend, ac *acmeContext, csr *x509.CertificateRequest) // (TLS) clients are mostly verifying against server's DNS SANs. maybeAugmentReqDataWithSuitableCN(ac, csr, data) - signingBundle, issuerId, err := ac.sc.fetchCAInfoWithIssuer(ac.Issuer.ID.String(), issuing.IssuanceUsage) + signingBundle, issuer, err := ac.sc.fetchCAInfoWithIssuer(ac.Issuer.ID.String(), issuing.IssuanceUsage) if err != nil { return nil, "", fmt.Errorf("failed loading CA %s: %w", ac.Issuer.ID.String(), err) } @@ -631,7 +631,7 @@ func issueCertFromCsr(b *backend, ac *acmeContext, csr *x509.CertificateRequest) return nil, "", fmt.Errorf("%w: refusing to sign CSR: %s", ErrBadCSR, err.Error()) } - if err = issuing.VerifyCertificate(ac.Context, ac.sc.Storage, issuerId, parsedBundle); err != nil { + if err = issuing.VerifyCertificate(ac.Issuer, parsedBundle); err != nil { return nil, "", fmt.Errorf("verification of parsed bundle failed: %w", err) } @@ -643,7 +643,7 @@ func issueCertFromCsr(b *backend, ac *acmeContext, csr *x509.CertificateRequest) } } - return parsedBundle, issuerId, err + return parsedBundle, issuer.ID, err } func parseCsrFromFinalize(data map[string]interface{}) (*x509.CertificateRequest, error) { diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index bef2c3dcba..2a8c804891 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -410,7 +410,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d var caErr error sc := b.makeStorageContext(ctx, req.Storage) - signingBundle, caErr := sc.fetchCAInfo(issuerName, issuing.IssuanceUsage) + signingBundle, issuer, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: @@ -421,14 +421,10 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d "error fetching CA certificate: %s", caErr)} } } - issuerId, err := issuing.ResolveIssuerReference(ctx, req.Storage, role.Issuer) - if err != nil { - if issuerId == issuing.IssuerRefNotFound { - b.Logger().Warn("could not resolve issuer reference, may be using a legacy CA bundle") - } else { - return nil, err - } + if issuer.ID == issuing.LegacyBundleShimID { + b.Logger().Warn("could not resolve issuer reference, using a legacy CA bundle") } + input := &inputBundle{ req: req, apiData: data, @@ -437,6 +433,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d b.adjustInputBundle(input) var parsedBundle *certutil.ParsedCertBundle var warnings []string + var err error if useCSR { parsedBundle, warnings, err = signCert(b.System(), input, signingBundle, false, useCSRValues) } else { @@ -463,7 +460,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d return nil, err } - if err = issuing.VerifyCertificate(sc.GetContext(), sc.GetStorage(), issuerId, parsedBundle); err != nil { + if err = issuing.VerifyCertificate(issuer, parsedBundle); err != nil { return nil, err } @@ -480,7 +477,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d // TODO: Should we clean up the original cert here? return nil, err } - err = storeCertMetadata(ctx, req.Storage, issuerId, role.Name, parsedBundle.Certificate, metadataBytes) + err = storeCertMetadata(ctx, req.Storage, issuer.ID, role.Name, parsedBundle.Certificate, metadataBytes) if err != nil { // TODO: Should we clean up the original cert here? return nil, err @@ -501,7 +498,7 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d resp = addWarnings(resp, warnings) b.pkiObserver.RecordPKIObservation(ctx, req, observe.ObservationTypePKIIssue, - observe.NewAdditionalPKIMetadata("issuer_id", issuerId), + observe.NewAdditionalPKIMetadata("issuer_id", issuer.ID), observe.NewAdditionalPKIMetadata("issuer_name", role.Issuer), observe.NewAdditionalPKIMetadata("signed", useCSR), observe.NewAdditionalPKIMetadata("role_name", role.Name), diff --git a/builtin/logical/pki/path_resign_crls.go b/builtin/logical/pki/path_resign_crls.go index 435edeeb62..81d832b179 100644 --- a/builtin/logical/pki/path_resign_crls.go +++ b/builtin/logical/pki/path_resign_crls.go @@ -661,7 +661,8 @@ func getCaBundle(sc *storageContext, issuerRef string) (*certutil.CAInfoBundle, return nil, fmt.Errorf("failed to resolve issuer %s: %w", issuerRefParam, err) } - return sc.fetchCAInfoByIssuerId(issuerId, issuing.CRLSigningUsage) + caBundle, _, err := sc.fetchCAInfoByIssuerId(issuerId, issuing.CRLSigningUsage) + return caBundle, err } func decodePemCrls(rawCrls []string) ([]*x509.RevocationList, error) { diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index e57c7aabbd..6ead8057b6 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -400,7 +400,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R var caErr error sc := b.makeStorageContext(ctx, req.Storage) - signingBundle, issuerId, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage) + signingBundle, issuer, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: @@ -448,7 +448,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R } } - if err := issuing.VerifyCertificate(sc.GetContext(), sc.GetStorage(), issuerId, parsedBundle); err != nil { + if err := issuing.VerifyCertificate(issuer, parsedBundle); err != nil { return nil, fmt.Errorf("verification of parsed bundle failed: %w", err) } @@ -477,7 +477,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R b.pkiObserver.RecordPKIObservation(ctx, req, observe.ObservationTypePKIIssuerSignIntermediate, observe.NewAdditionalPKIMetadata("issuer_name", issuerName), - observe.NewAdditionalPKIMetadata("issuer_id", issuerId), + observe.NewAdditionalPKIMetadata("issuer_id", issuer.ID), observe.NewAdditionalPKIMetadata("not_after", parsedBundle.Certificate.NotAfter.Format(time.RFC3339)), observe.NewAdditionalPKIMetadata("not_before", parsedBundle.Certificate.NotBefore.Format(time.RFC3339)), observe.NewAdditionalPKIMetadata("common_name", parsedBundle.Certificate.Subject.CommonName), @@ -601,7 +601,7 @@ func (b *backend) pathIssuerSignSelfIssued(ctx context.Context, req *logical.Req } sc := b.makeStorageContext(ctx, req.Storage) - signingBundle, issuerId, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage) + signingBundle, issuer, caErr := sc.fetchCAInfoWithIssuer(issuerName, issuing.IssuanceUsage) if caErr != nil { switch caErr.(type) { case errutil.UserError: @@ -670,7 +670,7 @@ func (b *backend) pathIssuerSignSelfIssued(ctx context.Context, req *logical.Req b.pkiObserver.RecordPKIObservation(ctx, req, observe.ObservationTypePKIIssuerSignSelfIssued, observe.NewAdditionalPKIMetadata("issuer_name", issuerName), - observe.NewAdditionalPKIMetadata("issuer_id", issuerId.String()), + observe.NewAdditionalPKIMetadata("issuer_id", issuer.ID.String()), observe.NewAdditionalPKIMetadata("issuing_ca", signingCB.IssuingCA), observe.NewAdditionalPKIMetadata("serial_number", parsing.SerialFromCert(cert)), observe.NewAdditionalPKIMetadata("not_after", cert.NotAfter.Format(time.RFC3339)), diff --git a/changelog/_10992.txt b/changelog/_10992.txt new file mode 100644 index 0000000000..c61e0a456c --- /dev/null +++ b/changelog/_10992.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Avoid loading issuer information multiple times per leaf certificate signing +```