Skip to content

Commit

Permalink
ca/linter: Port safety checks from ceremony tool (#7498)
Browse files Browse the repository at this point in the history
In #7005 several safety
checks were added to the `ceremony` tool:

This change extracts the `RawSubject` to `RawIssuer` DER byte comparison
into the `//linter` package proper so that it can serve both `//ca` and
`//cmd/ceremony`.

Adds a helper function `verifyTBSCertificateDeterminism` to `//ca`
similar to an existing check in `//cmd/ceremony`. This code is not
shared because we want `//cmd/ceremony` to largely stand alone from
boulder proper. The helper performs a byte comparison on the
`RawTBSCertificate` DER bytes for a given linting certificate and leaf
certificate. The goal is to verify that `x509.CreateCertificate` was
deterministic and produced identical DER bytes after each signing
operation.

Fixes #6965
  • Loading branch information
pgporada authored Jun 3, 2024
1 parent 764f6c2 commit 347ccf8
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 12 deletions.
77 changes: 76 additions & 1 deletion ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/miekg/pkcs11"
"github.com/prometheus/client_golang/prometheus"
"github.com/zmap/zlint/v3/lint"
"golang.org/x/crypto/cryptobyte"
cryptobyte_asn1 "golang.org/x/crypto/cryptobyte/asn1"
"golang.org/x/crypto/ocsp"
"google.golang.org/protobuf/types/known/timestamppb"

Expand Down Expand Up @@ -421,7 +423,7 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
ca.log.AuditInfof("Signing cert: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] precert=[%s]",
issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, hex.EncodeToString(precert.Raw))

_, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
lintCertBytes, issuanceToken, err := issuer.Prepare(certProfile.profile, issuanceReq)
if err != nil {
ca.log.AuditErrf("Preparing cert failed: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certProfileName=[%s] certProfileHash=[%x] err=[%v]",
issuer.Name(), serialHex, req.RegistrationID, names, certProfile.name, certProfile.hash, err)
Expand All @@ -436,6 +438,11 @@ func (ca *certificateAuthorityImpl) IssueCertificateForPrecertificate(ctx contex
return nil, berrors.InternalServerError("failed to sign certificate: %s", err)
}

err = tbsCertIsDeterministic(lintCertBytes, certDER)
if err != nil {
return nil, err
}

ca.signatureCount.With(prometheus.Labels{"purpose": string(certType), "issuer": issuer.Name()}).Inc()
ca.log.AuditInfof("Signing cert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] certificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
issuer.Name(), serialHex, req.RegistrationID, names, hex.EncodeToString(certDER), certProfile.name, certProfile.hash)
Expand Down Expand Up @@ -611,9 +618,77 @@ func (ca *certificateAuthorityImpl) issuePrecertificateInner(ctx context.Context
return nil, nil, berrors.InternalServerError("failed to sign precertificate: %s", err)
}

err = tbsCertIsDeterministic(lintCertBytes, certDER)
if err != nil {
return nil, nil, err
}

ca.signatureCount.With(prometheus.Labels{"purpose": string(precertType), "issuer": issuer.Name()}).Inc()
ca.log.AuditInfof("Signing precert success: issuer=[%s] serial=[%s] regID=[%d] names=[%s] precertificate=[%s] certProfileName=[%s] certProfileHash=[%x]",
issuer.Name(), serialHex, issueReq.RegistrationID, strings.Join(csr.DNSNames, ", "), hex.EncodeToString(certDER), certProfile.name, certProfile.hash)

return certDER, &certProfileWithID{certProfile.name, certProfile.hash, nil}, nil
}

// verifyTBSCertIsDeterministic verifies that x509.CreateCertificate signing
// operation is deterministic and produced identical DER bytes between the given
// lint certificate and leaf certificate. If the DER byte equality check fails
// it's mississuance, but it's better to know about the problem sooner than
// later. The caller is responsible for passing the appropriate valid
// certificate bytes in the correct position.
func tbsCertIsDeterministic(lintCertBytes []byte, leafCertBytes []byte) error {
if core.IsAnyNilOrZero(lintCertBytes, leafCertBytes) {
return fmt.Errorf("lintCertBytes of leafCertBytes were nil")
}

// extractTBSCertBytes is a partial copy of //crypto/x509/parser.go to
// extract the RawTBSCertificate field from given DER bytes. It the
// RawTBSCertificate field bytes or an error if the given bytes cannot be
// parsed. This is far more performant than parsing the entire *Certificate
// structure with x509.ParseCertificate().
//
// RFC 5280, Section 4.1
// Certificate ::= SEQUENCE {
// tbsCertificate TBSCertificate,
// signatureAlgorithm AlgorithmIdentifier,
// signatureValue BIT STRING }
//
// TBSCertificate ::= SEQUENCE {
// ..
extractTBSCertBytes := func(inputDERBytes *[]byte) ([]byte, error) {
input := cryptobyte.String(*inputDERBytes)

// Extract the Certificate bytes
if !input.ReadASN1(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("malformed certificate")
}

var tbs cryptobyte.String
// Extract the TBSCertificate bytes from the Certificate bytes
if !input.ReadASN1(&tbs, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("malformed tbs certificate")
}

if tbs.Empty() {
return nil, errors.New("parsed RawTBSCertificate field was empty")
}

return tbs, nil
}

lintRawTBSCert, err := extractTBSCertBytes(&lintCertBytes)
if err != nil {
return fmt.Errorf("while extracting lint TBS cert: %w", err)
}

leafRawTBSCert, err := extractTBSCertBytes(&leafCertBytes)
if err != nil {
return fmt.Errorf("while extracting leaf TBS cert: %w", err)
}

if !bytes.Equal(lintRawTBSCert, leafRawTBSCert) {
return fmt.Errorf("mismatch between lintCert and leafCert RawTBSCertificate DER bytes: \"%x\" != \"%x\"", lintRawTBSCert, leafRawTBSCert)
}

return nil
}
100 changes: 100 additions & 0 deletions ca/ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"encoding/asn1"
"errors"
"fmt"
"math/big"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -1295,3 +1296,102 @@ func TestGenerateSKID(t *testing.T) {
test.AssertEquals(t, cap(sha256skid), 20)
features.Reset()
}

func TestVerifyTBSCertIsDeterministic(t *testing.T) {
t.Parallel()

// Create first keypair and cert
testKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "unable to generate ECDSA private key")
template := &x509.Certificate{
NotAfter: time.Now().Add(1 * time.Hour),
DNSNames: []string{"example.com"},
SerialNumber: big.NewInt(1),
}
certDer1, err := x509.CreateCertificate(rand.Reader, template, template, &testKey.PublicKey, testKey)
test.AssertNotError(t, err, "unable to create certificate")

// Create second keypair and cert
testKey2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
test.AssertNotError(t, err, "unable to generate ECDSA private key")
template2 := &x509.Certificate{
NotAfter: time.Now().Add(2 * time.Hour),
DNSNames: []string{"example.net"},
SerialNumber: big.NewInt(2),
}
certDer2, err := x509.CreateCertificate(rand.Reader, template2, template2, &testKey2.PublicKey, testKey2)
test.AssertNotError(t, err, "unable to create certificate")

testCases := []struct {
name string
lintCertBytes []byte
leafCertBytes []byte
errorSubstr string
}{
{
name: "Both nil",
lintCertBytes: nil,
leafCertBytes: nil,
errorSubstr: "were nil",
},
{
name: "Missing a value, invalid input",
lintCertBytes: nil,
leafCertBytes: []byte{0x6, 0x6, 0x6},
errorSubstr: "were nil",
},
{
name: "Missing a value, valid input",
lintCertBytes: nil,
leafCertBytes: certDer1,
errorSubstr: "were nil",
},
{
name: "Mismatched bytes, invalid input",
lintCertBytes: []byte{0x6, 0x6, 0x6},
leafCertBytes: []byte{0x1, 0x2, 0x3},
errorSubstr: "malformed certificate",
},
{
name: "Mismatched bytes, invalider input",
lintCertBytes: certDer1,
leafCertBytes: []byte{0x1, 0x2, 0x3},
errorSubstr: "malformed certificate",
},
{
// This case is an example of when a linting cert's DER bytes are
// mismatched compared to then precert or final cert created from
// that linting cert's DER bytes.
name: "Mismatched bytes, valid input",
lintCertBytes: certDer1,
leafCertBytes: certDer2,
errorSubstr: "mismatch between",
},
{
// Take this with a grain of salt since this test is not actually
// creating a linting certificate and performing two
// x509.CreateCertificate() calls like
// ca.IssueCertificateForPrecertificate and
// ca.issuePrecertificateInner do. However, we're still going to
// verify the equality.
name: "Valid",
lintCertBytes: certDer1,
leafCertBytes: certDer1,
},
}

for _, testCase := range testCases {
// TODO(#7454) Remove this rebinding
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()
err := tbsCertIsDeterministic(testCase.lintCertBytes, testCase.leafCertBytes)
if testCase.errorSubstr != "" {
test.AssertError(t, err, "your lack of errors is disturbing")
test.AssertContains(t, err.Error(), testCase.errorSubstr)
} else {
test.AssertNotError(t, err, "unexpected error")
}
})
}
}
11 changes: 0 additions & 11 deletions cmd/ceremony/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,10 +645,6 @@ func rootCeremony(configBytes []byte) error {
if err != nil {
return err
}
// Verify that the lintCert is self-signed.
if !bytes.Equal(lintCert.RawSubject, lintCert.RawIssuer) {
return fmt.Errorf("mismatch between self-signed lintCert RawSubject and RawIssuer DER bytes: \"%x\" != \"%x\"", lintCert.RawSubject, lintCert.RawIssuer)
}
finalCert, err := signAndWriteCert(template, template, lintCert, keyInfo.key, signer, config.Outputs.CertificatePath)
if err != nil {
return err
Expand Down Expand Up @@ -697,10 +693,6 @@ func intermediateCeremony(configBytes []byte, ct certType) error {
if err != nil {
return err
}
// Verify that the lintCert (and therefore the eventual finalCert) corresponds to the specified issuer certificate.
if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
return fmt.Errorf("mismatch between issuer RawSubject and lintCert RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
}
finalCert, err := signAndWriteCert(template, issuer, lintCert, pub, signer, config.Outputs.CertificatePath)
if err != nil {
return err
Expand Down Expand Up @@ -780,9 +772,6 @@ func crossCertCeremony(configBytes []byte, ct certType) error {
if lintCert.NotBefore.Before(toBeCrossSigned.NotBefore) {
return fmt.Errorf("cross-signed subordinate CA's NotBefore predates the existing CA's NotBefore")
}
if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
return fmt.Errorf("mismatch between issuer RawSubject and lintCert RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
}
// BR 7.1.2.2.3 Cross-Certified Subordinate CA Extensions
if !slices.Equal(lintCert.ExtKeyUsage, toBeCrossSigned.ExtKeyUsage) {
return fmt.Errorf("lint cert and toBeCrossSigned cert EKUs differ")
Expand Down
11 changes: 11 additions & 0 deletions linter/linter.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package linter

import (
"bytes"
"crypto"
"crypto/ecdsa"
"crypto/rand"
Expand Down Expand Up @@ -239,6 +240,16 @@ func makeLintCert(tbs *x509.Certificate, subjectPubKey crypto.PublicKey, issuer
if err != nil {
return nil, nil, fmt.Errorf("failed to parse lint certificate: %w", err)
}
// RFC 5280, Sections 4.1.2.6 and 8
//
// When the subject of the certificate is a CA, the subject
// field MUST be encoded in the same way as it is encoded in the
// issuer field (Section 4.1.2.4) in all certificates issued by
// the subject CA.
if !bytes.Equal(issuer.RawSubject, lintCert.RawIssuer) {
return nil, nil, fmt.Errorf("mismatch between lint issuer RawSubject and lintCert.RawIssuer DER bytes: \"%x\" != \"%x\"", issuer.RawSubject, lintCert.RawIssuer)
}

return lintCertBytes, lintCert, nil
}

Expand Down

0 comments on commit 347ccf8

Please sign in to comment.