Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

did:x509: improved ca-fingerprint validation #3607

Merged
merged 6 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions network/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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()

Expand Down
6 changes: 3 additions & 3 deletions network/transport/grpc/connection_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -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})
Expand All @@ -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})
Expand Down
4 changes: 2 additions & 2 deletions network/transport/grpc/tls_offloading_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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")
Expand Down
108 changes: 54 additions & 54 deletions pki/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions storage/engine_azuresql_test.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
14 changes: 14 additions & 0 deletions storage/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions storage/orm/did_document_test.go
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down
4 changes: 2 additions & 2 deletions vcr/verifier/signature_verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand All @@ -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)
Expand Down
18 changes: 11 additions & 7 deletions vdr/didx509/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package didx509

import (
"bytes"
"crypto/x509"
"errors"
"fmt"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -78,9 +79,9 @@ type X509DidPolicy struct {

// X509DidReference represents a reference for an X.509 Decentralized Identifier (DID) including method, root certificate, and policies.
type X509DidReference struct {
Method HashAlgorithm
RootCertRef string
Policies []X509DidPolicy
Method HashAlgorithm
CAFingerprint string
reinkrul marked this conversation as resolved.
Show resolved Hide resolved
Policies []X509DidPolicy
}

// Resolve resolves a DID document given its identifier and corresponding metadata.
Expand All @@ -107,14 +108,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:509 ca-fingerprint refers to leaf certificate, must be either root or intermediate CA certificate")
reinkrul marked this conversation as resolved.
Show resolved Hide resolved
}
reinkrul marked this conversation as resolved.
Show resolved Hide resolved

// Validate certificate chain, checking signatures and whether the chain is complete
var chainWithoutLeaf []*x509.Certificate
Expand Down Expand Up @@ -225,7 +229,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, ":")
Expand Down
Loading
Loading