Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: PKI: Unset the SignatureAlgorithm for cross-signing of different key types #8157

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
183 changes: 166 additions & 17 deletions builtin/logical/pki/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2089,20 +2089,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{
Expand All @@ -2114,7 +2245,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",
Expand Down Expand Up @@ -2144,7 +2275,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",
Expand All @@ -2163,7 +2295,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",
Expand Down Expand Up @@ -2210,6 +2342,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
Expand Down
4 changes: 4 additions & 0 deletions builtin/logical/pki/path_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be a better idea to look at the public key from the certificate to be signed and set appropriately?


newCert, err := x509.CreateCertificate(rand.Reader, cert, signingBundle.Certificate, cert.PublicKey, signingBundle.PrivateKey)
if err != nil {
return nil, errwrap.Wrapf("error signing self-issued certificate: {{err}}", err)
Expand Down