Smaller PR feedback items

 - pki.mdx doc update
 - parens around logical.go comment to indicate DER encoded request is
   related to OCSP and not the snapshots
 - Use AllIssuers instead of writing them all out
 - Drop zero initialization of crl config's Disable flag if not present
 - Upgrade issuer on the fly instead of an initial migration

builtin/logical/pki/cert_util.go

@@ -151,8 +151,7 @@ func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuerID, usage issuerU
 	return caInfo, nil
 }
 
-func fetchCertBySerialBigInt(ctx context.Context, b *backend, req *logical.Request,
-	prefix string, serial *big.Int) (*logical.StorageEntry, error) {
+func fetchCertBySerialBigInt(ctx context.Context, b *backend, req *logical.Request, prefix string, serial *big.Int) (*logical.StorageEntry, error) {
 	return fetchCertBySerial(ctx, b, req, prefix, certutil.GetHexFormatted(serial.Bytes(), ":"))
 }
 

builtin/logical/pki/path_config_crl.go

@@ -98,7 +98,11 @@ func (b *backend) pathCRLWrite(ctx context.Context, req *logical.Request, d *fra
 		config.OcspDisable = ocspDisableRaw.(bool)
 	}
 
-	err = sc.writeRevocationConfig(config)
+	entry, err := logical.StorageEntryJSON("config/crl", config)
+	if err != nil {
+		return nil, err
+	}
+	err = sc.Storage.Put(sc.Context, entry)
 	if err != nil {
 		return nil, err
 	}

builtin/logical/pki/storage.go

@@ -31,6 +31,8 @@ const (
 
 	maxRolesToScanOnIssuerChange = 100
 	maxRolesToFindOnIssuerChange = 10
+
+	latestIssuerVersion = 1
 )
 
 type keyID string
@@ -85,7 +87,7 @@ const (
 	// 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 | OCSPSigningUsage
+	AllIssuerUsages = ReadOnlyUsage | IssuanceUsage | CRLSigningUsage | OCSPSigningUsage
 )
 
 var namedIssuerUsages = map[string]issuerUsage{
@@ -153,6 +155,8 @@ type issuerEntry struct {
 	RevocationTime       int64                     `json:"revocation_time"`
 	RevocationTimeUTC    time.Time                 `json:"revocation_time_utc"`
 	AIAURIs              *certutil.URLEntries      `json:"aia_uris,omitempty"`
+	Version              uint                      `json:"version"`
+	Dirty                bool                      `json:"-"`
 }
 
 type localCRLConfigEntry struct {
@@ -206,7 +210,6 @@ func (sc *storageContext) fetchKeyById(keyId keyID) (*keyEntry, error) {
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch pki key: %v", err)}
 	}
 	if entry == nil {
-		// FIXME: Dedicated/specific error for this?
 		return nil, errutil.UserError{Err: fmt.Sprintf("pki key id %s does not exist", keyId.String())}
 	}
 
@@ -563,7 +566,6 @@ func (sc *storageContext) fetchIssuerById(issuerId issuerID) (*issuerEntry, erro
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to fetch pki issuer: %v", err)}
 	}
 	if entry == nil {
-		// FIXME: Dedicated/specific error for this?
 		return nil, errutil.UserError{Err: fmt.Sprintf("pki issuer id %s does not exist", issuerId.String())}
 	}
 
@@ -572,7 +574,27 @@ func (sc *storageContext) fetchIssuerById(issuerId issuerID) (*issuerEntry, erro
 		return nil, errutil.InternalError{Err: fmt.Sprintf("unable to decode pki issuer with id %s: %v", issuerId.String(), err)}
 	}
 
-	return &issuer, nil
+	return sc.upgradeIssuerIfRequired(&issuer), nil
+}
+
+func (sc *storageContext) upgradeIssuerIfRequired(issuer *issuerEntry) *issuerEntry {
+	// *NOTE*: Don't attempt to write out the issuer here as it may cause ErrReadOnly that will direct the
+	// request all the way up to the primary cluster which would be horrible for local cluster operations such
+	// as generating a leaf cert or a revoke.
+	if issuer.Version == latestIssuerVersion {
+		return issuer
+	}
+
+	if issuer.Version == 0 {
+		// handle our new OCSPSigning usage flag for earlier versions
+		if issuer.Usage.HasUsage(CRLSigningUsage) && !issuer.Usage.HasUsage(OCSPSigningUsage) {
+			issuer.Usage.ToggleUsage(OCSPSigningUsage)
+		}
+	}
+
+	issuer.Version = latestIssuerVersion
+	issuer.Dirty = true
+	return issuer
 }
 
 func (sc *storageContext) writeIssuer(issuer *issuerEntry) error {
@@ -681,7 +703,8 @@ func (sc *storageContext) importIssuer(certValue string, issuerName string) (*is
 	result.Name = issuerName
 	result.Certificate = certValue
 	result.LeafNotAfterBehavior = certutil.ErrNotAfterBehavior
-	result.Usage.ToggleUsage(IssuanceUsage, CRLSigningUsage, OCSPSigningUsage)
+	result.Usage.ToggleUsage(AllIssuerUsages)
+	result.Version = latestIssuerVersion
 
 	// We shouldn't add CSRs or multiple certificates in this
 	countCertificates := strings.Count(result.Certificate, "-BEGIN ")
@@ -1052,28 +1075,14 @@ func (sc *storageContext) getRevocationConfig() (*crlConfig, error) {
 	}
 
 	var result crlConfig
-	result.Expiry = sc.Backend.crlLifetime.String()
-	result.Disable = false
-
 	if entry == nil {
+		result.Expiry = sc.Backend.crlLifetime.String()
 		return &result, nil
 	}
 
-	if err := entry.DecodeJSON(&result); err != nil {
+	if err = entry.DecodeJSON(&result); err != nil {
 		return nil, err
 	}
 
 	return &result, nil
 }
-
-func (sc *storageContext) writeRevocationConfig(config *crlConfig) error {
-	entry, err := logical.StorageEntryJSON("config/crl", config)
-	if err != nil {
-		return err
-	}
-	err = sc.Storage.Put(sc.Context, entry)
-	if err != nil {
-		return err
-	}
-	return nil
-}

builtin/logical/pki/storage_migrations.go

@@ -193,7 +193,7 @@ func getLegacyCertBundle(ctx context.Context, s logical.Storage) (*issuerEntry,
 		SerialNumber:         cb.SerialNumber,
 		LeafNotAfterBehavior: certutil.ErrNotAfterBehavior,
 	}
-	issuer.Usage.ToggleUsage(IssuanceUsage, CRLSigningUsage)
+	issuer.Usage.ToggleUsage(AllIssuerUsages)
 
 	return issuer, cb, nil
 }

builtin/logical/pki/storage_test.go

@@ -183,6 +183,8 @@ func genIssuerAndKey(t *testing.T, b *backend, s logical.Storage) (issuerEntry,
 		Certificate:  strings.TrimSpace(certBundle.Certificate) + "\n",
 		CAChain:      certBundle.CAChain,
 		SerialNumber: certBundle.SerialNumber,
+		Usage:        AllIssuerUsages,
+		Version:      latestIssuerVersion,
 	}
 
 	return pkiIssuer, pkiKey

http/logical.go

@@ -103,8 +103,8 @@ func buildLogicalRequestNoAuth(perfStandby bool, w http.ResponseWriter, r *http.
 		bufferedBody := newBufferedReader(r.Body)
 		r.Body = bufferedBody
 
-		// If we are uploading a snapshot or receiving an ocsp-request which
-		// is der encoded we don't want to parse it. Instead, we will simply
+		// If we are uploading a snapshot or receiving an ocsp-request (which
+		// is der encoded) we don't want to parse it. Instead, we will simply
 		// add the HTTP request to the logical request object for later consumption.
 		contentType := r.Header.Get("Content-Type")
 		if path == "sys/storage/raft/snapshot" || path == "sys/storage/raft/snapshot-force" || isOcspRequest(contentType) {

website/content/api-docs/secret/pki.mdx

@@ -1153,6 +1153,7 @@ At this time there are certain limitations of the OCSP implementation at this pa
  1. Only a single serial number within the request will appear in the response
  1. None of the extensions defined in the RFC are supported for requests or responses
  1. Ed25519 backed CA's are not supported for OCSP requests
+ 1. Note that this api will not work with the Vault client as both request and responses are DER encoded
 
 These are unauthenticated endpoints.