From 0a68335b196f9d1b0fe646575c3d5ba83f531613 Mon Sep 17 00:00:00 2001 From: Scott Miller Date: Tue, 14 Sep 2021 10:07:27 -0500 Subject: [PATCH] Allow signing self issued certs with a different public key algorithm. (#12514) * WIP: Unset the certificate's SignatureAlgorithm to allown cross-signing of different key types * Allow signing self issued certs with a different public key algorithm * Remove cruft * Remove stale import * changelog * eliminate errwrap * Add a test to cover the lack of opt-in flag * Better comment Co-authored-by: catsby --- builtin/logical/pki/backend_test.go | 197 +++++++++++++++++++++++++--- builtin/logical/pki/path_root.go | 57 ++++++++ changelog/12514.txt | 3 + 3 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 changelog/12514.txt diff --git a/builtin/logical/pki/backend_test.go b/builtin/logical/pki/backend_test.go index a3c85764e87c4..235f39a22f0fb 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", @@ -2185,6 +2317,20 @@ func TestBackend_SignSelfIssued(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) } @@ -2223,6 +2369,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..1964013d4c963 100644 --- a/builtin/logical/pki/path_root.go +++ b/builtin/logical/pki/path_root.go @@ -2,11 +2,17 @@ package pki import ( "context" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" + "crypto/rsa" "crypto/x509" "encoding/base64" "encoding/pem" + "errors" "fmt" + "golang.org/x/crypto/ed25519" "reflect" "strings" "time" @@ -103,6 +109,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,6 +438,25 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request cert.CRLDistributionPoints = urls.CRLDistributionPoints cert.OCSPServer = urls.OCSPServers + // 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, fmt.Errorf("error determining signing certificate algorithm type: %e", err) + } + certPubType, _, err := publicKeyType(cert.PublicKey) + if err != nil { + return nil, fmt.Errorf("error determining template algorithm type: %e", 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 { return nil, fmt.Errorf("error signing self-issued certificate: %w", err) @@ -448,6 +477,34 @@ func (b *backend) pathCASignSelfIssued(ctx context.Context, req *logical.Request }, nil } +// 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: + 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. ` 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. +```