ca: Use a pre-baked OCSP Must-Staple extension if requested

Also adds unit tests.

ca/ca.go

@@ -374,8 +374,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) {
@@ -404,10 +405,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
@@ -425,6 +426,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)
 }

ca/ca_test.go

@@ -1,6 +1,7 @@
 package ca
 
 import (
+ "bytes"
  "crypto/ecdsa"
  "crypto/elliptic"
  "crypto/rand"
@@ -18,29 +19,29 @@ 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{
+ uniquenames = append(uniquenames, acme.Identifier{Value: "192.0.2.1", Type: acme.IdentifierIP})
+ return core.Order{
  ID: "randomstring",
  AccountID: "accountid",
  ParsedCSR: &csr,
@@ -54,7 +55,84 @@ func TestOCSPMustStaple(t *testing.T) {
  },
  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)
+ }
 }