luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Allow old certs to be cross-signed (#16494) 

* Allow old certs to be cross-signed

In Vault 1.11, we introduced cross-signing support, but the earlier SKID
field change in Vault 1.10 causes problems: notably, certs created on
older versions of Vault (<=1.9) or outside of Vault (with a different
SKID method) cannot be cross-signed and validated in OpenSSL.

In particular, OpenSSL appears to be unique in requiring a SKID/AKID
match for chain building. If AKID and SKID are present on an otherwise
valid client/parent cert pair and the values are different, OpenSSL will
not build a valid path over those two, whereas most other chain
validation implementations will.

Regardless, to have proper cross-signing support, we really aught to
support copying an SKID. This adds such support to the sign-intermediate
endpoint. Support for the /issue endpoint is not added, as cross-signing
leaf certs isn't generally useful and can accept random SKIDs.

Resolves: #16461

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

* Add changelog

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

* Address review feedback, fix tests

Also adds a known-answer test using LE R3 CA's SKID.

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

* Address review feedback regarding separators

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2022-08-03 09:34:21 -04:00 committed by GitHub
parent dc0b4315f3
commit cf7105929f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 102 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
builtin/logical/pki/backend_test.go
View File

@ -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 {

23
builtin/logical/pki/cert_util.go
View File

@ -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,

24
builtin/logical/pki/chain_test.go
View File

@ -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",

24
builtin/logical/pki/path_sign_issuers.go
View File

@ -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
}

3
changelog/16494.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
secret/pki: Allow specifying SKID for cross-signed issuance from older Vault versions.
```

12
sdk/helper/certutil/helpers.go
View File

@ -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
}

3
sdk/helper/certutil/types.go
View File

@ -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 {

10
website/content/api-docs/secret/pki.mdx
View File

@ -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