Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions issuance/cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ type ProfileConfig struct {
MaxValidityPeriod config.Duration
MaxValidityBackdate config.Duration

// MaxCertificateSize causes rejection at the linting stage of any certificate
// that would be bigger than this many bytes. This should be considered a backstop
// and should be set higher than the corresponding limits at the WFE
// (MaxCumulativeIdentifierLength) and the RA (MaxNames * 253, per-profile),
// plus the longest possible signature size, plus some extra for certificate
// fields.
MaxCertificateSize int

// LintConfig is a path to a zlint config file, which can be used to control
// the behavior of zlint's "customizable lints".
LintConfig string
Expand All @@ -64,6 +72,8 @@ type Profile struct {
maxBackdate time.Duration
maxValidity time.Duration

maxCertificateSize int

lints lint.Registry
}

Expand Down Expand Up @@ -96,6 +106,7 @@ func NewProfile(profileConfig ProfileConfig) (*Profile, error) {
omitSKID: profileConfig.OmitSKID,
maxBackdate: profileConfig.MaxValidityBackdate.Duration,
maxValidity: profileConfig.MaxValidityPeriod.Duration,
maxCertificateSize: profileConfig.MaxCertificateSize,
lints: lints,
}

Expand Down Expand Up @@ -361,6 +372,10 @@ func (i *Issuer) Prepare(prof *Profile, req *IssuanceRequest) ([]byte, *issuance
return nil, nil, fmt.Errorf("tbsCertificate linting failed: %w", err)
}

if prof.maxCertificateSize > 0 && len(lintCertBytes) > prof.maxCertificateSize {
return nil, nil, fmt.Errorf("linting certificate too big (%d > %d)", len(lintCertBytes), prof.maxCertificateSize)
}

if len(req.precertDER) > 0 {
err = precert.Correspond(req.precertDER, lintCertBytes)
if err != nil {
Expand Down
35 changes: 35 additions & 0 deletions issuance/cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"encoding/base64"
"fmt"
"net"
"reflect"
"strings"
Expand Down Expand Up @@ -395,6 +396,40 @@ func TestIssue(t *testing.T) {
}
}

func TestIssueCertTooBig(t *testing.T) {
fc := clock.NewFake()
signer, err := newIssuer(defaultIssuerConfig(), issuerCert, issuerSigner, fc)
if err != nil {
t.Fatalf("newIssuer: %s", err)
}
pk, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
t.Fatalf("ecdsa.GenerateKey: %s", err)
}
var dnsNames []string
for i := 0; i < 1000; i++ {
dnsNames = append(dnsNames, fmt.Sprintf("%d.example.com", i))
}
profile := defaultProfile()
profile.maxCertificateSize = 1000
_, _, err = signer.Prepare(profile, &IssuanceRequest{
PublicKey: MarshalablePublicKey{pk.Public()},
SubjectKeyId: goodSKID,
Serial: []byte{1, 2, 3, 4, 5, 6, 7, 8, 9},
DNSNames: dnsNames,
NotBefore: fc.Now(),
NotAfter: fc.Now().Add(time.Hour - time.Second),
IncludeCTPoison: true,
})
if err == nil {
t.Errorf("signer.Prepare of big cert: got nil error, want an error")
}
expected := "linting certificate too big"
if !strings.Contains(err.Error(), expected) {
t.Errorf("signer.Prepare of big cert: got %q, want %q", err, expected)
}
}

func TestIssueDNSNamesOnly(t *testing.T) {
fc := clock.NewFake()
signer, err := newIssuer(defaultIssuerConfig(), issuerCert, issuerSigner, fc)
Expand Down
3 changes: 3 additions & 0 deletions test/config-next/ca.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"omitSKID": false,
"maxValidityPeriod": "7776000s",
"maxValidityBackdate": "1h5m",
"maxCertificateSize": 10000,
"lintConfig": "test/config-next/zlint.toml",
"ignoredLints": [
"w_subject_common_name_included",
Expand All @@ -69,6 +70,7 @@
"omitSKID": true,
"maxValidityPeriod": "160h",
"maxValidityBackdate": "1h5m",
"maxCertificateSize": 10000,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

verisimilitude nit: maybe make shortlived and modern have lower limits, since they also have lower maxNames limits?

"lintConfig": "test/config-next/zlint.toml",
"ignoredLints": [
"w_ext_subject_key_identifier_missing_sub_cert",
Expand All @@ -82,6 +84,7 @@
"omitSKID": true,
"maxValidityPeriod": "583200s",
"maxValidityBackdate": "1h5m",
"maxCertificateSize": 10000,
"lintConfig": "test/config-next/zlint.toml",
"ignoredLints": [
"w_ext_subject_key_identifier_missing_sub_cert",
Expand Down