diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index c027361f2..1f299f853 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -12,6 +12,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/base64" + "encoding/hex" "encoding/json" "encoding/pem" "fmt" @@ -583,6 +584,8 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s "signature_bits": 512, "format": "der", "not_before_duration": "2h", + // Let's Encrypt -- R3 SKID + "skid": "14:2E:B3:17:B7:58:56:CB:AE:50:09:40:E6:1F:AF:9D:8B:14:C2:C6", }, Check: func(resp *logical.Response) error { certString := resp.Data["certificate"].(string) @@ -602,6 +605,8 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s } cert := certs[0] + skid, _ := hex.DecodeString("142EB317B75856CBAE500940E61FAF9D8B14C2C6") + switch { case !reflect.DeepEqual(expected.IssuingCertificates, cert.IssuingCertificateURL): return fmt.Errorf("IssuingCertificateURL:\nexpected\n%#v\ngot\n%#v\n", expected.IssuingCertificates, cert.IssuingCertificateURL) @@ -613,6 +618,8 @@ func generateURLSteps(t *testing.T, caCert, caKey string, intdata, reqdata map[s return fmt.Errorf("DNSNames\nexpected\n%#v\ngot\n%#v\n", []string{"intermediate.cert.com"}, cert.DNSNames) case !reflect.DeepEqual(x509.SHA512WithRSA, cert.SignatureAlgorithm): return fmt.Errorf("Signature Algorithm:\nexpected\n%#v\ngot\n%#v\n", x509.SHA512WithRSA, cert.SignatureAlgorithm) + case !reflect.DeepEqual(skid, cert.SubjectKeyId): + return fmt.Errorf("SKID:\nexpected\n%#v\ngot\n%#v\n", skid, cert.SubjectKeyId) } if math.Abs(float64(time.Now().Add(-2*time.Hour).Unix()-cert.NotBefore.Unix())) > 10 { diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 867435ad3..9633ea959 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -10,6 +10,7 @@ import ( "crypto/x509/pkix" "encoding/asn1" "encoding/base64" + "encoding/hex" "encoding/pem" "errors" "fmt" @@ -1340,6 +1341,27 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn } } + // Parse SKID from the request for cross-signing. + var skid []byte + { + if rawSKIDValue, ok := data.apiData.GetOk("skid"); ok { + // Handle removing common separators to make copy/paste from tool + // output easier. Chromium uses space, OpenSSL uses colons, and at + // one point, Vault had preferred dash as a separator for hex + // strings. + var err error + skidValue := rawSKIDValue.(string) + for _, separator := range []string{":", "-", " "} { + skidValue = strings.ReplaceAll(skidValue, separator, "") + } + + skid, err = hex.DecodeString(skidValue) + if err != nil { + return nil, errutil.UserError{Err: fmt.Sprintf("cannot parse requested SKID value as hex: %v", err)} + } + } + } + creation := &certutil.CreationBundle{ Params: &certutil.CreationParameters{ Subject: subject, @@ -1359,6 +1381,7 @@ func generateCreationBundle(b *backend, data *inputBundle, caSign *certutil.CAIn BasicConstraintsValidForNonCA: data.role.BasicConstraintsValidForNonCA, NotBeforeDuration: data.role.NotBeforeDuration, ForceAppendCaChain: caSign != nil, + SKID: skid, }, SigningBundle: caSign, CSR: csr, diff --git a/builtin/logical/pki/chain_test.go b/builtin/logical/pki/chain_test.go index 97421e7b4..63f4622bb 100644 --- a/builtin/logical/pki/chain_test.go +++ b/builtin/logical/pki/chain_test.go @@ -1,9 +1,11 @@ package pki import ( + "bytes" "context" "crypto/x509" "crypto/x509/pkix" + "encoding/hex" "encoding/pem" "fmt" "strconv" @@ -113,6 +115,7 @@ type CBGenerateIntermediate struct { Existing bool Name string CommonName string + SKID string Parent string ImportErrorMessage string } @@ -151,6 +154,14 @@ func (c CBGenerateIntermediate) Run(t testing.TB, b *backend, s logical.Storage, if len(c.CommonName) > 0 { data["common_name"] = c.CommonName } + if len(c.SKID) > 0 { + // Copy the SKID from an existing, already-issued cert. + otherPEM := knownCerts[c.SKID] + otherCert := ToCertificate(t, otherPEM) + + data["skid"] = hex.EncodeToString(otherCert.SubjectKeyId) + } + resp, err = CBWrite(b, s, url, data) if err != nil { t.Fatalf("failed to sign CSR for issuer (%v): %v / body: %v", c.Name, err, data) @@ -158,6 +169,17 @@ func (c CBGenerateIntermediate) Run(t testing.TB, b *backend, s logical.Storage, knownCerts[c.Name] = strings.TrimSpace(resp.Data["certificate"].(string)) + // Verify SKID if one was requested. + if len(c.SKID) > 0 { + otherPEM := knownCerts[c.SKID] + otherCert := ToCertificate(t, otherPEM) + ourCert := ToCertificate(t, knownCerts[c.Name]) + + if !bytes.Equal(otherCert.SubjectKeyId, ourCert.SubjectKeyId) { + t.Fatalf("Expected two certs to have equal SKIDs but differed: them: %v vs us: %v", otherCert.SubjectKeyId, ourCert.SubjectKeyId) + } + } + // Set the signed intermediate url = "intermediate/set-signed" data = make(map[string]interface{}) @@ -829,6 +851,7 @@ var chainBuildingTestCases = []CBTestScenario{ Existing: true, Name: "cross-old-new", CommonName: "root-new", + SKID: "root-new-a", // Which old issuer is used here doesn't matter as they have // the same CN and key. Parent: "root-old-a", @@ -887,6 +910,7 @@ var chainBuildingTestCases = []CBTestScenario{ Existing: true, Name: "cross-new-old", CommonName: "root-old", + SKID: "root-old-a", // Which new issuer is used here doesn't matter as they have // the same CN and key. Parent: "root-new-a", diff --git a/builtin/logical/pki/path_sign_issuers.go b/builtin/logical/pki/path_sign_issuers.go index 75e2d7395..5f1ce76a0 100644 --- a/builtin/logical/pki/path_sign_issuers.go +++ b/builtin/logical/pki/path_sign_issuers.go @@ -7,15 +7,15 @@ import ( func pathIssuerSignIntermediate(b *backend) *framework.Path { pattern := "issuer/" + framework.GenericNameRegex(issuerRefParam) + "/sign-intermediate" - return pathIssuerSignIntermediateRaw(b, pattern) + return buildPathIssuerSignIntermediateRaw(b, pattern) } func pathSignIntermediate(b *backend) *framework.Path { pattern := "root/sign-intermediate" - return pathIssuerSignIntermediateRaw(b, pattern) + return buildPathIssuerSignIntermediateRaw(b, pattern) } -func pathIssuerSignIntermediateRaw(b *backend, pattern string) *framework.Path { +func buildPathIssuerSignIntermediateRaw(b *backend, pattern string) *framework.Path { fields := addIssuerRefField(map[string]*framework.FieldSchema{}) path := &framework.Path{ Pattern: pattern, @@ -67,6 +67,24 @@ SHA-2-512. Defaults to 0 to automatically detect based on key length }, } + fields["skid"] = &framework.FieldSchema{ + Type: framework.TypeString, + Default: "", + Description: `Value for the Subject Key Identifier field +(RFC 5280 Section 4.2.1.2). This value should ONLY be used when +cross-signing to mimic the existing certificate's SKID value; this +is necessary to allow certain TLS implementations (such as OpenSSL) +which use SKID/AKID matches in chain building to restrict possible +valid chains. + +Specified as a string in hex format. Default is empty, allowing +Vault to automatically calculate the SKID according to method one +in the above RFC section.`, + DisplayAttrs: &framework.DisplayAttributes{ + Value: "", + }, + } + return path } diff --git a/changelog/16494.txt b/changelog/16494.txt new file mode 100644 index 000000000..40cf3643a --- /dev/null +++ b/changelog/16494.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secret/pki: Allow specifying SKID for cross-signed issuance from older Vault versions. +``` diff --git a/sdk/helper/certutil/helpers.go b/sdk/helper/certutil/helpers.go index 457de9b02..fb691d14c 100644 --- a/sdk/helper/certutil/helpers.go +++ b/sdk/helper/certutil/helpers.go @@ -87,6 +87,16 @@ func GetSubjKeyID(privateKey crypto.Signer) ([]byte, error) { return getSubjectKeyID(privateKey.Public()) } +// Returns the explicit SKID when used for cross-signing, else computes a new +// SKID from the key itself. +func getSubjectKeyIDFromBundle(data *CreationBundle) ([]byte, error) { + if len(data.Params.SKID) > 0 { + return data.Params.SKID, nil + } + + return getSubjectKeyID(data.CSR.PublicKey) +} + func getSubjectKeyID(pub interface{}) ([]byte, error) { var publicKeyBytes []byte switch pub := pub.(type) { @@ -1066,7 +1076,7 @@ func signCertificate(data *CreationBundle, randReader io.Reader) (*ParsedCertBun return nil, err } - subjKeyID, err := getSubjectKeyID(data.CSR.PublicKey) + subjKeyID, err := getSubjectKeyIDFromBundle(data) if err != nil { return nil, err } diff --git a/sdk/helper/certutil/types.go b/sdk/helper/certutil/types.go index a5caa2e44..def940dba 100644 --- a/sdk/helper/certutil/types.go +++ b/sdk/helper/certutil/types.go @@ -796,6 +796,9 @@ type CreationParameters struct { // The duration the certificate will use NotBefore NotBeforeDuration time.Duration + + // The explicit SKID to use; especially useful for cross-signing. + SKID []byte } type CreationBundle struct { diff --git a/website/content/api-docs/secret/pki.mdx b/website/content/api-docs/secret/pki.mdx index d090709e4..1b68e4a2d 100644 --- a/website/content/api-docs/secret/pki.mdx +++ b/website/content/api-docs/secret/pki.mdx @@ -571,6 +571,16 @@ when signing an externally-owned intermediate. `signature_bits` value; only RSA issuers will change signature types based on this parameter. +- `skid` `(string: "")` - Value for the Subject Key Identifier field + (RFC 5280 Section 4.2.1.2). Specified as a string in hex format. Default + is empty, allowing Vault to automatically calculate the SKID according + to method one in the above RFC section. + +~> **Note**: This value should ONLY be used when cross-signing to mimic + the existing certificate's SKID value; this is necessary to allow + certain TLS implementations (such as OpenSSL) which use SKID/AKID + matches in chain building to restrict possible valid chains. + #### Sample Payload ```json