From f20ef587ce2dd0180ef3510dc72a0c652c3c5cd4 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 3 Dec 2024 15:52:09 +0100 Subject: [PATCH] #3587: validate certificate chain when resolving did:x509 DIDs --- core/tls.go | 25 ++++++++------ core/tls_test.go | 19 +++++++++++ vcr/verifier/signature_verifier_test.go | 2 +- vdr/didx509/resolver.go | 20 ++++++++++- vdr/didx509/resolver_test.go | 44 +++++++++++++++++++++++++ vdr/didx509/x509_utils.go | 2 +- vdr/didx509/x509_utils_test.go | 21 +++++------- 7 files changed, 108 insertions(+), 25 deletions(-) diff --git a/core/tls.go b/core/tls.go index 0fc1a56a1d..3ef10babaf 100644 --- a/core/tls.go +++ b/core/tls.go @@ -87,13 +87,23 @@ func LoadTrustStore(trustStoreFile string) (*TrustStore, error) { // ParseTrustStore creates a x509 certificate pool from the raw data func ParseTrustStore(data []byte) (*TrustStore, error) { - var err error - trustStore := new(TrustStore) - - trustStore.certificates, err = ParseCertificates(data) + certificates, err := ParseCertificates(data) if err != nil { return nil, err } + trustStore := BuildTrustStore(certificates) + + if err = validate(trustStore); err != nil { + return nil, err + } + + return trustStore, nil +} + +// BuildTrustStore creates a TrustStore from the given certificates, separating them into root and intermediate CAs +func BuildTrustStore(certificates []*x509.Certificate) *TrustStore { + trustStore := new(TrustStore) + trustStore.certificates = certificates trustStore.CertPool = NewCertPool(trustStore.certificates) for _, certificate := range trustStore.certificates { @@ -106,12 +116,7 @@ func ParseTrustStore(data []byte) (*TrustStore, error) { } } } - - if err = validate(trustStore); err != nil { - return nil, err - } - - return trustStore, nil + return trustStore } // validate returns an error if one of the certificates is invalid or does not form a chain to some root diff --git a/core/tls_test.go b/core/tls_test.go index 9201c3f747..da7bd5eb12 100644 --- a/core/tls_test.go +++ b/core/tls_test.go @@ -22,6 +22,7 @@ import ( "github.com/nuts-foundation/nuts-node/test/pki" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "os" "testing" ) @@ -54,3 +55,21 @@ func TestLoadTrustStore(t *testing.T) { assert.EqualError(t, err, "x509: certificate signed by unknown authority") }) } + +func TestBuildTrustStore(t *testing.T) { + t.Run("ok", func(t *testing.T) { + caBundleData, err := os.ReadFile("../pki/test/pkioverheid-server-bundle.pem") + require.NoError(t, err) + certs, err := ParseCertificates(caBundleData) + require.NoError(t, err) + + store := BuildTrustStore(certs) + + // Assert root certs + require.Len(t, store.RootCAs, 1) + assert.Equal(t, "CN=Staat der Nederlanden EV Root CA,O=Staat der Nederlanden,C=NL", store.RootCAs[0].Subject.String()) + // Assert intermediate certs + require.Len(t, store.IntermediateCAs, 2) + assert.Equal(t, "CN=Staat der Nederlanden Domein Server CA 2020,O=Staat der Nederlanden,C=NL", store.IntermediateCAs[1].Subject.String()) + }) +} diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index 5ec432dbc3..cf004813a6 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -488,7 +488,7 @@ func CertTemplate(serialNumber *big.Int) (*x509.Certificate, error) { } tmpl := x509.Certificate{ SerialNumber: serialNumber, - Subject: pkix.Name{Organization: []string{"JaegerTracing"}}, + Subject: pkix.Name{Organization: []string{"Example Org"}}, SignatureAlgorithm: x509.SHA256WithRSA, NotBefore: time.Now(), NotAfter: time.Now().Add(time.Hour * 24 * 30), // valid for a month diff --git a/vdr/didx509/resolver.go b/vdr/didx509/resolver.go index 12f524fdfe..f7629dd574 100644 --- a/vdr/didx509/resolver.go +++ b/vdr/didx509/resolver.go @@ -24,6 +24,7 @@ import ( "fmt" ssi "github.com/nuts-foundation/go-did" "github.com/nuts-foundation/go-did/did" + "github.com/nuts-foundation/nuts-node/core" "github.com/nuts-foundation/nuts-node/pki" "github.com/nuts-foundation/nuts-node/vdr/resolver" "strings" @@ -115,12 +116,29 @@ func (r Resolver) Resolve(id did.DID, metadata *resolver.ResolveMetadata) (*did. return nil, nil, err } + // Validate certificate chain, checking signatures and whether the chain is complete + var chainWithoutLeaf []*x509.Certificate + for _, curr := range chain { + if curr.Equal(validationCert) { + continue + } + chainWithoutLeaf = append(chainWithoutLeaf, curr) + } + trustStore := core.BuildTrustStore(chainWithoutLeaf) + verifiedChains, err := validationCert.Verify(x509.VerifyOptions{ + Intermediates: core.NewCertPool(trustStore.IntermediateCAs), + Roots: core.NewCertPool(trustStore.RootCAs), + }) + if err != nil { + return nil, nil, fmt.Errorf("did:509 certificate chain validation failed: %w", err) + } + err = validatePolicy(ref, validationCert) if err != nil { return nil, nil, err } - err = r.pkiValidator.ValidateStrict(chain) + err = r.pkiValidator.ValidateStrict(verifiedChains[0]) if err != nil { return nil, nil, err } diff --git a/vdr/didx509/resolver_test.go b/vdr/didx509/resolver_test.go index b83ddc228c..cac6441331 100644 --- a/vdr/didx509/resolver_test.go +++ b/vdr/didx509/resolver_test.go @@ -22,8 +22,10 @@ import ( "crypto/sha1" "crypto/sha512" "encoding/base64" + "encoding/pem" "errors" "fmt" + "github.com/lestrrat-go/jwx/v2/cert" "github.com/minio/sha256-simd" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/nuts-node/pki" @@ -227,6 +229,48 @@ func TestManager_Resolve_OtherName(t *testing.T) { assert.ErrorIs(t, err, ErrNoMatchingHeaderCredentials) metadata.JwtProtectedHeaders[X509CertThumbprintS256Header] = sha256Sum(signingCert.Raw) }) + t.Run("invalid signature of root certificate", func(t *testing.T) { + t.Skip("Can't test this right now, enable after https://github.com/nuts-foundation/nuts-node/issues/3587 has been fixed") + _, _, rootCert, _, _, err := BuildCertChain([]string{otherNameValue, otherNameValueSecondary}) + require.NoError(t, err) + + craftedCertChain := new(cert.Chain) + require.NoError(t, craftedCertChain.Add(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: rootCert.Raw}))) + // Do not add first cert, since it's the root CA cert, which should be the crafted certificate + for i := 1; i < certChain.Len(); i++ { + curr, b := certChain.Get(i) + require.True(t, b) + require.NoError(t, craftedCertChain.Add(curr)) + } + + metadata := resolver2.ResolveMetadata{} + metadata.JwtProtectedHeaders = make(map[string]interface{}) + metadata.JwtProtectedHeaders[X509CertChainHeader] = craftedCertChain + + _, _, err = resolver.Resolve(rootDID, &metadata) + require.ErrorContains(t, err, "did:509 certificate chain validation failed: x509: certificate signed by unknown authority") + }) + t.Run("invalid signature of leaf certificate", func(t *testing.T) { + _, _, _, _, craftedSigningCert, err := BuildCertChain([]string{otherNameValue, otherNameValueSecondary}) + require.NoError(t, err) + + craftedCertChain := new(cert.Chain) + // Do not add last cert, since it's the leaf, which should be the crafted certificate + for i := 0; i < certChain.Len()-1; i++ { + curr, b := certChain.Get(i) + require.True(t, b) + require.NoError(t, craftedCertChain.Add(curr)) + } + require.NoError(t, craftedCertChain.Add(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: craftedSigningCert.Raw}))) + + metadata := resolver2.ResolveMetadata{} + metadata.JwtProtectedHeaders = make(map[string]interface{}) + metadata.JwtProtectedHeaders[X509CertChainHeader] = craftedCertChain + metadata.JwtProtectedHeaders[X509CertThumbprintHeader] = sha1Sum(craftedSigningCert.Raw) + + _, _, err = resolver.Resolve(rootDID, &metadata) + require.ErrorContains(t, err, "did:509 certificate chain validation failed: x509: certificate signed by unknown authority") + }) t.Run("broken chain", func(t *testing.T) { expectedErr := errors.New("broken chain") validator.EXPECT().ValidateStrict(gomock.Any()).Return(expectedErr) diff --git a/vdr/didx509/x509_utils.go b/vdr/didx509/x509_utils.go index 4f6804db0f..8912c8826e 100644 --- a/vdr/didx509/x509_utils.go +++ b/vdr/didx509/x509_utils.go @@ -60,7 +60,7 @@ var ( ErrInvalidHash = fmt.Errorf("invalid hash") // ErrCertificateNotfound indicates that a certificate could not be found with the given hash. - ErrCertificateNotfound = fmt.Errorf("cannot locate a find a certificate with the given hash") + ErrCertificateNotfound = fmt.Errorf("cannot find a certificate with the given hash") // ErrInvalidPemBlock indicates that a PEM block is invalid or cannot be decoded properly. ErrInvalidPemBlock = fmt.Errorf("invalid PEM block") diff --git a/vdr/didx509/x509_utils_test.go b/vdr/didx509/x509_utils_test.go index 6fee677a05..911a09b2fa 100644 --- a/vdr/didx509/x509_utils_test.go +++ b/vdr/didx509/x509_utils_test.go @@ -52,7 +52,7 @@ func BuildCertChain(identifiers []string) (chainCerts [4]*x509.Certificate, chai return chainCerts, nil, nil, nil, nil, err } - intermediateL1Key, intermediateL1Cert, intermediateL1Pem, err := buildIntermediateCert(err, rootCert, rootKey) + intermediateL1Key, intermediateL1Cert, intermediateL1Pem, err := buildIntermediateCert(rootCert, rootKey, "Intermediate CA Level 1") if err != nil { return chainCerts, nil, nil, nil, nil, err } @@ -62,7 +62,7 @@ func BuildCertChain(identifiers []string) (chainCerts [4]*x509.Certificate, chai return chainCerts, nil, nil, nil, nil, err } - intermediateL2Key, intermediateL2Cert, intermediateL2Pem, err := buildIntermediateCert(err, intermediateL1Cert, intermediateL1Key) + intermediateL2Key, intermediateL2Cert, intermediateL2Pem, err := buildIntermediateCert(intermediateL1Cert, intermediateL1Key, "Intermediate CA Level 2") chainCerts[2] = intermediateL2Cert err = chain.Add(intermediateL2Pem) if err != nil { @@ -98,12 +98,12 @@ func buildSigningCert(identifiers []string, intermediateL2Cert *x509.Certificate return signingKey, signingCert, signingPEM, err } -func buildIntermediateCert(err error, parentCert *x509.Certificate, parentKey *rsa.PrivateKey) (*rsa.PrivateKey, *x509.Certificate, []byte, error) { +func buildIntermediateCert(parentCert *x509.Certificate, parentKey *rsa.PrivateKey, subjectName string) (*rsa.PrivateKey, *x509.Certificate, []byte, error) { intermediateL1Key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, nil, nil, err } - intermediateL1Tmpl, err := CertTemplate(nil) + intermediateL1Tmpl, err := CertTemplate(subjectName) if err != nil { return nil, nil, nil, err } @@ -119,7 +119,7 @@ func buildRootCert() (*rsa.PrivateKey, *x509.Certificate, []byte, error) { if err != nil { return nil, nil, nil, err } - rootCertTmpl, err := CertTemplate(nil) + rootCertTmpl, err := CertTemplate("Root CA") if err != nil { return nil, nil, nil, err } @@ -132,15 +132,12 @@ func buildRootCert() (*rsa.PrivateKey, *x509.Certificate, []byte, error) { // CertTemplate generates a template for a x509 certificate with a given serial number. If no serial number is provided, a random one is generated. // The certificate is valid for one month and uses SHA256 with RSA for the signature algorithm. -func CertTemplate(serialNumber *big.Int) (*x509.Certificate, error) { - // generate a random serial number (a real cert authority would have some logic behind this) - if serialNumber == nil { - serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 8) - serialNumber, _ = rand.Int(rand.Reader, serialNumberLimit) - } +func CertTemplate(subjectName string) (*x509.Certificate, error) { + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 8) + serialNumber, _ := rand.Int(rand.Reader, serialNumberLimit) tmpl := x509.Certificate{ SerialNumber: serialNumber, - Subject: pkix.Name{Organization: []string{"JaegerTracing"}}, + Subject: pkix.Name{Organization: []string{subjectName}}, SignatureAlgorithm: x509.SHA256WithRSA, NotBefore: time.Now(), NotAfter: time.Now().Add(time.Hour * 24 * 30), // valid for a month