upstream/vault
1
0
Fork 0
mirror of https://github.com/hashicorp/vault.git synced 2026-02-18 18:38:08 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Refactor PKI: Load issuer information once for VerifyCertificate (#10992) (#10994)

Browse source 
* Refactor PKI: Load issuer information once

* Add cl

Co-authored-by: Steven Clark <steven.clark@hashicorp.com>
This commit is contained in:
Vault Automation 2025-11-24 13:21:57 -05:00 committed by GitHub
parent 221a951094
commit df8ae716fe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 41 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
builtin/logical/pki/cert_util.go
View file

@ -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)
}

2
builtin/logical/pki/crl_util.go
View file

@ -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:

6
builtin/logical/pki/issuing/cert_verify.go
View file

@ -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
}

20
builtin/logical/pki/issuing/issuers.go
View file

@ -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) {

4
builtin/logical/pki/issuing/issuing_stubs_oss.go
View file

@ -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
}

6
builtin/logical/pki/path_acme_order.go
View file

@ -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) {

19
builtin/logical/pki/path_issue_sign.go
View file

@ -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),

3
builtin/logical/pki/path_resign_crls.go
View file

@ -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) {

10
builtin/logical/pki/path_root.go
View file

@ -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)),

3
changelog/_10992.txt Normal file
View file

@ -0,0 +1,3 @@
```release-note:improvement
secrets/pki: Avoid loading issuer information multiple times per leaf certificate signing
```