diff --git a/integration_tests/cli/pki-revocation-with-serial-number.sh b/integration_tests/cli/pki-revocation-with-serial-number.sh index dcd2d63e7..4a4a61cef 100755 --- a/integration_tests/cli/pki-revocation-with-serial-number.sh +++ b/integration_tests/cli/pki-revocation-with-serial-number.sh @@ -19,7 +19,6 @@ trustee_account="jack" second_trustee_account="alice" echo "Create a VendorAdmin Account" -create_new_account vendor_admin_account "VendorAdmin" test_divider diff --git a/x/pki/keeper/approved_certificates.go b/x/pki/keeper/approved_certificates.go index 80e658695..aee5efb28 100644 --- a/x/pki/keeper/approved_certificates.go +++ b/x/pki/keeper/approved_certificates.go @@ -74,26 +74,6 @@ func (k Keeper) RemoveApprovedCertificates( )) } -func (k Keeper) removeCertFromList(serialNumber string, certs *types.ApprovedCertificates) { - certIndex := -1 - - for i, cert := range certs.Certs { - if cert.SerialNumber == serialNumber { - certIndex = i - - break - } - } - if certIndex == -1 { - return - } - if certIndex == len(certs.Certs)-1 { - certs.Certs = certs.Certs[:certIndex] - } else { - certs.Certs = append(certs.Certs[:certIndex], certs.Certs[certIndex+1:]...) - } -} - // GetAllApprovedCertificates returns all approvedCertificates. func (k Keeper) GetAllApprovedCertificates(ctx sdk.Context) (list []types.ApprovedCertificates) { store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.ApprovedCertificatesKeyPrefix)) @@ -195,3 +175,33 @@ 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 + } + if certIndex == len(certs.Certs)-1 { + certs.Certs = certs.Certs[:certIndex] + } else { + 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/msg_server_approve_revoke_x_509_root_cert.go b/x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go index 611879fa2..134cc5cc5 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 @@ -54,30 +54,19 @@ func (k msgServer) ApproveRevokeX509RootCert(goCtx context.Context, msg *types.M if !found { return nil, pkitypes.NewErrCertificateDoesNotExist(msg.Subject, msg.SubjectKeyId) } - - var certBySerialNumber *types.Certificate - // Assign the approvals to the root certificate - for _, cert := range certificates.Certs { - if cert.IsRoot { - cert.Approvals = revocation.Approvals - } - if msg.SerialNumber != "" && cert.SerialNumber == msg.SerialNumber { - certBySerialNumber = cert - - break - } - } certID := types.CertificateIdentifier{ Subject: msg.Subject, SubjectKeyId: msg.SubjectKeyId, } - k.RemoveProposedCertificateRevocation(ctx, msg.Subject, msg.SubjectKeyId, msg.SerialNumber) k.AddRevokedRootCertificate(ctx, certID) + k.RemoveProposedCertificateRevocation(ctx, msg.Subject, msg.SubjectKeyId, msg.SerialNumber) + certBySerialNumber, _ := findCertificate(msg.SerialNumber, &certificates.Certs) if certBySerialNumber != nil { - k._removeAndRevokeBySerialNumber(ctx, certBySerialNumber, certID, certificates) + certBySerialNumber.Approvals = revocation.Approvals + k._removeAndRevokeBySerialNumber(ctx, certBySerialNumber, certificates) } else { - k._removeAndRevoke(ctx, certID, certificates) + k._removeAndRevoke(ctx, revocation.Approvals, certificates) } } else { k.SetProposedCertificateRevocation(ctx, revocation) @@ -86,30 +75,44 @@ func (k msgServer) ApproveRevokeX509RootCert(goCtx context.Context, msg *types.M return &types.MsgApproveRevokeX509RootCertResponse{}, nil } -func (k msgServer) _removeAndRevoke(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) { - k.AddRevokedCertificates(ctx, certificates) - k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId) - k.RevokeChildCertificates(ctx, certID.Subject, certID.SubjectKeyId) - +func (k msgServer) _removeAndRevoke(ctx sdk.Context, approvals []*types.Grant, certificates types.ApprovedCertificates) { + // Assign the approvals to the root certificate + for _, cert := range certificates.Certs { + if cert.IsRoot { + cert.Approvals = approvals + } + } + certID := types.CertificateIdentifier{ + Subject: certificates.Subject, + SubjectKeyId: certificates.SubjectKeyId, + } // remove from root certs index, add to revoked root certs k.RemoveApprovedRootCertificate(ctx, certID) + k.AddRevokedCertificates(ctx, certificates) + k.RemoveApprovedCertificates(ctx, certificates.Subject, certificates.SubjectKeyId) + k.RevokeChildCertificates(ctx, certificates.Subject, certificates.SubjectKeyId) // remove from subject -> subject key ID map - k.RemoveApprovedCertificateBySubject(ctx, certID.Subject, certID.SubjectKeyId) + k.RemoveApprovedCertificateBySubject(ctx, certificates.Subject, certificates.SubjectKeyId) // remove from subject key ID -> certificates map - k.RemoveApprovedCertificatesBySubjectKeyID(ctx, certID.Subject, certID.SubjectKeyId) + k.RemoveApprovedCertificatesBySubjectKeyID(ctx, certificates.Subject, certificates.SubjectKeyId) } -func (k msgServer) _removeAndRevokeBySerialNumber(ctx sdk.Context, cert *types.Certificate, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) { +func (k msgServer) _removeAndRevokeBySerialNumber(ctx sdk.Context, cert *types.Certificate, certificates types.ApprovedCertificates) { k.AddRevokedCertificates(ctx, types.ApprovedCertificates{ Subject: cert.Subject, SubjectKeyId: cert.SubjectKeyId, Certs: []*types.Certificate{cert}, }) - k.removeCertFromList(cert.SerialNumber, &certificates) + k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) if len(certificates.Certs) == 0 { k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) k.RevokeChildCertificates(ctx, cert.Subject, cert.SubjectKeyId) - k.RemoveApprovedRootCertificate(ctx, certID) + k.RemoveApprovedRootCertificate(ctx, + types.CertificateIdentifier{ + Subject: certificates.Subject, + SubjectKeyId: certificates.SubjectKeyId, + }, + ) k.RemoveApprovedCertificateBySubject(ctx, cert.Subject, cert.SubjectKeyId) k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId) } else { diff --git a/x/pki/keeper/msg_server_propose_revoke_x_509_root_cert.go b/x/pki/keeper/msg_server_propose_revoke_x_509_root_cert.go index 5dac4f158..dcbf4330d 100644 --- a/x/pki/keeper/msg_server_propose_revoke_x_509_root_cert.go +++ b/x/pki/keeper/msg_server_propose_revoke_x_509_root_cert.go @@ -33,7 +33,7 @@ func (k msgServer) ProposeRevokeX509RootCert(goCtx context.Context, msg *types.M // get corresponding approved certificates certificates, found := k.GetApprovedCertificates(ctx, msg.Subject, msg.SubjectKeyId) - if !found { + if !found || len(certificates.Certs) == 0 { return nil, pkitypes.NewErrCertificateDoesNotExist(msg.Subject, msg.SubjectKeyId) } @@ -46,14 +46,7 @@ func (k msgServer) ProposeRevokeX509RootCert(goCtx context.Context, msg *types.M } // fail if cert with serial number does not exist if msg.SerialNumber != "" { - found := false - for _, cert := range certificates.Certs { - if cert.SerialNumber == msg.SerialNumber { - found = true - - break - } - } + _, found = findCertificate(msg.SerialNumber, &certificates.Certs) if !found { return nil, pkitypes.NewErrCertificateBySerialNumberDoesNotExist( msg.Subject, msg.SubjectKeyId, msg.SerialNumber, 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 8429c4ef9..a6446c710 100644 --- a/x/pki/keeper/msg_server_revoke_x_509_cert.go +++ b/x/pki/keeper/msg_server_revoke_x_509_cert.go @@ -39,15 +39,7 @@ func (k msgServer) RevokeX509Cert(goCtx context.Context, msg *types.MsgRevokeX50 var certBySerialNumber *types.Certificate if msg.SerialNumber != "" { - found := false - for _, cert := range certificates.Certs { - if cert.SerialNumber == msg.SerialNumber { - certBySerialNumber = cert - found = true - - break - } - } + _, found = findCertificate(msg.SerialNumber, &certificates.Certs) if !found { return nil, pkitypes.NewErrCertificateBySerialNumberDoesNotExist(msg.Subject, msg.SubjectKeyId, msg.SerialNumber) } @@ -82,7 +74,7 @@ func (k msgServer) _removeAndRevokeX509CertBySerialNumber(ctx sdk.Context, cert SubjectKeyId: cert.SubjectKeyId, Certs: []*types.Certificate{cert}, }) - k.removeCertFromList(cert.SerialNumber, &certificates) + k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) if len(certificates.Certs) == 0 { k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) // Remove certificate identifier from issuer's ChildCertificates record