fixup! Configurable CRL

certificate-authority/service/grpc/signCertificate.go

@@ -103,11 +103,17 @@ func (s *CertificateAuthorityServer) updateSigningRecord(ctx context.Context, si
 		// revoke previous signing record
 		prevCred := prevSr.GetCredential()
 		if prevCred != nil {
-			_, err = s.store.RevokeCertificates(ctx, prevCred.GetIssuerId(), &store.RevocationListCertificate{
-				Serial:     prevCred.GetSerial(),
-				Expiration: prevCred.GetValidUntilDate(),
-				Revocation: time.Now().UnixNano(),
-			})
+			query := store.UpdateRevocationListQuery{
+				IssuerID: prevCred.GetIssuerId(),
+				RevokedCertificates: []*store.RevocationListCertificate{
+					{
+						Serial:     prevCred.GetSerial(),
+						ValidUntil: prevCred.GetValidUntilDate(),
+						Revocation: time.Now().UnixNano(),
+					},
+				},
+			}
+			_, err = s.store.UpdateRevocationList(ctx, &query)
 			if err != nil {
 				return err
 			}

certificate-authority/service/http/revocationList.go

@@ -1,10 +1,10 @@
 package http
 
 import (
+	"crypto"
 	"crypto/rand"
 	"crypto/x509"
 	"net/http"
-	"time"
 
 	"github.com/google/uuid"
 	"github.com/gorilla/mux"
@@ -21,40 +21,42 @@ func errCannotGetRevocationList(err error) error {
 	return pkgGrpc.ForwardErrorf(codes.Internal, "cannot get revocation list: %v", err)
 }
 
-func (requestHandler *RequestHandler) writeRevocationList(w http.ResponseWriter, r *http.Request) error {
-	vars := mux.Vars(r)
-	issuerID := vars[uri.IssuerIDKey]
-	if _, err := uuid.Parse(issuerID); err != nil {
-		return err
-	}
-	rl, err := requestHandler.store.GetRevocationList(r.Context(), issuerID, false)
-	if err != nil {
-		return err
-	}
+func createCRL(rl *store.RevocationList, issuer *x509.Certificate, priv crypto.Signer) ([]byte, error) {
 	number, err := store.ParseBigInt(rl.Number)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	now := time.Now()
-	signer := requestHandler.cas.GetSigner()
-	_, validFor := signer.GetCRLConfiguation()
 	template := &x509.RevocationList{
 		Number:     number,
-		ThisUpdate: pkgTime.Unix(0, rl.UpdatedAt),
-		NextUpdate: now.Add(validFor),
+		ThisUpdate: pkgTime.Unix(0, rl.IssuedAt),
+		NextUpdate: pkgTime.Unix(0, rl.ValidUntil),
 	}
 	for _, c := range rl.Certificates {
-		sn, err := store.ParseBigInt(c.Serial)
-		if err != nil {
-			return err
+		sn, errP := store.ParseBigInt(c.Serial)
+		if errP != nil {
+			return nil, errP
 		}
 		template.RevokedCertificateEntries = append(template.RevokedCertificateEntries, x509.RevocationListEntry{
 			SerialNumber:   sn,
 			RevocationTime: pkgTime.Unix(0, c.Revocation),
 		})
 	}
+	return x509.CreateRevocationList(rand.Reader, template, issuer, priv)
+}
 
-	crl, err := x509.CreateRevocationList(rand.Reader, template, signer.GetCertificate(), signer.GetPrivateKey())
+func (requestHandler *RequestHandler) writeRevocationList(w http.ResponseWriter, r *http.Request) error {
+	vars := mux.Vars(r)
+	issuerID := vars[uri.IssuerIDKey]
+	if _, err := uuid.Parse(issuerID); err != nil {
+		return err
+	}
+	signer := requestHandler.cas.GetSigner()
+	_, validFor := signer.GetCRLConfiguation()
+	rl, err := requestHandler.store.GetLatestIssuedOrIssueRevocationList(r.Context(), issuerID, validFor)
+	if err != nil {
+		return err
+	}
+	crl, err := createCRL(rl, signer.GetCertificate(), signer.GetPrivateKey())
 	if err != nil {
 		return err
 	}

certificate-authority/service/http/revocationList_test.go

@@ -77,13 +77,6 @@ func TestRevocationList(t *testing.T) {
 			},
 			wantErr: true,
 		},
-		{
-			name: "all certificates expired",
-			args: args{
-				issuer: test.GetIssuerID(0),
-			},
-			wantErr: true,
-		},
 		{
 			name: "valid",
 			args: args{

certificate-authority/store/cqldb/revocationList.go

@@ -2,6 +2,7 @@ package cqldb
 
 import (
 	"context"
+	"time"
 
 	"github.com/plgd-dev/hub/v2/certificate-authority/store"
 )
@@ -14,10 +15,10 @@ func (s *Store) InsertRevocationLists(context.Context, ...*store.RevocationList)
 	return store.ErrNotSupported
 }
 
-func (s *Store) RevokeCertificates(context.Context, string, ...*store.RevocationListCertificate) ([]string, error) {
+func (s *Store) UpdateRevocationList(context.Context, *store.UpdateRevocationListQuery) (*store.RevocationList, error) {
 	return nil, store.ErrNotSupported
 }
 
-func (s *Store) GetRevocationList(context.Context, string, bool) (*store.RevocationList, error) {
+func (s *Store) GetLatestIssuedOrIssueRevocationList(context.Context, string, time.Duration) (*store.RevocationList, error) {
 	return nil, store.ErrNotSupported
 }

certificate-authority/store/mongodb/revocationList.go

@@ -3,11 +3,9 @@ package mongodb
 import (
 	"context"
 	"errors"
-	"fmt"
 	"math/big"
 	"time"
 
-	"github.com/google/uuid"
 	"github.com/plgd-dev/hub/v2/certificate-authority/store"
 	"github.com/plgd-dev/hub/v2/pkg/mongodb"
 	"go.mongodb.org/mongo-driver/bson"
@@ -34,84 +32,127 @@ func (s *Store) InsertRevocationLists(ctx context.Context, rls ...*store.Revocat
 	return err
 }
 
+type revocationListUpdate struct {
+	originalRevocationList *store.RevocationList
+	certificatesToInsert   map[string]*store.RevocationListCertificate
+}
+
 // check the database and remove serials that are already in the array
-func (s *Store) getUniqueCertificates(ctx context.Context, issuerID string, certificates []*store.RevocationListCertificate) (*big.Int, map[string]*store.RevocationListCertificate, error) {
+func (s *Store) getRevocationListUpdate(ctx context.Context, query *store.UpdateRevocationListQuery) (revocationListUpdate, bool, error) {
 	cmap := make(map[string]*store.RevocationListCertificate)
-	for _, cert := range certificates {
+	for _, cert := range query.RevokedCertificates {
 		if _, ok := cmap[cert.Serial]; ok {
-			return nil, nil, fmt.Errorf("duplicate serial(%v)", cert.Serial)
+			s.logger.Debugf("skipping duplicate serial number(%v) in query", cert.Serial)
+			continue
 		}
 		if err := cert.Validate(); err != nil {
-			return nil, nil, err
+			return revocationListUpdate{}, false, err
 		}
 		cmap[cert.Serial] = cert
 	}
-	serials := maps.Keys(cmap)
-	filter := bson.M{
-		"_id": issuerID,
+	pl := mongo.Pipeline{
+		bson.D{{Key: mongodb.Match, Value: bson.D{{Key: "_id", Value: query.IssuerID}}}},
 	}
-	projection := bson.M{
-		store.NumberKey: 1,
-		store.CertificatesKey: bson.M{
-			"$filter": bson.M{
-				"input": "$" + store.CertificatesKey,
-				"as":    "cert",
-				"cond":  bson.M{mongodb.In: bson.A{"$$cert." + store.SerialKey, serials}},
+	if len(cmap) > 0 {
+		pl = append(pl, bson.D{{Key: "$addFields", Value: bson.M{
+			"duplicates": bson.M{
+				"$filter": bson.M{
+					"input": "$" + store.CertificatesKey,
+					"as":    "cert",
+					"cond":  bson.M{mongodb.In: bson.A{"$$cert." + store.SerialKey, maps.Keys(cmap)}},
+				},
 			},
-		},
+		}}})
 	}
-	var rl store.RevocationList
-	err := s.Collection(revocationListCol).FindOne(ctx, filter, options.FindOne().SetProjection(projection)).Decode(&rl)
+	cur, err := s.Collection(revocationListCol).Aggregate(ctx, pl)
 	if err != nil {
-		if errors.Is(err, mongo.ErrNoDocuments) {
-			return big.NewInt(0), cmap, nil
-		}
-		return nil, nil, err
+		return revocationListUpdate{}, false, err
 	}
-	number, err := store.ParseBigInt(rl.Number)
+	type revocationListWithNewCertificates struct {
+		*store.RevocationList `bson:",inline"`
+		Duplicates            []*store.RevocationListCertificate `bson:"duplicates,omitempty"`
+	}
+	var rl *revocationListWithNewCertificates
+	count, err := processCursor(ctx, cur, func(item *revocationListWithNewCertificates) error {
+		rl = item
+		return nil
+	})
 	if err != nil {
-		return nil, nil, err
+		return revocationListUpdate{}, false, err
+	}
+	if count == 0 {
+		return revocationListUpdate{
+			certificatesToInsert: cmap,
+		}, true, nil
 	}
-	for _, c := range rl.Certificates {
+	for _, c := range rl.Duplicates {
 		s.logger.Debugf("skipping duplicate serial number(%v)", c.Serial)
 		delete(cmap, c.Serial)
 	}
-	return number, cmap, nil
+	if len(cmap) == 0 && (!query.UpdateIfExpired || !rl.IsExpired()) {
+		return revocationListUpdate{
+			originalRevocationList: rl.RevocationList,
+		}, false, nil
+	}
+	return revocationListUpdate{
+		originalRevocationList: rl.RevocationList,
+		certificatesToInsert:   cmap,
+	}, true, nil
 }
 
-func (s *Store) RevokeCertificates(ctx context.Context, issuerID string, rl ...*store.RevocationListCertificate) ([]string, error) {
-	if _, err := uuid.Parse(issuerID); err != nil {
-		return nil, fmt.Errorf("invalid revocation list ID: %w", err)
+func (s *Store) UpdateRevocationList(ctx context.Context, query *store.UpdateRevocationListQuery) (*store.RevocationList, error) {
+	if err := query.Validate(); err != nil {
+		return nil, err
 	}
-	number, certificates, err := s.getUniqueCertificates(ctx, issuerID, rl)
+	upd, needsUpdate, err := s.getRevocationListUpdate(ctx, query)
 	if err != nil {
 		return nil, err
 	}
-	if len(certificates) == 0 {
-		return []string(nil), nil
+	if !needsUpdate {
+		return upd.originalRevocationList, nil
 	}
-	filter := bson.M{
-		"_id": issuerID,
+
+	if upd.originalRevocationList == nil {
+		newRL := &store.RevocationList{
+			Id:           query.IssuerID,
+			Number:       "1", // the sequence for the CRL number field starts from 1
+			IssuedAt:     query.IssuedAt,
+			ValidUntil:   query.ValidUntil,
+			Certificates: maps.Values(upd.certificatesToInsert),
+		}
+		if err = s.InsertRevocationLists(ctx, newRL); err != nil {
+			return nil, err
+		}
+		return newRL, nil
 	}
-	now := time.Now()
-	// increment by 1, the sequence for the CRL number field should start from 1
-	number.Add(number, big.NewInt(1))
-	set := bson.M{
-		store.UpdatedAtKey: now.UnixNano(),
-		store.NumberKey:    number.String(),
+
+	number, err := store.ParseBigInt(upd.originalRevocationList.Number)
+	if err != nil {
+		return nil, err
+	}
+	filter := bson.M{
+		"_id":           query.IssuerID,
+		store.NumberKey: number.String(),
 	}
+	nextNumber := number.Add(number, big.NewInt(1))
 	update := bson.M{
-		"$push": bson.M{
-			store.CertificatesKey: bson.M{"$each": maps.Values(certificates)},
-		},
-		"$set": set,
-		"$setOnInsert": bson.M{
-			"_id": issuerID,
+		"$set": bson.M{
+			store.NumberKey:     nextNumber.String(),
+			store.IssuedAtKey:   query.IssuedAt,
+			store.ValidUntilKey: query.ValidUntil,
 		},
 	}
-	opts := options.Update().SetUpsert(true)
-	_, err = s.Collection(revocationListCol).UpdateOne(ctx, filter, update, opts)
-	return maps.Keys(certificates), err
+	if len(upd.certificatesToInsert) > 0 {
+		update["$push"] = bson.M{
+			store.CertificatesKey: bson.M{"$each": maps.Values(upd.certificatesToInsert)},
+		}
+	}
+	opts := options.FindOneAndUpdate().SetReturnDocument(options.After)
+	var updatedRL store.RevocationList
+	if err = s.Collection(revocationListCol).FindOneAndUpdate(ctx, filter, update, opts).Decode(&updatedRL); err != nil {
+		return nil, err
+	}
+	return &updatedRL, nil
 }
 
 func (s *Store) GetRevocationList(ctx context.Context, issuerID string, includeExpired bool) (*store.RevocationList, error) {
@@ -123,21 +164,20 @@ func (s *Store) GetRevocationList(ctx context.Context, issuerID string, includeE
 	if !includeExpired {
 		filter[store.CertificatesKey] = bson.M{
 			"$elemMatch": bson.M{
-				// non-expired certificates
-				store.ExpirationKey: bson.M{"$gte": now},
+				store.ValidUntilKey: bson.M{"$gte": now}, // non-expired certificates
 			},
 		}
 		projection := bson.M{
-			"_id":              1,
-			store.NumberKey:    1,
-			store.UpdatedAtKey: 1,
+			"_id":               1,
+			store.NumberKey:     1,
+			store.IssuedAtKey:   1,
+			store.ValidUntilKey: 1,
 			store.CertificatesKey: bson.M{
 				"$filter": bson.M{
 					"input": "$" + store.CertificatesKey,
 					"as":    "cert",
 					"cond": bson.M{
-						// non-expired certificates
-						"$gte": []interface{}{"$$cert." + store.ExpirationKey, now},
+						"$gte": []interface{}{"$$cert." + store.ValidUntilKey, now}, // non-expired certificates
 					},
 				},
 			},
@@ -155,3 +195,21 @@ func (s *Store) GetRevocationList(ctx context.Context, issuerID string, includeE
 	}
 	return &rl, nil
 }
+
+func (s *Store) GetLatestIssuedOrIssueRevocationList(ctx context.Context, issuerID string, validFor time.Duration) (*store.RevocationList, error) {
+	rl, err := s.GetRevocationList(ctx, issuerID, true)
+	if err != nil {
+		return nil, err
+	}
+	if rl.IssuedAt > 0 && !rl.IsExpired() {
+		return rl, nil
+	}
+	issuedAt := time.Now()
+	validUntil := issuedAt.Add(validFor)
+	return s.UpdateRevocationList(ctx, &store.UpdateRevocationListQuery{
+		IssuerID:        issuerID,
+		IssuedAt:        issuedAt.UnixNano(),
+		ValidUntil:      validUntil.UnixNano(),
+		UpdateIfExpired: true,
+	})
+}