diff --git a/cmd/ceremony/crl.go b/cmd/ceremony/crl.go index 65d00a7b6..98790d906 100644 --- a/cmd/ceremony/crl.go +++ b/cmd/ceremony/crl.go @@ -3,14 +3,13 @@ package main import ( "crypto" "crypto/x509" - "crypto/x509/pkix" - "encoding/asn1" "encoding/pem" "errors" "fmt" "math/big" "time" + "github.com/letsencrypt/boulder/crl/idp" "github.com/letsencrypt/boulder/linter" ) @@ -37,7 +36,7 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex return nil, errors.New("nextUpdate must be less than 12 months after thisUpdate") } // Add the Issuing Distribution Point extension. - idp, err := makeIDPExt() + idp, err := idp.MakeCACertsExt() if err != nil { return nil, fmt.Errorf("creating IDP extension: %w", err) } @@ -60,30 +59,3 @@ func generateCRL(signer crypto.Signer, issuer *x509.Certificate, thisUpdate, nex return pem.EncodeToMemory(&pem.Block{Type: "X509 CRL", Bytes: crlBytes}), nil } - -// issuingDistributionPoint represents the ASN.1 IssuingDistributionPoint -// SEQUENCE as defined in RFC 5280 Section 5.2.5. We only use one of the fields, -// all others are omitted. -// https://datatracker.ietf.org/doc/html/rfc5280#page-66 -type issuingDistributionPoint struct { - OnlyContainsCACerts bool `asn1:"optional,tag:2"` -} - -// makeIDPExt returns a critical IssuingDistributionPoint extension enabling the -// OnlyContainsCACerts boolean. -func makeIDPExt() (*pkix.Extension, error) { - val := issuingDistributionPoint{ - OnlyContainsCACerts: true, - } - - valBytes, err := asn1.Marshal(val) - if err != nil { - return nil, err - } - - return &pkix.Extension{ - Id: asn1.ObjectIdentifier{2, 5, 29, 28}, // id-ce-issuingDistributionPoint - Value: valBytes, - Critical: true, - }, nil -} diff --git a/crl/idp/idp.go b/crl/idp/idp.go new file mode 100644 index 000000000..054096288 --- /dev/null +++ b/crl/idp/idp.go @@ -0,0 +1,102 @@ +package idp + +import ( + "crypto/x509/pkix" + "encoding/asn1" + "errors" + "fmt" +) + +var idpOID = asn1.ObjectIdentifier{2, 5, 29, 28} // id-ce-issuingDistributionPoint + +// issuingDistributionPoint represents the ASN.1 IssuingDistributionPoint +// SEQUENCE as defined in RFC 5280 Section 5.2.5. We only use three of the +// fields, so the others are omitted. +type issuingDistributionPoint struct { + DistributionPoint distributionPointName `asn1:"optional,tag:0"` + OnlyContainsUserCerts bool `asn1:"optional,tag:1"` + OnlyContainsCACerts bool `asn1:"optional,tag:2"` +} + +// distributionPointName represents the ASN.1 DistributionPointName CHOICE as +// defined in RFC 5280 Section 4.2.1.13. We only use one of the fields, so the +// others are omitted. +type distributionPointName struct { + // Technically, FullName is of type GeneralNames, which is of type SEQUENCE OF + // GeneralName. But GeneralName itself is of type CHOICE, and the ans1.Marhsal + // function doesn't support marshalling structs to CHOICEs, so we have to use + // asn1.RawValue and encode the GeneralName ourselves. + FullName []asn1.RawValue `asn1:"optional,tag:0"` +} + +// MakeUserCertsExt returns a critical IssuingDistributionPoint extension +// containing the given URLs and with the OnlyContainsUserCerts boolean set to +// true. +func MakeUserCertsExt(urls []string) (pkix.Extension, error) { + var gns []asn1.RawValue + for _, url := range urls { + gns = append(gns, asn1.RawValue{ // GeneralName + Class: 2, // context-specific + Tag: 6, // uniformResourceIdentifier, IA5String + Bytes: []byte(url), + }) + } + + val := issuingDistributionPoint{ + DistributionPoint: distributionPointName{FullName: gns}, + OnlyContainsUserCerts: true, + } + + valBytes, err := asn1.Marshal(val) + if err != nil { + return pkix.Extension{}, err + } + + return pkix.Extension{ + Id: idpOID, + Value: valBytes, + Critical: true, + }, nil +} + +// MakeCACertsExt returns a critical IssuingDistributionPoint extension +// asserting the OnlyContainsCACerts boolean. +func MakeCACertsExt() (*pkix.Extension, error) { + val := issuingDistributionPoint{ + OnlyContainsCACerts: true, + } + + valBytes, err := asn1.Marshal(val) + if err != nil { + return nil, err + } + + return &pkix.Extension{ + Id: idpOID, + Value: valBytes, + Critical: true, + }, nil +} + +// GetIDPURIs returns the URIs contained within the issuingDistributionPoint +// extension, if present, or an error otherwise. +func GetIDPURIs(exts []pkix.Extension) ([]string, error) { + for _, ext := range exts { + if ext.Id.Equal(idpOID) { + val := issuingDistributionPoint{} + rest, err := asn1.Unmarshal(ext.Value, &val) + if err != nil { + return nil, fmt.Errorf("parsing IssuingDistributionPoint extension: %w", err) + } + if len(rest) != 0 { + return nil, fmt.Errorf("parsing IssuingDistributionPoint extension: got %d unexpected trailing bytes", len(rest)) + } + var uris []string + for _, generalName := range val.DistributionPoint.FullName { + uris = append(uris, string(generalName.Bytes)) + } + return uris, nil + } + } + return nil, errors.New("no IssuingDistributionPoint extension found") +} diff --git a/crl/idp/idp_test.go b/crl/idp/idp_test.go new file mode 100644 index 000000000..a142a5913 --- /dev/null +++ b/crl/idp/idp_test.go @@ -0,0 +1,40 @@ +package idp + +import ( + "encoding/hex" + "testing" + + "github.com/letsencrypt/boulder/test" +) + +func TestMakeUserCertsExt(t *testing.T) { + t.Parallel() + dehex := func(s string) []byte { r, _ := hex.DecodeString(s); return r } + tests := []struct { + name string + urls []string + want []byte + }{ + { + name: "one (real) url", + urls: []string{"http://prod.c.lencr.org/20506757847264211/126.crl"}, + want: dehex("303AA035A0338631687474703A2F2F70726F642E632E6C656E63722E6F72672F32303530363735373834373236343231312F3132362E63726C8101FF"), + }, + { + name: "two urls", + urls: []string{"http://old.style/12345678/90.crl", "http://new.style/90.crl"}, + want: dehex("3042A03DA03B8620687474703A2F2F6F6C642E7374796C652F31323334353637382F39302E63726C8617687474703A2F2F6E65772E7374796C652F39302E63726C8101FF"), + }, + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, err := MakeUserCertsExt(tc.urls) + test.AssertNotError(t, err, "should never fail to marshal asn1 to bytes") + test.AssertDeepEquals(t, got.Id, idpOID) + test.AssertEquals(t, got.Critical, true) + test.AssertDeepEquals(t, got.Value, tc.want) + }) + } +} diff --git a/crl/storer/storer.go b/crl/storer/storer.go index 296852415..10b1753c7 100644 --- a/crl/storer/storer.go +++ b/crl/storer/storer.go @@ -5,13 +5,12 @@ import ( "context" "crypto/sha256" "crypto/x509" - "crypto/x509/pkix" - "encoding/asn1" "encoding/base64" "errors" "fmt" "io" "math/big" + "slices" "time" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -22,6 +21,7 @@ import ( "google.golang.org/protobuf/types/known/emptypb" "github.com/letsencrypt/boulder/crl" + "github.com/letsencrypt/boulder/crl/idp" cspb "github.com/letsencrypt/boulder/crl/storer/proto" "github.com/letsencrypt/boulder/issuance" blog "github.com/letsencrypt/boulder/log" @@ -186,15 +186,30 @@ func (cs *crlStorer) UploadCRL(stream cspb.CRLStorer_UploadCRLServer) error { return fmt.Errorf("parsing previous CRL for %s: %w", crlId, err) } - idp := getIDPExt(crl.Extensions) - prevIdp := getIDPExt(prevCRL.Extensions) - if !bytes.Equal(idp, prevIdp) { - return fmt.Errorf("IDP does not match previous: %x != %x", idp, prevIdp) - } - if crl.Number.Cmp(prevCRL.Number) <= 0 { return fmt.Errorf("crlNumber not strictly increasing: %d <= %d", crl.Number, prevCRL.Number) } + + idpURIs, err := idp.GetIDPURIs(crl.Extensions) + if err != nil { + return fmt.Errorf("getting IDP for %s: %w", crlId, err) + } + + prevURIs, err := idp.GetIDPURIs(prevCRL.Extensions) + if err != nil { + return fmt.Errorf("getting previous IDP for %s: %w", crlId, err) + } + + uriMatch := false + for _, uri := range idpURIs { + if slices.Contains(prevURIs, uri) { + uriMatch = true + break + } + } + if !uriMatch { + return fmt.Errorf("IDP does not match previous: %v !∩ %v", idpURIs, prevURIs) + } } // Finally actually upload the new CRL. @@ -230,13 +245,3 @@ func (cs *crlStorer) UploadCRL(stream cspb.CRLStorer_UploadCRLServer) error { return stream.SendAndClose(&emptypb.Empty{}) } - -// getIDPExt returns the contents of the issuingDistributionPoint extension, if present. -func getIDPExt(exts []pkix.Extension) []byte { - for _, ext := range exts { - if ext.Id.Equal(asn1.ObjectIdentifier{2, 5, 29, 28}) { // id-ce-issuingDistributionPoint - return ext.Value - } - } - return nil -} diff --git a/crl/storer/storer_test.go b/crl/storer/storer_test.go index a07d6cc12..0c80dfe4b 100644 --- a/crl/storer/storer_test.go +++ b/crl/storer/storer_test.go @@ -7,6 +7,7 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/x509" + "crypto/x509/pkix" "errors" "io" "math/big" @@ -20,6 +21,7 @@ import ( "google.golang.org/grpc" "google.golang.org/protobuf/types/known/emptypb" + "github.com/letsencrypt/boulder/crl/idp" cspb "github.com/letsencrypt/boulder/crl/storer/proto" "github.com/letsencrypt/boulder/issuance" blog "github.com/letsencrypt/boulder/log" @@ -307,6 +309,9 @@ func TestUploadCRLSuccess(t *testing.T) { storer, iss := setupTestUploadCRL(t) errs := make(chan error, 1) + idpExt, err := idp.MakeUserCertsExt([]string{"http://c.ex.org"}) + test.AssertNotError(t, err, "creating test IDP extension") + ins := make(chan *cspb.UploadCRLRequest) go func() { errs <- storer.UploadCRL(&fakeUploadCRLServerStream{input: ins}) @@ -329,6 +334,7 @@ func TestUploadCRLSuccess(t *testing.T) { RevokedCertificateEntries: []x509.RevocationListEntry{ {SerialNumber: big.NewInt(123), RevocationTime: time.Now().Add(-time.Hour)}, }, + ExtraExtensions: []pkix.Extension{idpExt}, }, iss.Cert.Certificate, iss.Signer, @@ -346,6 +352,7 @@ func TestUploadCRLSuccess(t *testing.T) { RevokedCertificateEntries: []x509.RevocationListEntry{ {SerialNumber: big.NewInt(123), RevocationTime: time.Now().Add(-time.Hour)}, }, + ExtraExtensions: []pkix.Extension{idpExt}, }, iss.Cert.Certificate, iss.Signer, diff --git a/issuance/crl.go b/issuance/crl.go index 2f36d695c..c962bb03f 100644 --- a/issuance/crl.go +++ b/issuance/crl.go @@ -3,8 +3,6 @@ package issuance import ( "crypto/rand" "crypto/x509" - "crypto/x509/pkix" - "encoding/asn1" "fmt" "math/big" "time" @@ -12,6 +10,7 @@ import ( "github.com/zmap/zlint/v3/lint" "github.com/letsencrypt/boulder/config" + "github.com/letsencrypt/boulder/crl/idp" "github.com/letsencrypt/boulder/linter" ) @@ -92,7 +91,7 @@ func (i *Issuer) IssueCRL(prof *CRLProfile, req *CRLRequest) ([]byte, error) { // contain the new-style CRLDP URL instead. idps = append(idps, fmt.Sprintf("%s/%d/%d.crl", req.DeprecatedIDPBaseURL, i.NameID(), req.Shard)) } - idp, err := makeIDPExt(idps) + idp, err := idp.MakeUserCertsExt(idps) if err != nil { return nil, fmt.Errorf("creating IDP extension: %w", err) } @@ -115,51 +114,3 @@ func (i *Issuer) IssueCRL(prof *CRLProfile, req *CRLRequest) ([]byte, error) { return crlBytes, nil } - -// distributionPointName represents the ASN.1 DistributionPointName CHOICE as -// defined in RFC 5280 Section 4.2.1.13. We only use one of the fields, so the -// others are omitted. -type distributionPointName struct { - // Technically, FullName is of type GeneralNames, which is of type SEQUENCE OF - // GeneralName. But GeneralName itself is of type CHOICE, and the ans1.Marhsal - // function doesn't support marshalling structs to CHOICEs, so we have to use - // asn1.RawValue and encode the GeneralName ourselves. - FullName []asn1.RawValue `asn1:"optional,tag:0"` -} - -// issuingDistributionPoint represents the ASN.1 IssuingDistributionPoint -// SEQUENCE as defined in RFC 5280 Section 5.2.5. We only use two of the fields, -// so the others are omitted. -type issuingDistributionPoint struct { - DistributionPoint distributionPointName `asn1:"optional,tag:0"` - OnlyContainsUserCerts bool `asn1:"optional,tag:1"` -} - -// makeIDPExt returns a critical IssuingDistributionPoint extension containing -// the given URLs and with the OnlyContainsUserCerts boolean set to true. -func makeIDPExt(urls []string) (pkix.Extension, error) { - var gns []asn1.RawValue - for _, url := range urls { - gns = append(gns, asn1.RawValue{ // GeneralName - Class: 2, // context-specific - Tag: 6, // uniformResourceIdentifier, IA5String - Bytes: []byte(url), - }) - } - - val := issuingDistributionPoint{ - DistributionPoint: distributionPointName{FullName: gns}, - OnlyContainsUserCerts: true, - } - - valBytes, err := asn1.Marshal(val) - if err != nil { - return pkix.Extension{}, err - } - - return pkix.Extension{ - Id: asn1.ObjectIdentifier{2, 5, 29, 28}, // id-ce-issuingDistributionPoint - Value: valBytes, - Critical: true, - }, nil -} diff --git a/issuance/crl_test.go b/issuance/crl_test.go index 2f56d8b68..adc5e089f 100644 --- a/issuance/crl_test.go +++ b/issuance/crl_test.go @@ -2,8 +2,6 @@ package issuance import ( "crypto/x509" - "encoding/asn1" - "encoding/hex" "math/big" "testing" "time" @@ -148,35 +146,3 @@ func TestIssueCRL(t *testing.T) { test.AssertError(t, err, "crl issuance with no IDP should fail") test.AssertContains(t, err.Error(), "must contain an issuingDistributionPoint") } - -func TestMakeIDPExt(t *testing.T) { - t.Parallel() - dehex := func(s string) []byte { r, _ := hex.DecodeString(s); return r } - tests := []struct { - name string - urls []string - want []byte - }{ - { - name: "one (real) url", - urls: []string{"http://prod.c.lencr.org/20506757847264211/126.crl"}, - want: dehex("303AA035A0338631687474703A2F2F70726F642E632E6C656E63722E6F72672F32303530363735373834373236343231312F3132362E63726C8101FF"), - }, - { - name: "two urls", - urls: []string{"http://old.style/12345678/90.crl", "http://new.style/90.crl"}, - want: dehex("3042A03DA03B8620687474703A2F2F6F6C642E7374796C652F31323334353637382F39302E63726C8617687474703A2F2F6E65772E7374796C652F39302E63726C8101FF"), - }, - } - for _, tc := range tests { - tc := tc - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - got, err := makeIDPExt(tc.urls) - test.AssertNotError(t, err, "should never fail to marshal asn1 to bytes") - test.AssertDeepEquals(t, got.Id, asn1.ObjectIdentifier{2, 5, 29, 28}) - test.AssertEquals(t, got.Critical, true) - test.AssertDeepEquals(t, got.Value, tc.want) - }) - } -}