diff --git a/integration_tests/cli/pki-noc-certs.sh b/integration_tests/cli/pki-noc-certs.sh index b083c7e14..4fe99d3fb 100755 --- a/integration_tests/cli/pki-noc-certs.sh +++ b/integration_tests/cli/pki-noc-certs.sh @@ -315,9 +315,11 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\"" echo $result | jq -echo "Request NOC certificate by VID must not contain revoked root certificates" +echo "Request NOC root certificate by VID = $vid must not contain revoked root certificates" result=$(dcld query pki noc-x509-root-certs --vid="$vid") -check_response "$result" "Not Found" +check_response "$result" "\"subject\": \"$noc_root_cert_2_subject\"" +check_response "$result" "\"subjectKeyId\": \"$noc_root_cert_2_subject_key_id\"" +check_response "$result" "\"serialNumber\": \"$noc_root_cert_2_serial_number\"" response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\"" response_does_not_contain "$result" "\"subjectKeyId\": \"$noc_root_cert_1_subject_key_id\"" response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial_number\"" 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 b17e1ebce..63b43162c 100755 --- a/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh +++ b/integration_tests/cli/pki-noc-revocation-with-revoking-child.sh @@ -100,7 +100,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\"" echo $result | jq -echo "Request NOC certificate by VID should be empty" +echo "Request NOC root certificate by VID = $vid should be empty" result=$(dcld query pki noc-x509-root-certs --vid="$vid") check_response "$result" "Not Found" response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\"" @@ -109,14 +109,14 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\"" echo $result | jq -echo "Request all certificates by subject should be empty" +echo "Request all certificates by NOC root certificate's subject should be empty" result=$(dcld query pki all-subject-x509-certs --subject="$noc_root_cert_1_subject") check_response "$result" "Not Found" response_does_not_contain "$result" "\"$noc_root_cert_1_subject\"" response_does_not_contain "$result" "\"$noc_root_cert_1_subject_key_id\"" echo $result | jq -echo "Request all certificates by subjectKeyId should be empty" +echo "Request all certificates by NOC root certificate's subjectKeyId should be empty" result=$(dcld query pki x509-cert --subject-key-id="$noc_root_cert_1_subject_key_id") check_response "$result" "Not Found" response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\"" diff --git a/integration_tests/cli/pki-noc-revocation-with-serial-number.sh b/integration_tests/cli/pki-noc-revocation-with-serial-number.sh index 89a0ad125..5dc2de123 100755 --- a/integration_tests/cli/pki-noc-revocation-with-serial-number.sh +++ b/integration_tests/cli/pki-noc-revocation-with-serial-number.sh @@ -101,7 +101,7 @@ response_does_not_contain "$result" "\"subjectKeyId\": \"$noc_root_cert_1_subjec response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial_number\"" echo $result | jq -echo "Request NOC certificate by VID should contain only one root certificate with serialNumber=$noc_root_cert_1_copy_serial_number" +echo "Request NOC root certificate by VID = $vid should contain only one root certificate with serialNumber=$noc_root_cert_1_copy_serial_number" result=$(dcld query pki noc-x509-root-certs --vid="$vid") check_response "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\"" check_response "$result" "\"subject\": \"$noc_root_cert_1_subject\"" @@ -192,7 +192,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\"" echo $result | jq -echo "Request NOC certificate by VID should be empty" +echo "Request NOC root certificate by VID = $vid should be empty" result=$(dcld query pki noc-x509-root-certs --vid="$vid") check_response "$result" "Not Found" response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\"" diff --git a/types/pki/errors.go b/types/pki/errors.go index bfaca2bee..620b7a188 100644 --- a/types/pki/errors.go +++ b/types/pki/errors.go @@ -357,6 +357,13 @@ func NewErrMessageRemoveRoot(subject string, subjectKeyID string) error { ) } +func NewErrMessageExistingCertIsNotRoot(subject string, subjectKeyID string) error { + return sdkerrors.Wrapf(ErrInappropriateCertificateType, + "The existing certificate with the same combination of subject (%v) and subjectKeyID (%v) is not a root certificate", + subject, subjectKeyID, + ) +} + func NewErrUnsupportedOperation(e interface{}) error { return sdkerrors.Wrapf(ErrUnsupportedOperation, "%v", e) } diff --git a/x/pki/handler_revoke_noc_root_cert_test.go b/x/pki/handler_revoke_noc_root_cert_test.go index 50de4015b..560b38c35 100644 --- a/x/pki/handler_revoke_noc_root_cert_test.go +++ b/x/pki/handler_revoke_noc_root_cert_test.go @@ -81,7 +81,7 @@ func TestHandler_RevokeNocX509RootCert_CertificateExists(t *testing.T) { Vid: testconstants.Vid, }, nocRoorCert: testconstants.RootCertPem, - err: sdkerrors.ErrUnauthorized, + err: pkitypes.ErrInappropriateCertificateType, }, { name: "ExistingNotNocCert", @@ -235,9 +235,10 @@ func TestHandler_RevokeNocX509RootCert_RevokeDefault(t *testing.T) { require.Equal(t, 0, len(aprCertsBySubjectKeyID)) // query noc root certificate by VID - _, err = queryNocRootCertificates(setup, testconstants.Vid) - require.Error(t, err) - require.Equal(t, codes.NotFound, status.Code(err)) + nocRootCerts, err := queryNocRootCertificates(setup, testconstants.Vid) + require.NoError(t, err) + require.Equal(t, 1, len(nocRootCerts.Certs)) + require.Equal(t, testconstants.NocRootCert2SubjectKeyID, nocRootCerts.Certs[0].SubjectKeyId) // Child certificate should not be revoked _, err = queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID) diff --git a/x/pki/keeper/child_certificates.go b/x/pki/keeper/child_certificates.go index 433182ac7..54a9c6fa2 100644 --- a/x/pki/keeper/child_certificates.go +++ b/x/pki/keeper/child_certificates.go @@ -111,10 +111,10 @@ func (k msgServer) RevokeChildCertificates(ctx sdk.Context, issuer string, autho // Revoke certificates with this subject/subjectKeyID combination certificates, _ := k.GetApprovedCertificates(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId) k.AddRevokedCertificates(ctx, certificates) - // FIXME: Should be replaced + // FIXME: Below two lines is not in the context of RevokeChildCertificates method. In future current implementation must be refactored if len(certificates.Certs) > 0 { // If cert is NOC then remove it from NOC certificates list - k.RemoveNocCertificates(ctx, certificates.Certs[0].Vid) + k.RemoveNocCertificate(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId, certificates.Certs[0].Vid) } k.RemoveApprovedCertificates(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId) 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 433610f89..6ac5e6eae 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 @@ -30,7 +30,7 @@ func (k msgServer) RevokeNocRootX509Cert(goCtx context.Context, msg *types.MsgRe cert := certificates.Certs[0] if !cert.IsRoot { - return nil, pkitypes.NewErrUnauthorizedCertIssuer(cert.Subject, cert.SubjectKeyId) + return nil, pkitypes.NewErrMessageExistingCertIsNotRoot(cert.Subject, cert.SubjectKeyId) } // Existing certificate must be NOC certificate if !cert.IsNoc { @@ -71,7 +71,7 @@ func (k msgServer) _revokeNocRootCertificates(ctx sdk.Context, certificates type k.AddRevokedNocRootCertificates(ctx, types.RevokedNocRootCertificates(certificates)) // Remove certs from NOC and approved lists - k.RemoveNocRootCertificates(ctx, vid) + k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId) k.RemoveApprovedCertificates(ctx, certificates.Subject, certificates.SubjectKeyId) // remove from subject -> subject key ID map k.RemoveApprovedCertificateBySubject(ctx, certificates.Subject, certificates.SubjectKeyId) @@ -108,16 +108,12 @@ func (k msgServer) _revokeNocRootCertificate( k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates) if len(certificates.Certs) == 0 { + k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId) k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId) - k.RemoveNocRootCertificates(ctx, vid) k.RemoveApprovedCertificateBySubject(ctx, cert.Subject, cert.SubjectKeyId) k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId) } else { - certs := types.NocRootCertificates{ - Vid: vid, - Certs: certificates.Certs, - } - k.SetNocRootCertificates(ctx, certs) + k.RemoveNocRootCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber) k.SetApprovedCertificates(ctx, certificates) k.SetApprovedCertificatesBySubjectKeyID( ctx, diff --git a/x/pki/keeper/noc_certificates.go b/x/pki/keeper/noc_certificates.go index 537dc90fa..2149820f2 100644 --- a/x/pki/keeper/noc_certificates.go +++ b/x/pki/keeper/noc_certificates.go @@ -71,6 +71,27 @@ func (k Keeper) RemoveNocCertificates( )) } +func (k Keeper) RemoveNocCertificate(ctx sdk.Context, subject, subjectKeyID string, vid int32) { + certs, found := k.GetNocCertificates(ctx, vid) + if !found { + return + } + + for i := 0; i < len(certs.Certs); { + if certs.Certs[i].Subject == subject && certs.Certs[i].SubjectKeyId == subjectKeyID { + certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...) + } else { + i++ + } + } + + if len(certs.Certs) == 0 { + k.RemoveNocCertificates(ctx, vid) + } else { + k.SetNocCertificates(ctx, certs) + } +} + // GetAllNocCertificates returns all nocCertificates. func (k Keeper) GetAllNocCertificates(ctx sdk.Context) (list []types.NocCertificates) { store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.NocCertificatesKeyPrefix)) diff --git a/x/pki/keeper/noc_root_certificates.go b/x/pki/keeper/noc_root_certificates.go index ede9b6796..742aa8eee 100644 --- a/x/pki/keeper/noc_root_certificates.go +++ b/x/pki/keeper/noc_root_certificates.go @@ -70,6 +70,54 @@ func (k Keeper) RemoveNocRootCertificates( )) } +func (k Keeper) RemoveNocRootCertificate(ctx sdk.Context, vid int32, subject, subjectKeyID string) { + certs, found := k.GetNocRootCertificates(ctx, vid) + if !found { + return + } + + certsBefore := len(certs.Certs) + for i := 0; i < len(certs.Certs); { + cert := certs.Certs[i] + if cert.Subject == subject && cert.SubjectKeyId == subjectKeyID { + certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...) + } else { + i++ + } + } + + if len(certs.Certs) == 0 { + k.RemoveNocRootCertificates(ctx, vid) + } else if certsBefore > len(certs.Certs) { // Update state only if any certificate is removed + k.SetNocRootCertificates(ctx, certs) + } +} + +func (k Keeper) RemoveNocRootCertificateBySerialNumber(ctx sdk.Context, vid int32, subject, subjectKeyID, serialNumber string) { + certs, found := k.GetNocRootCertificates(ctx, vid) + if !found { + return + } + + certsBefore := len(certs.Certs) + for i := 0; i < len(certs.Certs); { + cert := certs.Certs[i] + if cert.Subject == subject && + cert.SubjectKeyId == subjectKeyID && + cert.SerialNumber == serialNumber { + certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...) + } else { + i++ + } + } + + if len(certs.Certs) == 0 { + k.RemoveNocRootCertificates(ctx, vid) + } else if certsBefore > len(certs.Certs) { // Update state only if any certificate is removed + k.SetNocRootCertificates(ctx, certs) + } +} + // GetAllNocRootCertificates returns all nocRootCertificates. func (k Keeper) GetAllNocRootCertificates(ctx sdk.Context) (list []types.NocRootCertificates) { store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.NocRootCertificatesKeyPrefix))