fixup! Implement Certificate Revocation List

Author: Danielius1922

certificate-authority/service/grpc/signer.go

@@ -25,7 +25,7 @@ type Signer struct {
 	hubID       string
 	crl         struct {
 		serverAddress string
-		validFor      time.Duration // TODO: schedule the CRL to be regenerated -> increment the Number, ThisUpdate and NextUpdate fields
+		validFor      time.Duration
 	}
 }
 

certificate-authority/service/http/requestHandler.go

@@ -13,27 +13,24 @@ import (
 	"github.com/plgd-dev/hub/v2/certificate-authority/service/uri"
 	"github.com/plgd-dev/hub/v2/certificate-authority/store"
 	"github.com/plgd-dev/hub/v2/http-gateway/serverMux"
-	"github.com/plgd-dev/hub/v2/pkg/log"
 )
 
 // RequestHandler for handling incoming request
 type RequestHandler struct {
 	config *Config
 	mux    *runtime.ServeMux
 
-	cas    *grpcService.CertificateAuthorityServer
-	store  store.Store
-	logger log.Logger
+	cas   *grpcService.CertificateAuthorityServer
+	store store.Store
 }
 
 // NewHTTP returns HTTP handler
-func NewRequestHandler(config *Config, r *mux.Router, cas *grpcService.CertificateAuthorityServer, s store.Store, logger log.Logger) (*RequestHandler, error) {
+func NewRequestHandler(config *Config, r *mux.Router, cas *grpcService.CertificateAuthorityServer, s store.Store) (*RequestHandler, error) {
 	requestHandler := &RequestHandler{
 		config: config,
 		mux:    serverMux.New(),
 		cas:    cas,
 		store:  s,
-		logger: logger,
 	}
 
 	if config.CRLEnabled {

certificate-authority/service/http/service.go

@@ -49,7 +49,7 @@ func New(serviceName string, config Config, s store.Store, ca *grpcService.Certi
 		return nil, fmt.Errorf("cannot create http service: %w", err)
 	}
 
-	requestHandler, err := NewRequestHandler(&config, service.GetRouter(), ca, s, logger)
+	requestHandler, err := NewRequestHandler(&config, service.GetRouter(), ca, s)
 	if err != nil {
 		_ = service.Close()
 		return nil, err

certificate-authority/store/mongodb/revocationList.go

@@ -3,6 +3,7 @@ package mongodb
 import (
 	"context"
 	"errors"
+	"fmt"
 	"math/big"
 	"time"
 
@@ -121,6 +122,9 @@ func (s *Store) UpdateRevocationList(ctx context.Context, query *store.UpdateRev
 			Certificates: maps.Values(upd.certificatesToInsert),
 		}
 		if err = s.InsertRevocationLists(ctx, newRL); err != nil {
+			if mongo.IsDuplicateKeyError(err) {
+				return nil, fmt.Errorf("%w: %w", store.ErrDuplicateID, err)
+			}
 			return nil, err
 		}
 		return newRL, nil
@@ -157,7 +161,7 @@ func (s *Store) UpdateRevocationList(ctx context.Context, query *store.UpdateRev
 	var updatedRL store.RevocationList
 	if err = s.Collection(revocationListCol).FindOneAndUpdate(ctx, filter, update, opts).Decode(&updatedRL); err != nil {
 		if errors.Is(err, mongo.ErrNoDocuments) {
-			return nil, store.ErrNotFound
+			return nil, fmt.Errorf("%w: %w", store.ErrNotFound, err)
 		}
 		return nil, err
 	}

certificate-authority/store/mongodb/revocationList_test.go

@@ -2,13 +2,18 @@ package mongodb_test
 
 import (
 	"context"
+	"errors"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/google/uuid"
 	"github.com/plgd-dev/hub/v2/certificate-authority/store"
 	"github.com/plgd-dev/hub/v2/certificate-authority/test"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
 )
 
 func TestUpdateRevocationList(t *testing.T) {
@@ -179,8 +184,64 @@ func TestUpdateRevocationList(t *testing.T) {
 	}
 }
 
-func TestParallelUpdateRevocationList(*testing.T) {
-	// TODO: run 10 parallel updates, they all should eventually succeed
+func TestParallelUpdateRevocationList(t *testing.T) {
+	s, cleanUpStore := test.NewMongoStore(t)
+	defer cleanUpStore()
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
+	defer cancel()
+
+	issuerID := uuid.NewString()
+	count := 10
+	certificates := make(map[int]*store.RevocationListCertificate)
+	for i := range count {
+		certificates[i] = test.GetCertificate(i, time.Now(), time.Now().Add(time.Hour))
+	}
+
+	var failed atomic.Bool
+	failed.Store(false)
+	var wg sync.WaitGroup
+	wg.Add(10)
+	for i := range count {
+		go func(index int) {
+			defer wg.Done()
+			cert, ok := certificates[index]
+			assert.True(t, ok)
+			var err error
+			for range 100 { // parallel execution should eventually succeed in cases when we get duplicate _id and
+				q := &store.UpdateRevocationListQuery{
+					IssuerID:            issuerID,
+					RevokedCertificates: []*store.RevocationListCertificate{cert},
+				}
+				_, err = s.UpdateRevocationList(ctx, q)
+				if errors.Is(err, store.ErrDuplicateID) || errors.Is(err, store.ErrNotFound) {
+					continue
+				}
+				if err == nil {
+					break
+				}
+				failed.Store(true)
+				assert.NoError(t, err)
+			}
+			assert.NoError(t, err)
+		}(i)
+	}
+	wg.Wait()
+	require.False(t, failed.Load())
+
+	rl, err := s.GetLatestIssuedOrIssueRevocationList(ctx, issuerID, time.Hour)
+	require.NoError(t, err)
+
+	require.NotEmpty(t, rl.IssuedAt)
+	require.NotEmpty(t, rl.ValidUntil)
+	expected := &store.RevocationList{
+		Id:           issuerID,
+		Number:       "1",
+		IssuedAt:     rl.IssuedAt,
+		ValidUntil:   rl.ValidUntil,
+		Certificates: maps.Values(certificates),
+	}
+	test.CheckRevocationList(t, expected, rl, true)
 }
 
 func TestGetRevocationList(t *testing.T) {

certificate-authority/store/mongodb/signingRecords.go

@@ -11,6 +11,7 @@ import (
 	"go.mongodb.org/mongo-driver/bson"
 	"go.mongodb.org/mongo-driver/mongo"
 	"go.mongodb.org/mongo-driver/mongo/options"
+	"golang.org/x/exp/maps"
 )
 
 const signingRecordsCol = "signedCertificateRecords"
@@ -124,7 +125,7 @@ func (s *Store) RevokeSigningRecords(ctx context.Context, ownerID string, query
 		ids          []string
 		certificates []*store.RevocationListCertificate
 	}
-	idFilter := []string{}
+	idFilter := make(map[string]struct{})
 	irs := make(map[string]issuersRecord)
 	err := s.LoadSigningRecords(ctx, ownerID, &pb.GetSigningRecordsRequest{
 		IdFilter:       query.GetIdFilter(),
@@ -134,8 +135,8 @@ func (s *Store) RevokeSigningRecords(ctx context.Context, ownerID string, query
 		if credential == nil {
 			return nil
 		}
+		idFilter[v.GetId()] = struct{}{}
 		if credential.GetValidUntilDate() <= now {
-			idFilter = append(idFilter, v.GetId())
 			return nil
 		}
 		record := irs[credential.GetIssuerId()]
@@ -162,8 +163,6 @@ func (s *Store) RevokeSigningRecords(ctx context.Context, ownerID string, query
 		if err != nil {
 			return 0, err
 		}
-		// TODO
-		// idFilter = append(idFilter, serials...)
 	}
 
 	if len(idFilter) == 0 {
@@ -172,7 +171,7 @@ func (s *Store) RevokeSigningRecords(ctx context.Context, ownerID string, query
 
 	// delete the signing records
 	return s.DeleteSigningRecords(ctx, ownerID, &pb.DeleteSigningRecordsRequest{
-		IdFilter: idFilter,
+		IdFilter: maps.Keys(idFilter),
 	})
 }
 

certificate-authority/store/store.go

@@ -13,6 +13,7 @@ import (
 var (
 	ErrNotSupported = errors.New("not supported")
 	ErrNotFound     = errors.New("no document found")
+	ErrDuplicateID  = errors.New("duplicate ID")
 )
 
 type (

certificate-authority/test/revocationList.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"sort"
 	"strconv"
 	"testing"
 	"time"
@@ -71,6 +72,12 @@ func CheckRevocationList(t *testing.T, expected, actual *store.RevocationList, i
 	require.Equal(t, expected.IssuedAt, actual.IssuedAt)
 	require.Equal(t, expected.ValidUntil, actual.ValidUntil)
 	require.Len(t, actual.Certificates, len(expected.Certificates))
+	sort.Slice(actual.Certificates, func(i, j int) bool {
+		return actual.Certificates[i].Serial < actual.Certificates[j].Serial
+	})
+	sort.Slice(expected.Certificates, func(i, j int) bool {
+		return expected.Certificates[i].Serial < expected.Certificates[j].Serial
+	})
 	for i := range actual.Certificates {
 		require.Equal(t, expected.Certificates[i].Serial, actual.Certificates[i].Serial)
 		require.Equal(t, expected.Certificates[i].ValidUntil, actual.Certificates[i].ValidUntil)