fixup! Implement Certificate Revocation List

certificate-authority/service/grpc/server.go

@@ -2,7 +2,10 @@ package grpc
 
 import (
 	"bytes"
+	"context"
+	"errors"
 	"fmt"
+	"time"
 
 	"github.com/plgd-dev/hub/v2/certificate-authority/pb"
 	"github.com/plgd-dev/hub/v2/certificate-authority/store"
@@ -84,6 +87,24 @@ func NewCertificateAuthorityServer(ownerClaim, hubID, crlServerAddress string, s
 	return s, nil
 }
 
+func (s *CertificateAuthorityServer) initStore(issuerID string) error {
+	if s.store.SupportsRevocationList() {
+		ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+		defer cancel()
+		err := s.store.InsertRevocationLists(ctx, &store.RevocationList{
+			Id:     issuerID,
+			Number: "1",
+		})
+		if errors.Is(err, store.ErrDuplicateID) {
+			return nil
+		}
+		if err != nil {
+			return fmt.Errorf("failed to create revocation list: %w", err)
+		}
+	}
+	return nil
+}
+
 func (s *CertificateAuthorityServer) Close() {
 	for _, ca := range s.signerConfig.caPoolArray {
 		if !ca.IsFile() {
@@ -115,6 +136,9 @@ func (s *CertificateAuthorityServer) load() (bool, error) {
 	if oldSigner != nil && len(signer.certificate) == len(oldSigner.certificate) && bytes.Equal(signer.certificate[0].Raw, oldSigner.certificate[0].Raw) {
 		return false, nil
 	}
+	if err = s.initStore(signer.GetIssuerID()); err != nil {
+		return false, err
+	}
 	return s.signer.CompareAndSwap(oldSigner, signer), nil
 }
 

certificate-authority/service/grpc/signCertificate.go

@@ -93,60 +93,38 @@ func (s *CertificateAuthorityServer) getSigningRecord(ctx context.Context, signi
 	return originalSr, nil
 }
 
-func (s *CertificateAuthorityServer) updateRevocationListForSigningRecord(ctx context.Context, sr, prevSr *pb.SigningRecord) error {
-	if prevSr != nil {
-		// revoke previous signing record
-		prevCred := prevSr.GetCredential()
-		if prevCred != nil {
-			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 fmt.Errorf("failed to update revocation list: %w", err)
-			}
-		}
+func (s *CertificateAuthorityServer) revokeSigningRecord(ctx context.Context, revokedSr *pb.SigningRecord) error {
+	revokedCred := revokedSr.GetCredential()
+	if revokedCred == nil { // nothing to revoke
 		return nil
 	}
-	cred := sr.GetCredential()
-	if cred != nil {
-		// create new RevocationList if it doesn't exist
-		err := s.store.InsertRevocationLists(ctx, &store.RevocationList{
-			Id:     cred.GetIssuerId(),
-			Number: "1",
-		})
-		if errors.Is(err, store.ErrDuplicateID) {
-			return nil
-		}
-		if err != nil {
-			return fmt.Errorf("failed to create revocation list: %w", err)
-		}
+	query := store.UpdateRevocationListQuery{
+		IssuerID: revokedCred.GetIssuerId(),
+		RevokedCertificates: []*store.RevocationListCertificate{
+			{
+				Serial:     revokedCred.GetSerial(),
+				ValidUntil: revokedCred.GetValidUntilDate(),
+				Revocation: time.Now().UnixNano(),
+			},
+		},
 	}
-	return nil
+	_, err := s.store.UpdateRevocationList(ctx, &query)
+	return err
 }
 
 func (s *CertificateAuthorityServer) updateSigningRecord(ctx context.Context, signingRecord *pb.SigningRecord) error {
-	// try to get previous signing record
 	prevSr, err := s.getSigningRecord(ctx, signingRecord)
 	if err != nil {
 		return err
 	}
-	if s.store.SupportsRevocationList() {
-		err = s.updateRevocationListForSigningRecord(ctx, signingRecord, prevSr)
+	if s.store.SupportsRevocationList() && prevSr != nil {
+		err = s.revokeSigningRecord(ctx, prevSr)
 		if err != nil {
-			return err
+			return fmt.Errorf("failed to revoke original signing record: %w", err)
 		}
 	}
 	// upsert new one
-	err = s.store.UpdateSigningRecord(ctx, signingRecord)
-	return err
+	return s.store.UpdateSigningRecord(ctx, signingRecord)
 }
 
 func (s *CertificateAuthorityServer) SignCertificate(ctx context.Context, req *pb.SignCertificateRequest) (*pb.SignCertificateResponse, error) {

certificate-authority/service/grpc/signer.go

@@ -148,15 +148,19 @@ func (s *Signer) GetPrivateKey() *ecdsa.PrivateKey {
 	return s.privateKey
 }
 
-func (s *Signer) GetCRLConfiguation() (string, time.Duration) {
+func (s *Signer) GetIssuerID() string {
+	return s.issuerID
+}
+
+func (s *Signer) GetCRLConfiguration() (string, time.Duration) {
 	return s.crl.serverAddress, s.crl.validFor
 }
 
 func (s *Signer) IsCRLEnabled() bool {
 	return s.crl.serverAddress != ""
 }
 
-func (s *Signer) newCertificateSigner(identitySigner bool, opts ...func(cfg *certificateSigner.SignerConfig)) *certificateSigner.CertificateSigner {
+func (s *Signer) newCertificateSigner(identitySigner bool, opts ...func(cfg *certificateSigner.SignerConfig)) (*certificateSigner.CertificateSigner, error) {
 	if identitySigner {
 		return certificateSigner.NewIdentityCertificateSigner(s.certificate, s.privateKey, opts...)
 	}
@@ -181,7 +185,10 @@ func (s *Signer) sign(ctx context.Context, isIdentityCertificate bool, csr []byt
 			[]string{path.Join(s.crl.serverAddress, uri.SigningRevocationListBase, s.issuerID)},
 		))
 	}
-	signer := s.newCertificateSigner(isIdentityCertificate, opts...)
+	signer, err := s.newCertificateSigner(isIdentityCertificate, opts...)
+	if err != nil {
+		return nil, nil, err
+	}
 	cert, err := signer.Sign(ctx, csr)
 	if err != nil {
 		return nil, nil, err

certificate-authority/service/http/revocationList.go

@@ -49,18 +49,17 @@ func createCRL(rl *store.RevocationList, issuer *x509.Certificate, priv crypto.S
 	return x509.CreateRevocationList(rand.Reader, template, issuer, priv)
 }
 
-func (requestHandler *requestHandler) tryGetRevocationList(ctx context.Context, issuerID string, validFor time.Duration, tries int) (*store.RevocationList, error) {
-	for i := 0; i < tries; i++ {
-		rl, err := requestHandler.store.GetLatestIssuedOrIssueRevocationList(ctx, issuerID, validFor)
-		if err == nil {
-			return rl, nil
-		}
-		if errors.Is(err, store.ErrNotFound) {
-			continue
-		}
-		return nil, err
+func (requestHandler *requestHandler) tryGetRevocationList(ctx context.Context, issuerID string, validFor time.Duration) (*store.RevocationList, error) {
+	rl, err := requestHandler.store.GetLatestIssuedOrIssueRevocationList(ctx, issuerID, validFor)
+	if err == nil {
+		return rl, nil
+	}
+	if errors.Is(err, store.ErrNotFound) || errors.Is(err, store.ErrDuplicateID) {
+		// this only occurs if some parallel request updated the revocation list first, in that case we should
+		// just retrieve the updated one
+		return requestHandler.store.GetRevocationList(ctx, issuerID, true)
 	}
-	return nil, store.ErrNotFound
+	return nil, err
 }
 
 func (requestHandler *requestHandler) writeRevocationList(w http.ResponseWriter, r *http.Request) error {
@@ -70,10 +69,8 @@ func (requestHandler *requestHandler) writeRevocationList(w http.ResponseWriter,
 		return err
 	}
 	signer := requestHandler.cas.GetSigner()
-	_, validFor := signer.GetCRLConfiguation()
-	// TODO: make configurable
-	const tries = 3
-	rl, err := requestHandler.tryGetRevocationList(r.Context(), issuerID, validFor, tries)
+	_, validFor := signer.GetCRLConfiguration()
+	rl, err := requestHandler.tryGetRevocationList(r.Context(), issuerID, validFor)
 	if err != nil {
 		return err
 	}

certificate-authority/service/http/revocationList_test.go

@@ -3,11 +3,14 @@ package http_test
 import (
 	"context"
 	"crypto/x509"
+	"errors"
 	"io"
 	"net/http"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
 	certAuthURI "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/certificate-authority/test"
@@ -18,6 +21,7 @@ import (
 	"github.com/plgd-dev/hub/v2/test/config"
 	oauthTest "github.com/plgd-dev/hub/v2/test/oauth-server/test"
 	testService "github.com/plgd-dev/hub/v2/test/service"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/maps"
 )
@@ -175,7 +179,7 @@ func TestRevocationList(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			request := httpgwTest.NewRequest(http.MethodGet, certAuthURI.SigningRevocationList, nil).Host(config.CERTIFICATE_AUTHORITY_HTTP_HOST).AuthToken(token).AddIssuerID(tt.args.issuer).Build()
+			request := httpgwTest.NewRequest(http.MethodGet, certAuthURI.SigningRevocationList, nil).Host(config.CERTIFICATE_AUTHORITY_HTTP_HOST).AddIssuerID(tt.args.issuer).Build()
 			httpResp := httpgwTest.HTTPDo(t, request)
 			respBody, err := io.ReadAll(httpResp.Body)
 			require.NoError(t, err)
@@ -190,3 +194,95 @@ func TestRevocationList(t *testing.T) {
 		})
 	}
 }
+
+func TestParallelIssueAndUpdateRevocationList(t *testing.T) {
+	if config.ACTIVE_DATABASE() == database.CqlDB {
+		t.Skip("revocation list not supported for CqlDB")
+	}
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*30)
+	defer cancel()
+
+	shutDown := testService.SetUpServices(context.Background(), t, testService.SetUpServicesOAuth|testService.SetUpServicesMachine2MachineOAuth)
+	defer shutDown()
+	caShutdown := test.New(t, test.MakeConfig(t))
+	defer caShutdown()
+	s, cleanUpStore := test.NewStore(t)
+	defer cleanUpStore()
+
+	token := oauthTest.GetDefaultAccessToken(t)
+	ctx = pkgGrpc.CtxWithToken(ctx, token)
+
+	issuerID := uuid.NewString()
+	const iterations = 100
+	const iterationsPerHttpRequest = 10
+	certificates := make([]*store.RevocationListCertificate, iterations)
+	for i := 0; i < len(certificates); i++ {
+		certificates[i] = test.GetCertificate(i, time.Now(), time.Now().Add(time.Hour))
+	}
+
+	q := &store.UpdateRevocationListQuery{
+		IssuerID: issuerID,
+	}
+	_, err := s.UpdateRevocationList(ctx, q)
+	require.NoError(t, err)
+
+	var wg sync.WaitGroup
+	wg.Add(iterations)
+	doUpdate := func(i int) {
+		go func(index int) {
+			defer wg.Done()
+			for {
+				cert := certificates[index]
+				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
+				}
+				assert.NoError(t, err)
+			}
+		}(i)
+	}
+
+	wg.Add(iterations)
+	doIssue := func() {
+		go func() {
+			defer wg.Done()
+			for {
+				_, err := s.GetLatestIssuedOrIssueRevocationList(ctx, issuerID, time.Second)
+				if errors.Is(err, store.ErrDuplicateID) || errors.Is(err, store.ErrNotFound) {
+					continue
+				}
+				if err == nil {
+					break
+				}
+				assert.NoError(t, err)
+			}
+		}()
+	}
+
+	wg.Add(iterations / iterationsPerHttpRequest)
+	doIssueByHttp := func() {
+		go func() {
+			defer wg.Done()
+			request := httpgwTest.NewRequest(http.MethodGet, certAuthURI.SigningRevocationList, nil).Host(config.CERTIFICATE_AUTHORITY_HTTP_HOST).AddIssuerID(issuerID).Build()
+			httpResp := httpgwTest.HTTPDo(t, request)
+			_ = httpResp.Body.Close()
+			assert.Equal(t, http.StatusOK, httpResp.StatusCode)
+		}()
+	}
+
+	for iter := range iterations {
+		doIssue()
+		doUpdate(iter)
+		if iter%iterationsPerHttpRequest == 0 {
+			doIssueByHttp()
+		}
+	}
+	wg.Wait()
+}

certificate-authority/store/cqldb/revocationList.go

@@ -19,6 +19,10 @@ func (s *Store) UpdateRevocationList(context.Context, *store.UpdateRevocationLis
 	return nil, store.ErrNotSupported
 }
 
+func (s *Store) GetRevocationList(context.Context, string, bool) (*store.RevocationList, error) {
+	return nil, store.ErrNotSupported
+}
+
 func (s *Store) GetLatestIssuedOrIssueRevocationList(context.Context, string, time.Duration) (*store.RevocationList, error) {
 	return nil, store.ErrNotSupported
 }

certificate-authority/store/revocationList.go

@@ -59,9 +59,11 @@ func ParseBigInt(s string) (*big.Int, error) {
 	return &number, nil
 }
 
-// TODO: use some delta to check expiration
+// IsExpired checks whether the revocation list is expired
 func (rl *RevocationList) IsExpired() bool {
-	return rl.ValidUntil <= time.Now().UnixNano()
+	// the crl is expiring soon, so we treat it as already expired
+	const delta = time.Minute
+	return rl.ValidUntil <= time.Now().Add(delta).UnixNano()
 }
 
 func (rl *RevocationList) Validate() error {

certificate-authority/store/store.go

@@ -48,6 +48,8 @@ type Store interface {
 	SupportsRevocationList() bool
 	// InsertRevocationLists adds revocations lists to the database
 	InsertRevocationLists(ctx context.Context, rls ...*RevocationList) error
+	// Get revocation list for given issuer
+	GetRevocationList(ctx context.Context, issuerID string, includeExpired bool) (*RevocationList, error)
 	// UpdateRevocationList updates revocation list number and validity and adds certificates to revocation list. Returns the updated revocation list.
 	UpdateRevocationList(ctx context.Context, query *UpdateRevocationListQuery) (*RevocationList, error)
 	// Get valid latest issued or issue a new one revocation list

go.mod

@@ -32,7 +32,7 @@ require (
 	github.com/panjf2000/ants/v2 v2.10.0
 	github.com/pion/dtls/v3 v3.0.2
 	github.com/pion/logging v0.2.2
-	github.com/plgd-dev/device/v2 v2.5.3-0.20240916150018-cc07b737d112
+	github.com/plgd-dev/device/v2 v2.5.4-0.20241030122710-df5db5c42749
 	github.com/plgd-dev/go-coap/v3 v3.3.5
 	github.com/plgd-dev/kit/v2 v2.0.0-20211006190727-057b33161b90
 	github.com/pseudomuto/protoc-gen-doc v1.5.1

go.sum

@@ -266,8 +266,8 @@ github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/profile v1.7.0 h1:hnbDkaNWPCLMO9wGLdBFTIZvzDrDfBM2072E1S9gJkA=
 github.com/pkg/profile v1.7.0/go.mod h1:8Uer0jas47ZQMJ7VD+OHknK4YDY07LPUC6dEvqDjvNo=
-github.com/plgd-dev/device/v2 v2.5.3-0.20240916150018-cc07b737d112 h1:nSgyZUOfQr1l6E3cXOcNzogmE13uOkfZ4mh/aK+HhyQ=
-github.com/plgd-dev/device/v2 v2.5.3-0.20240916150018-cc07b737d112/go.mod h1:TXeTvVt0hi22FwhxaGxM1NiRwDDi1RmSgIrX9foWlic=
+github.com/plgd-dev/device/v2 v2.5.4-0.20241030122710-df5db5c42749 h1:0MXPEr9awv9j896AH8dwpCNjXg8rwVVPLnF/ZJNjWw8=
+github.com/plgd-dev/device/v2 v2.5.4-0.20241030122710-df5db5c42749/go.mod h1:TXeTvVt0hi22FwhxaGxM1NiRwDDi1RmSgIrX9foWlic=
 github.com/plgd-dev/go-coap/v2 v2.0.4-0.20200819112225-8eb712b901bc/go.mod h1:+tCi9Q78H/orWRtpVWyBgrr4vKFo2zYtbbxUllerBp4=
 github.com/plgd-dev/go-coap/v2 v2.4.1-0.20210517130748-95c37ac8e1fa/go.mod h1:rA7fc7ar+B/qa+Q0hRqv7yj/EMtIlmo1l7vkQGSrHPU=
 github.com/plgd-dev/go-coap/v3 v3.3.5 h1:GBdBwM/9JtJhbHxBhbzXAc40yaWvdYX16+vN0ShoX7w=