diff --git a/integration_tests/cli/pki-noc-certs.sh b/integration_tests/cli/pki-noc-certs.sh index 286723fb9..35addefc2 100755 --- a/integration_tests/cli/pki-noc-certs.sh +++ b/integration_tests/cli/pki-noc-certs.sh @@ -424,7 +424,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_cert_1_serial_numb response_does_not_contain "$result" "\"serialNumber\": \"$noc_cert_1_copy_serial_number\"" echo $result | jq -echo "Request NOC certificate by VID = $vid should contain ony leaf certificate" +echo "Request NOC certificate by VID = $vid should contain one leaf certificate" result=$(dcld query pki noc-x509-certs --vid="$vid") echo $result | jq check_response "$result" "\"subject\": \"$noc_leaf_cert_1_subject\"" diff --git a/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh b/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh index 3768890a3..9026344b8 100755 --- a/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh +++ b/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh @@ -204,7 +204,7 @@ check_response "$result" "\"serialNumber\": \"$noc_cert_2_serial_number\"" check_response "$result" "\"serialNumber\": \"$noc_cert_2_copy_serial_number\"" check_response "$result" "\"serialNumber\": \"$noc_leaf_cert_2_serial_number\"" -echo "$vendor_account Vendor revokes root NOC certificate by setting \"revoke-child\" flag to true, it should revoke child certificates too" +echo "$vendor_account Vendor revokes non-root NOC certificate by setting \"revoke-child\" flag to true, it should revoke child certificates too" result=$(echo "$passphrase" | dcld tx pki revoke-noc-x509-cert --subject="$noc_cert_2_subject" --subject-key-id="$noc_cert_2_subject_key_id" --revoke-child=true --from=$vendor_account --yes) check_response "$result" "\"code\": 0" diff --git a/x/pki/handler_revoke_noc_cert_test.go b/x/pki/handler_revoke_noc_cert_test.go index ef1a06a5a..938ebe22a 100644 --- a/x/pki/handler_revoke_noc_cert_test.go +++ b/x/pki/handler_revoke_noc_cert_test.go @@ -202,12 +202,6 @@ func TestHandler_RevokeNocX509Cert_RevokeDefault(t *testing.T) { require.Equal(t, testconstants.NocCert1Subject, revokedNocCerts.Subject) require.Equal(t, testconstants.NocCert1SubjectKeyID, revokedNocCerts.SubjectKeyId) - revokedCerts, err := queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID) - require.NoError(t, err) - require.Equal(t, 2, len(revokedCerts.Certs)) - require.Equal(t, testconstants.NocCert1Subject, revokedCerts.Subject) - require.Equal(t, testconstants.NocCert1SubjectKeyID, revokedCerts.SubjectKeyId) - // query noc certificate by Subject _, err = queryApprovedCertificatesBySubject(setup, testconstants.NocCert1Subject) require.Error(t, err) @@ -396,11 +390,6 @@ func TestHandler_RevokeNocX509Cert_RevokeBySerialNumber(t *testing.T) { require.Equal(t, 1, len(revokedNocCerts.Certs)) require.Equal(t, testconstants.NocCert1SerialNumber, revokedNocCerts.Certs[0].SerialNumber) - revokedCerts, err := queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID) - require.NoError(t, err) - require.Equal(t, 1, len(revokedCerts.Certs)) - require.Equal(t, testconstants.NocCert1SerialNumber, revokedCerts.Certs[0].SerialNumber) - // Child certificate should not be revoked _, err = queryRevokedCertificates(setup, testconstants.NocLeafCert1Subject, testconstants.NocLeafCert1SubjectKeyID) require.Equal(t, codes.NotFound, status.Code(err)) diff --git a/x/pki/keeper/approved_certificates.go b/x/pki/keeper/approved_certificates.go index 20fa5215d..3fe09b856 100644 --- a/x/pki/keeper/approved_certificates.go +++ b/x/pki/keeper/approved_certificates.go @@ -155,29 +155,3 @@ func (k Keeper) verifyCertificate(ctx sdk.Context, fmt.Sprintf("Certificate verification failed for certificate with subject=%v and subjectKeyID=%v", x509Certificate.Subject, x509Certificate.SubjectKeyID)) } - -func (k Keeper) removeCertFromList(issuer string, serialNumber string, certs *types.ApprovedCertificates) { - certIndex := -1 - - for i, cert := range certs.Certs { - if cert.SerialNumber == serialNumber && cert.Issuer == issuer { - certIndex = i - - break - } - } - if certIndex == -1 { - return - } - certs.Certs = append(certs.Certs[:certIndex], certs.Certs[certIndex+1:]...) -} - -func findCertificate(serialNumber string, certificates *[]*types.Certificate) (*types.Certificate, bool) { - for _, cert := range *certificates { - if cert.SerialNumber == serialNumber { - return cert, true - } - } - - return nil, false -} diff --git a/x/pki/keeper/approved_certificates_by_subject_key_id.go b/x/pki/keeper/approved_certificates_by_subject_key_id.go index ae8e47963..485598613 100644 --- a/x/pki/keeper/approved_certificates_by_subject_key_id.go +++ b/x/pki/keeper/approved_certificates_by_subject_key_id.go @@ -98,6 +98,12 @@ func (k Keeper) RemoveApprovedCertificatesBySubjectKeyID( } } +func (k Keeper) RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx sdk.Context, subject, subjectKeyID, serialNumber string) { + k._removeCertificatesFromSubjectKeyIDState(ctx, subjectKeyID, func(cert *types.Certificate) bool { + return cert.Subject == subject && cert.SubjectKeyId == subjectKeyID && cert.SerialNumber == serialNumber + }) +} + // GetAllApprovedCertificatesBySubjectKeyID returns all approvedCertificatesBySubjectKeyId. func (k Keeper) GetAllApprovedCertificatesBySubjectKeyID(ctx sdk.Context) (list []types.ApprovedCertificatesBySubjectKeyId) { store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.ApprovedCertificatesBySubjectKeyIDKeyPrefix)) @@ -113,3 +119,29 @@ func (k Keeper) GetAllApprovedCertificatesBySubjectKeyID(ctx sdk.Context) (list return } + +func (k Keeper) _removeCertificatesFromSubjectKeyIDState(ctx sdk.Context, subjectKeyID string, filter func(cert *types.Certificate) bool) { + certs, found := k.GetApprovedCertificatesBySubjectKeyID(ctx, subjectKeyID) + if !found { + return + } + + numCertsBefore := len(certs.Certs) + for i := 0; i < len(certs.Certs); { + cert := certs.Certs[i] + if filter(cert) { + certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...) + } else { + i++ + } + } + + if len(certs.Certs) == 0 { + store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.ApprovedCertificatesBySubjectKeyIDKeyPrefix)) + store.Delete(types.ApprovedCertificatesBySubjectKeyIDKey( + subjectKeyID, + )) + } else if numCertsBefore > len(certs.Certs) { // Update state only if any certificate is removed + k.SetApprovedCertificatesBySubjectKeyID(ctx, certs) + } +} diff --git a/x/pki/keeper/keeper.go b/x/pki/keeper/keeper.go index 85c1ae684..6300493cd 100644 --- a/x/pki/keeper/keeper.go +++ b/x/pki/keeper/keeper.go @@ -76,3 +76,29 @@ func (k Keeper) EnsureVidMatches(ctx sdk.Context, owner string, signer string) e return nil } + +func removeCertFromList(issuer string, serialNumber string, certs *[]*types.Certificate) { + certIndex := -1 + + for i, cert := range *certs { + if cert.SerialNumber == serialNumber && cert.Issuer == issuer { + certIndex = i + + break + } + } + if certIndex == -1 { + return + } + *certs = append((*certs)[:certIndex], (*certs)[certIndex+1:]...) +} + +func findCertificate(serialNumber string, certificates *[]*types.Certificate) (*types.Certificate, bool) { + for _, cert := range *certificates { + if cert.SerialNumber == serialNumber { + return cert, true + } + } + + return nil, false +} diff --git a/x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go b/x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go index ef4c4e2e1..d1afa6951 100644 --- a/x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go +++ b/x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go @@ -117,7 +117,7 @@ func (k msgServer) _revokeRootCertificate( SubjectKeyId: cert.SubjectKeyId, Certs: []*types.Certificate{cert}, }) - k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) + removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs) if len(certificates.Certs) == 0 { k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) @@ -131,9 +131,6 @@ func (k msgServer) _revokeRootCertificate( k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId) } else { k.SetApprovedCertificates(ctx, certificates) - k.SetApprovedCertificatesBySubjectKeyID( - ctx, - types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs}, - ) + k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber) } } diff --git a/x/pki/keeper/msg_server_remove_x_509_cert.go b/x/pki/keeper/msg_server_remove_x_509_cert.go index 3aec721b0..af4d89ae3 100644 --- a/x/pki/keeper/msg_server_remove_x_509_cert.go +++ b/x/pki/keeper/msg_server_remove_x_509_cert.go @@ -53,18 +53,19 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50 // remove from subject with serialNumber map k.RemoveUniqueCertificate(ctx, certBySerialNumber.Issuer, certBySerialNumber.SerialNumber) - certs := types.ApprovedCertificates{ - Subject: msg.Subject, - SubjectKeyId: msg.SubjectKeyId, - Certs: certificates, - } - k.removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &certs) - if foundApproved { - k._removeApprovedX509Cert(ctx, certID, certs) + removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &aprCerts.Certs) + k._removeApprovedX509Cert(ctx, certID, &aprCerts, msg.SerialNumber) } if foundRevoked { - k._removeRevokedX509Cert(ctx, certID, certs) + certs := types.ApprovedCertificates{ + Subject: revCerts.Subject, + SubjectKeyId: revCerts.SubjectKeyId, + Certs: revCerts.Certs, + } + removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &certs.Certs) + revCerts.Certs = certs.Certs + k._removeRevokedX509Cert(ctx, certID, &revCerts) } } else { k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId) @@ -83,31 +84,24 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50 return &types.MsgRemoveX509CertResponse{}, nil } -func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) { +func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates *types.ApprovedCertificates, serialNumber string) { if len(certificates.Certs) == 0 { k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId) k.RemoveApprovedCertificateBySubject(ctx, certID.Subject, certID.SubjectKeyId) k.RemoveApprovedCertificatesBySubjectKeyID(ctx, certID.Subject, certID.SubjectKeyId) } else { - k.SetApprovedCertificates(ctx, certificates) - k.SetApprovedCertificatesBySubjectKeyID( - ctx, - types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: certID.SubjectKeyId, Certs: certificates.Certs}, - ) + k.SetApprovedCertificates(ctx, *certificates) + k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, certID.Subject, certID.SubjectKeyId, serialNumber) } } -func (k msgServer) _removeRevokedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) { +func (k msgServer) _removeRevokedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates *types.RevokedCertificates) { if len(certificates.Certs) == 0 { k.RemoveRevokedCertificates(ctx, certID.Subject, certID.SubjectKeyId) } else { k.SetRevokedCertificates( ctx, - types.RevokedCertificates{ - Subject: certID.Subject, - SubjectKeyId: certID.SubjectKeyId, - Certs: certificates.Certs, - }, + *certificates, ) } } diff --git a/x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go b/x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go index 6ac5e6eae..c979e8a47 100644 --- a/x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go +++ b/x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go @@ -105,7 +105,7 @@ func (k msgServer) _revokeNocRootCertificate( } k.AddRevokedNocRootCertificates(ctx, revNocCerts) - k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) + removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs) if len(certificates.Certs) == 0 { k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId) @@ -115,10 +115,7 @@ func (k msgServer) _revokeNocRootCertificate( } else { k.RemoveNocRootCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber) k.SetApprovedCertificates(ctx, certificates) - k.SetApprovedCertificatesBySubjectKeyID( - ctx, - types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs}, - ) + k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber) } return nil diff --git a/x/pki/keeper/msg_server_revoke_noc_x_509_cert.go b/x/pki/keeper/msg_server_revoke_noc_x_509_cert.go index 3e4334f52..361dbcb78 100644 --- a/x/pki/keeper/msg_server_revoke_noc_x_509_cert.go +++ b/x/pki/keeper/msg_server_revoke_noc_x_509_cert.go @@ -80,7 +80,7 @@ func (k msgServer) _revokeNocCertificate( } k.AddRevokedCertificates(ctx, revCerts) - k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) + removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs) if len(certificates.Certs) == 0 { k.RemoveNocCertificate(ctx, certificates.Subject, certificates.SubjectKeyId, vid) k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) @@ -88,10 +88,7 @@ func (k msgServer) _revokeNocCertificate( k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId) } else { k.RemoveNocCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber) - k.SetApprovedCertificatesBySubjectKeyID( - ctx, - types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs}, - ) + k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber) k.SetApprovedCertificates(ctx, certificates) } diff --git a/x/pki/keeper/msg_server_revoke_x_509_cert.go b/x/pki/keeper/msg_server_revoke_x_509_cert.go index 99c2ad0a0..15c7bbe15 100644 --- a/x/pki/keeper/msg_server_revoke_x_509_cert.go +++ b/x/pki/keeper/msg_server_revoke_x_509_cert.go @@ -77,7 +77,7 @@ func (k msgServer) _revokeX509Certificate(ctx sdk.Context, cert *types.Certifica SubjectKeyId: cert.SubjectKeyId, Certs: []*types.Certificate{cert}, }) - k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) + removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs) if len(certificates.Certs) == 0 { k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) // Remove certificate identifier from issuer's ChildCertificates record @@ -87,9 +87,6 @@ func (k msgServer) _revokeX509Certificate(ctx sdk.Context, cert *types.Certifica k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId) } else { k.SetApprovedCertificates(ctx, certificates) - k.SetApprovedCertificatesBySubjectKeyID( - ctx, - types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs}, - ) + k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, cert.SerialNumber) } }