fixup! Implement Certificate Revocation List

certificate-authority/service/grpc/signCertificate.go

@@ -26,27 +26,6 @@ func (s *CertificateAuthorityServer) validateRequest(csr []byte) error {
 	return nil
 }
 
-func (s *CertificateAuthorityServer) updateSigningIdentityCertificateRecord(ctx context.Context, updateSigningRecord *pb.SigningRecord) (*pb.SigningRecord, error) {
-	var found bool
-	now := time.Now().UnixNano()
-	err := s.store.LoadSigningRecords(ctx, updateSigningRecord.GetOwner(), &store.SigningRecordsQuery{
-		CommonNameFilter: []string{updateSigningRecord.GetCommonName()},
-	}, func(sr *store.SigningRecord) (err error) {
-		if updateSigningRecord.GetPublicKey() != sr.GetPublicKey() && sr.GetCredential().GetValidUntilDate() > now {
-			return fmt.Errorf("common name %v with different public key fingerprint exist", sr.GetCommonName())
-		}
-		found = true
-		return nil
-	})
-	if err != nil {
-		return nil, err
-	}
-	if found {
-		return s.store.UpdateSigningRecord(ctx, updateSigningRecord)
-	}
-	return nil, s.store.CreateSigningRecord(ctx, updateSigningRecord)
-}
-
 func toSigningRecord(owner, issuerID string, template *x509.Certificate) (*pb.SigningRecord, error) {
 	publicKeyRaw, err := x509.MarshalPKIXPublicKey(template.PublicKey)
 	if err != nil {
@@ -81,15 +60,62 @@ func toSigningRecord(owner, issuerID string, template *x509.Certificate) (*pb.Si
 	}, nil
 }
 
-func (s *CertificateAuthorityServer) updateSigningRecord(ctx context.Context, signingRecord *pb.SigningRecord) (*pb.SigningRecord, error) {
-	var checkForIdentity bool
-	if signingRecord.GetDeviceId() != "" && signingRecord.GetDeviceId() != signingRecord.GetOwner() {
-		checkForIdentity = true
-	}
+func (s *CertificateAuthorityServer) getSigningRecord(ctx context.Context, signingRecord *pb.SigningRecord) (*pb.SigningRecord, error) {
+	checkForIdentity := signingRecord.GetDeviceId() != "" && signingRecord.GetDeviceId() != signingRecord.GetOwner()
+	var err error
+	var originalSr *store.SigningRecord
 	if checkForIdentity {
-		return s.updateSigningIdentityCertificateRecord(ctx, signingRecord)
+		now := time.Now().UnixNano()
+		err = s.store.LoadSigningRecords(ctx, signingRecord.GetOwner(), &store.SigningRecordsQuery{
+			CommonNameFilter: []string{signingRecord.GetCommonName()},
+		}, func(sr *store.SigningRecord) (err error) {
+			// _id is calculated as uuid.NewSHA1(uuid.NameSpaceX500, CommonName + PublicKey) -> thus same CommonName and PublicKey == same _id
+			if signingRecord.GetPublicKey() != sr.GetPublicKey() &&
+				sr.GetCredential().GetValidUntilDate() > now {
+				return fmt.Errorf("common name %v with different public key fingerprint exist", sr.GetCommonName())
+			}
+			if signingRecord.GetId() == sr.GetId() {
+				originalSr = sr
+			}
+			return nil
+		})
+	} else {
+		err = s.store.LoadSigningRecords(ctx, signingRecord.GetOwner(), &store.SigningRecordsQuery{
+			IdFilter: []string{signingRecord.GetId()},
+		}, func(sr *store.SigningRecord) (err error) {
+			originalSr = sr
+			return nil
+		})
 	}
-	return s.store.UpdateSigningRecord(ctx, signingRecord)
+	if err != nil {
+		return nil, err
+	}
+	return originalSr, nil
+}
+
+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 prevSr != nil {
+		// 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(),
+			})
+			if err != nil {
+				return err
+			}
+		}
+	}
+	// upsert new one
+	err = s.store.UpdateSigningRecord(ctx, signingRecord)
+	return err
 }
 
 func (s *CertificateAuthorityServer) SignCertificate(ctx context.Context, req *pb.SignCertificateRequest) (*pb.SignCertificateResponse, error) {
@@ -111,23 +137,10 @@ func (s *CertificateAuthorityServer) SignCertificate(ctx context.Context, req *p
 		return nil, logger.LogAndReturnError(status.Errorf(codes.InvalidArgument, fmtError, errors.New("cannot create signing record")))
 	}
 	credential.CertificatePem = string(cert)
-	replacedRecord, err := s.updateSigningRecord(ctx, signingRecord)
-	if err != nil {
+	if err := s.updateSigningRecord(ctx, signingRecord); err != nil {
 		return nil, logger.LogAndReturnError(status.Errorf(codes.InvalidArgument, fmtError, err))
 	}
 	logger.With("crt", string(cert)).Debugf("CertificateAuthorityServer.SignCertificate")
-	replacedCredential := replacedRecord.GetCredential()
-	if replacedRecord != nil {
-		err = s.store.RevokeCertificates(ctx, replacedCredential.GetIssuerId(), &store.RevocationListCertificate{
-			Serial:     replacedCredential.GetSerial(),
-			Expiration: replacedCredential.GetValidUntilDate(),
-			Revocation: time.Now().UnixNano(),
-		})
-		if err != nil {
-			// TODO: what to do here? remove the new signing record? restore the original?
-			panic(err)
-		}
-	}
 
 	return &pb.SignCertificateResponse{
 		Certificate: cert,

certificate-authority/service/grpc/signCertificate_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
 	"github.com/plgd-dev/device/v2/pkg/security/generateCertificate"
 	"github.com/plgd-dev/hub/v2/certificate-authority/pb"
 	caTest "github.com/plgd-dev/hub/v2/certificate-authority/test"
@@ -90,16 +91,20 @@ func testSigningByFunction(t *testing.T, signFn ClientSignFunc, csr ...[]byte) {
 	}
 }
 
-func createCSR(t *testing.T, commonName string) []byte {
-	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
-	require.NoError(t, err)
+func createCSRWithKey(t *testing.T, commonName string, priv *ecdsa.PrivateKey) []byte {
 	var cfg generateCertificate.Configuration
 	cfg.Subject.CommonName = commonName
 	csr, err := generateCertificate.GenerateCSR(cfg, priv)
 	require.NoError(t, err)
 	return csr
 }
 
+func createCSR(t *testing.T, commonName string) []byte {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	return createCSRWithKey(t, commonName, priv)
+}
+
 func TestCertificateAuthorityServerSignCSR(t *testing.T) {
 	csr := createCSR(t, "aa")
 	testSigningByFunction(t, func(ctx context.Context, c pb.CertificateAuthorityClient, req *pb.SignCertificateRequest) (*pb.SignCertificateResponse, error) {
@@ -145,6 +150,53 @@ func TestCertificateAuthorityServerSignCSRWithDifferentPublicKeys(t *testing.T)
 	require.NoError(t, err)
 }
 
+func TestCertificateAuthorityServerSignCSRWithSameDevice(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
+	defer cancel()
+
+	cfg := caTest.MakeConfig(t)
+	cfg.APIs.GRPC.Authorization.Endpoints = append(cfg.APIs.GRPC.Authorization.Endpoints, validator.AuthorityConfig{
+		Authority: "https://" + config.M2M_OAUTH_SERVER_HTTP_HOST + m2mOauthUri.Base,
+		HTTP:      config.MakeHttpClientConfig(),
+	})
+
+	m2mCfg := m2mOauthTest.MakeConfig(t)
+	serviceOAuthClient := m2mOauthTest.ServiceOAuthClient
+	serviceOAuthClient.InsertTokenClaims = map[string]interface{}{
+		config.OWNER_CLAIM: oauthService.DeviceUserID,
+	}
+	m2mCfg.OAuthSigner.Clients[0] = &serviceOAuthClient
+
+	tearDown := service.SetUp(ctx, t, service.WithCAConfig(cfg), service.WithM2MOAuthConfig(m2mCfg))
+	defer tearDown()
+
+	ctx = pkgGrpc.CtxWithToken(ctx, m2mOauthTest.GetDefaultAccessToken(t))
+
+	conn, err := grpc.NewClient(config.CERTIFICATE_AUTHORITY_HOST, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
+		RootCAs: test.GetRootCertificatePool(t),
+	})))
+	require.NoError(t, err)
+	c := pb.NewCertificateAuthorityClient(conn)
+
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	deviceID := uuid.NewString()
+
+	csr := createCSRWithKey(t, "uuid:"+deviceID, priv)
+	_, err = c.SignIdentityCertificate(ctx, &pb.SignCertificateRequest{CertificateSigningRequest: csr})
+	require.NoError(t, err)
+
+	csr1 := createCSRWithKey(t, "uuid:"+deviceID, priv)
+	_, err = c.SignIdentityCertificate(ctx, &pb.SignCertificateRequest{CertificateSigningRequest: csr1})
+	require.NoError(t, err)
+
+	priv2, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+	csr2 := createCSRWithKey(t, "uuid:"+deviceID, priv2)
+	_, err = c.SignIdentityCertificate(ctx, &pb.SignCertificateRequest{CertificateSigningRequest: csr2})
+	require.Error(t, err)
+}
+
 func TestCertificateAuthorityServerSignCSRWithEmptyCommonName(t *testing.T) {
 	csr := createCSR(t, "")
 	testSigningByFunction(t, func(ctx context.Context, c pb.CertificateAuthorityClient, req *pb.SignCertificateRequest) (*pb.SignCertificateResponse, error) {

certificate-authority/service/grpc/signIdentityCertificate.go

@@ -5,10 +5,8 @@ import (
 	"crypto/x509/pkix"
 	"errors"
 	"fmt"
-	"time"
 
 	"github.com/plgd-dev/hub/v2/certificate-authority/pb"
-	"github.com/plgd-dev/hub/v2/certificate-authority/store"
 	"github.com/plgd-dev/hub/v2/identity-store/events"
 	"github.com/plgd-dev/hub/v2/pkg/net/grpc"
 	"google.golang.org/grpc/codes"
@@ -58,23 +56,10 @@ func (s *CertificateAuthorityServer) SignIdentityCertificate(ctx context.Context
 		return nil, logger.LogAndReturnError(status.Errorf(codes.InvalidArgument, fmtError, "cannot create signing record"))
 	}
 	signingRecord.Credential.CertificatePem = string(cert)
-	replacedRecord, err := s.updateSigningRecord(ctx, signingRecord)
-	if err != nil {
+	if err := s.updateSigningRecord(ctx, signingRecord); err != nil {
 		return nil, logger.LogAndReturnError(status.Errorf(codes.InvalidArgument, fmtError, err))
 	}
 	logger.With("crt", string(cert)).Debugf("CertificateAuthorityServer.SignIdentityCertificate")
-	replacedCredential := replacedRecord.GetCredential()
-	if replacedCredential != nil {
-		err = s.store.RevokeCertificates(ctx, replacedCredential.GetIssuerId(), &store.RevocationListCertificate{
-			Serial:     replacedCredential.GetSerial(),
-			Expiration: replacedCredential.GetValidUntilDate(),
-			Revocation: time.Now().UnixNano(),
-		})
-		if err != nil {
-			// TODO: what to do here? remove the new signing record? restore the original?
-			panic(err)
-		}
-	}
 
 	return &pb.SignCertificateResponse{
 		Certificate: cert,

certificate-authority/store/cqldb/signingRecords.go

@@ -86,16 +86,16 @@ func (s *Store) CreateSigningRecord(ctx context.Context, signingRecord *store.Si
 	return nil
 }
 
-func (s *Store) UpdateSigningRecord(ctx context.Context, signingRecord *store.SigningRecord) (*store.SigningRecord, error) {
+func (s *Store) UpdateSigningRecord(ctx context.Context, signingRecord *store.SigningRecord) error {
 	// To set TTL for whole row, we need to reinsert(update with upsert) the row,
 	// because Cassandra does not allow to update TTL to primary key columns
 	// More info https://opensource.docs.scylladb.com/stable/cql/time-to-live.html
 	insertQuery, err := s.getInsertQuery(signingRecord, true)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
-	return nil, s.Session().Query(insertQuery).WithContext(ctx).Exec()
+	return s.Session().Query(insertQuery).WithContext(ctx).Exec()
 }
 
 func stringsToCQLSet(filter []string, str bool) string {

certificate-authority/store/cqldb/signingRecords_test.go

@@ -181,7 +181,7 @@ func TestStoreUpdateSigningRecord(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := s.UpdateSigningRecord(ctx, tt.args.sub)
+			err := s.UpdateSigningRecord(ctx, tt.args.sub)
 			if tt.wantErr {
 				require.Error(t, err)
 				return
@@ -592,7 +592,7 @@ func BenchmarkSigningRecords(b *testing.B) {
 			for _, l := range data {
 				go func(l *pb.SigningRecord) {
 					defer wg.Done()
-					_, err := s.UpdateSigningRecord(ctx, l)
+					err := s.UpdateSigningRecord(ctx, l)
 					assert.NoError(b, err)
 				}(l)
 			}

certificate-authority/store/mongodb/signingRecords.go

@@ -25,24 +25,17 @@ func (s *Store) CreateSigningRecord(ctx context.Context, signingRecord *store.Si
 	return err
 }
 
-func (s *Store) UpdateSigningRecord(ctx context.Context, signingRecord *store.SigningRecord) (*store.SigningRecord, error) {
+func (s *Store) UpdateSigningRecord(ctx context.Context, signingRecord *store.SigningRecord) error {
 	if err := signingRecord.Validate(); err != nil {
-		return nil, err
+		return err
 	}
-	var replacedRecord *store.SigningRecord
 	filter := bson.M{"_id": signingRecord.GetId()}
-	opts := options.FindOneAndReplace().SetReturnDocument(options.Before).SetUpsert(true)
-	result := s.Collection(signingRecordsCol).FindOneAndReplace(ctx, filter, signingRecord, opts)
-	if result.Err() == nil {
-		replacedRecord = &store.SigningRecord{}
-		if err := result.Decode(replacedRecord); err != nil {
-			return nil, err
-		}
+	upsert := true
+	opts := &options.UpdateOptions{
+		Upsert: &upsert,
 	}
-	if result.Err() != nil && !errors.Is(result.Err(), mongo.ErrNoDocuments) {
-		return nil, result.Err()
-	}
-	return replacedRecord, nil
+	_, err := s.Collection(signingRecordsCol).UpdateOne(ctx, filter, bson.M{"$set": signingRecord}, opts)
+	return err
 }
 
 func toCommonNameQueryFilter(owner string, commonName string) bson.D {

certificate-authority/store/mongodb/signingRecords_test.go

@@ -95,7 +95,7 @@ func TestStoreUpdateSigningRecord(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			_, err := s.UpdateSigningRecord(ctx, tt.args.sub)
+			err := s.UpdateSigningRecord(ctx, tt.args.sub)
 			if tt.wantErr {
 				require.Error(t, err)
 				return

certificate-authority/store/store.go

@@ -33,7 +33,7 @@ type Store interface {
 	// CreateSigningRecord creates a new signing record. If the record already exists, it will throw an error.
 	CreateSigningRecord(ctx context.Context, record *SigningRecord) error
 	// UpdateSigningRecord updates an existing signing record. If the record does not exist, it will create a new one.
-	UpdateSigningRecord(ctx context.Context, record *SigningRecord) (*SigningRecord, error)
+	UpdateSigningRecord(ctx context.Context, record *SigningRecord) error
 	DeleteSigningRecords(ctx context.Context, ownerID string, query *DeleteSigningRecordsQuery) (int64, error)
 	LoadSigningRecords(ctx context.Context, ownerID string, query *SigningRecordsQuery, p Process[SigningRecord]) error