From da4c7702248911f5062f4b46bf0de0230d77fc08 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 18:42:19 -0800 Subject: [PATCH] ca: Pass OCSP Must-Staple from CSR into generated certificate (#436) Fixes #433 --- README.md | 5 +- ca/ca.go | 30 ++++++++++- ca/ca_test.go | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 169 insertions(+), 4 deletions(-) create mode 100644 ca/ca_test.go diff --git a/README.md b/README.md index 645468d1..fb168463 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,8 @@ # Pebble -[![Build Status](https://travis-ci.org/letsencrypt/pebble.svg?branch=master)](https://travis-ci.org/letsencrypt/pebble) -[![Coverage Status](https://coveralls.io/repos/github/letsencrypt/pebble/badge.svg?branch=cpu-goveralls)](https://coveralls.io/github/letsencrypt/pebble?branch=cpu-goveralls) +[![Checks](https://github.com/letsencrypt/pebble/actions/workflows/checks.yml/badge.svg)](https://github.com/letsencrypt/pebble/actions/workflows/checks.yml) +[![Tests](https://github.com/letsencrypt/pebble/actions/workflows/tests.yml/badge.svg)](https://github.com/letsencrypt/pebble/actions/workflows/tests.yml) +[![Coverage Status](https://coveralls.io/repos/github/letsencrypt/pebble/badge.svg)](https://coveralls.io/github/letsencrypt/pebble) [![Go Report Card](https://goreportcard.com/badge/github.com/letsencrypt/pebble)](https://goreportcard.com/report/github.com/letsencrypt/pebble) A miniature version of [Boulder](https://github.com/letsencrypt/boulder), Pebble diff --git a/ca/ca.go b/ca/ca.go index d35b5f6d..c16d7f4a 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -1,6 +1,7 @@ package ca import ( + "bytes" "crypto" "crypto/rand" "crypto/rsa" @@ -252,7 +253,7 @@ func (ca *CAImpl) newChain(intermediateKey crypto.Signer, intermediateSubject pk return c } -func (ca *CAImpl) newCertificate(domains []string, ips []net.IP, key crypto.PublicKey, accountID, notBefore, notAfter string) (*core.Certificate, error) { +func (ca *CAImpl) newCertificate(domains []string, ips []net.IP, key crypto.PublicKey, accountID, notBefore, notAfter string, extensions []pkix.Extension) (*core.Certificate, error) { if len(domains) == 0 && len(ips) == 0 { return nil, errors.New("must specify at least one domain name or IP address") } @@ -299,6 +300,7 @@ func (ca *CAImpl) newCertificate(domains []string, ips []net.IP, key crypto.Publ KeyUsage: x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth}, SubjectKeyId: subjectKeyID, + ExtraExtensions: extensions, BasicConstraintsValid: true, IsCA: false, } @@ -373,6 +375,22 @@ func New(log *log.Logger, db *db.MemoryStore, ocspResponderURL string, alternate return ca } +var ocspMustStapleExt = pkix.Extension{ + Id: asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24}, + Value: []byte{0x30, 0x03, 0x02, 0x01, 0x05}, +} + +// Returns whether the given extensions array contains an OCSP Must-Staple +// extension. +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) CompleteOrder(order *core.Order) { // Lock the order for reading order.RLock() @@ -397,9 +415,17 @@ func (ca *CAImpl) CompleteOrder(order *core.Order) { authz.RUnlock() } + // Build a list of approved extensions to include in the certificate + var extensions []pkix.Extension + 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 csr := order.ParsedCSR - cert, err := ca.newCertificate(csr.DNSNames, csr.IPAddresses, csr.PublicKey, order.AccountID, order.NotBefore, order.NotAfter) + cert, err := ca.newCertificate(csr.DNSNames, csr.IPAddresses, csr.PublicKey, order.AccountID, order.NotBefore, order.NotAfter, extensions) if err != nil { ca.log.Printf("Error: unable to issue order: %s", err.Error()) return diff --git a/ca/ca_test.go b/ca/ca_test.go new file mode 100644 index 00000000..17d03470 --- /dev/null +++ b/ca/ca_test.go @@ -0,0 +1,138 @@ +package ca + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/asn1" + "log" + "net" + "os" + "testing" + "time" + + "github.com/letsencrypt/pebble/v2/acme" + "github.com/letsencrypt/pebble/v2/core" + "github.com/letsencrypt/pebble/v2/db" +) + +var ( + ocspID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} + ocspValue = []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) + } + csr := x509.CertificateRequest{ + DNSNames: []string{"fake.domain"}, + IPAddresses: []net.IP{[]byte{192, 0, 2, 1}}, + PublicKey: &privateKey.PublicKey, + Extensions: extensions, + } + return core.Order{ + ID: "randomstring", + AccountID: "accountid", + ParsedCSR: &csr, + BeganProcessing: true, + Order: acme.Order{ + Status: acme.StatusPending, + Expires: time.Now().AddDate(0, 0, 1).UTC().Format(time.RFC3339), + 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 +} + +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/ duplicate 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++ + } + } + if numOCSPMustStapleExtensions != 1 { + t.Errorf("Expected exactly 1 OCSP Must-Staple extension, found %d", numOCSPMustStapleExtensions) + } +}