From 8acbf7f4804be71215277088b3490ade80eeaa44 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Wed, 3 Aug 2022 12:42:24 -0400 Subject: [PATCH] Add PSS support to PKI Secrets Engine (#16519) * Add PSS signature support to Vault PKI engine Signed-off-by: Alexander Scheel * 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 * Add use_pss, revocation_signature_algorithm docs Signed-off-by: Alexander Scheel * Add PSS to signature role issuance test matrix Signed-off-by: Alexander Scheel * Add changelog Signed-off-by: Alexander Scheel * Allow roots to self-identify revocation alg When using PSS support with a managed key, sometimes the underlying device will not support PKCS#1v1.5 signatures. This results in CRL building failing, unless we update the entry's signature algorithm prior to building the CRL for the new root. With a RSA-type key and use_pss=true, we use the signature bits value to decide which hash function to use for PSS support. Signed-off-by: Alexander Scheel * Add clearer error message on failed import When CRL building fails during cert/key import, due to PSS failures, give a better indication to the user that import succeeded its just CRL building that failed. This tells them the parameter to adjust on the issuer and warns that CRL building will fail until this is fixed. Signed-off-by: Alexander Scheel * Add case insensitive SigAlgo matching Signed-off-by: Alexander Scheel * Convert UsePSS back to regular bool Signed-off-by: Alexander Scheel * Refactor PSS->certTemplate into helper function Signed-off-by: Alexander Scheel * Proper string output on rev_sig_alg display Signed-off-by: Alexander Scheel * Copy root's SignatureAlgorithm for CRL building Signed-off-by: Alexander Scheel --- builtin/logical/pki/backend_test.go | 43 ++++++++---- builtin/logical/pki/ca_util.go | 1 + builtin/logical/pki/cert_util.go | 2 + builtin/logical/pki/crl_test.go | 56 +++++++++++++++ builtin/logical/pki/crl_util.go | 1 + builtin/logical/pki/fields.go | 7 ++ builtin/logical/pki/path_fetch_issuers.go | 82 +++++++++++++++++++--- builtin/logical/pki/path_intermediate.go | 7 +- builtin/logical/pki/path_issue_sign.go | 8 +++ builtin/logical/pki/path_manage_issuers.go | 8 +++ builtin/logical/pki/path_roles.go | 11 +++ builtin/logical/pki/path_root.go | 20 ++++++ builtin/logical/pki/path_sign_issuers.go | 7 ++ builtin/logical/pki/storage.go | 46 ++++++++++++ builtin/logical/pki/test_helpers.go | 4 ++ changelog/16519.txt | 3 + sdk/helper/certutil/helpers.go | 65 ++++++++++------- sdk/helper/certutil/types.go | 2 + website/content/api-docs/secret/pki.mdx | 24 +++++++ 19 files changed, 351 insertions(+), 46 deletions(-) create mode 100644 changelog/16519.txt diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 1f299f853..db365a4f8 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -3552,6 +3552,7 @@ func TestReadWriteDeleteRoles(t *testing.T) { "allowed_serial_numbers": []interface{}{}, "generate_lease": false, "signature_bits": json.Number("256"), + "use_pss": false, "allowed_domains": []interface{}{}, "allowed_uri_sans_template": false, "enforce_hostnames": true, @@ -4459,6 +4460,7 @@ type KeySizeRegression struct { // Signature Bits presently is only specified on the role. RoleSignatureBits []int + RoleUsePSS bool // These are tuples; must be of the same length. TestKeyTypes []string @@ -4502,6 +4504,7 @@ func RoleKeySizeRegressionHelper(t *testing.T, b *backend, s logical.Storage, in "key_type": test.RoleKeyType, "key_bits": roleKeyBits, "signature_bits": roleSignatureBits, + "use_pss": test.RoleUsePSS, }) if err != nil { t.Fatal(err) @@ -4527,6 +4530,15 @@ func RoleKeySizeRegressionHelper(t *testing.T, b *backend, s logical.Storage, in t.Fatalf("key size regression test [%d] failed: haveErr: %v, expectErr: %v, err: %v, resp: %v, test case: %v, caKeyType: %v, caKeyBits: %v, role: %v, keyType: %v, keyBits: %v", index, haveErr, test.ExpectError, err, resp, test, caKeyType, caKeyBits, role, keyType, keyBits) } + if resp != nil && test.RoleUsePSS && caKeyType == "rsa" { + leafCert := parseCert(t, resp.Data["certificate"].(string)) + switch leafCert.SignatureAlgorithm { + case x509.SHA256WithRSAPSS, x509.SHA384WithRSAPSS, x509.SHA512WithRSAPSS: + default: + t.Fatalf("key size regression test [%d] failed on role %v: unexpected signature algorithm; expected RSA-type CA to sign a leaf cert with PSS algorithm; got %v", index, role, leafCert.SignatureAlgorithm.String()) + } + } + tested += 1 } } @@ -4548,36 +4560,41 @@ func TestBackend_Roles_KeySizeRegression(t *testing.T) { testCases := []KeySizeRegression{ // RSA with default parameters should fail to issue smaller RSA keys // and any size ECDSA/Ed25519 keys. - /* 0 */ {"rsa", []int{0, 2048}, []int{0, 256, 384, 512}, []string{"rsa", "ec", "ec", "ec", "ec", "ed25519"}, []int{1024, 224, 256, 384, 521, 0}, true}, + /* 0 */ {"rsa", []int{0, 2048}, []int{0, 256, 384, 512}, false, []string{"rsa", "ec", "ec", "ec", "ec", "ed25519"}, []int{1024, 224, 256, 384, 521, 0}, true}, // But it should work to issue larger RSA keys. - /* 1 */ {"rsa", []int{0, 2048}, []int{0, 256, 384, 512}, []string{"rsa", "rsa"}, []int{2048, 3072}, false}, + /* 1 */ {"rsa", []int{0, 2048}, []int{0, 256, 384, 512}, false, []string{"rsa", "rsa"}, []int{2048, 3072}, false}, // EC with default parameters should fail to issue smaller EC keys // and any size RSA/Ed25519 keys. - /* 2 */ {"ec", []int{0}, []int{0}, []string{"rsa", "ec", "ed25519"}, []int{2048, 224, 0}, true}, + /* 2 */ {"ec", []int{0}, []int{0}, false, []string{"rsa", "ec", "ed25519"}, []int{2048, 224, 0}, true}, // But it should work to issue larger EC keys. Note that we should be // independent of signature bits as that's computed from the issuer // type (for EC based issuers). - /* 3 */ {"ec", []int{224}, []int{0, 256, 384, 521}, []string{"ec", "ec", "ec", "ec"}, []int{224, 256, 384, 521}, false}, - /* 4 */ {"ec", []int{0, 256}, []int{0, 256, 384, 521}, []string{"ec", "ec", "ec"}, []int{256, 384, 521}, false}, - /* 5 */ {"ec", []int{384}, []int{0, 256, 384, 521}, []string{"ec", "ec"}, []int{384, 521}, false}, - /* 6 */ {"ec", []int{521}, []int{0, 256, 384, 512}, []string{"ec"}, []int{521}, false}, + /* 3 */ {"ec", []int{224}, []int{0, 256, 384, 521}, false, []string{"ec", "ec", "ec", "ec"}, []int{224, 256, 384, 521}, false}, + /* 4 */ {"ec", []int{0, 256}, []int{0, 256, 384, 521}, false, []string{"ec", "ec", "ec"}, []int{256, 384, 521}, false}, + /* 5 */ {"ec", []int{384}, []int{0, 256, 384, 521}, false, []string{"ec", "ec"}, []int{384, 521}, false}, + /* 6 */ {"ec", []int{521}, []int{0, 256, 384, 512}, false, []string{"ec"}, []int{521}, false}, // Ed25519 should reject RSA and EC keys. - /* 7 */ {"ed25519", []int{0}, []int{0}, []string{"rsa", "ec", "ec"}, []int{2048, 256, 521}, true}, + /* 7 */ {"ed25519", []int{0}, []int{0}, false, []string{"rsa", "ec", "ec"}, []int{2048, 256, 521}, true}, // But it should work to issue Ed25519 keys. - /* 8 */ {"ed25519", []int{0}, []int{0}, []string{"ed25519"}, []int{0}, false}, + /* 8 */ {"ed25519", []int{0}, []int{0}, false, []string{"ed25519"}, []int{0}, false}, // Any key type should reject insecure RSA key sizes. - /* 9 */ {"any", []int{0}, []int{0, 256, 384, 512}, []string{"rsa", "rsa"}, []int{512, 1024}, true}, + /* 9 */ {"any", []int{0}, []int{0, 256, 384, 512}, false, []string{"rsa", "rsa"}, []int{512, 1024}, true}, // But work for everything else. - /* 10 */ {"any", []int{0}, []int{0, 256, 384, 512}, []string{"rsa", "rsa", "ec", "ec", "ec", "ec", "ed25519"}, []int{2048, 3072, 224, 256, 384, 521, 0}, false}, + /* 10 */ {"any", []int{0}, []int{0, 256, 384, 512}, false, []string{"rsa", "rsa", "ec", "ec", "ec", "ec", "ed25519"}, []int{2048, 3072, 224, 256, 384, 521, 0}, false}, // RSA with larger than default key size should reject smaller ones. - /* 11 */ {"rsa", []int{3072}, []int{0, 256, 384, 512}, []string{"rsa"}, []int{2048}, true}, + /* 11 */ {"rsa", []int{3072}, []int{0, 256, 384, 512}, false, []string{"rsa"}, []int{2048}, true}, + + // We should be able to sign with PSS with any CA key type. + /* 12 */ {"rsa", []int{0}, []int{0, 256, 384, 512}, true, []string{"rsa"}, []int{2048}, false}, + /* 13 */ {"ec", []int{0}, []int{0}, true, []string{"ec"}, []int{256}, false}, + /* 14 */ {"ed25519", []int{0}, []int{0}, true, []string{"ed25519"}, []int{0}, false}, } - if len(testCases) != 12 { + if len(testCases) != 15 { t.Fatalf("misnumbered test case entries will make it hard to find bugs: %v", len(testCases)) } diff --git a/builtin/logical/pki/ca_util.go b/builtin/logical/pki/ca_util.go index 2b7d79317..d2a59f34b 100644 --- a/builtin/logical/pki/ca_util.go +++ b/builtin/logical/pki/ca_util.go @@ -49,6 +49,7 @@ func getGenerationParams(sc *storageContext, data *framework.FieldData) (exporte KeyType: keyType, KeyBits: keyBits, SignatureBits: data.Get("signature_bits").(int), + UsePSS: data.Get("use_pss").(bool), AllowLocalhost: true, AllowAnyName: true, AllowIPSANs: true, diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 9633ea959..4aff5b37f 100644 --- a/builtin/logical/pki/cert_util.go +++ b/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) @@ -1373,6 +1374,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn KeyType: data.role.KeyType, KeyBits: data.role.KeyBits, SignatureBits: data.role.SignatureBits, + UsePSS: data.role.UsePSS, NotAfter: notAfter, KeyUsage: x509.KeyUsage(parseKeyUsages(data.role.KeyUsage)), ExtKeyUsage: parseExtKeyUsages(data.role), diff --git a/builtin/logical/pki/crl_test.go b/builtin/logical/pki/crl_test.go index de8ff0eee..428777e41 100644 --- a/builtin/logical/pki/crl_test.go +++ b/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) } diff --git a/builtin/logical/pki/crl_util.go b/builtin/logical/pki/crl_util.go index ed572fa86..6fa29a2b7 100644 --- a/builtin/logical/pki/crl_util.go +++ b/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) diff --git a/builtin/logical/pki/fields.go b/builtin/logical/pki/fields.go index 22100f388..3b6ed9773 100644 --- a/builtin/logical/pki/fields.go +++ b/builtin/logical/pki/fields.go @@ -319,6 +319,13 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length }, } + fields["use_pss"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Default: false, + Description: `Whether or not to use PSS signatures when using a +RSA key-type issuer. Defaults to false.`, + } + fields["key_type"] = &framework.FieldSchema{ Type: framework.TypeString, Default: "rsa", diff --git a/builtin/logical/pki/path_fetch_issuers.go b/builtin/logical/pki/path_fetch_issuers.go index 461b9aab6..6049db52f 100644 --- a/builtin/logical/pki/path_fetch_issuers.go +++ b/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. @@ -179,16 +191,22 @@ func respondReadIssuer(issuer *issuerEntry) (*logical.Response, error) { respManualChain = append(respManualChain, string(entity)) } + revSigAlgStr := issuer.RevocationSigAlg.String() + if issuer.RevocationSigAlg == x509.UnknownSignatureAlgorithm { + revSigAlgStr = "" + } + 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": revSigAlgStr, }, }, nil } @@ -258,6 +276,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[strings.ToLower(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 +312,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 +466,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[strings.ToLower(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 { diff --git a/builtin/logical/pki/path_intermediate.go b/builtin/logical/pki/path_intermediate.go index f13bf75ef..b33c22f36 100644 --- a/builtin/logical/pki/path_intermediate.go +++ b/builtin/logical/pki/path_intermediate.go @@ -66,13 +66,18 @@ func (b *backend) pathGenerateIntermediate(ctx context.Context, req *logical.Req // Nasty hack part two. :-) For generation of CSRs, certutil presently doesn't // support configuration of this. However, because we need generation parameters, // which create a role and attempt to read this parameter, we need to provide - // a value (which will be ignored). Hence, we stub in the missing parameter here, + // a value (which will be ignored). Hence, we stub in the missing parameters here, // including its schema, just enough for it to work.. data.Schema["signature_bits"] = &framework.FieldSchema{ Type: framework.TypeInt, Default: 0, } data.Raw["signature_bits"] = 0 + data.Schema["use_pss"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Default: false, + } + data.Raw["use_pss"] = false sc := b.makeStorageContext(ctx, req.Storage) exported, format, role, errorResp := getGenerationParams(sc, data) diff --git a/builtin/logical/pki/path_issue_sign.go b/builtin/logical/pki/path_issue_sign.go index 23ea27920..79b47bdf2 100644 --- a/builtin/logical/pki/path_issue_sign.go +++ b/builtin/logical/pki/path_issue_sign.go @@ -151,6 +151,13 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length }, } + ret.Fields["use_pss"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Default: false, + Description: `Whether or not to use PSS signatures when using a +RSA key-type issuer. Defaults to false.`, + } + return ret } @@ -211,6 +218,7 @@ func (b *backend) pathSignVerbatim(ctx context.Context, req *logical.Request, da ExtKeyUsage: data.Get("ext_key_usage").([]string), ExtKeyUsageOIDs: data.Get("ext_key_usage_oids").([]string), SignatureBits: data.Get("signature_bits").(int), + UsePSS: data.Get("use_pss").(bool), } *entry.AllowWildcardCertificates = true diff --git a/builtin/logical/pki/path_manage_issuers.go b/builtin/logical/pki/path_manage_issuers.go index 969e03377..f6eb73bb1 100644 --- a/builtin/logical/pki/path_manage_issuers.go +++ b/builtin/logical/pki/path_manage_issuers.go @@ -82,6 +82,7 @@ with Active Directory Certificate Services.`, // signed certificate's bits (that's on the /sign-intermediate // endpoints). Remove it from the list of fields to avoid confusion. delete(ret.Fields, "signature_bits") + delete(ret.Fields, "use_pss") return ret } @@ -238,6 +239,13 @@ func (b *backend) pathImportIssuers(ctx context.Context, req *logical.Request, d if len(createdIssuers) > 0 { err := b.crlBuilder.rebuild(ctx, b, req, true) if err != nil { + // Before returning, check if the error message includes the + // string "PSS". If so, it indicates we might've wanted to modify + // this issuer, so convert the error to a warning. + if strings.Contains(err.Error(), "PSS") || strings.Contains(err.Error(), "pss") { + err = fmt.Errorf("Rebuilding the CRL failed with a message relating to the PSS signature algorithm. This likely means the revocation_signature_algorithm needs to be set on the newly imported issuer(s) because a managed key supports only the PSS algorithm; by default PKCS#1v1.5 was used to build the CRLs. CRLs will not be generated until this has been addressed, however the import was successful. The original error is reproduced below:\n\n\t%v", err) + } + return nil, err } } diff --git a/builtin/logical/pki/path_roles.go b/builtin/logical/pki/path_roles.go index 1aab44686..e3a3b9096 100644 --- a/builtin/logical/pki/path_roles.go +++ b/builtin/logical/pki/path_roles.go @@ -238,6 +238,13 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length (SHA-2-256 for RSA keys, and matching the curve size for NIST P-Curves).`, }, + "use_pss": { + Type: framework.TypeBool, + Default: false, + Description: `Whether or not to use PSS signatures when using a +RSA key-type issuer. Defaults to false.`, + }, + "key_usage": { Type: framework.TypeCommaStringSlice, Default: []string{"DigitalSignature", "KeyAgreement", "KeyEncipherment"}, @@ -673,6 +680,7 @@ func (b *backend) pathRoleCreate(ctx context.Context, req *logical.Request, data KeyType: data.Get("key_type").(string), KeyBits: data.Get("key_bits").(int), SignatureBits: data.Get("signature_bits").(int), + UsePSS: data.Get("use_pss").(bool), UseCSRCommonName: data.Get("use_csr_common_name").(bool), UseCSRSANs: data.Get("use_csr_sans").(bool), KeyUsage: data.Get("key_usage").([]string), @@ -869,6 +877,7 @@ func (b *backend) pathRolePatch(ctx context.Context, req *logical.Request, data KeyType: getWithExplicitDefault(data, "key_type", oldEntry.KeyType).(string), KeyBits: getWithExplicitDefault(data, "key_bits", oldEntry.KeyBits).(int), SignatureBits: getWithExplicitDefault(data, "signature_bits", oldEntry.SignatureBits).(int), + UsePSS: getWithExplicitDefault(data, "use_pss", oldEntry.UsePSS).(bool), UseCSRCommonName: getWithExplicitDefault(data, "use_csr_common_name", oldEntry.UseCSRCommonName).(bool), UseCSRSANs: getWithExplicitDefault(data, "use_csr_sans", oldEntry.UseCSRSANs).(bool), KeyUsage: getWithExplicitDefault(data, "key_usage", oldEntry.KeyUsage).([]string), @@ -1063,6 +1072,7 @@ type roleEntry struct { UseCSRSANs bool `json:"use_csr_sans"` KeyType string `json:"key_type"` KeyBits int `json:"key_bits"` + UsePSS bool `json:"use_pss"` SignatureBits int `json:"signature_bits"` MaxPathLength *int `json:",omitempty"` KeyUsageOld string `json:"key_usage,omitempty"` @@ -1118,6 +1128,7 @@ func (r *roleEntry) ToResponseData() map[string]interface{} { "key_type": r.KeyType, "key_bits": r.KeyBits, "signature_bits": r.SignatureBits, + "use_pss": r.UsePSS, "key_usage": r.KeyUsage, "ext_key_usage": r.ExtKeyUsage, "ext_key_usage_oids": r.ExtKeyUsageOIDs, diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 1c46e881c..7ef6151b8 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -236,6 +236,25 @@ func (b *backend) pathCAGenerateRoot(ctx context.Context, req *logical.Request, resp.Data["key_id"] = myKey.ID resp.Data["key_name"] = myKey.Name + // Update the issuer to reflect the PSS status here for revocation; this + // allows CRL building to succeed if the root is using a managed key with + // only PSS support. + if input.role.KeyType == "rsa" && input.role.UsePSS { + // The one time that it is safe (and good) to copy the + // SignatureAlgorithm field off the certificate (for the purposes of + // detecting PSS support) is when we've freshly generated it AND it + // is a root (exactly this endpoint). + // + // For intermediates, this doesn't hold (not this endpoint) as that + // reflects the parent key's preferences. For imports, this doesn't + // hold as the old system might've allowed other signature types that + // the new system (whether Vault or a managed key) doesn't. + myIssuer.RevocationSigAlg = parsedBundle.Certificate.SignatureAlgorithm + if err := sc.writeIssuer(myIssuer); err != nil { + return nil, fmt.Errorf("unable to store PSS-updated issuer: %v", err) + } + } + // Also store it as just the certificate identified by serial number, so it // can be revoked err = req.Storage.Put(ctx, &logical.StorageEntry{ @@ -290,6 +309,7 @@ func (b *backend) pathIssuerSignIntermediate(ctx context.Context, req *logical.R EnforceHostnames: false, KeyType: "any", SignatureBits: data.Get("signature_bits").(int), + UsePSS: data.Get("use_pss").(bool), AllowedOtherSANs: []string{"*"}, AllowedSerialNumbers: []string{"*"}, AllowedURISANs: []string{"*"}, diff --git a/builtin/logical/pki/path_sign_issuers.go b/builtin/logical/pki/path_sign_issuers.go index 5f1ce76a0..cadbac555 100644 --- a/builtin/logical/pki/path_sign_issuers.go +++ b/builtin/logical/pki/path_sign_issuers.go @@ -85,6 +85,13 @@ in the above RFC section.`, }, } + fields["use_pss"] = &framework.FieldSchema{ + Type: framework.TypeBool, + Default: false, + Description: `Whether or not to use PSS signatures when using a +RSA key-type issuer. Defaults to false.`, + } + return path } diff --git a/builtin/logical/pki/storage.go b/builtin/logical/pki/storage.go index 40b891098..3019a0951 100644 --- a/builtin/logical/pki/storage.go +++ b/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 { diff --git a/builtin/logical/pki/test_helpers.go b/builtin/logical/pki/test_helpers.go index e91dce2bb..80e9a7bc5 100644 --- a/builtin/logical/pki/test_helpers.go +++ b/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{})) } diff --git a/changelog/16519.txt b/changelog/16519.txt new file mode 100644 index 000000000..1325202e6 --- /dev/null +++ b/changelog/16519.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secret/pki: Add RSA PSS signature support for issuing certificates, signing CRLs +``` diff --git a/sdk/helper/certutil/helpers.go b/sdk/helper/certutil/helpers.go index fb691d14c..b69eb36aa 100644 --- a/sdk/helper/certutil/helpers.go +++ b/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 { @@ -766,6 +781,29 @@ func CreateCertificateWithKeyGenerator(data *CreationBundle, randReader io.Reade return createCertificate(data, randReader, keyGenerator) } +// Set correct correct RSA sig algo +func certTemplateSetSigAlgo(certTemplate *x509.Certificate, data *CreationBundle) { + if data.Params.UsePSS { + switch data.Params.SignatureBits { + case 256: + certTemplate.SignatureAlgorithm = x509.SHA256WithRSAPSS + case 384: + certTemplate.SignatureAlgorithm = x509.SHA384WithRSAPSS + case 512: + certTemplate.SignatureAlgorithm = x509.SHA512WithRSAPSS + } + } else { + switch data.Params.SignatureBits { + case 256: + certTemplate.SignatureAlgorithm = x509.SHA256WithRSA + case 384: + certTemplate.SignatureAlgorithm = x509.SHA384WithRSA + case 512: + certTemplate.SignatureAlgorithm = x509.SHA512WithRSA + } + } +} + func createCertificate(data *CreationBundle, randReader io.Reader, privateKeyGenerator KeyGenerator) (*ParsedCertBundle, error) { var err error result := &ParsedCertBundle{} @@ -834,14 +872,7 @@ func createCertificate(data *CreationBundle, randReader io.Reader, privateKeyGen if data.SigningBundle != nil { switch data.SigningBundle.PrivateKeyType { case RSAPrivateKey: - switch data.Params.SignatureBits { - case 256: - certTemplate.SignatureAlgorithm = x509.SHA256WithRSA - case 384: - certTemplate.SignatureAlgorithm = x509.SHA384WithRSA - case 512: - certTemplate.SignatureAlgorithm = x509.SHA512WithRSA - } + certTemplateSetSigAlgo(certTemplate, data) case Ed25519PrivateKey: certTemplate.SignatureAlgorithm = x509.PureEd25519 case ECPrivateKey: @@ -863,14 +894,7 @@ func createCertificate(data *CreationBundle, randReader io.Reader, privateKeyGen switch data.Params.KeyType { case "rsa": - switch data.Params.SignatureBits { - case 256: - certTemplate.SignatureAlgorithm = x509.SHA256WithRSA - case 384: - certTemplate.SignatureAlgorithm = x509.SHA384WithRSA - case 512: - certTemplate.SignatureAlgorithm = x509.SHA512WithRSA - } + certTemplateSetSigAlgo(certTemplate, data) case "ed25519": certTemplate.SignatureAlgorithm = x509.PureEd25519 case "ec": @@ -1097,14 +1121,7 @@ func signCertificate(data *CreationBundle, randReader io.Reader) (*ParsedCertBun switch data.SigningBundle.PrivateKeyType { case RSAPrivateKey: - switch data.Params.SignatureBits { - case 256: - certTemplate.SignatureAlgorithm = x509.SHA256WithRSA - case 384: - certTemplate.SignatureAlgorithm = x509.SHA384WithRSA - case 512: - certTemplate.SignatureAlgorithm = x509.SHA512WithRSA - } + certTemplateSetSigAlgo(certTemplate, data) case ECPrivateKey: switch data.Params.SignatureBits { case 256: diff --git a/sdk/helper/certutil/types.go b/sdk/helper/certutil/types.go index def940dba..03aba8499 100644 --- a/sdk/helper/certutil/types.go +++ b/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 { @@ -782,6 +783,7 @@ type CreationParameters struct { PolicyIdentifiers []string BasicConstraintsValidForNonCA bool SignatureBits int + UsePSS bool ForceAppendCaChain bool // Only used when signing a CA cert diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index 1b68e4a2d..38f1f260b 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -581,6 +581,10 @@ when signing an externally-owned intermediate. certain TLS implementations (such as OpenSSL) which use SKID/AKID matches in chain building to restrict possible valid chains. +- `use_pss` `(bool: false)` - Specifies whether or not to use PSS signatures + over PKCS#1v1.5 signatures when a RSA-type issuer is used. Ignored for + ECDSA/Ed25519 issuers. + #### Sample Payload ```json @@ -771,6 +775,10 @@ have access.** `signature_bits` value; only RSA issuers will change signature types based on this parameter. +- `use_pss` `(bool: false)` - Specifies whether or not to use PSS signatures + over PKCS#1v1.5 signatures when a RSA-type issuer is used. Ignored for + ECDSA/Ed25519 issuers. + #### Sample Payload ```json @@ -1898,6 +1906,18 @@ do so, import a new issuer and a new `issuer_id` will be assigned. could be marked `usage=read-only`, freezing the CRL. Finally, after a grace period, the issuer could be deleted. +- `revocation_signature_algorithm` `(string: "")` - Which signature algorithm + to use when building CRLs. See Go's [`x509.SignatureAlgorithm`](https://pkg.go.dev/crypto/x509#SignatureAlgorithm) + constant for possible values. This flag allows control over hash function + and signature scheme (PKCS#1v1.5 vs PSS). The default (empty string) value + is for Go to select the signature algorithm automatically, which may not + always work. + +~> Note: This can fail if the underlying key does not support the requested + signature algorithm; this may not always be known at modification time. + This most commonly needs to be modified when using PKCS#11 managed keys + with the `CKM_RSA_PKCS_PSS` mechanism type. + #### Sample Payload ```json @@ -2370,6 +2390,10 @@ request is denied. `signature_bits` value; only RSA issuers will change signature types based on this parameter. +- `use_pss` `(bool: false)` - Specifies whether or not to use PSS signatures + over PKCS#1v1.5 signatures when a RSA-type issuer is used. Ignored for + ECDSA/Ed25519 issuers. + - `key_usage` `(list: ["DigitalSignature", "KeyAgreement", "KeyEncipherment"])` - Specifies the allowed key usage constraint on issued certificates. Valid values can be found at https://golang.org/pkg/crypto/x509/#KeyUsage - simply