From 0477dfecc91eb250372b46fc5e5787ee554d4a4d Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Thu, 22 Feb 2024 13:18:37 -0800 Subject: [PATCH 1/7] ca: Pass any CSR extensions into the generated certificate This will allow for extensions such as OCSP stapling. --- ca/ca.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ca/ca.go b/ca/ca.go index d35b5f6d..8571b627 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -252,7 +252,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 +299,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, } @@ -399,7 +400,7 @@ func (ca *CAImpl) CompleteOrder(order *core.Order) { // 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, csr.Extensions) if err != nil { ca.log.Printf("Error: unable to issue order: %s", err.Error()) return From cef2453bef26b63ef005127b920f39e3846c31e3 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Mon, 26 Feb 2024 12:18:03 -0800 Subject: [PATCH 2/7] ca: only pass the OCSP Must-Staple extension --- ca/ca.go | 15 ++++++++++++- ca/ca_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 ca/ca_test.go diff --git a/ca/ca.go b/ca/ca.go index 8571b627..9b5821e1 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -1,6 +1,7 @@ package ca import ( + "bytes" "crypto" "crypto/rand" "crypto/rsa" @@ -374,6 +375,10 @@ 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}) +} + func (ca *CAImpl) CompleteOrder(order *core.Order) { // Lock the order for reading order.RLock() @@ -398,9 +403,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 + for _, ext := range order.ParsedCSR.Extensions { + if isOCSPMustStapleExtension(ext) { + extensions = append(extensions, ext) + } + } + // 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, csr.Extensions) + 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..136f2db4 --- /dev/null +++ b/ca/ca_test.go @@ -0,0 +1,60 @@ +package ca + +import ( + "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" +) + +func TestOCSPMustStaple(t *testing.T) { + logger := log.New(os.Stdout, "Pebble ", log.LstdFlags) + 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}}, + 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}, + }, + }, + } + var uniquenames []acme.Identifier + uniquenames = append(uniquenames, acme.Identifier{Value: "13.12.13.12", Type: acme.IdentifierIP}) + order := &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: uniquenames, + 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(), + } + + ca.CompleteOrder(order) + log.Printf("cert: %+v", order.CertificateObject.Cert) +} From 856df11ae362552e16a2b08afd840c015e96adb1 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 12:11:06 -0800 Subject: [PATCH 3/7] ca: Use a pre-baked OCSP Must-Staple extension if requested Also adds unit tests. --- ca/ca.go | 22 +++++++--- ca/ca_test.go | 112 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 24 deletions(-) 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) + } } From 392fa468fe27d025d86eeaa5ebef08f35918fdd6 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 12:35:11 -0800 Subject: [PATCH 4/7] ca: minor comment additions --- ca/ca.go | 20 +++++++++++--------- ca/ca_test.go | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ca/ca.go b/ca/ca.go index e9e39c0a..c16d7f4a 100644 --- a/ca/ca.go +++ b/ca/ca.go @@ -380,6 +380,17 @@ var ocspMustStapleExt = pkix.Extension{ 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() @@ -427,15 +438,6 @@ 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 61046ed9..a5efe91b 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -109,7 +109,7 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { t.Error("Expected foundOCSPExtension.Critical to be false, but it was true") } - // Test w/ several extensions + // Test w/ duplicate extensions ca = makeCa() order = makeCertOrderWithExtensions([]pkix.Extension{ { From ffa3247743f4c4a1d2244b6f0442e0c81f03a376 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 14:48:54 -0800 Subject: [PATCH 5/7] ca_test: appease the linter --- ca/ca_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ca/ca_test.go b/ca/ca_test.go index a5efe91b..ca28c882 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -20,7 +20,7 @@ import ( ) var ocspId asn1.ObjectIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} -var ocspValue []byte = []byte{0x30, 0x03, 0x02, 0x01, 0x05} +var ocspValue = []byte{0x30, 0x03, 0x02, 0x01, 0x05} func makeCa() *CAImpl { logger := log.New(os.Stdout, "Pebble ", log.LstdFlags) @@ -127,7 +127,7 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { numOCSPMustStapleExtensions := 0 for _, ext := range order.CertificateObject.Cert.Extensions { if ext.Id.Equal(ocspId) && bytes.Equal(ext.Value, ocspValue) { - numOCSPMustStapleExtensions += 1 + numOCSPMustStapleExtensions++ } } if numOCSPMustStapleExtensions != 1 { From 6abd7139bc998d7de0899213e23170139a91747b Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 15:46:03 -0800 Subject: [PATCH 6/7] ca_test: run gofumpt --- ca/ca_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ca/ca_test.go b/ca/ca_test.go index ca28c882..2f922cf8 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -19,8 +19,10 @@ import ( "github.com/letsencrypt/pebble/v2/db" ) -var ocspId asn1.ObjectIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} -var ocspValue = []byte{0x30, 0x03, 0x02, 0x01, 0x05} +var ( + ocspId asn1.ObjectIdentifier = 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) From 1400bf973dc332845897de991130c0d11263c635 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Tue, 27 Feb 2024 16:02:24 -0800 Subject: [PATCH 7/7] ca_test: more linter fixes --- ca/ca_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ca/ca_test.go b/ca/ca_test.go index 2f922cf8..17d03470 100644 --- a/ca/ca_test.go +++ b/ca/ca_test.go @@ -20,8 +20,8 @@ import ( ) var ( - ocspId asn1.ObjectIdentifier = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} - ocspValue = []byte{0x30, 0x03, 0x02, 0x01, 0x05} + ocspID = asn1.ObjectIdentifier{1, 3, 6, 1, 5, 5, 7, 1, 24} + ocspValue = []byte{0x30, 0x03, 0x02, 0x01, 0x05} ) func makeCa() *CAImpl { @@ -59,7 +59,7 @@ func makeCertOrderWithExtensions(extensions []pkix.Extension) core.Order { func getOCSPMustStapleExtension(cert *x509.Certificate) *pkix.Extension { for _, ext := range cert.Extensions { - if ext.Id.Equal(ocspId) && bytes.Equal(ext.Value, ocspValue) { + if ext.Id.Equal(ocspID) && bytes.Equal(ext.Value, ocspValue) { return &ext } } @@ -81,7 +81,7 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { ca := makeCa() order := makeCertOrderWithExtensions([]pkix.Extension{ { - Id: ocspId, + Id: ocspID, Critical: false, Value: ocspValue, }, @@ -98,7 +98,7 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { ca = makeCa() order = makeCertOrderWithExtensions([]pkix.Extension{ { - Id: ocspId, + Id: ocspID, Critical: true, Value: ocspValue, }, @@ -115,12 +115,12 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { ca = makeCa() order = makeCertOrderWithExtensions([]pkix.Extension{ { - Id: ocspId, + Id: ocspID, Critical: true, Value: ocspValue, }, { - Id: ocspId, + Id: ocspID, Critical: true, Value: ocspValue, }, @@ -128,7 +128,7 @@ func TestSettingOCSPMustStapleExtension(t *testing.T) { ca.CompleteOrder(&order) numOCSPMustStapleExtensions := 0 for _, ext := range order.CertificateObject.Cert.Extensions { - if ext.Id.Equal(ocspId) && bytes.Equal(ext.Value, ocspValue) { + if ext.Id.Equal(ocspID) && bytes.Equal(ext.Value, ocspValue) { numOCSPMustStapleExtensions++ } }