From 71d9c33802d1f1cdb92559b0846e21adb47e1c4b Mon Sep 17 00:00:00 2001 From: Kit Haines Date: Thu, 15 Sep 2022 15:38:33 -0400 Subject: [PATCH] Add "plumbing" for surfacing warnings, and warning overwriting ttl (#17073) * Add "plumbing" for surfacing warnings, and add warning about TTL > maxTTL when issuing a cert. --- builtin/logical/pki/cert_util.go | 146 ++++++++++++----------- builtin/logical/pki/cert_util_test.go | 4 +- builtin/logical/pki/path_intermediate.go | 4 +- builtin/logical/pki/path_issue_sign.go | 7 +- builtin/logical/pki/path_root.go | 8 +- builtin/logical/pki/storage_test.go | 2 +- builtin/logical/pki/util.go | 7 ++ changelog/17073.txt | 3 + 8 files changed, 101 insertions(+), 80 deletions(-) create mode 100644 changelog/17073.txt diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 71c314fec..1b2ae8240 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -213,7 +213,7 @@ func fetchCertBySerial(ctx context.Context, b *backend, req *logical.Request, pr } // Retrieve the old-style path. We disregard errors here because they - // always manifest on windows, and thus the initial check for a revoked + // always manifest on Windows, and thus the initial check for a revoked // cert fails would return an error when the cert isn't revoked, preventing // the happy path from working. certEntry, _ = req.Storage.Get(ctx, legacyPath) @@ -259,7 +259,7 @@ func validateURISAN(b *backend, data *inputBundle, uri string) bool { return valid } -// Validates a given common name, ensuring its either a email or a hostname +// Validates a given common name, ensuring it's either an email or a hostname // after validating it according to the role parameters, or disables // validation altogether. func validateCommonName(b *backend, data *inputBundle, name string) string { @@ -661,25 +661,25 @@ func generateCert(sc *storageContext, input *inputBundle, caSign *certutil.CAInfoBundle, isCA bool, - randomSource io.Reader) (*certutil.ParsedCertBundle, error, + randomSource io.Reader) (*certutil.ParsedCertBundle, []string, error, ) { ctx := sc.Context b := sc.Backend if input.role == nil { - return nil, errutil.InternalError{Err: "no role found in data bundle"} + return nil, nil, errutil.InternalError{Err: "no role found in data bundle"} } if input.role.KeyType == "rsa" && input.role.KeyBits < 2048 { - return nil, errutil.UserError{Err: "RSA keys < 2048 bits are unsafe and not supported"} + return nil, nil, errutil.UserError{Err: "RSA keys < 2048 bits are unsafe and not supported"} } - data, err := generateCreationBundle(b, input, caSign, nil) + data, warnings, err := generateCreationBundle(b, input, caSign, nil) if err != nil { - return nil, err + return nil, nil, err } if data.Params == nil { - return nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} + return nil, nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} } if isCA { @@ -691,7 +691,7 @@ func generateCert(sc *storageContext, // issuer entry yet, we default to the global URLs. entries, err := getGlobalAIAURLs(ctx, sc.Storage) if err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)} } data.Params.URLs = entries @@ -705,56 +705,56 @@ func generateCert(sc *storageContext, parsedBundle, err := generateCABundle(sc, input, data, randomSource) if err != nil { - return nil, err + return nil, nil, err } - return parsedBundle, nil + return parsedBundle, warnings, nil } // N.B.: This is only meant to be used for generating intermediate CAs. // It skips some sanity checks. -func generateIntermediateCSR(sc *storageContext, input *inputBundle, randomSource io.Reader) (*certutil.ParsedCSRBundle, error) { +func generateIntermediateCSR(sc *storageContext, input *inputBundle, randomSource io.Reader) (*certutil.ParsedCSRBundle, []string, error) { b := sc.Backend - creation, err := generateCreationBundle(b, input, nil, nil) + creation, warnings, err := generateCreationBundle(b, input, nil, nil) if err != nil { - return nil, err + return nil, nil, err } if creation.Params == nil { - return nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} + return nil, nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} } addBasicConstraints := input.apiData != nil && input.apiData.Get("add_basic_constraints").(bool) parsedBundle, err := generateCSRBundle(sc, input, creation, addBasicConstraints, randomSource) if err != nil { - return nil, err + return nil, nil, err } - return parsedBundle, nil + return parsedBundle, warnings, nil } func signCert(b *backend, data *inputBundle, caSign *certutil.CAInfoBundle, isCA bool, - useCSRValues bool) (*certutil.ParsedCertBundle, error, + useCSRValues bool) (*certutil.ParsedCertBundle, []string, error, ) { if data.role == nil { - return nil, errutil.InternalError{Err: "no role found in data bundle"} + return nil, nil, errutil.InternalError{Err: "no role found in data bundle"} } csrString := data.apiData.Get("csr").(string) if csrString == "" { - return nil, errutil.UserError{Err: "\"csr\" is empty"} + return nil, nil, errutil.UserError{Err: "\"csr\" is empty"} } pemBlock, _ := pem.Decode([]byte(csrString)) if pemBlock == nil { - return nil, errutil.UserError{Err: "csr contains no data"} + return nil, nil, errutil.UserError{Err: "csr contains no data"} } csr, err := x509.ParseCertificateRequest(pemBlock.Bytes) if err != nil { - return nil, errutil.UserError{Err: fmt.Sprintf("certificate request could not be parsed: %v", err)} + return nil, nil, errutil.UserError{Err: fmt.Sprintf("certificate request could not be parsed: %v", err)} } // This switch validates that the CSR key type matches the role and sets @@ -766,14 +766,14 @@ func signCert(b *backend, case "rsa": // Verify that the key matches the role type if csr.PublicKeyAlgorithm != x509.RSA { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "role requires keys of type %s", data.role.KeyType)} } pubKey, ok := csr.PublicKey.(*rsa.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } actualKeyType = "rsa" @@ -781,13 +781,13 @@ func signCert(b *backend, case "ec": // Verify that the key matches the role type if csr.PublicKeyAlgorithm != x509.ECDSA { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "role requires keys of type %s", data.role.KeyType)} } pubKey, ok := csr.PublicKey.(*ecdsa.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } actualKeyType = "ec" @@ -795,14 +795,14 @@ func signCert(b *backend, case "ed25519": // Verify that the key matches the role type if csr.PublicKeyAlgorithm != x509.Ed25519 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "role requires keys of type %s", data.role.KeyType)} } _, ok := csr.PublicKey.(ed25519.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } actualKeyType = "ed25519" @@ -814,10 +814,10 @@ func signCert(b *backend, case x509.RSA: pubKey, ok := csr.PublicKey.(*rsa.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } if pubKey.N.BitLen() < 2048 { - return nil, errutil.UserError{Err: "RSA keys < 2048 bits are unsafe and not supported"} + return nil, nil, errutil.UserError{Err: "RSA keys < 2048 bits are unsafe and not supported"} } actualKeyType = "rsa" @@ -825,7 +825,7 @@ func signCert(b *backend, case x509.ECDSA: pubKey, ok := csr.PublicKey.(*ecdsa.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } actualKeyType = "ec" @@ -833,16 +833,16 @@ func signCert(b *backend, case x509.Ed25519: _, ok := csr.PublicKey.(ed25519.PublicKey) if !ok { - return nil, errutil.UserError{Err: "could not parse CSR's public key"} + return nil, nil, errutil.UserError{Err: "could not parse CSR's public key"} } actualKeyType = "ed25519" actualKeyBits = 0 default: - return nil, errutil.UserError{Err: "Unknown key type in CSR: " + csr.PublicKeyAlgorithm.String()} + return nil, nil, errutil.UserError{Err: "Unknown key type in CSR: " + csr.PublicKeyAlgorithm.String()} } default: - return nil, errutil.InternalError{Err: fmt.Sprintf("unsupported key type value: %s", data.role.KeyType)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unsupported key type value: %s", data.role.KeyType)} } // Before validating key lengths, update our KeyBits/SignatureBits based @@ -861,7 +861,7 @@ func signCert(b *backend, // for signing operations if data.role.KeyBits, data.role.SignatureBits, err = certutil.ValidateDefaultOrValueKeyTypeSignatureLength( actualKeyType, 0, data.role.SignatureBits); err != nil { - return nil, errutil.InternalError{Err: fmt.Sprintf("unknown internal error updating default values: %v", err)} + return nil, nil, errutil.InternalError{Err: fmt.Sprintf("unknown internal error updating default values: %v", err)} } // We're using the KeyBits field as a minimum value below, and P-224 is safe @@ -883,31 +883,31 @@ func signCert(b *backend, // that we always validate both RSA and ECDSA key sizes. if actualKeyType == "rsa" { if actualKeyBits < data.role.KeyBits { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "role requires a minimum of a %d-bit key, but CSR's key is %d bits", data.role.KeyBits, actualKeyBits)} } if actualKeyBits < 2048 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "Vault requires a minimum of a 2048-bit key, but CSR's key is %d bits", actualKeyBits)} } } else if actualKeyType == "ec" { if actualKeyBits < data.role.KeyBits { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "role requires a minimum of a %d-bit key, but CSR's key is %d bits", data.role.KeyBits, actualKeyBits)} } } - creation, err := generateCreationBundle(b, data, caSign, csr) + creation, warnings, err := generateCreationBundle(b, data, caSign, csr) if err != nil { - return nil, err + return nil, nil, err } if creation.Params == nil { - return nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} + return nil, nil, errutil.InternalError{Err: "nil parameters received from parameter bundle generation"} } creation.Params.IsCA = isCA @@ -919,10 +919,10 @@ func signCert(b *backend, parsedBundle, err := certutil.SignCertificate(creation) if err != nil { - return nil, err + return nil, nil, err } - return parsedBundle, nil + return parsedBundle, warnings, nil } // otherNameRaw describes a name related to a certificate which is not in one @@ -1037,10 +1037,11 @@ func forEachSAN(extension []byte, callback func(tag int, data []byte) error) err // generateCreationBundle is a shared function that reads parameters supplied // from the various endpoints and generates a CreationParameters with the // parameters that can be used to issue or sign -func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAInfoBundle, csr *x509.CertificateRequest) (*certutil.CreationBundle, error) { +func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAInfoBundle, csr *x509.CertificateRequest) (*certutil.CreationBundle, []string, error) { // Read in names -- CN, DNS and email addresses var cn string var ridSerialNumber string + var warnings []string dnsNames := []string{} emailAddresses := []string{} { @@ -1050,7 +1051,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if cn == "" { cn = data.apiData.Get("common_name").(string) if cn == "" && data.role.RequireCN { - return nil, errutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true, unless "require_cn" is set to false`} + return nil, nil, errutil.UserError{Err: `the common_name field is required, or must be provided in a CSR with "use_csr_common_name" set to true, unless "require_cn" is set to false`} } } @@ -1083,7 +1084,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn ) converted, err := p.ToASCII(cn) if err != nil { - return nil, errutil.UserError{Err: err.Error()} + return nil, nil, errutil.UserError{Err: err.Error()} } if hostnameRegex.MatchString(converted) { dnsNames = append(dnsNames, converted) @@ -1107,7 +1108,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn ) converted, err := p.ToASCII(v) if err != nil { - return nil, errutil.UserError{Err: err.Error()} + return nil, nil, errutil.UserError{Err: err.Error()} } if hostnameRegex.MatchString(converted) { dnsNames = append(dnsNames, converted) @@ -1122,7 +1123,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if cn != "" { badName := validateCommonName(b, data, cn) if len(badName) != 0 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "common name %s not allowed by this role", badName)} } } @@ -1130,7 +1131,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if ridSerialNumber != "" { badName := validateSerialNumber(data, ridSerialNumber) if len(badName) != 0 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "serial_number %s not allowed by this role", badName)} } } @@ -1138,13 +1139,13 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn // Check for bad email and/or DNS names badName := validateNames(b, data, dnsNames) if len(badName) != 0 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "subject alternate name %s not allowed by this role", badName)} } badName = validateNames(b, data, emailAddresses) if len(badName) != 0 { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "email address %s not allowed by this role", badName)} } } @@ -1162,7 +1163,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if data.role.UseCSRSANs && csr != nil && len(csr.Extensions) > 0 { others, err := getOtherSANsFromX509Extensions(csr.Extensions) if err != nil { - return nil, errutil.UserError{Err: fmt.Errorf("could not parse requested other SAN: %w", err).Error()} + return nil, nil, errutil.UserError{Err: fmt.Errorf("could not parse requested other SAN: %w", err).Error()} } for _, other := range others { otherSANsInput = append(otherSANsInput, other.String()) @@ -1171,17 +1172,17 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if len(otherSANsInput) > 0 { requested, err := parseOtherSANs(otherSANsInput) if err != nil { - return nil, errutil.UserError{Err: fmt.Errorf("could not parse requested other SAN: %w", err).Error()} + return nil, nil, errutil.UserError{Err: fmt.Errorf("could not parse requested other SAN: %w", err).Error()} } badOID, badName, err := validateOtherSANs(data, requested) switch { case err != nil: - return nil, errutil.UserError{Err: err.Error()} + return nil, nil, errutil.UserError{Err: err.Error()} case len(badName) > 0: - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "other SAN %s not allowed for OID %s by this role", badName, badOID)} case len(badOID) > 0: - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "other SAN OID %s not allowed by this role", badOID)} default: otherSANs = requested @@ -1194,7 +1195,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if csr != nil && data.role.UseCSRSANs { if len(csr.IPAddresses) > 0 { if !data.role.AllowIPSANs { - return nil, errutil.UserError{Err: "IP Subject Alternative Names are not allowed in this role, but was provided some via CSR"} + return nil, nil, errutil.UserError{Err: "IP Subject Alternative Names are not allowed in this role, but was provided some via CSR"} } ipAddresses = csr.IPAddresses } @@ -1202,13 +1203,13 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn ipAlt := data.apiData.Get("ip_sans").([]string) if len(ipAlt) > 0 { if !data.role.AllowIPSANs { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "IP Subject Alternative Names are not allowed in this role, but was provided %s", ipAlt)} } for _, v := range ipAlt { parsedIP := net.ParseIP(v) if parsedIP == nil { - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "the value %q is not a valid IP address", v)} } ipAddresses = append(ipAddresses, parsedIP) @@ -1222,7 +1223,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if csr != nil && data.role.UseCSRSANs { if len(csr.URIs) > 0 { if len(data.role.AllowedURISANs) == 0 { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: "URI Subject Alternative Names are not allowed in this role, but were provided via CSR", } } @@ -1231,7 +1232,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn for _, uri := range csr.URIs { valid := validateURISAN(b, data, uri.String()) if !valid { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: "URI Subject Alternative Names were provided via CSR which are not valid for this role", } } @@ -1243,7 +1244,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn uriAlt := data.apiData.Get("uri_sans").([]string) if len(uriAlt) > 0 { if len(data.role.AllowedURISANs) == 0 { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: "URI Subject Alternative Names are not allowed in this role, but were provided via the API", } } @@ -1251,14 +1252,14 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn for _, uri := range uriAlt { valid := validateURISAN(b, data, uri) if !valid { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: "URI Subject Alternative Names were provided via the API which are not valid for this role", } } parsedURI, err := url.Parse(uri) if parsedURI == nil || err != nil { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: fmt.Sprintf( "the provided URI Subject Alternative Name %q is not a valid URI", uri), } @@ -1300,7 +1301,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn } if ttl > 0 && notAfterAlt != "" { - return nil, errutil.UserError{ + return nil, nil, errutil.UserError{ Err: "Either ttl or not_after should be provided. Both should not be provided in the same request.", } } @@ -1320,13 +1321,14 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn maxTTL = b.System().MaxLeaseTTL() } if ttl > maxTTL { + warnings = append(warnings, fmt.Sprintf("TTL %q is longer than permitted maxTTL %q, so maxTTL is being used", ttl, maxTTL)) ttl = maxTTL } if notAfterAlt != "" { notAfter, err = time.Parse(time.RFC3339, notAfterAlt) if err != nil { - return nil, errutil.UserError{Err: err.Error()} + return nil, nil, errutil.UserError{Err: err.Error()} } } else { notAfter = time.Now().Add(ttl) @@ -1334,7 +1336,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn if caSign != nil && notAfter.After(caSign.Certificate.NotAfter) { // If it's not self-signed, verify that the issued certificate // won't be valid past the lifetime of the CA certificate, and - // act accordingly. This is dependent based on the issuers's + // act accordingly. This is dependent based on the issuer's // LeafNotAfterBehavior argument. switch caSign.LeafNotAfterBehavior { case certutil.PermitNotAfterBehavior: @@ -1344,7 +1346,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn case certutil.ErrNotAfterBehavior: fallthrough default: - return nil, errutil.UserError{Err: fmt.Sprintf( + return nil, nil, errutil.UserError{Err: fmt.Sprintf( "cannot satisfy request, as TTL would result in notAfter %s that is beyond the expiration of the CA certificate at %s", notAfter.Format(time.RFC3339Nano), caSign.Certificate.NotAfter.Format(time.RFC3339Nano))} } } @@ -1366,7 +1368,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn skid, err = hex.DecodeString(skidValue) if err != nil { - return nil, errutil.UserError{Err: fmt.Sprintf("cannot parse requested SKID value as hex: %v", err)} + return nil, nil, errutil.UserError{Err: fmt.Sprintf("cannot parse requested SKID value as hex: %v", err)} } } } @@ -1400,7 +1402,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn // Don't deal with URLs or max path length if it's self-signed, as these // normally come from the signing bundle if caSign == nil { - return creation, nil + return creation, warnings, nil } // This will have been read in from the getGlobalAIAURLs function @@ -1426,7 +1428,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn } } - return creation, nil + return creation, warnings, nil } func convertRespToPKCS8(resp *logical.Response) error { @@ -1586,7 +1588,7 @@ const ( ) // Note: Taken from the Go source code since it's not public, plus changed to not marshal -// marshalSANs marshals a list of addresses into a the contents of an X.509 +// marshalSANs marshals a list of addresses into the contents of an X.509 // SubjectAlternativeName extension. func marshalSANs(dnsNames, emailAddresses []string, ipAddresses []net.IP, uris []*url.URL) []asn1.RawValue { var rawValues []asn1.RawValue diff --git a/builtin/logical/pki/cert_util_test.go b/builtin/logical/pki/cert_util_test.go index 635cfce98..ba2e6f810 100644 --- a/builtin/logical/pki/cert_util_test.go +++ b/builtin/logical/pki/cert_util_test.go @@ -110,7 +110,7 @@ func TestPki_MultipleOUs(t *testing.T) { OU: []string{"Z", "E", "V"}, }, } - cb, err := generateCreationBundle(&b, input, nil, nil) + cb, _, err := generateCreationBundle(&b, input, nil, nil) if err != nil { t.Fatalf("Error: %v", err) } @@ -212,7 +212,7 @@ func TestPki_PermitFQDNs(t *testing.T) { name := name testCase := testCase t.Run(name, func(t *testing.T) { - cb, err := generateCreationBundle(&b, testCase.input, nil, nil) + cb, _, err := generateCreationBundle(&b, testCase.input, nil, nil) if err != nil { t.Fatalf("Error: %v", err) } diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index f634cde74..c87584221 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -95,7 +95,7 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req req: req, apiData: data, } - parsedBundle, err := generateIntermediateCSR(sc, input, b.Backend.GetRandomReader()) + parsedBundle, warnings, err := generateIntermediateCSR(sc, input, b.Backend.GetRandomReader()) if err != nil { switch err.(type) { case errutil.UserError: @@ -161,6 +161,8 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req } resp.Data["key_id"] = myKey.ID + resp = addWarnings(resp, warnings) + return resp, nil } diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index e523de074..dedcc82d4 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -309,10 +309,11 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d } var parsedBundle *certutil.ParsedCertBundle var err error + var warnings []string if useCSR { - parsedBundle, err = signCert(b, input, signingBundle, false, useCSRValues) + parsedBundle, warnings, err = signCert(b, input, signingBundle, false, useCSRValues) } else { - parsedBundle, err = generateCert(sc, input, signingBundle, false, rand.Reader) + parsedBundle, warnings, err = generateCert(sc, input, signingBundle, false, rand.Reader) } if err != nil { switch err.(type) { @@ -426,6 +427,8 @@ func (b *backend) pathIssueSignCert(ctx context.Context, req *logical.Request, d } } + resp = addWarnings(resp, warnings) + return resp, nil } diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index f39af232a..64c56cef6 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -146,7 +146,7 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, apiData: data, role: role, } - parsedBundle, err := generateCert(sc, input, nil, true, b.Backend.GetRandomReader()) + parsedBundle, warnings, err := generateCert(sc, input, nil, true, b.Backend.GetRandomReader()) if err != nil { switch err.(type) { case errutil.UserError: @@ -275,6 +275,8 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, resp.AddWarning("Max path length of the generated certificate is zero. This certificate cannot be used to issue intermediate CA certificates.") } + resp = addWarnings(resp, warnings) + return resp, nil } @@ -355,7 +357,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R apiData: data, role: role, } - parsedBundle, err := signCert(b, input, signingBundle, true, useCSRValues) + parsedBundle, warnings, err := signCert(b, input, signingBundle, true, useCSRValues) if err != nil { switch err.(type) { case errutil.UserError: @@ -451,6 +453,8 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R resp.AddWarning("Max path length of the signed certificate is zero. This certificate cannot be used to issue intermediate CA certificates.") } + resp = addWarnings(resp, warnings) + return resp, nil } diff --git a/builtin/logical/pki/storage_test.go b/builtin/logical/pki/storage_test.go index 856175f04..3ff2de6d5 100644 --- a/builtin/logical/pki/storage_test.go +++ b/builtin/logical/pki/storage_test.go @@ -251,7 +251,7 @@ func genCertBundle(t *testing.T, b *backend, s logical.Storage) *certutil.CertBu apiData: apiData, role: role, } - parsedCertBundle, err := generateCert(sc, input, nil, true, b.GetRandomReader()) + parsedCertBundle, _, err := generateCert(sc, input, nil, true, b.GetRandomReader()) require.NoError(t, err) certBundle, err := parsedCertBundle.ToCertBundle() diff --git a/builtin/logical/pki/util.go b/builtin/logical/pki/util.go index ba2ca7a65..87e572fd9 100644 --- a/builtin/logical/pki/util.go +++ b/builtin/logical/pki/util.go @@ -350,3 +350,10 @@ func (sc *storageContext) isIfModifiedSinceBeforeLastModified(helper *IfModified return false, nil } + +func addWarnings(resp *logical.Response, warnings []string) *logical.Response { + for _, warning := range warnings { + resp.AddWarning(warning) + } + return resp +} diff --git a/changelog/17073.txt b/changelog/17073.txt new file mode 100644 index 000000000..96ab50bbc --- /dev/null +++ b/changelog/17073.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Add a warning to any successful response when the requested TTL is overwritten by MaxTTL +``` \ No newline at end of file