Skip to content

Commit 246cd54

Browse files
committed
[FAB-3240] Intermediate CA certs validation
This change-set modifies the bccspmsp’s setup method to ensure that intermediate CA certificates are not revoked. This is done by setting up first the CRLs and then by validating CA certificates. Tests have been added to verify that the setup fails when the setup configuration contains an intermediate CA that has been revoked. Change-Id: I8d4ef9e61de09b8f2d3909a65d02a4f5ba055038 Signed-off-by: Angelo De Caro <[email protected]>
1 parent 51b9f4e commit 246cd54

File tree

8 files changed

+233
-93
lines changed

8 files changed

+233
-93
lines changed

msp/mspimpl.go

+136-93
Original file line numberDiff line numberDiff line change
@@ -368,23 +368,6 @@ func (msp *bccspmsp) Setup(conf1 *m.MSPConfig) error {
368368
msp.admins[i] = id
369369
}
370370

371-
// ensure that our CAs are properly formed
372-
for _, cert := range append(append([]Identity{}, msp.rootCerts...), msp.intermediateCerts...) {
373-
if !isCACert(cert.(*identity).cert) {
374-
return fmt.Errorf("CA Certificate did not have the Subject Key Identifier extension, (SN: %s)", cert.(*identity).cert.SerialNumber)
375-
}
376-
}
377-
378-
// setup the signer (if present)
379-
if conf.SigningIdentity != nil {
380-
sid, err := msp.getSigningIdentityFromConf(conf.SigningIdentity)
381-
if err != nil {
382-
return err
383-
}
384-
385-
msp.signer = sid
386-
}
387-
388371
// setup the CRL (if present)
389372
msp.CRL = make([]*pkix.CertificateList, len(conf.RevocationList))
390373
for i, crlbytes := range conf.RevocationList {
@@ -401,6 +384,27 @@ func (msp *bccspmsp) Setup(conf1 *m.MSPConfig) error {
401384
msp.CRL[i] = crl
402385
}
403386

387+
// ensure that our CAs are properly formed and that they are valid
388+
for _, id := range append(append([]Identity{}, msp.rootCerts...), msp.intermediateCerts...) {
389+
if !isCACert(id.(*identity).cert) {
390+
return fmt.Errorf("CA Certificate did not have the Subject Key Identifier extension, (SN: %s)", id.(*identity).cert.SerialNumber)
391+
}
392+
393+
if err := msp.validateCAIdentity(id.(*identity)); err != nil {
394+
return fmt.Errorf("CA Certificate is not valid, (SN: %s) [%s]", id.(*identity).cert.SerialNumber, err)
395+
}
396+
}
397+
398+
// setup the signer (if present)
399+
if conf.SigningIdentity != nil {
400+
sid, err := msp.getSigningIdentityFromConf(conf.SigningIdentity)
401+
if err != nil {
402+
return err
403+
}
404+
405+
msp.signer = sid
406+
}
407+
404408
// setup the OUs
405409
if err := msp.setupOUs(conf); err != nil {
406410
return err
@@ -461,82 +465,7 @@ func (msp *bccspmsp) Validate(id Identity) error {
461465
// this is how I can validate it given the
462466
// root of trust this MSP has
463467
case *identity:
464-
validationChain, err := msp.getCertificationChainForBCCSPIdentity(id)
465-
if err != nil {
466-
return fmt.Errorf("Could not obtain certification chain, err %s", err)
467-
}
468-
469-
// here we know that the identity is valid; now we have to check whether it has been revoked
470-
471-
// identify the SKI of the CA that signed this cert
472-
SKI, err := getSubjectKeyIdentifierFromCert(validationChain[1])
473-
if err != nil {
474-
return fmt.Errorf("Could not obtain Subject Key Identifier for signer cert, err %s", err)
475-
}
476-
477-
// check whether one of the CRLs we have has this cert's
478-
// SKI as its AuthorityKeyIdentifier
479-
for _, crl := range msp.CRL {
480-
aki, err := getAuthorityKeyIdentifierFromCrl(crl)
481-
if err != nil {
482-
return fmt.Errorf("Could not obtain Authority Key Identifier for crl, err %s", err)
483-
}
484-
485-
// check if the SKI of the cert that signed us matches the AKI of any of the CRLs
486-
if bytes.Equal(aki, SKI) {
487-
// we have a CRL, check whether the serial number is revoked
488-
for _, rc := range crl.TBSCertList.RevokedCertificates {
489-
if rc.SerialNumber.Cmp(id.cert.SerialNumber) == 0 {
490-
// We have found a CRL whose AKI matches the SKI of
491-
// the CA (root or intermediate) that signed the
492-
// certificate that is under validation. As a
493-
// precaution, we verify that said CA is also the
494-
// signer of this CRL.
495-
err = validationChain[1].CheckCRLSignature(crl)
496-
if err != nil {
497-
// the CA cert that signed the certificate
498-
// that is under validation did not sign the
499-
// candidate CRL - skip
500-
mspLogger.Warningf("Invalid signature over the identified CRL, error %s", err)
501-
continue
502-
}
503-
504-
// A CRL also includes a time of revocation so that
505-
// the CA can say "this cert is to be revoked starting
506-
// from this time"; however here we just assume that
507-
// revocation applies instantaneously from the time
508-
// the MSP config is committed and used so we will not
509-
// make use of that field
510-
return errors.New("The certificate has been revoked")
511-
}
512-
}
513-
}
514-
}
515-
516-
// Check that the identity's OUs are compatible with those recognized by this MSP,
517-
// meaning that the intersection is not empty.
518-
if len(msp.ouIdentifiers) > 0 {
519-
found := false
520-
521-
for _, OU := range id.GetOrganizationalUnits() {
522-
certificationIDs, exists := msp.ouIdentifiers[OU.OrganizationalUnitIdentifier]
523-
524-
if exists {
525-
for _, certificationID := range certificationIDs {
526-
if bytes.Equal(certificationID, OU.CertifiersIdentifier) {
527-
found = true
528-
break
529-
}
530-
}
531-
}
532-
}
533-
534-
if !found {
535-
return fmt.Errorf("None of the identity's organizational units [%v] are in MSP %s", id.GetOrganizationalUnits(), msp.name)
536-
}
537-
}
538-
539-
return nil
468+
return msp.validateIdentity(id)
540469
default:
541470
return fmt.Errorf("Identity type not recognized")
542471
}
@@ -899,3 +828,117 @@ func (msp *bccspmsp) sanitizeCert(cert *x509.Certificate) (*x509.Certificate, er
899828
}
900829
return cert, nil
901830
}
831+
832+
func (msp *bccspmsp) validateIdentity(id *identity) error {
833+
validationChain, err := msp.getCertificationChainForBCCSPIdentity(id)
834+
if err != nil {
835+
return fmt.Errorf("Could not obtain certification chain, err %s", err)
836+
}
837+
838+
err = msp.validateIdentityAgainstChain(id, validationChain)
839+
if err != nil {
840+
return fmt.Errorf("Could not validate identity against certification chain, err %s", err)
841+
}
842+
843+
err = msp.validateIdentityOUs(id)
844+
if err != nil {
845+
return fmt.Errorf("Could not validate identity's OUs, err %s", err)
846+
}
847+
848+
return nil
849+
}
850+
851+
func (msp *bccspmsp) validateCAIdentity(id *identity) error {
852+
if !id.cert.IsCA {
853+
return errors.New("Only CA identities can be validated")
854+
}
855+
856+
validationChain, err := msp.getUniqueValidationChain(id.cert)
857+
if err != nil {
858+
return fmt.Errorf("Could not obtain certification chain, err %s", err)
859+
}
860+
if len(validationChain) == 1 {
861+
// validationChain[0] is the root CA certificate
862+
return nil
863+
}
864+
865+
return msp.validateIdentityAgainstChain(id, validationChain)
866+
}
867+
868+
func (msp *bccspmsp) validateIdentityAgainstChain(id *identity, validationChain []*x509.Certificate) error {
869+
// here we know that the identity is valid; now we have to check whether it has been revoked
870+
871+
// identify the SKI of the CA that signed this cert
872+
SKI, err := getSubjectKeyIdentifierFromCert(validationChain[1])
873+
if err != nil {
874+
return fmt.Errorf("Could not obtain Subject Key Identifier for signer cert, err %s", err)
875+
}
876+
877+
// check whether one of the CRLs we have has this cert's
878+
// SKI as its AuthorityKeyIdentifier
879+
for _, crl := range msp.CRL {
880+
aki, err := getAuthorityKeyIdentifierFromCrl(crl)
881+
if err != nil {
882+
return fmt.Errorf("Could not obtain Authority Key Identifier for crl, err %s", err)
883+
}
884+
885+
// check if the SKI of the cert that signed us matches the AKI of any of the CRLs
886+
if bytes.Equal(aki, SKI) {
887+
// we have a CRL, check whether the serial number is revoked
888+
for _, rc := range crl.TBSCertList.RevokedCertificates {
889+
if rc.SerialNumber.Cmp(id.cert.SerialNumber) == 0 {
890+
// We have found a CRL whose AKI matches the SKI of
891+
// the CA (root or intermediate) that signed the
892+
// certificate that is under validation. As a
893+
// precaution, we verify that said CA is also the
894+
// signer of this CRL.
895+
err = validationChain[1].CheckCRLSignature(crl)
896+
if err != nil {
897+
// the CA cert that signed the certificate
898+
// that is under validation did not sign the
899+
// candidate CRL - skip
900+
mspLogger.Warningf("Invalid signature over the identified CRL, error %s", err)
901+
continue
902+
}
903+
904+
// A CRL also includes a time of revocation so that
905+
// the CA can say "this cert is to be revoked starting
906+
// from this time"; however here we just assume that
907+
// revocation applies instantaneously from the time
908+
// the MSP config is committed and used so we will not
909+
// make use of that field
910+
return errors.New("The certificate has been revoked")
911+
}
912+
}
913+
}
914+
}
915+
916+
return nil
917+
}
918+
919+
func (msp *bccspmsp) validateIdentityOUs(id *identity) error {
920+
// Check that the identity's OUs are compatible with those recognized by this MSP,
921+
// meaning that the intersection is not empty.
922+
if len(msp.ouIdentifiers) > 0 {
923+
found := false
924+
925+
for _, OU := range id.GetOrganizationalUnits() {
926+
certificationIDs, exists := msp.ouIdentifiers[OU.OrganizationalUnitIdentifier]
927+
928+
if exists {
929+
for _, certificationID := range certificationIDs {
930+
if bytes.Equal(certificationID, OU.CertifiersIdentifier) {
931+
found = true
932+
break
933+
}
934+
}
935+
}
936+
}
937+
938+
if !found {
939+
return fmt.Errorf("None of the identity's organizational units [%v] are in MSP %s", id.GetOrganizationalUnits(), msp.name)
940+
}
941+
}
942+
943+
return nil
944+
}

msp/revocation_test.go

+25
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@ package msp
1919
import (
2020
"testing"
2121

22+
"path/filepath"
23+
24+
"github.com/hyperledger/fabric/bccsp/sw"
2225
"github.com/hyperledger/fabric/protos/msp"
2326
"github.com/stretchr/testify/assert"
2427
)
@@ -58,3 +61,25 @@ func TestIdentityPolicyPrincipalAgainstRevokedIdentity(t *testing.T) {
5861
err = id.SatisfiesPrincipal(principal)
5962
assert.Error(t, err)
6063
}
64+
65+
func TestRevokedIntermediateCA(t *testing.T) {
66+
// testdata/revokedica
67+
// 1) a key and a signcert (used to populate the default signing identity);
68+
// 2) cacert is the CA that signed the intermediate;
69+
// 3) a revocation list that revokes the intermediate CA cert
70+
dir := "testdata/revokedica"
71+
conf, err := GetLocalMspConfig(dir, nil, "DEFAULT")
72+
assert.NoError(t, err)
73+
74+
thisMSP, err := NewBccspMsp()
75+
assert.NoError(t, err)
76+
ks, err := sw.NewFileBasedKeyStore(nil, filepath.Join(dir, "keystore"), true)
77+
assert.NoError(t, err)
78+
csp, err := sw.New(256, "SHA2", ks)
79+
assert.NoError(t, err)
80+
thisMSP.(*bccspmsp).bccsp = csp
81+
82+
err = thisMSP.Setup(conf)
83+
assert.Error(t, err)
84+
assert.Contains(t, err.Error(), "CA Certificate is not valid, ")
85+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICLzCCAdagAwIBAgIQU9G+E1HIAZHCLdZ3j8yxOjAKBggqhkjOPQQDAjBJMQsw
3+
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4+
YW5jaXNjbzENMAsGA1UEAxMEaWNhMTAeFw0xNzA1MTEwNzQ2MDdaFw0yNzA1MDkw
5+
NzQ2MDdaMEoxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYD
6+
VQQHEw1TYW4gRnJhbmNpc2NvMQ4wDAYDVQQDEwV1c2VyMTBZMBMGByqGSM49AgEG
7+
CCqGSM49AwEHA0IABJ5KYN0OaMyduXw1t5U07pV29vsSAra4blFQHPy+x2LMY/kV
8+
xkaQDbUGAuSCOP0wceqUvXEkExL5Ui0uGcNK4t6jgZ4wgZswDgYDVR0PAQH/BAQD
9+
AgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwZgYDVR0jBF8w
10+
XYBbMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAERq5JuY0xQ3oypuerfulObUxH
11+
7wWRMatxz6+EuBPj9uqeMdfEs2Tx2DOBdb6jMvAAM0OUG32kn24T3XZZ4Ap/3DAK
12+
BggqhkjOPQQDAgNHADBEAiAyV131BUkiTGeHLiv9dZRLftognxidV4hPPNNG80hv
13+
YgIgKkOoJMdkDtU0VDXSZBFRlKpNidPlbreK+6FOcivS7Js=
14+
-----END CERTIFICATE-----
+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICEjCCAbigAwIBAgIRAKhZ5EvzGvy83SuaCfWfeC8wCgYIKoZIzj0EAwIwVTEL
3+
MAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBG
4+
cmFuY2lzY28xDDAKBgNVBAoTA29yZzELMAkGA1UEAxMCY2EwHhcNMTcwNTExMDc0
5+
NjA3WhcNMjcwNTA5MDc0NjA3WjBVMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2Fs
6+
aWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEMMAoGA1UEChMDb3JnMQsw
7+
CQYDVQQDEwJjYTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABDVH2BNZpXh1h9BA
8+
JvGD+I/cRJPGHGPIifjGGZ+lQc+j5MrrZC0+n/W+ypTO6d4GSbZgAFa1IZm2N2+c
9+
kK5Yny+jaTBnMA4GA1UdDwEB/wQEAwIBpjAZBgNVHSUEEjAQBgRVHSUABggrBgEF
10+
BQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdDgQiBCCzijNEqkR0yCs22TAE2iPN
11+
nM2XGeBpIjKp7G65nTVT2TAKBggqhkjOPQQDAgNIADBFAiEAzF8huxRNn8J2zchq
12+
SW6SBybbkxstTNt+OaIhVwRjJ5cCIEKuXlo7TYMngHiChqI8D9CNKjuMqrtGYGAI
13+
gQrXKTsw
14+
-----END CERTIFICATE-----

msp/testdata/revokedica/crls/crl.pem

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
-----BEGIN X509 CRL-----
2+
MIIBMzCB2gIBATAKBggqhkjOPQQDAjBVMQswCQYDVQQGEwJVUzETMBEGA1UECBMK
3+
Q2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEMMAoGA1UEChMDb3Jn
4+
MQswCQYDVQQDEwJjYRcNMTcwNTExMDc0NjA3WhcNMjcwNTA5MDc0NjA3WjAjMCEC
5+
EEzQohO1frOTEWQE+9Ws8nsXDTE3MDUxMTA3NDYwN1qgLzAtMCsGA1UdIwQkMCKA
6+
ILOKM0SqRHTIKzbZMATaI82czZcZ4GkiMqnsbrmdNVPZMAoGCCqGSM49BAMCA0gA
7+
MEUCIQCz/DcyUVInAUW3D/+618a/UovNdXT7guOhjMCx8nGufAIgbtoVSX6VnMc/
8+
7ZQ6p4XhR0XMZxxD0oIKNSuqtGsEkEo=
9+
-----END X509 CRL-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICbjCCAhWgAwIBAgIQTNCiE7V+s5MRZAT71azyezAKBggqhkjOPQQDAjBVMQsw
3+
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4+
YW5jaXNjbzEMMAoGA1UEChMDb3JnMQswCQYDVQQDEwJjYTAeFw0xNzA1MTEwNzQ2
5+
MDdaFw0yNzA1MDkwNzQ2MDdaMEkxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxp
6+
Zm9ybmlhMRYwFAYDVQQHEw1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQDEwRpY2ExMFkw
7+
EwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAERq5JuY0xQ3oypuerfulObUxH7wWRMatx
8+
z6+EuBPj9uqeMdfEs2Tx2DOBdb6jMvAAM0OUG32kn24T3XZZ4Ap/3KOB0jCBzzAO
9+
BgNVHQ8BAf8EBAMCAaYwGQYDVR0lBBIwEAYEVR0lAAYIKwYBBQUHAwEwDwYDVR0T
10+
AQH/BAUwAwEB/zBkBgNVHQ4EXQRbMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE
11+
Rq5JuY0xQ3oypuerfulObUxH7wWRMatxz6+EuBPj9uqeMdfEs2Tx2DOBdb6jMvAA
12+
M0OUG32kn24T3XZZ4Ap/3DArBgNVHSMEJDAigCCzijNEqkR0yCs22TAE2iPNnM2X
13+
GeBpIjKp7G65nTVT2TAKBggqhkjOPQQDAgNHADBEAiA7lweLUGOiPDiicv1UA11e
14+
BWqsyR19QoaNkRxcdFVNIgIgE+GKKxeomIceln8PJgMIdPfWrRkiK6kVCMF1E/AU
15+
MNo=
16+
-----END CERTIFICATE-----
+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgYV78DPlUOeRDAsOl
3+
VfZMheUFtsloDxt2jMQ2pEKHG9GhRANCAASeSmDdDmjMnbl8NbeVNO6Vdvb7EgK2
4+
uG5RUBz8vsdizGP5FcZGkA21BgLkgjj9MHHqlL1xJBMS+VItLhnDSuLe
5+
-----END PRIVATE KEY-----
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIICLzCCAdagAwIBAgIQU9G+E1HIAZHCLdZ3j8yxOjAKBggqhkjOPQQDAjBJMQsw
3+
CQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNU2FuIEZy
4+
YW5jaXNjbzENMAsGA1UEAxMEaWNhMTAeFw0xNzA1MTEwNzQ2MDdaFw0yNzA1MDkw
5+
NzQ2MDdaMEoxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYD
6+
VQQHEw1TYW4gRnJhbmNpc2NvMQ4wDAYDVQQDEwV1c2VyMTBZMBMGByqGSM49AgEG
7+
CCqGSM49AwEHA0IABJ5KYN0OaMyduXw1t5U07pV29vsSAra4blFQHPy+x2LMY/kV
8+
xkaQDbUGAuSCOP0wceqUvXEkExL5Ui0uGcNK4t6jgZ4wgZswDgYDVR0PAQH/BAQD
9+
AgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwZgYDVR0jBF8w
10+
XYBbMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAERq5JuY0xQ3oypuerfulObUxH
11+
7wWRMatxz6+EuBPj9uqeMdfEs2Tx2DOBdb6jMvAAM0OUG32kn24T3XZZ4Ap/3DAK
12+
BggqhkjOPQQDAgNHADBEAiAyV131BUkiTGeHLiv9dZRLftognxidV4hPPNNG80hv
13+
YgIgKkOoJMdkDtU0VDXSZBFRlKpNidPlbreK+6FOcivS7Js=
14+
-----END CERTIFICATE-----

0 commit comments

Comments
 (0)