Commit cbe884f5 authored by Jason Yellick's avatar Jason Yellick
Browse files

FAB-12893 Fix MSP SKI error reporting



The MSP claims "CA Certificate did not have the Subject Key Identifier
extension, (SN: %s)" whenever a certificate does not have an SKI set or
when the certificate does not have the "IsCA" attribute set. In the latter
case, it is very confusing to the user because when they inspect the
certificate the SKI will indeed be present while the error indicates
otherwise.

Additionally, the error prints the cert serial number as decimal, which is
very unhelpful because tools typically report the serial number as hex.

Change-Id: I4cdc0b33d39617e108adaa5999669a6599dd9999
Signed-off-by: default avatarJason Yellick <jyellick@us.ibm.com>
parent d5eb2ab9
......@@ -197,11 +197,14 @@ func (msp *bccspmsp) setupCRLs(conf *m.FabricMSPConfig) error {
return nil
}
func (msp *bccspmsp) finalizeSetupCAs(config *m.FabricMSPConfig) error {
func (msp *bccspmsp) finalizeSetupCAs() error {
// ensure that our CAs are properly formed and that they are valid
for _, id := range append(append([]Identity{}, msp.rootCerts...), msp.intermediateCerts...) {
if !isCACert(id.(*identity).cert) {
return errors.Errorf("CA Certificate did not have the Subject Key Identifier extension, (SN: %s)", id.(*identity).cert.SerialNumber)
if !id.(*identity).cert.IsCA {
return errors.Errorf("CA Certificate did not have the CA attribute, (SN: %x)", id.(*identity).cert.SerialNumber)
}
if _, err := getSubjectKeyIdentifierFromCert(id.(*identity).cert); err != nil {
return errors.WithMessage(err, fmt.Sprintf("CA Certificate problem with Subject Key Identifier extension, (SN: %x)", id.(*identity).cert.SerialNumber))
}
if err := msp.validateCAIdentity(id.(*identity)); err != nil {
......@@ -351,8 +354,11 @@ func (msp *bccspmsp) setupTLSCAs(conf *m.FabricMSPConfig) error {
continue
}
if !isCACert(cert) {
return errors.Errorf("CA Certificate did not have the Subject Key Identifier extension, (SN: %s)", cert.SerialNumber)
if !cert.IsCA {
return errors.Errorf("CA Certificate did not have the CA attribute, (SN: %x)", cert.SerialNumber)
}
if _, err := getSubjectKeyIdentifierFromCert(cert); err != nil {
return errors.WithMessage(err, fmt.Sprintf("CA Certificate problem with Subject Key Identifier extension, (SN: %x)", cert.SerialNumber))
}
if err := msp.validateTLSCAIdentity(cert, opts); err != nil {
......@@ -399,7 +405,7 @@ func (msp *bccspmsp) preSetupV1(conf *m.FabricMSPConfig) error {
}
// Finalize setup of the CAs
if err := msp.finalizeSetupCAs(conf); err != nil {
if err := msp.finalizeSetupCAs(); err != nil {
return err
}
......
/*
Copyright IBM Corp. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/
package msp
import (
"crypto/x509"
"testing"
"github.com/hyperledger/fabric/protos/msp"
"github.com/onsi/gomega"
)
var (
caCert = `-----BEGIN CERTIFICATE-----
MIIB8jCCAZigAwIBAgIRANxd4D3sY0656NqOh8Rha0AwCgYIKoZIzj0EAwIwWDEL
MAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBG
cmFuY2lzY28xDTALBgNVBAoTBE9yZzIxDTALBgNVBAMTBE9yZzIwHhcNMTcwNTA4
MDkzMDM0WhcNMjcwNTA2MDkzMDM0WjBYMQswCQYDVQQGEwJVUzETMBEGA1UECBMK
Q2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzENMAsGA1UEChMET3Jn
MjENMAsGA1UEAxMET3JnMjBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDYy+qzS
J/8CMfhpBFhUhhz+7up4+lwjBWDSS01koszNh8camHTA8vS4ZsN+DZ2DRsSmRZgs
tG2oogLLIdh6Z1CjQzBBMA4GA1UdDwEB/wQEAwIBpjAPBgNVHSUECDAGBgRVHSUA
MA8GA1UdEwEB/wQFMAMBAf8wDQYDVR0OBAYEBAECAwQwCgYIKoZIzj0EAwIDSAAw
RQIgWnMmH0yxAjub3qfzxQioHKQ8+WvUjAXm0ejId9Q+rDICIQDr30UCPj+SXzOb
Cu4psMMBfLujKoiBNdLE1KEpt8lN1g==
-----END CERTIFICATE-----`
nonCACert = `-----BEGIN CERTIFICATE-----
MIICNjCCAd2gAwIBAgIRAMnf9/dmV9RvCCVw9pZQUfUwCgYIKoZIzj0EAwIwgYEx
CzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1TYW4g
RnJhbmNpc2NvMRkwFwYDVQQKExBvcmcxLmV4YW1wbGUuY29tMQwwCgYDVQQLEwND
T1AxHDAaBgNVBAMTE2NhLm9yZzEuZXhhbXBsZS5jb20wHhcNMTcxMTEyMTM0MTEx
WhcNMjcxMTEwMTM0MTExWjBpMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZv
cm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEMMAoGA1UECxMDQ09QMR8wHQYD
VQQDExZwZWVyMC5vcmcxLmV4YW1wbGUuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0D
AQcDQgAEZ8S4V71OBJpyMIVZdwYdFXAckItrpvSrCf0HQg40WW9XSoOOO76I+Umf
EkmTlIJXP7/AyRRSRU38oI8Ivtu4M6NNMEswDgYDVR0PAQH/BAQDAgeAMAwGA1Ud
EwEB/wQCMAAwKwYDVR0jBCQwIoAginORIhnPEFZUhXm6eWBkm7K7Zc8R4/z7LW4H
ossDlCswCgYIKoZIzj0EAwIDRwAwRAIgVikIUZzgfuFsGLQHWJUVJCU7pDaETkaz
PzFgsCiLxUACICgzJYlW7nvZxP7b6tbeu3t8mrhMXQs956mD4+BoKuNI
-----END CERTIFICATE-----`
caWithoutSKI = `-----BEGIN CERTIFICATE-----
MIIDVjCCAj6gAwIBAgIJAKsK4xHz4yA2MA0GCSqGSIb3DQEBCwUAMFsxCzAJBgNV
BAYTAlVTMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
aWRnaXRzIFB0eSBMdGQxFDASBgNVBAMMC2ZhYnJpYy50ZXN0MB4XDTE4MTExNTE5
MTA1MloXDTI5MTAyODE5MTA1MlowWzELMAkGA1UEBhMCVVMxEzARBgNVBAgMClNv
bWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDEUMBIG
A1UEAwwLZmFicmljLnRlc3QwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB
AQDjpNeST0vgoT+MNTFiI6pB6cCXlF5drW+b3BVlGYtvRK7y6szSV+XH46kxyGt3
038tuVUOuPTyc40LxWQngGO8H5zwRYV5ELu57cfeLnI9MArOF4mUSQ5lkrG7zq4F
neDDSYWGfItetsNc75ut+HiN0KK6gZ1xMG7Op8mFCwlVvDCJ8tJjhltwta3ZbDIC
eLeNYtqvyZul+bNRIw883XXY1hBW8BW+tW0r0YTQPdXEwp/yEBkZhhkCmkt1l0tM
utfkxFsUM1kWqqG/NUuz7BqQ9FL59btXeYirD3+njLTERNdzDMEAn2aOgVwWAnye
KnOZ1P51T+YJAgTyQilf7py9AgMBAAGjHTAbMAwGA1UdEwQFMAMBAf8wCwYDVR0P
BAQDAgEGMA0GCSqGSIb3DQEBCwUAA4IBAQCBtomvDwLqQh89IfjPpbwOduQDWyqp
BxGIlSNBaZkHR9WlnzRl13HZ4JklsaT/DRhKcnB5EuUHMHKUdPuhjx94F51WxlYc
f0wttSk8l5LfPAvLfL3/NwTT2YcyICA0glWF4D8FDUPKRTiOerR9KByrn4ktIjzd
vpx58pjg15TqKgrZF2h+TJ5jFa48O1wBvtMhP8WL6/6O+NjOEP56UnXPGie/3HLC
yvhEkMILRkzGUfd091cpuNxd+aGA37mZbwc+8UBpYbZFhq3NORL8zSxUQLzm1NcV
U98sznvJPRCkRiwYp5L9C5Xq72CHG/3M6cmoN0Cl0xjZicfpfnZSA/ix
-----END CERTIFICATE-----`
)
func TestTLSCAValidation(t *testing.T) {
gt := gomega.NewGomegaWithT(t)
t.Run("GoodCert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
err := mspImpl.setupTLSCAs(&msp.FabricMSPConfig{
TlsRootCerts: [][]byte{[]byte(caCert)},
})
gt.Expect(err).NotTo(gomega.HaveOccurred())
})
t.Run("NonCACert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
err := mspImpl.setupTLSCAs(&msp.FabricMSPConfig{
TlsRootCerts: [][]byte{[]byte(nonCACert)},
})
gt.Expect(err).To(gomega.MatchError("CA Certificate did not have the CA attribute, (SN: c9dff7f76657d46f082570f6965051f5)"))
})
t.Run("NoSKICert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
err := mspImpl.setupTLSCAs(&msp.FabricMSPConfig{
TlsRootCerts: [][]byte{[]byte(caWithoutSKI)},
})
gt.Expect(err).To(gomega.MatchError("CA Certificate problem with Subject Key Identifier extension, (SN: ab0ae311f3e32036): subjectKeyIdentifier not found in certificate"))
})
}
func TestCAValidation(t *testing.T) {
gt := gomega.NewGomegaWithT(t)
t.Run("GoodCert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
cert, err := mspImpl.getCertFromPem([]byte(caCert))
gt.Expect(err).NotTo(gomega.HaveOccurred())
mspImpl.opts.Roots.AddCert(cert)
mspImpl.rootCerts = []Identity{&identity{cert: cert}}
err = mspImpl.finalizeSetupCAs()
gt.Expect(err).NotTo(gomega.HaveOccurred())
})
t.Run("NonCACert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
cert, err := mspImpl.getCertFromPem([]byte(nonCACert))
gt.Expect(err).NotTo(gomega.HaveOccurred())
mspImpl.opts.Roots.AddCert(cert)
mspImpl.rootCerts = []Identity{&identity{cert: cert}}
err = mspImpl.finalizeSetupCAs()
gt.Expect(err).To(gomega.MatchError("CA Certificate did not have the CA attribute, (SN: c9dff7f76657d46f082570f6965051f5)"))
})
t.Run("NoSKICert", func(t *testing.T) {
mspImpl := &bccspmsp{
opts: &x509.VerifyOptions{Roots: x509.NewCertPool(), Intermediates: x509.NewCertPool()},
}
cert, err := mspImpl.getCertFromPem([]byte(caWithoutSKI))
gt.Expect(err).NotTo(gomega.HaveOccurred())
mspImpl.opts.Roots.AddCert(cert)
mspImpl.rootCerts = []Identity{&identity{cert: cert}}
err = mspImpl.finalizeSetupCAs()
gt.Expect(err).To(gomega.MatchError("CA Certificate problem with Subject Key Identifier extension, (SN: ab0ae311f3e32036): subjectKeyIdentifier not found in certificate"))
})
}
......@@ -282,19 +282,3 @@ func getSubjectKeyIdentifierFromCert(cert *x509.Certificate) ([]byte, error) {
return nil, errors.New("subjectKeyIdentifier not found in certificate")
}
// isCACert does a few checks on the certificate,
// assuming it's a CA; it returns true if all looks good
// and false otherwise
func isCACert(cert *x509.Certificate) bool {
_, err := getSubjectKeyIdentifierFromCert(cert)
if err != nil {
return false
}
if !cert.IsCA {
return false
}
return true
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment