From 7049424c16979844980042d9f957b09c0a157ba9 Mon Sep 17 00:00:00 2001 From: Steven Clark Date: Tue, 30 Jul 2024 12:20:10 -0400 Subject: [PATCH] Allow SignCert callers to override CSR signature checks (#27914) - We are leveraging this new feature flag to ignore the CSR's signature as we are constructing a CSR based on the information from a CMPv2 message. --- builtin/logical/pki/cert_util.go | 8 ++ builtin/logical/pki/issuing/issue_common.go | 2 + builtin/logical/pki/issuing/sign_cert.go | 24 ++++-- sdk/helper/certutil/certutil_test.go | 95 +++++++++++++++++++++ sdk/helper/certutil/helpers.go | 7 +- sdk/helper/certutil/types.go | 5 ++ 6 files changed, 131 insertions(+), 10 deletions(-) diff --git a/builtin/logical/pki/cert_util.go b/builtin/logical/pki/cert_util.go index 63c40c530f..5304a54db8 100644 --- a/builtin/logical/pki/cert_util.go +++ b/builtin/logical/pki/cert_util.go @@ -468,6 +468,10 @@ func (i SignCertInputFromDataFields) GetPermittedDomains() []string { return i.data.Get("permitted_dns_domains").([]string) } +func (i SignCertInputFromDataFields) IgnoreCSRSignature() bool { + return false +} + func signCert(sysView logical.SystemView, data *inputBundle, caSign *certutil.CAInfoBundle, isCA bool, useCSRValues bool) (*certutil.ParsedCertBundle, []string, error) { if data.role == nil { return nil, nil, errutil.InternalError{Err: "no role found in data bundle"} @@ -498,6 +502,10 @@ type CreationBundleInputFromFieldData struct { data *framework.FieldData } +func (cb CreationBundleInputFromFieldData) IgnoreCSRSignature() bool { + return false +} + func (cb CreationBundleInputFromFieldData) GetCommonName() string { return cb.data.Get("common_name").(string) } diff --git a/builtin/logical/pki/issuing/issue_common.go b/builtin/logical/pki/issuing/issue_common.go index cd4335fecf..b1a67e2feb 100644 --- a/builtin/logical/pki/issuing/issue_common.go +++ b/builtin/logical/pki/issuing/issue_common.go @@ -91,6 +91,7 @@ type CreationBundleInput interface { GetOptionalSkid() (interface{}, bool) IsUserIdInSchema() (interface{}, bool) GetUserIds() []string + IgnoreCSRSignature() bool } // GenerateCreationBundle is a shared function that reads parameters supplied @@ -428,6 +429,7 @@ func GenerateCreationBundle(b logical.SystemView, role *RoleEntry, entityInfo En NotBeforeDuration: role.NotBeforeDuration, ForceAppendCaChain: caSign != nil, SKID: skid, + IgnoreCSRSignature: cb.IgnoreCSRSignature(), }, SigningBundle: caSign, CSR: csr, diff --git a/builtin/logical/pki/issuing/sign_cert.go b/builtin/logical/pki/issuing/sign_cert.go index 773e8b2a96..c6548f6952 100644 --- a/builtin/logical/pki/issuing/sign_cert.go +++ b/builtin/logical/pki/issuing/sign_cert.go @@ -23,20 +23,26 @@ type SignCertInput interface { GetPermittedDomains() []string } -func NewBasicSignCertInput(csr *x509.CertificateRequest, isCA bool, useCSRValues bool) BasicSignCertInput { +func NewBasicSignCertInput(csr *x509.CertificateRequest, isCA, useCSRValues bool) BasicSignCertInput { + return NewBasicSignCertInputWithIgnore(csr, isCA, useCSRValues, false) +} + +func NewBasicSignCertInputWithIgnore(csr *x509.CertificateRequest, isCA, useCSRValues, ignoreCsrSignature bool) BasicSignCertInput { return BasicSignCertInput{ - isCA: isCA, - useCSRValues: useCSRValues, - csr: csr, + isCA: isCA, + useCSRValues: useCSRValues, + csr: csr, + ignoreCsrSignature: ignoreCsrSignature, } } var _ SignCertInput = BasicSignCertInput{} type BasicSignCertInput struct { - isCA bool - useCSRValues bool - csr *x509.CertificateRequest + isCA bool + useCSRValues bool + csr *x509.CertificateRequest + ignoreCsrSignature bool } func (b BasicSignCertInput) GetTTL() int { @@ -91,6 +97,10 @@ func (b BasicSignCertInput) GetCSR() (*x509.CertificateRequest, error) { return b.csr, nil } +func (b BasicSignCertInput) IgnoreCSRSignature() bool { + return b.ignoreCsrSignature +} + func (b BasicSignCertInput) IsCA() bool { return b.isCA } diff --git a/sdk/helper/certutil/certutil_test.go b/sdk/helper/certutil/certutil_test.go index 8b55094612..4ebab01827 100644 --- a/sdk/helper/certutil/certutil_test.go +++ b/sdk/helper/certutil/certutil_test.go @@ -1006,6 +1006,101 @@ func TestBasicConstraintExtension(t *testing.T) { }) } +// TestIgnoreCSRSigning Make sure we validate the CSR by default and that we can override +// the behavior disabling CSR signature checks +func TestIgnoreCSRSigning(t *testing.T) { + t.Parallel() + + caKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("failed generating ca key: %v", err) + } + subjKeyID, err := GetSubjKeyID(caKey) + if err != nil { + t.Fatalf("failed generating ca subject key id: %v", err) + } + caCertTemplate := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "root.localhost", + }, + SubjectKeyId: subjKeyID, + DNSNames: []string{"root.localhost"}, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + SerialNumber: big.NewInt(mathrand.Int63()), + NotBefore: time.Now().Add(-30 * time.Second), + NotAfter: time.Now().Add(262980 * time.Hour), + BasicConstraintsValid: true, + IsCA: true, + } + caBytes, err := x509.CreateCertificate(rand.Reader, caCertTemplate, caCertTemplate, caKey.Public(), caKey) + if err != nil { + t.Fatalf("failed creating ca certificate: %v", err) + } + caCert, err := x509.ParseCertificate(caBytes) + if err != nil { + t.Fatalf("failed parsing ca certificate: %v", err) + } + + signingBundle := &CAInfoBundle{ + ParsedCertBundle: ParsedCertBundle{ + PrivateKeyType: ECPrivateKey, + PrivateKey: caKey, + CertificateBytes: caBytes, + Certificate: caCert, + CAChain: nil, + }, + URLs: &URLEntries{}, + } + + key := genEdDSA(t) + csr := &x509.CertificateRequest{ + PublicKeyAlgorithm: x509.ECDSA, + PublicKey: key.Public(), + Subject: pkix.Name{ + CommonName: "test.dadgarcorp.com", + }, + } + t.Run(fmt.Sprintf("ignore-csr-disabled"), func(t *testing.T) { + params := &CreationParameters{ + URLs: &URLEntries{}, + } + data := &CreationBundle{ + Params: params, + SigningBundle: signingBundle, + CSR: csr, + } + + _, err := SignCertificate(data) + if err == nil { + t.Fatalf("should have failed signing csr with ignore csr signature disabled") + } + if !strings.Contains(err.Error(), "request signature invalid") { + t.Fatalf("expected error to contain 'request signature invalid': got: %v", err) + } + }) + + t.Run(fmt.Sprintf("ignore-csr-enabled"), func(t *testing.T) { + params := &CreationParameters{ + IgnoreCSRSignature: true, + URLs: &URLEntries{}, + } + data := &CreationBundle{ + Params: params, + SigningBundle: signingBundle, + CSR: csr, + } + + cert, err := SignCertificate(data) + if err != nil { + t.Fatalf("failed to sign certificate: %v", err) + } + + if err := cert.Verify(); err != nil { + t.Fatalf("signature verification failed: %v", err) + } + }) +} + func genRsaKey(t *testing.T) *rsa.PrivateKey { key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { diff --git a/sdk/helper/certutil/helpers.go b/sdk/helper/certutil/helpers.go index 61bf7560f3..674878cbef 100644 --- a/sdk/helper/certutil/helpers.go +++ b/sdk/helper/certutil/helpers.go @@ -1186,9 +1186,10 @@ func signCertificate(data *CreationBundle, randReader io.Reader) (*ParsedCertBun return nil, errutil.UserError{Err: "nil csr given to signCertificate"} } - err := data.CSR.CheckSignature() - if err != nil { - return nil, errutil.UserError{Err: "request signature invalid"} + if !data.Params.IgnoreCSRSignature { + if err := data.CSR.CheckSignature(); err != nil { + return nil, errutil.UserError{Err: "request signature invalid"} + } } result := &ParsedCertBundle{} diff --git a/sdk/helper/certutil/types.go b/sdk/helper/certutil/types.go index bfdc153c48..cdfc912e92 100644 --- a/sdk/helper/certutil/types.go +++ b/sdk/helper/certutil/types.go @@ -819,6 +819,11 @@ type CreationParameters struct { // The explicit SKID to use; especially useful for cross-signing. SKID []byte + + // Ignore validating the CSR's signature. This should only be enabled if the + // sender of the CSR has proven proof of possession of the associated + // private key by some other means, otherwise keep this set to false. + IgnoreCSRSignature bool } type CreationBundle struct {