Use issuer's RevocationSigAlg for CRL signing

We introduce a new parameter on issuers, revocation_signature_algorithm
to control the signature algorithm used during CRL signing. This is
because the SignatureAlgorithm value from the certificate itself is
incorrect for this purpose: a RSA root could sign an ECDSA intermediate
with say, SHA256WithRSA, but when the intermediate goes to sign a CRL,
it must use ECDSAWithSHA256 or equivalent instead of SHA256WithRSA. When
coupled with support for PSS-only keys, allowing the user to set the
signature algorithm value as desired seems like the best approach.

Signed-off-by: Alexander Scheel <[email protected]>

builtin/logical/pki/cert_util.go

@@ -138,6 +138,7 @@ func (sc *storageContext) fetchCAInfoByIssuerId(issuerId issuerID, usage issuerU
  ParsedCertBundle: *parsedBundle,
  URLs: nil,
  LeafNotAfterBehavior: entry.LeafNotAfterBehavior,
+ RevocationSigAlg: entry.RevocationSigAlg,
  }
 
  entries, err := getURLs(sc.Context, sc.Storage)

builtin/logical/pki/crl_test.go

@@ -2,6 +2,7 @@ package pki
 
 import (
  "context"
+ "encoding/asn1"
  "strings"
  "testing"
  "time"
@@ -25,6 +26,61 @@ func TestBackend_CRL_EnableDisableRoot(t *testing.T) {
  crlEnableDisableTestForBackend(t, b, s, []string{caSerial})
 }
 
+func TestBackend_CRL_AllKeyTypeSigAlgos(t *testing.T) {
+ type testCase struct {
+ KeyType string
+ KeyBits int
+ SigAlgo string
+ }
+
+ testCases := []testCase{
+ {"rsa", 2048, "SHA256WithRSA"},
+ {"rsa", 2048, "SHA384WithRSA"},
+ {"rsa", 2048, "SHA512WithRSA"},
+ {"rsa", 2048, "SHA256WithRSAPSS"},
+ {"rsa", 2048, "SHA384WithRSAPSS"},
+ {"rsa", 2048, "SHA512WithRSAPSS"},
+ {"ec", 256, "ECDSAWithSHA256"},
+ {"ec", 384, "ECDSAWithSHA384"},
+ {"ec", 521, "ECDSAWithSHA512"},
+ {"ed25519", 0, "PureEd25519"},
+ }
+
+ for index, tc := range testCases {
+ t.Logf("tv %v", index)
+ b, s := createBackendWithStorage(t)
+
+ resp, err := CBWrite(b, s, "root/generate/internal", map[string]interface{}{
+ "ttl": "40h",
+ "common_name": "myvault.com",
+ "key_type": tc.KeyType,
+ "key_bits": tc.KeyBits,
+ })
+ if err != nil {
+ t.Fatalf("tc %v: %v", index, err)
+ }
+ caSerial := resp.Data["serial_number"].(string)
+
+ _, err = CBPatch(b, s, "issuer/default", map[string]interface{}{
+ "revocation_signature_algorithm": tc.SigAlgo,
+ })
+ if err != nil {
+ t.Fatalf("tc %v: %v", index, err)
+ }
+
+ crlEnableDisableTestForBackend(t, b, s, []string{caSerial})
+
+ crl := getParsedCrlFromBackend(t, b, s, "crl")
+ if strings.HasSuffix(tc.SigAlgo, "PSS") {
+ algo := crl.SignatureAlgorithm
+ pssOid := asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 10}
+ if !algo.Algorithm.Equal(pssOid) {
+ t.Fatalf("tc %v failed: expected sig-alg to be %v / got %v", index, pssOid, algo)
+ }
+ }
+ }
+}
+
 func TestBackend_CRL_EnableDisableIntermediateWithRoot(t *testing.T) {
  crlEnableDisableIntermediateTestForBackend(t, true)
 }

builtin/logical/pki/crl_util.go

@@ -622,6 +622,7 @@ WRITE:
  Number: big.NewInt(crlNumber),
  ThisUpdate: time.Now(),
  NextUpdate: time.Now().Add(crlLifetime),
+ SignatureAlgorithm: signingBundle.RevocationSigAlg,
  }
 
  crlBytes, err := x509.CreateRevocationList(rand.Reader, revocationListTemplate, signingBundle.Certificate, signingBundle.PrivateKey)

builtin/logical/pki/path_fetch_issuers.go

@@ -2,6 +2,7 @@ package pki
 
 import (
  "context"
+ "crypto/x509"
  "encoding/pem"
  "fmt"
  "strings"
@@ -106,6 +107,17 @@ this issuer; valid values are "read-only", "issuing-certificates", and
 and always set.`,
  Default: []string{"read-only", "issuing-certificates", "crl-signing"},
  }
+ fields["revocation_signature_algorithm"] = &framework.FieldSchema{
+ Type: framework.TypeString,
+ Description: `Which x509.SignatureAlgorithm name to use for
+signing CRLs. This parameter allows differentiation between PKCS#1v1.5
+and PSS keys and choice of signature hash algorithm. The default (empty
+string) value is for Go to select the signature algorithm. This can fail
+if the underlying key does not support the requested signature algorithm,
+which may not be known at modification time (such as with PKCS#11 managed
+RSA keys).`,
+ Default: "",
+ }
 
  return &framework.Path{
  // Returns a JSON entry.
@@ -181,14 +193,15 @@ func respondReadIssuer(issuer *issuerEntry) (*logical.Response, error) {
 
  return &logical.Response{
  Data: map[string]interface{}{
- "issuer_id": issuer.ID,
- "issuer_name": issuer.Name,
- "key_id": issuer.KeyID,
- "certificate": issuer.Certificate,
- "manual_chain": respManualChain,
- "ca_chain": issuer.CAChain,
- "leaf_not_after_behavior": issuer.LeafNotAfterBehavior.String(),
- "usage": issuer.Usage.Names(),
+ "issuer_id": issuer.ID,
+ "issuer_name": issuer.Name,
+ "key_id": issuer.KeyID,
+ "certificate": issuer.Certificate,
+ "manual_chain": respManualChain,
+ "ca_chain": issuer.CAChain,
+ "leaf_not_after_behavior": issuer.LeafNotAfterBehavior.String(),
+ "usage": issuer.Usage.Names(),
+ "revocation_signature_algorithm": issuer.RevocationSigAlg,
  },
  }, nil
 }
@@ -258,6 +271,23 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
  return logical.ErrorResponse(fmt.Sprintf("Unable to parse specified usages: %v - valid values are %v", rawUsage, AllIssuerUsages.Names())), nil
  }
 
+ // Revocation signature algorithm changes
+ revSigAlgStr := data.Get("revocation_signature_algorithm").(string)
+ revSigAlg, present := certutil.SignatureAlgorithmNames[revSigAlgStr]
+ if !present && revSigAlgStr != "" {
+ var knownAlgos []string
+ for algoName := range certutil.SignatureAlgorithmNames {
+ knownAlgos = append(knownAlgos, algoName)
+ }
+
+ return logical.ErrorResponse(fmt.Sprintf("Unknown signature algorithm value: %v - valid values are %v", revSigAlg, strings.Join(knownAlgos, ", "))), nil
+ } else if revSigAlgStr == "" {
+ revSigAlg = x509.UnknownSignatureAlgorithm
+ }
+ if err := issuer.CanMaybeSignWithAlgo(revSigAlg); err != nil {
+ return nil, err
+ }
+
  modified := false
 
  var oldName string
@@ -277,6 +307,11 @@ func (b *backend) pathUpdateIssuer(ctx context.Context, req *logical.Request, da
  modified = true
  }
 
+ if revSigAlg != issuer.RevocationSigAlg {
+ issuer.RevocationSigAlg = revSigAlg
+ modified = true
+ }
+
  var updateChain bool
  var constructedChain []issuerID
  for index, newPathRef := range newPath {
@@ -426,6 +461,32 @@ func (b *backend) pathPatchIssuer(ctx context.Context, req *logical.Request, dat
  }
  }
 
+ // Revocation signature algorithm changes
+ rawRevSigAlg, ok := data.GetOk("revocation_signature_algorithm")
+ if ok {
+ revSigAlgStr := rawRevSigAlg.(string)
+ revSigAlg, present := certutil.SignatureAlgorithmNames[revSigAlgStr]
+ if !present && revSigAlgStr != "" {
+ var knownAlgos []string
+ for algoName := range certutil.SignatureAlgorithmNames {
+ knownAlgos = append(knownAlgos, algoName)
+ }
+
+ return logical.ErrorResponse(fmt.Sprintf("Unknown signature algorithm value: %v - valid values are %v", revSigAlg, strings.Join(knownAlgos, ", "))), nil
+ } else if revSigAlgStr == "" {
+ revSigAlg = x509.UnknownSignatureAlgorithm
+ }
+
+ if err := issuer.CanMaybeSignWithAlgo(revSigAlg); err != nil {
+ return nil, err
+ }
+
+ if revSigAlg != issuer.RevocationSigAlg {
+ issuer.RevocationSigAlg = revSigAlg
+ modified = true
+ }
+ }
+
  // Manual Chain Changes
  newPathData, ok := data.GetOk("manual_chain")
  if ok {

builtin/logical/pki/storage.go

@@ -145,6 +145,7 @@ type issuerEntry struct {
  SerialNumber string `json:"serial_number"`
  LeafNotAfterBehavior certutil.NotAfterBehavior `json:"not_after_behavior"`
  Usage issuerUsage `json:"usage"`
+ RevocationSigAlg x509.SignatureAlgorithm `json:"revocation_signature_algorithm"`
 }
 
 type localCRLConfigEntry struct {
@@ -427,6 +428,51 @@ func (i issuerEntry) EnsureUsage(usage issuerUsage) error {
  return fmt.Errorf("unknown delta between usages: %v -> %v / for issuer [%v]", usage.Names(), i.Usage.Names(), issuerRef)
 }
 
+func (i issuerEntry) CanMaybeSignWithAlgo(algo x509.SignatureAlgorithm) error {
+ // Hack: Go isn't kind enough expose its lovely signatureAlgorithmDetails
+ // informational struct for our usage. However, we don't want to actually
+ // fetch the private key and attempt a signature with this algo (as we'll
+ // mint new, previously unsigned material in the process that could maybe
+ // be potentially abused if it leaks).
+ //
+ // So...
+ //
+ // ...we maintain our own mapping of cert.PKI<->sigAlgos. Notably, we
+ // exclude DSA support as the PKI engine has never supported DSA keys.
+ if algo == x509.UnknownSignatureAlgorithm {
+ // Special cased to indicate upgrade and letting Go automatically
+ // chose the correct value.
+ return nil
+ }
+
+ cert, err := i.GetCertificate()
+ if err != nil {
+ return fmt.Errorf("unable to parse issuer's potential signature algorithm types: %v", err)
+ }
+
+ switch cert.PublicKeyAlgorithm {
+ case x509.RSA:
+ switch algo {
+ case x509.SHA256WithRSA, x509.SHA384WithRSA, x509.SHA512WithRSA,
+ x509.SHA256WithRSAPSS, x509.SHA384WithRSAPSS,
+ x509.SHA512WithRSAPSS:
+ return nil
+ }
+ case x509.ECDSA:
+ switch algo {
+ case x509.ECDSAWithSHA256, x509.ECDSAWithSHA384, x509.ECDSAWithSHA512:
+ return nil
+ }
+ case x509.Ed25519:
+ switch algo {
+ case x509.PureEd25519:
+ return nil
+ }
+ }
+
+ return fmt.Errorf("unable to use issuer of type %v to sign with %v key type", cert.PublicKeyAlgorithm.String(), algo.String())
+}
+
 func (sc *storageContext) listIssuers() ([]issuerID, error) {
  strList, err := sc.Storage.List(sc.Context, issuerPrefix)
  if err != nil {

builtin/logical/pki/test_helpers.go

@@ -284,6 +284,10 @@ func CBWrite(b *backend, s logical.Storage, path string, data map[string]interfa
  return CBReq(b, s, logical.UpdateOperation, path, data)
 }
 
+func CBPatch(b *backend, s logical.Storage, path string, data map[string]interface{}) (*logical.Response, error) {
+ return CBReq(b, s, logical.PatchOperation, path, data)
+}
+
 func CBList(b *backend, s logical.Storage, path string) (*logical.Response, error) {
  return CBReq(b, s, logical.ListOperation, path, make(map[string]interface{}))
 }

sdk/helper/certutil/helpers.go

@@ -49,6 +49,21 @@ var expectedNISTPCurveHashBits = map[int]int{
  521: 512,
 }
 
+// Mapping of constant names<->constant values for SignatureAlgorithm
+var SignatureAlgorithmNames = map[string]x509.SignatureAlgorithm{
+ "SHA256WithRSA": x509.SHA256WithRSA,
+ "SHA384WithRSA": x509.SHA384WithRSA,
+ "SHA512WithRSA": x509.SHA512WithRSA,
+ "ECDSAWithSHA256": x509.ECDSAWithSHA256,
+ "ECDSAWithSHA384": x509.ECDSAWithSHA384,
+ "ECDSAWithSHA512": x509.ECDSAWithSHA512,
+ "SHA256WithRSAPSS": x509.SHA256WithRSAPSS,
+ "SHA384WithRSAPSS": x509.SHA384WithRSAPSS,
+ "SHA512WithRSAPSS": x509.SHA512WithRSAPSS,
+ "PureEd25519": x509.PureEd25519,
+ "Ed25519": x509.PureEd25519, // Duplicated for clarity; most won't expect the "Pure" prefix.
+}
+
 // GetHexFormatted returns the byte buffer formatted in hex with
 // the specified separator between bytes.
 func GetHexFormatted(buf []byte, sep string) string {

sdk/helper/certutil/types.go

@@ -710,6 +710,7 @@ type CAInfoBundle struct {
  ParsedCertBundle
  URLs *URLEntries
  LeafNotAfterBehavior NotAfterBehavior
+ RevocationSigAlg x509.SignatureAlgorithm
 }
 
 func (b *CAInfoBundle) GetCAChain() []*CertBlock {