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 <clint@ctshryock.com>
2021-09-14 10:07:27 -05:00 · 2021-09-14 10:07:27 -05:00 · 6f18a9b6be
parent 89271bf0ca
commit 6f18a9b6be
3 changed files with 242 additions and 19 deletions
--- a/builtin/logical/pki/backend_test.go
+++ b/builtin/logical/pki/backend_test.go
@ -2103,22 +2103,6 @@ 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",
@ -2128,7 +2112,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",
@ -2158,7 +2142,7 @@ 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",
@ -2177,7 +2161,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",
@ -2224,6 +2208,185 @@ func TestBackend_SignSelfIssued(t *testing.T) {
 	}
 }

+// 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{
+		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("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)
+	}
+	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)
+	}
+}
+
+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
--- 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.
 `
--- a/changelog/12514.txt
+++ b/changelog/12514.txt
@ -0,0 +1,3 @@
+```release-note:improvement
+secrets/pki: Allow signing of self-issued certs with a different signature algorithm.
+```