From d7de0cd7204cdc720e9b3917472d44ab14b5afa7 Mon Sep 17 00:00:00 2001
From: Artem Barger <artem@bargr.net>
Date: Mon, 3 Jul 2023 14:46:36 +0300
Subject: [PATCH] [Fix]: Provide a meaningful error for cert sanitization

This commit takes care to handle the error where certificate
sanitization procedure fails to construct certificate chain due to
misconfiguration. Before this commit, the peer will simply fail with
panic without clear explanation of what exactly was wrong.

Addresses (#4302).

Signed-off-by: Artem Barger <artem@bargr.net>
---
 msp/mspimpl.go           | 10 ++++++++--
 msp/mspimplsetup_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/msp/mspimpl.go b/msp/mspimpl.go
index 0df2081c49a..8e9ff61978f 100644
--- a/msp/mspimpl.go
+++ b/msp/mspimpl.go
@@ -13,6 +13,7 @@ import (
 	"encoding/asn1"
 	"encoding/hex"
 	"encoding/pem"
+	"fmt"
 	"strings"
 
 	"github.com/golang/protobuf/proto"
@@ -919,9 +920,14 @@ func (msp *bccspmsp) sanitizeCert(cert *x509.Certificate) (*x509.Certificate, er
 		}
 
 		// ok, this is no a root CA cert, and now we
-		// then we have chain of certs and can get parent
+		// have chain of certs and can extract parent
 		// to sanitize the cert whenever it's intermediate or leaf certificate
-		parentCert := chain[1]
+		var parentCert *x509.Certificate
+		if len(chain) <= 1 {
+			return nil, fmt.Errorf("failed to traverse certificate verification chain"+
+				" for leaf or intermediate certificate, with subject %s", cert.Subject)
+		}
+		parentCert = chain[1]
 
 		// Sanitize
 		return sanitizeECDSASignedCert(cert, parentCert)
diff --git a/msp/mspimplsetup_test.go b/msp/mspimplsetup_test.go
index 2963f272bec..fd719d7fb0c 100644
--- a/msp/mspimplsetup_test.go
+++ b/msp/mspimplsetup_test.go
@@ -11,6 +11,9 @@ import (
 	"testing"
 
 	"github.com/hyperledger/fabric-protos-go/msp"
+	"github.com/hyperledger/fabric/bccsp"
+	"github.com/hyperledger/fabric/bccsp/sw"
+	"github.com/hyperledger/fabric/common/crypto/tlsgen"
 
 	"github.com/onsi/gomega"
 )
@@ -127,6 +130,43 @@ func TestTLSCAValidation(t *testing.T) {
 	})
 }
 
+func TestMalformedCertsChainSetup(t *testing.T) {
+	gt := gomega.NewGomegaWithT(t)
+
+	ca, err := tlsgen.NewCA()
+	gt.Expect(err).NotTo(gomega.HaveOccurred())
+
+	inter, err := ca.NewIntermediateCA()
+	gt.Expect(err).NotTo(gomega.HaveOccurred())
+
+	cp, err := sw.NewDefaultSecurityLevelWithKeystore(sw.NewDummyKeyStore())
+	gt.Expect(err).NotTo(gomega.HaveOccurred())
+
+	cp.GetHash(&bccsp.SHA256Opts{})
+	mspImpl := &bccspmsp{
+		opts:  &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
+		bccsp: cp,
+		cryptoConfig: &msp.FabricCryptoConfig{
+			IdentityIdentifierHashFunction: "SHA256",
+		},
+	}
+
+	// Add root CA certificate
+	// cert, err := mspImpl.getCertFromPem([]byte(ca.CertBytes()))
+	certInter, err := mspImpl.getCertFromPem([]byte(inter.CertBytes()))
+	gt.Expect(err).NotTo(gomega.HaveOccurred())
+	mspImpl.opts.Roots.AddCert(certInter)
+	mspImpl.rootCerts = []Identity{&identity{cert: certInter}}
+
+	err = mspImpl.finalizeSetupCAs()
+	gt.Expect(err).NotTo(gomega.HaveOccurred())
+
+	// Extract identity from the leaf certificate
+	_, _, err = mspImpl.getIdentityFromConf(inter.CertBytes())
+	gt.Expect(err).To(gomega.HaveOccurred())
+	gt.Expect(err.Error()).To(gomega.ContainSubstring("failed to traverse certificate verification chain"))
+}
+
 func TestCAValidation(t *testing.T) {
 	gt := gomega.NewGomegaWithT(t)