Always return PKI configs for CRLs, URLs (#15470)

* Always return non-nil CRL configuration

When using the default CRL configuration (as none has been set), return
the default configuration rather than inferring it in buildCRL. This
additionally allows us to return the default configuration on GET
operations to /config/crl.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Always return non-nil URL configuration

When using the default (empty) URL configuration as none has been set,
return the default configuration rather than inferring it inside of
fetchCAInfoByIssuerId or generateCert. This additionally allows us to
return the default configuration on GET operations to /config/urls.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

builtin/logical/pki/cert_util.go

@@ -143,13 +143,6 @@ func fetchCAInfoByIssuerId(ctx context.Context, b *backend, req *logical.Request
 	if err != nil {
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)}
 	}
-	if entries == nil {
-		entries = &certutil.URLEntries{
-			IssuingCertificates:   []string{},
-			CRLDistributionPoints: []string{},
-			OCSPServers:           []string{},
-		}
-	}
 	caInfo.URLs = entries
 
 	return caInfo, nil
@@ -633,13 +626,6 @@ func generateCert(ctx context.Context,
 			if err != nil {
 				return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch URL information: %v", err)}
 			}
-			if entries == nil {
-				entries = &certutil.URLEntries{
-					IssuingCertificates:   []string{},
-					CRLDistributionPoints: []string{},
-					OCSPServers:           []string{},
-				}
-			}
 			data.Params.URLs = entries
 
 			if input.role.MaxPathLength == nil {

builtin/logical/pki/crl_util.go

@@ -569,28 +569,26 @@ func buildCRL(ctx context.Context, b *backend, req *logical.Request, forceNew bo
 	crlLifetime := b.crlLifetime
 	var revokedCerts []pkix.RevokedCertificate
 
-	if crlInfo != nil {
+	if crlInfo.Expiry != "" {
-		if crlInfo.Expiry != "" {
+		crlDur, err := time.ParseDuration(crlInfo.Expiry)
-			crlDur, err := time.ParseDuration(crlInfo.Expiry)
+		if err != nil {
-			if err != nil {
+			return errutil.InternalError{Err: fmt.Sprintf("error parsing CRL duration of %s", crlInfo.Expiry)}
-				return errutil.InternalError{Err: fmt.Sprintf("error parsing CRL duration of %s", crlInfo.Expiry)}
+		}
-			}
+		crlLifetime = crlDur
-			crlLifetime = crlDur
+	}
+
+	if crlInfo.Disable {
+		if !forceNew {
+			return nil
 		}
 
-		if crlInfo.Disable {
+		// NOTE: in this case, the passed argument (revoked) is not added
-			if !forceNew {
+		// to the revokedCerts list. This is because we want to sign an
-				return nil
+		// **empty** CRL (as the CRL was disabled but we've specified the
-			}
+		// forceNew option). In previous versions of Vault (1.10 series and
-
+		// earlier), we'd have queried the certs below, whereas we now have
-			// NOTE: in this case, the passed argument (revoked) is not added
+		// an assignment from a pre-queried list.
-			// to the revokedCerts list. This is because we want to sign an
+		goto WRITE
-			// **empty** CRL (as the CRL was disabled but we've specified the
-			// forceNew option). In previous versions of Vault (1.10 series and
-			// earlier), we'd have queried the certs below, whereas we now have
-			// an assignment from a pre-queried list.
-			goto WRITE
-		}
 	}
 
 	revokedCerts = revoked

builtin/logical/pki/path_config_crl.go

@@ -54,11 +54,15 @@ func (b *backend) CRL(ctx context.Context, s logical.Storage) (*crlConfig, error
 	if err != nil {
 		return nil, err
 	}
-	if entry == nil {
-		return nil, nil
-	}
 
 	var result crlConfig
+	result.Expiry = b.crlLifetime.String()
+	result.Disable = false
+
+	if entry == nil {
+		return &result, nil
+	}
+
 	if err := entry.DecodeJSON(&result); err != nil {
 		return nil, err
 	}
@@ -71,9 +75,6 @@ func (b *backend) pathCRLRead(ctx context.Context, req *logical.Request, _ *fram
 	if err != nil {
 		return nil, err
 	}
-	if config == nil {
-		return nil, nil
-	}
 
 	return &logical.Response{
 		Data: map[string]interface{}{
@@ -88,9 +89,6 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
 	if err != nil {
 		return nil, err
 	}
-	if config == nil {
-		config = &crlConfig{}
-	}
 
 	if expiryRaw, ok := d.GetOk("expiry"); ok {
 		expiry := expiryRaw.(string)

builtin/logical/pki/path_config_urls.go

@@ -63,16 +63,22 @@ func getURLs(ctx context.Context, req *logical.Request) (*certutil.URLEntries, e
 	if err != nil {
 		return nil, err
 	}
-	if entry == nil {
+
-		return nil, nil
+	entries := &certutil.URLEntries{
+		IssuingCertificates:   []string{},
+		CRLDistributionPoints: []string{},
+		OCSPServers:           []string{},
 	}
 
-	var entries certutil.URLEntries
+	if entry == nil {
-	if err := entry.DecodeJSON(&entries); err != nil {
+		return entries, nil
+	}
+
+	if err := entry.DecodeJSON(entries); err != nil {
 		return nil, err
 	}
 
-	return &entries, nil
+	return entries, nil
 }
 
 func writeURLs(ctx context.Context, req *logical.Request, entries *certutil.URLEntries) error {
@@ -97,9 +103,6 @@ func (b *backend) pathReadURL(ctx context.Context, req *logical.Request, _ *fram
 	if err != nil {
 		return nil, err
 	}
-	if entries == nil {
-		return nil, nil
-	}
 
 	resp := &logical.Response{
 		Data: structs.New(entries).Map(),
@@ -113,13 +116,6 @@ func (b *backend) pathWriteURL(ctx context.Context, req *logical.Request, data *
 	if err != nil {
 		return nil, err
 	}
-	if entries == nil {
-		entries = &certutil.URLEntries{
-			IssuingCertificates:   []string{},
-			CRLDistributionPoints: []string{},
-			OCSPServers:           []string{},
-		}
-	}
 
 	if urlsInt, ok := data.GetOk("issuing_certificates"); ok {
 		entries.IssuingCertificates = urlsInt.([]string)

changelog/15470.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Always return CRLs, URLs configurations, even if using the default value.
+```