diff --git a/ca/ca.go b/ca/ca.go index 9b5821e1..e9e39c0a 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -375,8 +375,9 @@ func New(log *log.Logger, db *db.MemoryStore, ocspResponderURL string, alternate return ca } -func isOCSPMustStapleExtension(ext pkix.Extension) bool { - return ext.Id.Equal(asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}) && bytes.Equal(ext.Value, []byte{0x30, 0x03, 0x02, 0x01, 0x05}) +var ocspMustStapleExt = pkix.Extension{ + Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}, + Value: []byte{0x30, 0x03, 0x02, 0x01, 0x05}, } func (ca *CAImpl) CompleteOrder(order *core.Order) { @@ -405,10 +406,10 @@ func (ca *CAImpl) CompleteOrder(order *core.Order) { // Build a list of approved extensions to include in the certificate var extensions []pkix.Extension - for _, ext := range order.ParsedCSR.Extensions { - if isOCSPMustStapleExtension(ext) { - extensions = append(extensions, ext) - } + if extensionsContainsOCSPMustStaple(order.ParsedCSR.Extensions) { + // If the user requested an OCSP Must-Staple extension, use our + // pre-baked one to ensure a reasonable value for Critical + extensions = append(extensions, ocspMustStapleExt) } // issue a certificate for the csr @@ -426,6 +427,15 @@ func (ca *CAImpl) CompleteOrder(order *core.Order) { order.Unlock() } +func extensionsContainsOCSPMustStaple(extensions []pkix.Extension) bool { + for _, ext := range extensions { + if ext.Id.Equal(ocspMustStapleExt.Id) && bytes.Equal(ext.Value, ocspMustStapleExt.Value) { + return true + } + } + return false +} + func (ca *CAImpl) GetNumberOfRootCerts() int { return len(ca.chains) } diff --git a/ca/ca_test.go b/ca/ca_test.go index 136f2db4..61046ed9 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -1,6 +1,7 @@ package ca import ( + "bytes" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" @@ -18,29 +19,27 @@ import ( "github.com/letsencrypt/pebble/v2/db" ) -func TestOCSPMustStaple(t *testing.T) { +var ocspId asn1.ObjectIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} +var ocspValue []byte = []byte{0x30, 0x03, 0x02, 0x01, 0x05} + +func makeCa() *CAImpl { logger := log.New(os.Stdout, "Pebble ", log.LstdFlags) + db := db.NewMemoryStore() + return New(logger, db, "", 0, 1, 0) +} + +func makeCertOrderWithExtensions(extensions []pkix.Extension) core.Order { privateKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { panic(err) } - db := db.NewMemoryStore() - ca := New(logger, db, "", 0, 1, 0) csr := x509.CertificateRequest{ - DNSNames: []string{"test.org"}, - IPAddresses: []net.IP{[]byte{10, 255, 0, 0}}, + DNSNames: []string{"fake.domain"}, + IPAddresses: []net.IP{[]byte{192, 0, 2, 1}}, PublicKey: &privateKey.PublicKey, - Extensions: []pkix.Extension{ - { - Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}, - Critical: false, - Value: []byte{0x30, 0x03, 0x02, 0x01, 0x05}, - }, - }, + Extensions: extensions, } - var uniquenames []acme.Identifier - uniquenames = append(uniquenames, acme.Identifier{Value: "13.12.13.12", Type: acme.IdentifierIP}) - order := &core.Order{ + return core.Order{ ID: "randomstring", AccountID: "accountid", ParsedCSR: &csr, @@ -48,13 +47,90 @@ func TestOCSPMustStaple(t *testing.T) { Order: acme.Order{ Status: acme.StatusPending, Expires: time.Now().AddDate(0, 0, 1).UTC().Format(time.RFC3339), - Identifiers: uniquenames, + Identifiers: []acme.Identifier{}, NotBefore: time.Now().UTC().Format(time.RFC3339), NotAfter: time.Now().AddDate(30, 0, 0).UTC().Format(time.RFC3339), }, ExpiresDate: time.Now().AddDate(0, 0, 1).UTC(), } +} + +func getOCSPMustStapleExtension(cert *x509.Certificate) *pkix.Extension { + for _, ext := range cert.Extensions { + if ext.Id.Equal(ocspId) && bytes.Equal(ext.Value, ocspValue) { + return &ext + } + } + return nil +} - ca.CompleteOrder(order) - log.Printf("cert: %+v", order.CertificateObject.Cert) +func TestNoExtensions(t *testing.T) { + ca := makeCa() + order := makeCertOrderWithExtensions([]pkix.Extension{}) + ca.CompleteOrder(&order) + foundOCSPExtension := getOCSPMustStapleExtension(order.CertificateObject.Cert) + if foundOCSPExtension != nil { + t.Error("Expected no OCSP Must-Staple extension in complete cert, but found one") + } +} + +func TestSettingOCSPMustStapleExtension(t *testing.T) { + // Base case + ca := makeCa() + order := makeCertOrderWithExtensions([]pkix.Extension{ + { + Id: ocspId, + Critical: false, + Value: ocspValue, + }, + }) + ca.CompleteOrder(&order) + foundOCSPExtension := getOCSPMustStapleExtension(order.CertificateObject.Cert) + if foundOCSPExtension == nil { + t.Error("Expected OCSP Must-Staple extension in complete cert, but didn't find it") + } else if foundOCSPExtension.Critical { + t.Error("Expected foundOCSPExtension.Critical to be false, but it was true") + } + + // Test w/ improperly set Critical value + ca = makeCa() + order = makeCertOrderWithExtensions([]pkix.Extension{ + { + Id: ocspId, + Critical: true, + Value: ocspValue, + }, + }) + ca.CompleteOrder(&order) + foundOCSPExtension = getOCSPMustStapleExtension(order.CertificateObject.Cert) + if foundOCSPExtension == nil { + t.Error("Expected OCSP Must-Staple extension in complete cert, but didn't find it") + } else if foundOCSPExtension.Critical { + t.Error("Expected foundOCSPExtension.Critical to be false, but it was true") + } + + // Test w/ several extensions + ca = makeCa() + order = makeCertOrderWithExtensions([]pkix.Extension{ + { + Id: ocspId, + Critical: true, + Value: ocspValue, + }, + { + Id: ocspId, + Critical: true, + Value: ocspValue, + }, + }) + ca.CompleteOrder(&order) + numOCSPMustStapleExtensions := 0 + for _, ext := range order.CertificateObject.Cert.Extensions { + if ext.Id.Equal(ocspId) && bytes.Equal(ext.Value, ocspValue) { + numOCSPMustStapleExtensions += 1 + } + } + if numOCSPMustStapleExtensions != 1 { + t.Errorf("Expected exactly 1 OCSP Must-Staple extension, found %d", numOCSPMustStapleExtensions) + } }