From f20ef587ce2dd0180ef3510dc72a0c652c3c5cd4 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 3 Dec 2024 15:52:09 +0100 Subject: [PATCH 1/2] #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 From 7a3badf35fba1d1f83d61916fd054bcee16d8337 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Wed, 4 Dec 2024 10:18:15 +0100 Subject: [PATCH 2/2] Fix broken test --- vcr/verifier/signature_verifier_test.go | 63 ++++++++++--------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index cf004813a6..64b4044980 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -344,13 +344,11 @@ func buildX509Credential(chainPems *cert.Chain, signingCert *x509.Certificate, r } func buildCertChain(ura string) (*cert.Chain, *x509.Certificate, *rsa.PrivateKey, *x509.Certificate, error) { - chain := [4]*x509.Certificate{} - chainPems := &cert.Chain{} rootKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, nil, nil, nil, err } - rootCertTmpl, err := CertTemplate(nil) + rootCertTmpl, err := CertTemplate("Root CA") if err != nil { return nil, nil, nil, nil, err } @@ -361,57 +359,47 @@ func buildCertChain(ura string) (*cert.Chain, *x509.Certificate, *rsa.PrivateKey if err != nil { return nil, nil, nil, nil, err } - chain[3] = rootCert - err = chainPems.Add(rootPem) - if err != nil { - return nil, nil, nil, nil, err - } intermediateL1Key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, nil, nil, nil, err } - intermediateL1Tmpl, err := CertTemplate(nil) + intermediateL1Tmpl, err := CertTemplate("Intermediate CA Level 1") if err != nil { return nil, nil, nil, nil, err } + intermediateL1Tmpl.IsCA = true intermediateL1Tmpl.KeyUsage = x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature intermediateL1Tmpl.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth} intermediateL1Cert, intermediateL1Pem, err := CreateCert(intermediateL1Tmpl, rootCertTmpl, &intermediateL1Key.PublicKey, rootKey) if err != nil { return nil, nil, nil, nil, err } - chain[2] = intermediateL1Cert - err = chainPems.Add(intermediateL1Pem) - if err != nil { - return nil, nil, nil, nil, err - } intermediateL2Key, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, nil, nil, nil, err } - intermediateL2Tmpl, err := CertTemplate(nil) + intermediateL2Tmpl, err := CertTemplate("Intermediate CA Level 2") if err != nil { return nil, nil, nil, nil, err } + intermediateL2Tmpl.IsCA = true intermediateL2Tmpl.KeyUsage = x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature intermediateL2Tmpl.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth, x509.ExtKeyUsageClientAuth} intermediateL2Cert, intermediateL2Pem, err := CreateCert(intermediateL2Tmpl, intermediateL1Cert, &intermediateL2Key.PublicKey, intermediateL1Key) if err != nil { return nil, nil, nil, nil, err } - chain[1] = intermediateL2Cert - err = chainPems.Add(intermediateL2Pem) + + signingKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { return nil, nil, nil, nil, err } - - signingKey, err := rsa.GenerateKey(rand.Reader, 2048) + signingTmpl, err := CertTemplate("Leaf") if err != nil { return nil, nil, nil, nil, err } - signingTmpl, err := CertTemplate(nil) signingTmpl.Subject.SerialNumber = ura signingTmpl.KeyUsage = x509.KeyUsageDigitalSignature signingTmpl.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth} @@ -419,13 +407,17 @@ func buildCertChain(ura string) (*cert.Chain, *x509.Certificate, *rsa.PrivateKey if err != nil { return nil, nil, nil, nil, err } - chain[0] = signingCert - err = chainPems.Add(signingPEM) - if err != nil { - return nil, nil, nil, nil, err + + certChain := &cert.Chain{} + for _, str := range []string{signingPEM, intermediateL2Pem, intermediateL1Pem, rootPem} { + fixedPem := strings.ReplaceAll(str, "\n", "\\n") + err = certChain.Add([]byte(fixedPem)) + if err != nil { + return nil, nil, nil, nil, err + } } - chainPems, err = fixChainHeaders(chainPems) - return chainPems, rootCert, signingKey, signingCert, nil + + return certChain, rootCert, signingKey, signingCert, nil } func testUraCredential(did string, ura string) (*vc.VerifiableCredential, error) { @@ -480,15 +472,13 @@ func x509VerifierTestSetup(t testing.TB) (signatureVerifier, *pki.MockValidator) } // CertTemplate is a helper function to create a cert template with a serial number and other required fields -func CertTemplate(serialNumber *big.Int) (*x509.Certificate, error) { +func CertTemplate(subjectName string) (*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) - } + 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{"Example Org"}}, + Subject: pkix.Name{Organization: []string{subjectName}}, SignatureAlgorithm: x509.SHA256WithRSA, NotBefore: time.Now(), NotAfter: time.Now().Add(time.Hour * 24 * 30), // valid for a month @@ -498,19 +488,18 @@ func CertTemplate(serialNumber *big.Int) (*x509.Certificate, error) { } // CreateCert invokes x509.CreateCertificate and returns it in the x509.Certificate format -func CreateCert(template, parent *x509.Certificate, pub interface{}, parentPriv interface{}) (cert *x509.Certificate, certPEM []byte, err error) { - +func CreateCert(template, parent *x509.Certificate, pub interface{}, parentPriv interface{}) (cert *x509.Certificate, certPEM string, err error) { certDER, err := x509.CreateCertificate(rand.Reader, template, parent, pub, parentPriv) if err != nil { - return nil, nil, err + return nil, "", err } // parse the resulting certificate so we can use it again cert, err = x509.ParseCertificate(certDER) if err != nil { - return nil, nil, err + return nil, "", err } // PEM encode the certificate (this is a standard TLS encoding) b := pem.Block{Type: "CERTIFICATE", Bytes: certDER} - certPEM = pem.EncodeToMemory(&b) + certPEM = string(pem.EncodeToMemory(&b)) return cert, certPEM, err }