did:x509: improved ca-fingerprint validation (#3607)

Author: reinkrul

network/network_test.go

@@ -1257,7 +1257,7 @@ func TestNetwork_checkHealth(t *testing.T) {
 	t.Run("TLS", func(t *testing.T) {
 		t.Run("up", func(t *testing.T) {
 			mockPKIValidator := pki.NewMockValidator(gomock.NewController(t))
-			mockPKIValidator.EXPECT().Validate([]*x509.Certificate{certificate.Leaf})
+			mockPKIValidator.EXPECT().CheckCRL([]*x509.Certificate{certificate.Leaf})
 			n := Network{
 				trustStore:   trustStore,
 				certificate:  certificate,
@@ -1270,7 +1270,7 @@ func TestNetwork_checkHealth(t *testing.T) {
 		})
 		t.Run("revoked/denied certificate", func(t *testing.T) {
 			mockPKIValidator := pki.NewMockValidator(gomock.NewController(t))
-			mockPKIValidator.EXPECT().Validate([]*x509.Certificate{certificate.Leaf}).Return(errors.New("custom error"))
+			mockPKIValidator.EXPECT().CheckCRL([]*x509.Certificate{certificate.Leaf}).Return(errors.New("custom error"))
 			n := Network{
 				trustStore:   trustStore,
 				certificate:  certificate,
@@ -1322,7 +1322,7 @@ func TestNetwork_checkHealth(t *testing.T) {
 			cxt.network.certificate = certificate
 			cxt.network.nodeDID = *nodeDID
 			cxt.didStore.EXPECT().Resolve(*nodeDID, nil).MinTimes(1).Return(completeDocument, &resolver.DocumentMetadata{}, nil)
-			cxt.pkiValidator.EXPECT().Validate([]*x509.Certificate{certificate.Leaf})
+			cxt.pkiValidator.EXPECT().CheckCRL([]*x509.Certificate{certificate.Leaf})
 
 			health := cxt.network.CheckHealth()
 

network/transport/grpc/connection_manager_test.go

@@ -1223,7 +1223,7 @@ func Test_grpcConnectionManager_revalidatePeers(t *testing.T) {
 	cert := testPKI.Certificate().Leaf
 
 	t.Run("ok", func(t *testing.T) {
-		mockValidator.EXPECT().Validate([]*x509.Certificate{cert})
+		mockValidator.EXPECT().CheckCRL([]*x509.Certificate{cert})
 		cm, err := NewGRPCConnectionManager(Config{pkiValidator: mockValidator}, nil, *nodeDID, nil)
 		require.NoError(t, err)
 		connection := NewStubConnection(transport.Peer{Certificate: cert})
@@ -1234,7 +1234,7 @@ func Test_grpcConnectionManager_revalidatePeers(t *testing.T) {
 		assert.Equal(t, 0, connection.disconnectCalls)
 	})
 	t.Run("denied", func(t *testing.T) {
-		mockValidator.EXPECT().Validate([]*x509.Certificate{cert}).Return(pki.ErrCertBanned)
+		mockValidator.EXPECT().CheckCRL([]*x509.Certificate{cert}).Return(pki.ErrCertBanned)
 		cm, err := NewGRPCConnectionManager(Config{pkiValidator: mockValidator}, nil, *nodeDID, nil)
 		require.NoError(t, err)
 		connection := NewStubConnection(transport.Peer{Certificate: cert})
@@ -1245,7 +1245,7 @@ func Test_grpcConnectionManager_revalidatePeers(t *testing.T) {
 		assert.Equal(t, 1, connection.disconnectCalls)
 	})
 	t.Run("denied multiple", func(t *testing.T) {
-		mockValidator.EXPECT().Validate([]*x509.Certificate{cert}).Return(pki.ErrCertBanned).Times(3)
+		mockValidator.EXPECT().CheckCRL([]*x509.Certificate{cert}).Return(pki.ErrCertBanned).Times(3)
 		cm, err := NewGRPCConnectionManager(Config{pkiValidator: mockValidator}, nil, *nodeDID, nil)
 		require.NoError(t, err)
 		connection := NewStubConnection(transport.Peer{Certificate: cert})

network/transport/grpc/tls_offloading_test.go

@@ -74,7 +74,7 @@ func Test_tlsOffloadingAuthenticator(t *testing.T) {
 			var peerInfo *peer.Peer
 			var success bool
 			serverStream.ctx = contextWithMD(encodedCert)
-			pkiMock.EXPECT().Validate(gomock.Any())
+			pkiMock.EXPECT().CheckCRL(gomock.Any())
 
 			err := auth.intercept(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error {
 				peerInfo, success = peer.FromContext(wrappedStream.Context())
@@ -99,7 +99,7 @@ func Test_tlsOffloadingAuthenticator(t *testing.T) {
 		})
 		t.Run("certificate revoked/banned", func(t *testing.T) {
 			serverStream.ctx = contextWithMD(encodedCert)
-			pkiMock.EXPECT().Validate(gomock.Any()).Return(errors.New("custom error"))
+			pkiMock.EXPECT().CheckCRL(gomock.Any()).Return(errors.New("custom error"))
 
 			err := auth.intercept(nil, serverStream, nil, func(srv interface{}, wrappedStream grpc.ServerStream) error {
 				t.Fatal("should not be called")

pki/mock.go

@@ -1,3 +1,21 @@
+/*
+ * Copyright (C) 2024 Nuts community
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
 package storage
 
 import (

storage/engine_azuresql_test.go

@@ -1,3 +1,21 @@
+/*
+ * Copyright (C) 2024 Nuts community
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
 package orm
 
 import (

storage/mock.go

@@ -100,7 +100,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) {
 
 		t.Run("happy flow", func(t *testing.T) {
 			sv, validator := x509VerifierTestSetup(t)
-			validator.EXPECT().ValidateStrict(gomock.Any()).Return(nil)
+			validator.EXPECT().CheckCRLStrict(gomock.Any()).Return(nil)
 			err = sv.VerifySignature(*cred, nil)
 			assert.NoError(t, err)
 		})
@@ -121,7 +121,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) {
 			assert.NoError(t, err)
 			sv, validator := x509VerifierTestSetup(t)
 			expectedError := errors.New("wrong ura")
-			validator.EXPECT().ValidateStrict(gomock.Any()).Return(expectedError)
+			validator.EXPECT().CheckCRLStrict(gomock.Any()).Return(expectedError)
 			err = sv.VerifySignature(*cred, nil)
 			assert.Error(t, err)
 			assert.ErrorIs(t, err, expectedError)

storage/orm/did_document_test.go

@@ -19,6 +19,7 @@
 package didx509
 
 import (
+	"bytes"
 	"crypto/x509"
 	"errors"
 	"fmt"
@@ -47,8 +48,8 @@ const (
 
 var (
 
-	// ErrX509ChainMissing is returned when the x509 root certificate chain is not present in the metadata.
-	ErrX509ChainMissing = errors.New("x509 rootCert chain is missing")
+	// ErrX509ChainMissing indicates that no x5c header was found in the provided metadata.
+	ErrX509ChainMissing = errors.New("no x5c header found")
 
 	// ErrNoCertsInHeaders indicates that no x5t or x5t#S256 header was found in the provided metadata.
 	ErrNoCertsInHeaders = errors.New("no x5t or x5t#S256 header found")
@@ -76,11 +77,14 @@ type X509DidPolicy struct {
 	Value string
 }
 
-// X509DidReference represents a reference for an X.509 Decentralized Identifier (DID) including method, root certificate, and policies.
+// X509DidReference represents a reference for an X.509 Decentralized Identifier (DID).
 type X509DidReference struct {
-	Method      HashAlgorithm
-	RootCertRef string
-	Policies    []X509DidPolicy
+	// Method specifies the hash algorithm that was used to generate CAFingerprint from the raw DER bytes of the CA certificate.
+	Method HashAlgorithm
+	// CAFingerprint is the fingerprint of the CA certificate.
+	CAFingerprint string
+	// Policies contain the fields that are included in the did:x509, which must be validated against the certificates.
+	Policies []X509DidPolicy
 }
 
 // Resolve resolves a DID document given its identifier and corresponding metadata.
@@ -107,14 +111,17 @@ func (r Resolver) Resolve(id did.DID, metadata *resolver.ResolveMetadata) (*did.
 	if err != nil {
 		return nil, nil, fmt.Errorf("did:x509 x5c certificate parsing: %w", err)
 	}
-	_, err = findCertificateByHash(chain, ref.RootCertRef, ref.Method)
+	caFingerprintCert, err := findCertificateByHash(chain, ref.CAFingerprint, ref.Method)
 	if err != nil {
 		return nil, nil, err
 	}
 	validationCert, err := findValidationCertificate(metadata, chain)
 	if err != nil {
 		return nil, nil, err
 	}
+	if bytes.Equal(caFingerprintCert.Raw, validationCert.Raw) {
+		return nil, nil, fmt.Errorf("did:x509 ca-fingerprint refers to leaf certificate, must be either root or intermediate CA certificate")
+	}
 
 	// Validate certificate chain, checking signatures and whether the chain is complete
 	var chainWithoutLeaf []*x509.Certificate
@@ -225,7 +232,7 @@ func parseX509Did(id did.DID) (*X509DidReference, error) {
 		return nil, ErrDidVersion
 	}
 	ref.Method = HashAlgorithm(didParts[1])
-	ref.RootCertRef = didParts[2]
+	ref.CAFingerprint = didParts[2]
 
 	for _, policyString := range policyStrings {
 		policyFragments := strings.Split(policyString, ":")