From 7f42a41ededf8cebe1b29b43e902412f3420bab9 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sun, 12 Jan 2025 16:08:39 +0800 Subject: [PATCH 1/5] ceremony: Replace x509.Certificate.PolicyIdentifiers with Policies Closes https://github.com/letsencrypt/boulder/issues/7148. Signed-off-by: Eng Zer Jun --- cmd/ceremony/cert.go | 21 ++------------------- cmd/ceremony/cert_test.go | 15 +-------------- 2 files changed, 3 insertions(+), 33 deletions(-) diff --git a/cmd/ceremony/cert.go b/cmd/ceremony/cert.go index 7f3893593e2..96a24218818 100644 --- a/cmd/ceremony/cert.go +++ b/cmd/ceremony/cert.go @@ -10,8 +10,6 @@ import ( "fmt" "io" "math/big" - "strconv" - "strings" "time" ) @@ -173,21 +171,6 @@ func (profile *certProfile) verifyProfile(ct certType) error { return nil } -func parseOID(oidStr string) (asn1.ObjectIdentifier, error) { - var oid asn1.ObjectIdentifier - for _, a := range strings.Split(oidStr, ".") { - i, err := strconv.Atoi(a) - if err != nil { - return nil, err - } - if i <= 0 { - return nil, errors.New("OID components must be >= 1") - } - oid = append(oid, i) - } - return oid, nil -} - var stringToKeyUsage = map[string]x509.KeyUsage{ "Digital Signature": x509.KeyUsageDigitalSignature, "CRL Sign": x509.KeyUsageCRLSign, @@ -318,11 +301,11 @@ func makeTemplate(randReader io.Reader, profile *certProfile, pubKey []byte, tbc } for _, policyConfig := range profile.Policies { - oid, err := parseOID(policyConfig.OID) + oid, err := x509.ParseOID(policyConfig.OID) if err != nil { return nil, err } - cert.PolicyIdentifiers = append(cert.PolicyIdentifiers, oid) + cert.Policies = append(cert.Policies, oid) } return cert, nil diff --git a/cmd/ceremony/cert_test.go b/cmd/ceremony/cert_test.go index 95a2b33755f..f4f8bfaf498 100644 --- a/cmd/ceremony/cert_test.go +++ b/cmd/ceremony/cert_test.go @@ -6,7 +6,6 @@ import ( "crypto/rsa" "crypto/x509" "crypto/x509/pkix" - "encoding/asn1" "encoding/hex" "errors" "fmt" @@ -37,18 +36,6 @@ func realRand(_ pkcs11.SessionHandle, length int) ([]byte, error) { return r, err } -func TestParseOID(t *testing.T) { - _, err := parseOID("") - test.AssertError(t, err, "parseOID accepted an empty OID") - _, err = parseOID("a.b.c") - test.AssertError(t, err, "parseOID accepted an OID containing non-ints") - _, err = parseOID("1.0.2") - test.AssertError(t, err, "parseOID accepted an OID containing zero") - oid, err := parseOID("1.2.3") - test.AssertNotError(t, err, "parseOID failed with a valid OID") - test.Assert(t, oid.Equal(asn1.ObjectIdentifier{1, 2, 3}), "parseOID returned incorrect OID") -} - func TestMakeSubject(t *testing.T) { profile := &certProfile{ CommonName: "common name", @@ -126,7 +113,7 @@ func TestMakeTemplateRoot(t *testing.T) { test.AssertEquals(t, len(cert.IssuingCertificateURL), 1) test.AssertEquals(t, cert.IssuingCertificateURL[0], profile.IssuerURL) test.AssertEquals(t, cert.KeyUsage, x509.KeyUsageDigitalSignature|x509.KeyUsageCRLSign) - test.AssertEquals(t, len(cert.PolicyIdentifiers), 2) + test.AssertEquals(t, len(cert.Policies), 2) test.AssertEquals(t, len(cert.ExtKeyUsage), 0) cert, err = makeTemplate(randReader, profile, pubKey, nil, intermediateCert) From 8003a3b7a2ae7f2c3f4d3ea8b0e86ed317a7d6ae Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Sun, 12 Jan 2025 16:39:15 +0800 Subject: [PATCH 2/5] Preserve PolicyIdentifiers field ZLint requires the field to be set. Signed-off-by: Eng Zer Jun --- cmd/ceremony/cert.go | 27 +++++++++++++++++++++++++-- cmd/ceremony/cert_test.go | 14 ++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/cmd/ceremony/cert.go b/cmd/ceremony/cert.go index 96a24218818..40bc0860b82 100644 --- a/cmd/ceremony/cert.go +++ b/cmd/ceremony/cert.go @@ -10,6 +10,8 @@ import ( "fmt" "io" "math/big" + "strconv" + "strings" "time" ) @@ -171,6 +173,21 @@ func (profile *certProfile) verifyProfile(ct certType) error { return nil } +func parseOID(oidStr string) (asn1.ObjectIdentifier, error) { + var oid asn1.ObjectIdentifier + for _, a := range strings.Split(oidStr, ".") { + i, err := strconv.Atoi(a) + if err != nil { + return nil, err + } + if i <= 0 { + return nil, errors.New("OID components must be >= 1") + } + oid = append(oid, i) + } + return oid, nil +} + var stringToKeyUsage = map[string]x509.KeyUsage{ "Digital Signature": x509.KeyUsageDigitalSignature, "CRL Sign": x509.KeyUsageCRLSign, @@ -301,11 +318,17 @@ func makeTemplate(randReader io.Reader, profile *certProfile, pubKey []byte, tbc } for _, policyConfig := range profile.Policies { - oid, err := x509.ParseOID(policyConfig.OID) + asnOID, err := parseOID(policyConfig.OID) + if err != nil { + return nil, err + } + cert.PolicyIdentifiers = append(cert.PolicyIdentifiers, asnOID) + + x509OID, err := x509.ParseOID(policyConfig.OID) if err != nil { return nil, err } - cert.Policies = append(cert.Policies, oid) + cert.Policies = append(cert.Policies, x509OID) } return cert, nil diff --git a/cmd/ceremony/cert_test.go b/cmd/ceremony/cert_test.go index f4f8bfaf498..688c683812e 100644 --- a/cmd/ceremony/cert_test.go +++ b/cmd/ceremony/cert_test.go @@ -6,6 +6,7 @@ import ( "crypto/rsa" "crypto/x509" "crypto/x509/pkix" + "encoding/asn1" "encoding/hex" "errors" "fmt" @@ -36,6 +37,18 @@ func realRand(_ pkcs11.SessionHandle, length int) ([]byte, error) { return r, err } +func TestParseOID(t *testing.T) { + _, err := parseOID("") + test.AssertError(t, err, "parseOID accepted an empty OID") + _, err = parseOID("a.b.c") + test.AssertError(t, err, "parseOID accepted an OID containing non-ints") + _, err = parseOID("1.0.2") + test.AssertError(t, err, "parseOID accepted an OID containing zero") + oid, err := parseOID("1.2.3") + test.AssertNotError(t, err, "parseOID failed with a valid OID") + test.Assert(t, oid.Equal(asn1.ObjectIdentifier{1, 2, 3}), "parseOID returned incorrect OID") +} + func TestMakeSubject(t *testing.T) { profile := &certProfile{ CommonName: "common name", @@ -113,6 +126,7 @@ func TestMakeTemplateRoot(t *testing.T) { test.AssertEquals(t, len(cert.IssuingCertificateURL), 1) test.AssertEquals(t, cert.IssuingCertificateURL[0], profile.IssuerURL) test.AssertEquals(t, cert.KeyUsage, x509.KeyUsageDigitalSignature|x509.KeyUsageCRLSign) + test.AssertEquals(t, len(cert.PolicyIdentifiers), 2) test.AssertEquals(t, len(cert.Policies), 2) test.AssertEquals(t, len(cert.ExtKeyUsage), 0) From 4ad9f89b9fd9c3674d124b92610e5da3f3470d52 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Tue, 14 Jan 2025 09:22:49 +0800 Subject: [PATCH 3/5] Populate `x509.Certificate.Policies` in all places Reference: https://github.com/letsencrypt/boulder/pull/7940#pullrequestreview-2548339909 Signed-off-by: Eng Zer Jun --- cmd/ceremony/cert.go | 2 +- cmd/cert-checker/main_test.go | 9 ++++++--- issuance/cert.go | 3 +++ issuance/cert_test.go | 16 +++++++++------- linter/linter.go | 1 + 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/cmd/ceremony/cert.go b/cmd/ceremony/cert.go index 40bc0860b82..0fcf6ce1f27 100644 --- a/cmd/ceremony/cert.go +++ b/cmd/ceremony/cert.go @@ -326,7 +326,7 @@ func makeTemplate(randReader io.Reader, profile *certProfile, pubKey []byte, tbc x509OID, err := x509.ParseOID(policyConfig.OID) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to parse %s as OID: %w", policyConfig.OID, err) } cert.Policies = append(cert.Policies, x509OID) } diff --git a/cmd/cert-checker/main_test.go b/cmd/cert-checker/main_test.go index 137b05767e2..e190ec626a8 100644 --- a/cmd/cert-checker/main_test.go +++ b/cmd/cert-checker/main_test.go @@ -18,7 +18,6 @@ import ( mrand "math/rand/v2" "os" "slices" - "sort" "strings" "sync" "testing" @@ -585,6 +584,9 @@ func TestIgnoredLint(t *testing.T) { checker := newChecker(saDbMap, clock.NewFake(), pa, kp, time.Hour, testValidityDurations, blog.NewMock()) serial := big.NewInt(1337) + x509OID, err := x509.OIDFromInts([]uint64{1, 2, 3}) + test.AssertNotError(t, err, "failed to create x509.OID") + template := &x509.Certificate{ Subject: pkix.Name{ CommonName: "CPU's Cool CA", @@ -597,6 +599,7 @@ func TestIgnoredLint(t *testing.T) { PolicyIdentifiers: []asn1.ObjectIdentifier{ {1, 2, 3}, }, + Policies: []x509.OID{x509OID}, BasicConstraintsValid: true, IsCA: true, IssuingCertificateURL: []string{"http://aia.example.org"}, @@ -639,12 +642,12 @@ func TestIgnoredLint(t *testing.T) { "zlint info: w_ct_sct_policy_count_unsatisfied Certificate had 0 embedded SCTs. Browser policy may require 2 for this certificate.", "zlint error: e_scts_from_same_operator Certificate had too few embedded SCTs; browser policy requires 2.", } - sort.Strings(expectedProblems) + slices.Sort(expectedProblems) // Check the certificate with a nil ignore map. This should return the // expected zlint problems. _, problems := checker.checkCert(context.Background(), cert, nil) - sort.Strings(problems) + slices.Sort(problems) test.AssertDeepEquals(t, problems, expectedProblems) // Check the certificate again with an ignore map that excludes the affected diff --git a/issuance/cert.go b/issuance/cert.go index 0c97b1b84c5..6f9a87500e2 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -193,6 +193,8 @@ func (i *Issuer) requestValid(clk clock.Clock, prof *Profile, req *IssuanceReque } func (i *Issuer) generateTemplate() *x509.Certificate { + x509OID, _ := x509.OIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) + template := &x509.Certificate{ SignatureAlgorithm: i.sigAlg, OCSPServer: []string{i.ocspURL}, @@ -200,6 +202,7 @@ func (i *Issuer) generateTemplate() *x509.Certificate { BasicConstraintsValid: true, // Baseline Requirements, Section 7.1.6.1: domain-validated PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, + Policies: []x509.OID{x509OID}, } // TODO(#7294): Use i.crlURLBase and a shard calculation to create a diff --git a/issuance/cert_test.go b/issuance/cert_test.go index 80f8c5d4674..f98602927f1 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -22,9 +22,7 @@ import ( "github.com/letsencrypt/boulder/test" ) -var ( - goodSKID = []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9} -) +var goodSKID = []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9} func defaultProfile() *Profile { p, _ := NewProfile(defaultProfileConfig()) @@ -33,7 +31,7 @@ func defaultProfile() *Profile { func TestGenerateValidity(t *testing.T) { fc := clock.NewFake() - fc.Set(time.Date(2015, time.June, 04, 11, 04, 38, 0, time.UTC)) + fc.Set(time.Date(2015, time.June, 0o4, 11, 0o4, 38, 0, time.UTC)) tests := []struct { name string @@ -46,15 +44,15 @@ func TestGenerateValidity(t *testing.T) { name: "normal usage", backdate: time.Hour, // 90% of one hour is 54 minutes validity: 7 * 24 * time.Hour, - notBefore: time.Date(2015, time.June, 04, 10, 10, 38, 0, time.UTC), + notBefore: time.Date(2015, time.June, 0o4, 10, 10, 38, 0, time.UTC), notAfter: time.Date(2015, time.June, 11, 10, 10, 37, 0, time.UTC), }, { name: "zero backdate", backdate: 0, validity: 7 * 24 * time.Hour, - notBefore: time.Date(2015, time.June, 04, 11, 04, 38, 0, time.UTC), - notAfter: time.Date(2015, time.June, 11, 11, 04, 37, 0, time.UTC), + notBefore: time.Date(2015, time.June, 0o4, 11, 0o4, 38, 0, time.UTC), + notAfter: time.Date(2015, time.June, 11, 11, 0o4, 37, 0, time.UTC), }, } @@ -315,6 +313,9 @@ func TestGenerateTemplate(t *testing.T) { actual := issuer.generateTemplate() + x509OID, err := x509.OIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) + test.AssertNotError(t, err, "failed to create x509.OID") + expected := &x509.Certificate{ BasicConstraintsValid: true, SignatureAlgorithm: x509.SHA256WithRSA, @@ -322,6 +323,7 @@ func TestGenerateTemplate(t *testing.T) { OCSPServer: []string{"http://ocsp"}, CRLDistributionPoints: nil, PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, + Policies: []x509.OID{x509OID}, } test.AssertDeepEquals(t, actual, expected) diff --git a/linter/linter.go b/linter/linter.go index e9bf33b85a2..249e5ab91f3 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -195,6 +195,7 @@ func makeIssuer(realIssuer *x509.Certificate, lintSigner crypto.Signer) (*x509.C PermittedIPRanges: realIssuer.PermittedIPRanges, PermittedURIDomains: realIssuer.PermittedURIDomains, PolicyIdentifiers: realIssuer.PolicyIdentifiers, + Policies: realIssuer.Policies, SerialNumber: realIssuer.SerialNumber, Subject: realIssuer.Subject, SubjectKeyId: realIssuer.SubjectKeyId, From 506c51b85435702a5792eb50b4751ce083b336f2 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Wed, 15 Jan 2025 03:28:38 +0800 Subject: [PATCH 4/5] Apply code suggestions Reference: https://github.com/letsencrypt/boulder/pull/7940#pullrequestreview-2550627240 Signed-off-by: Eng Zer Jun --- issuance/cert.go | 15 ++++++++++++--- issuance/cert_test.go | 17 ++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/issuance/cert.go b/issuance/cert.go index 6f9a87500e2..a53dfa5242c 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -192,9 +192,18 @@ func (i *Issuer) requestValid(clk clock.Clock, prof *Profile, req *IssuanceReque return nil } -func (i *Issuer) generateTemplate() *x509.Certificate { - x509OID, _ := x509.OIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) +// Baseline Requirements, Section 7.1.6.1: domain-validated +var domainValidatedOID = mustOIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) + +func mustOIDFromInts(oid []uint64) x509.OID { + x509OID, err := x509.OIDFromInts(oid) + if err != nil { + panic(fmt.Errorf("failed to create OID using ints %v: %w", oid, err)) + } + return x509OID +} +func (i *Issuer) generateTemplate() *x509.Certificate { template := &x509.Certificate{ SignatureAlgorithm: i.sigAlg, OCSPServer: []string{i.ocspURL}, @@ -202,7 +211,7 @@ func (i *Issuer) generateTemplate() *x509.Certificate { BasicConstraintsValid: true, // Baseline Requirements, Section 7.1.6.1: domain-validated PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, - Policies: []x509.OID{x509OID}, + Policies: []x509.OID{domainValidatedOID}, } // TODO(#7294): Use i.crlURLBase and a shard calculation to create a diff --git a/issuance/cert_test.go b/issuance/cert_test.go index f98602927f1..67deedf6c79 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -22,7 +22,9 @@ import ( "github.com/letsencrypt/boulder/test" ) -var goodSKID = []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9} +var ( + goodSKID = []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9} +) func defaultProfile() *Profile { p, _ := NewProfile(defaultProfileConfig()) @@ -31,7 +33,7 @@ func defaultProfile() *Profile { func TestGenerateValidity(t *testing.T) { fc := clock.NewFake() - fc.Set(time.Date(2015, time.June, 0o4, 11, 0o4, 38, 0, time.UTC)) + fc.Set(time.Date(2015, time.June, 04, 11, 04, 38, 0, time.UTC)) tests := []struct { name string @@ -44,15 +46,15 @@ func TestGenerateValidity(t *testing.T) { name: "normal usage", backdate: time.Hour, // 90% of one hour is 54 minutes validity: 7 * 24 * time.Hour, - notBefore: time.Date(2015, time.June, 0o4, 10, 10, 38, 0, time.UTC), + notBefore: time.Date(2015, time.June, 04, 10, 10, 38, 0, time.UTC), notAfter: time.Date(2015, time.June, 11, 10, 10, 37, 0, time.UTC), }, { name: "zero backdate", backdate: 0, validity: 7 * 24 * time.Hour, - notBefore: time.Date(2015, time.June, 0o4, 11, 0o4, 38, 0, time.UTC), - notAfter: time.Date(2015, time.June, 11, 11, 0o4, 37, 0, time.UTC), + notBefore: time.Date(2015, time.June, 04, 11, 04, 38, 0, time.UTC), + notAfter: time.Date(2015, time.June, 11, 11, 04, 37, 0, time.UTC), }, } @@ -313,9 +315,6 @@ func TestGenerateTemplate(t *testing.T) { actual := issuer.generateTemplate() - x509OID, err := x509.OIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) - test.AssertNotError(t, err, "failed to create x509.OID") - expected := &x509.Certificate{ BasicConstraintsValid: true, SignatureAlgorithm: x509.SHA256WithRSA, @@ -323,7 +322,7 @@ func TestGenerateTemplate(t *testing.T) { OCSPServer: []string{"http://ocsp"}, CRLDistributionPoints: nil, PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, - Policies: []x509.OID{x509OID}, + Policies: []x509.OID{domainValidatedOID}, } test.AssertDeepEquals(t, actual, expected) From acad8901045c43d7e71f8a2cca4ac8ee5ab7b620 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Thu, 23 Jan 2025 01:12:22 +0800 Subject: [PATCH 5/5] Apply code suggestions Reference: https://github.com/letsencrypt/boulder/pull/7940#pullrequestreview-2567790613 Signed-off-by: Eng Zer Jun --- issuance/cert.go | 14 +++++++------- issuance/cert_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/issuance/cert.go b/issuance/cert.go index a53dfa5242c..0884b3eb6bb 100644 --- a/issuance/cert.go +++ b/issuance/cert.go @@ -193,15 +193,15 @@ func (i *Issuer) requestValid(clk clock.Clock, prof *Profile, req *IssuanceReque } // Baseline Requirements, Section 7.1.6.1: domain-validated -var domainValidatedOID = mustOIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) - -func mustOIDFromInts(oid []uint64) x509.OID { - x509OID, err := x509.OIDFromInts(oid) +var domainValidatedASN1OID = asn1.ObjectIdentifier{2, 23, 140, 1, 2, 1} +var domainValidatedOID = func() x509.OID { + x509OID, err := x509.OIDFromInts([]uint64{2, 23, 140, 1, 2, 1}) if err != nil { - panic(fmt.Errorf("failed to create OID using ints %v: %w", oid, err)) + // This should never happen, as the OID is hardcoded. + panic(fmt.Errorf("failed to create OID using ints %v: %s", x509OID, err)) } return x509OID -} +}() func (i *Issuer) generateTemplate() *x509.Certificate { template := &x509.Certificate{ @@ -210,7 +210,7 @@ func (i *Issuer) generateTemplate() *x509.Certificate { IssuingCertificateURL: []string{i.issuerURL}, BasicConstraintsValid: true, // Baseline Requirements, Section 7.1.6.1: domain-validated - PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, + PolicyIdentifiers: []asn1.ObjectIdentifier{domainValidatedASN1OID}, Policies: []x509.OID{domainValidatedOID}, } diff --git a/issuance/cert_test.go b/issuance/cert_test.go index 67deedf6c79..d5824a73aeb 100644 --- a/issuance/cert_test.go +++ b/issuance/cert_test.go @@ -321,7 +321,7 @@ func TestGenerateTemplate(t *testing.T) { IssuingCertificateURL: []string{"http://issuer"}, OCSPServer: []string{"http://ocsp"}, CRLDistributionPoints: nil, - PolicyIdentifiers: []asn1.ObjectIdentifier{{2, 23, 140, 1, 2, 1}}, + PolicyIdentifiers: []asn1.ObjectIdentifier{domainValidatedASN1OID}, Policies: []x509.OID{domainValidatedOID}, }