From ce0b24adf811f910f54820da0385ff2a7e4f0da9 Mon Sep 17 00:00:00 2001 From: catsby Date: Tue, 14 Jan 2020 16:22:45 -0600 Subject: [PATCH 1/8] WIP: Unset the certificate's SignatureAlgorithm to allown cross-signing of different key types --- builtin/logical/pki/backend_test.go | 183 +++++++++++++++++++++++++--- builtin/logical/pki/path_root.go | 4 + 2 files changed, 170 insertions(+), 17 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 3f23068f98a23..8050a12008e82 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2102,20 +2102,151 @@ func TestBackend_SignSelfIssued(t *testing.T) { t.Fatal(err) } - getSelfSigned := func(subject, issuer *x509.Certificate) (string, *x509.Certificate) { - selfSigned, err := x509.CreateCertificate(rand.Reader, subject, issuer, key.Public(), key) - if err != nil { - t.Fatal(err) - } - cert, err := x509.ParseCertificate(selfSigned) - if err != nil { - t.Fatal(err) - } - pemSS := strings.TrimSpace(string(pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE", - Bytes: selfSigned, - }))) - return pemSS, cert + template := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "foo.bar.com", + }, + SerialNumber: big.NewInt(1234), + IsCA: false, + BasicConstraintsValid: true, + } + + ss, _ := getSelfSigned(t, template, template, key) + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/sign-self-issued", + Storage: storage, + Data: map[string]interface{}{ + "certificate": ss, + }, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("got nil response") + } + if !resp.IsError() { + t.Fatalf("expected error due to non-CA; got: %#v", *resp) + } + + // Set CA to true, but leave issuer alone + template.IsCA = true + + issuer := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "bar.foo.com", + }, + SerialNumber: big.NewInt(2345), + IsCA: true, + BasicConstraintsValid: true, + } + ss, ssCert := getSelfSigned(t, template, issuer, key) + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/sign-self-issued", + Storage: storage, + Data: map[string]interface{}{ + "certificate": ss, + }, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("got nil response") + } + if !resp.IsError() { + t.Fatalf("expected error due to different issuer; cert info is\nIssuer\n%#v\nSubject\n%#v\n", ssCert.Issuer, ssCert.Subject) + } + + ss, ssCert = getSelfSigned(t, template, template, key) + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/sign-self-issued", + Storage: storage, + Data: map[string]interface{}{ + "certificate": ss, + }, + }) + if err != nil { + t.Fatal(err) + } + if resp == nil { + t.Fatal("got nil response") + } + if resp.IsError() { + t.Fatalf("error in response: %s", resp.Error().Error()) + } + + newCertString := resp.Data["certificate"].(string) + block, _ := pem.Decode([]byte(newCertString)) + newCert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatal(err) + } + + signingBundle, err := fetchCAInfo(context.Background(), &logical.Request{Storage: storage}) + if err != nil { + t.Fatal(err) + } + if reflect.DeepEqual(newCert.Subject, newCert.Issuer) { + t.Fatal("expected different subject/issuer") + } + if !reflect.DeepEqual(newCert.Issuer, signingBundle.Certificate.Subject) { + t.Fatalf("expected matching issuer/CA subject\n\nIssuer:\n%#v\nSubject:\n%#v\n", newCert.Issuer, signingBundle.Certificate.Subject) + } + if bytes.Equal(newCert.AuthorityKeyId, newCert.SubjectKeyId) { + t.Fatal("expected different authority/subject") + } + if !bytes.Equal(newCert.AuthorityKeyId, signingBundle.Certificate.SubjectKeyId) { + t.Fatal("expected authority on new cert to be same as signing subject") + } + if newCert.Subject.CommonName != "foo.bar.com" { + t.Fatalf("unexpected common name on new cert: %s", newCert.Subject.CommonName) + } +} + +// TestBackend_SignSelfIssued_DifferentTypes is a copy of +// TestBackend_SignSelfIssued, but uses a different key type for the internal +// root (EC instead of RSA). This verifies that we can cross-sign CAs that are +// different key types, at the cost of verifying the algorithm used +func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { + // create the backend + config := logical.TestBackendConfig() + storage := &logical.InmemStorage{} + config.StorageView = storage + + b := Backend(config) + err := b.Setup(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + // generate root + rootData := map[string]interface{}{ + "common_name": "test.com", + "ttl": "172800", + "key_type": "ec", + "key_bits": "521", + } + + resp, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/generate/internal", + Storage: storage, + Data: rootData, + }) + if resp != nil && resp.IsError() { + t.Fatalf("failed to generate root, %#v", *resp) + } + if err != nil { + t.Fatal(err) + } + + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) } template := &x509.Certificate{ @@ -2127,7 +2258,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { BasicConstraintsValid: true, } - ss, _ := getSelfSigned(template, template) + ss, _ := getSelfSigned(t, template, template, key) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "root/sign-self-issued", @@ -2157,7 +2288,8 @@ func TestBackend_SignSelfIssued(t *testing.T) { IsCA: true, BasicConstraintsValid: true, } - ss, ssCert := getSelfSigned(template, issuer) + + ss, ssCert := getSelfSigned(t, template, issuer, key) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "root/sign-self-issued", @@ -2176,7 +2308,7 @@ func TestBackend_SignSelfIssued(t *testing.T) { t.Fatalf("expected error due to different issuer; cert info is\nIssuer\n%#v\nSubject\n%#v\n", ssCert.Issuer, ssCert.Subject) } - ss, ssCert = getSelfSigned(template, template) + ss, ssCert = getSelfSigned(t, template, template, key) resp, err = b.HandleRequest(context.Background(), &logical.Request{ Operation: logical.UpdateOperation, Path: "root/sign-self-issued", @@ -2223,6 +2355,23 @@ func TestBackend_SignSelfIssued(t *testing.T) { } } +func getSelfSigned(t *testing.T, subject, issuer *x509.Certificate, key *rsa.PrivateKey) (string, *x509.Certificate) { + t.Helper() + selfSigned, err := x509.CreateCertificate(rand.Reader, subject, issuer, key.Public(), key) + if err != nil { + t.Fatal(err) + } + cert, err := x509.ParseCertificate(selfSigned) + if err != nil { + t.Fatal(err) + } + pemSS := strings.TrimSpace(string(pem.EncodeToMemory(&pem.Block{ + Type: "CERTIFICATE", + Bytes: selfSigned, + }))) + return pemSS, cert +} + // This is a really tricky test because the Go stdlib asn1 package is incapable // of doing the right thing with custom OID SANs (see comments in the package, // it's readily admitted that it's too magic) but that means that any diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 29bee4baae10b..763581afbe0e1 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -428,6 +428,10 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request cert.CRLDistributionPoints = urls.CRLDistributionPoints cert.OCSPServer = urls.OCSPServers + // clear out the signature algorithm. This allows cross-signing certificates + // of different key types, at the cost of verifying the algorithm used. + cert.SignatureAlgorithm = x509.UnknownSignatureAlgorithm + newCert, err := x509.CreateCertificate(rand.Reader, cert, signingBundle.Certificate, cert.PublicKey, signingBundle.PrivateKey) if err != nil { return nil, fmt.Errorf("error signing self-issued certificate: %w", err) From 13bd6b089507842b160abfa67b46f14eb08a6f86 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Wed, 8 Sep 2021 13:53:05 -0500 Subject: [PATCH 2/8] Allow signing self issued certs with a different public key algorithm --- builtin/logical/pki/backend_test.go | 3 +- builtin/logical/pki/path_root.go | 70 +++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index 8050a12008e82..d4c16a1f6e932 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2295,7 +2295,8 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { Path: "root/sign-self-issued", Storage: storage, Data: map[string]interface{}{ - "certificate": ss, + "certificate": ss, + "allow_different_signature_algorithm": "true", }, }) if err != nil { diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 763581afbe0e1..3797f905887c4 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -2,11 +2,18 @@ package pki import ( "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" + "crypto/rsa" "crypto/x509" + "encoding/asn1" "encoding/base64" "encoding/pem" + "errors" "fmt" + "golang.org/x/crypto/ed25519" "reflect" "strings" "time" @@ -103,6 +110,10 @@ func pathSignSelfIssued(b *backend) *framework.Path { Type: framework.TypeString, Description: `PEM-format self-issued certificate to be signed.`, }, + "allow_different_signature_algorithm": &framework.FieldSchema{ + Type: framework.TypeBool, + Description: `If true, allow the public key type of the signer to differ from the self issued certificate.`, + }, }, HelpSynopsis: pathSignSelfIssuedHelpSyn, @@ -428,9 +439,24 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request cert.CRLDistributionPoints = urls.CRLDistributionPoints cert.OCSPServer = urls.OCSPServers - // clear out the signature algorithm. This allows cross-signing certificates - // of different key types, at the cost of verifying the algorithm used. - cert.SignatureAlgorithm = x509.UnknownSignatureAlgorithm + // If the requested signature algorithm isn't the same as the signing certificate, and + // the user has requested a cross-algorithm signature, reset the template's signing algorithm + // to that of the signing key + signingPubType, signingAlgorithm, err := publicKeyType(signingBundle.Certificate.PublicKey) + if err != nil { + return nil, errwrap.Wrapf("error determining signing certificate algorithm type", err) + } + certPubType, _, err := publicKeyType(cert.PublicKey) + if err != nil { + return nil, errwrap.Wrapf("error determining template algorithm type", err) + } + + if signingPubType != certPubType { + b, ok := data.GetOk("allow_different_signature_algorithm") + if ok && b.(bool) { + cert.SignatureAlgorithm = signingAlgorithm + } + } newCert, err := x509.CreateCertificate(rand.Reader, cert, signingBundle.Certificate, cert.PublicKey, signingBundle.PrivateKey) if err != nil { @@ -452,6 +478,44 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request }, nil } +var ( + oidSignatureECDSAWithSHA256 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 2} + oidSignatureECDSAWithSHA384 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 3} + oidSignatureECDSAWithSHA512 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 4} + oidSignatureSHA256WithRSA = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 11} + oidSignatureEd25519 = asn1.ObjectIdentifier{1, 3, 101, 112} +) + +// Adapted from x509.go, may need to be updated! +func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAlgo x509.SignatureAlgorithm, err error) { + switch pub := pub.(type) { + case *rsa.PublicKey: + pubType = x509.RSA + sigAlgo = x509.SHA256WithRSA + + case *ecdsa.PublicKey: + pubType = x509.ECDSA + + switch pub.Curve { + case elliptic.P224(), elliptic.P256(): + sigAlgo = x509.ECDSAWithSHA256 + case elliptic.P384(): + sigAlgo = x509.ECDSAWithSHA384 + case elliptic.P521(): + sigAlgo = x509.ECDSAWithSHA512 + default: + err = errors.New("x509: unknown elliptic curve") + } + + case ed25519.PublicKey: + pubType = x509.Ed25519 + sigAlgo = x509.PureEd25519 + default: + err = errors.New("x509: only RSA, ECDSA and Ed25519 keys supported") + } + return +} + const pathGenerateRootHelpSyn = ` Generate a new CA certificate and private key used for signing. ` From 507ecaf05ecde6883c65f42066a87a66db7d6537 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Wed, 8 Sep 2021 13:54:53 -0500 Subject: [PATCH 3/8] Remove cruft --- builtin/logical/pki/path_root.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 3797f905887c4..81694ab8104b3 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -478,21 +478,12 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request }, nil } -var ( - oidSignatureECDSAWithSHA256 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 2} - oidSignatureECDSAWithSHA384 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 3} - oidSignatureECDSAWithSHA512 = asn1.ObjectIdentifier{1, 2, 840, 10045, 4, 3, 4} - oidSignatureSHA256WithRSA = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 1, 11} - oidSignatureEd25519 = asn1.ObjectIdentifier{1, 3, 101, 112} -) - -// Adapted from x509.go, may need to be updated! +// Adapted from x509.go, may need to be updated in the future func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAlgo x509.SignatureAlgorithm, err error) { switch pub := pub.(type) { case *rsa.PublicKey: pubType = x509.RSA sigAlgo = x509.SHA256WithRSA - case *ecdsa.PublicKey: pubType = x509.ECDSA @@ -506,7 +497,6 @@ func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAl default: err = errors.New("x509: unknown elliptic curve") } - case ed25519.PublicKey: pubType = x509.Ed25519 sigAlgo = x509.PureEd25519 From ead65c9cd1995a604e66ec4c7d964082c3a4f1ba Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Wed, 8 Sep 2021 13:59:35 -0500 Subject: [PATCH 4/8] Remove stale import --- builtin/logical/pki/path_root.go | 1 - 1 file changed, 1 deletion(-) diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 81694ab8104b3..0b09885d1b784 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -8,7 +8,6 @@ import ( "crypto/rand" "crypto/rsa" "crypto/x509" - "encoding/asn1" "encoding/base64" "encoding/pem" "errors" From 84b1894d9a5420d23ffcf68e936c179088959f26 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Wed, 8 Sep 2021 14:00:54 -0500 Subject: [PATCH 5/8] changelog --- changelog/12514.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/12514.txt diff --git a/changelog/12514.txt b/changelog/12514.txt new file mode 100644 index 0000000000000..8f00061e2ef31 --- /dev/null +++ b/changelog/12514.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/pki: Allow signing of self-issued certs with a different signature algorithm. +``` From 8f3ef443840aa90f422a65185495d0c7d41a2dac Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Wed, 8 Sep 2021 14:48:17 -0500 Subject: [PATCH 6/8] eliminate errwrap --- builtin/logical/pki/path_root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 0b09885d1b784..619fca493a1a8 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -443,11 +443,11 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request // to that of the signing key signingPubType, signingAlgorithm, err := publicKeyType(signingBundle.Certificate.PublicKey) if err != nil { - return nil, errwrap.Wrapf("error determining signing certificate algorithm type", err) + return nil, fmt.Errorf("error determining signing certificate algorithm type: %e", err) } certPubType, _, err := publicKeyType(cert.PublicKey) if err != nil { - return nil, errwrap.Wrapf("error determining template algorithm type", err) + return nil, fmt.Errorf("error determining template algorithm type: %e", err) } if signingPubType != certPubType { From e7fd51fcf5914dcd7d49d656c86d2fb07819a3e4 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Thu, 9 Sep 2021 10:07:30 -0500 Subject: [PATCH 7/8] Add a test to cover the lack of opt-in flag --- builtin/logical/pki/backend_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index d4c16a1f6e932..93e64890d37bc 100644 --- a/builtin/logical/pki/backend_test.go +++ b/builtin/logical/pki/backend_test.go @@ -2295,8 +2295,7 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { Path: "root/sign-self-issued", Storage: storage, Data: map[string]interface{}{ - "certificate": ss, - "allow_different_signature_algorithm": "true", + "certificate": ss, }, }) if err != nil { @@ -2318,6 +2317,20 @@ func TestBackend_SignSelfIssued_DifferentTypes(t *testing.T) { "certificate": ss, }, }) + if err == nil { + t.Fatal("expected error due to different signature algo but not opted-in") + } + + ss, ssCert = getSelfSigned(t, template, template, key) + resp, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "root/sign-self-issued", + Storage: storage, + Data: map[string]interface{}{ + "certificate": ss, + "allow_different_signature_algorithm": "true", + }, + }) if err != nil { t.Fatal(err) } From a9f1c2f7ad74ecb462d19298205c8c4ec6325be5 Mon Sep 17 00:00:00 2001 From: "Scott G. Miller" Date: Tue, 14 Sep 2021 09:44:08 -0500 Subject: [PATCH 8/8] Better comment --- builtin/logical/pki/path_root.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/logical/pki/path_root.go b/builtin/logical/pki/path_root.go index 619fca493a1a8..1964013d4c963 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -477,7 +477,8 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request }, nil } -// Adapted from x509.go, may need to be updated in the future +// Adapted from similar code in https://github.com/golang/go/blob/4a4221e8187189adcc6463d2d96fe2e8da290132/src/crypto/x509/x509.go#L1342, +// may need to be updated in the future. func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAlgo x509.SignatureAlgorithm, err error) { switch pub := pub.(type) { case *rsa.PublicKey: @@ -485,7 +486,6 @@ func publicKeyType(pub crypto.PublicKey) (pubType x509.PublicKeyAlgorithm, sigAl sigAlgo = x509.SHA256WithRSA case *ecdsa.PublicKey: pubType = x509.ECDSA - switch pub.Curve { case elliptic.P224(), elliptic.P256(): sigAlgo = x509.ECDSAWithSHA256