#535 Add new txn to remove non-root certificates

Resolve MR's comments

Signed-off-by: Abdulbois <abdulbois.tursunov@dsr-corporation.com>
Signed-off-by: Abdulbois <abdulbois123@gmail.com>

Author: Abdulbois

integration_tests/cli/pki-remove-x509-certificates.sh

@@ -76,19 +76,31 @@ check_response "$result" "\"serialNumber\": \"$intermediate_cert_2_serial_number
 check_response "$result" "\"serialNumber\": \"$leaf_cert_serial_number\""
 response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_1_serial_number\""
 
-echo "Request all approved intermediate certificates should contain only one certificate with serialNumber 4"
+echo "Request approved certificates by an intermediate certificate's subject and subjectKeyId should contain only one certificate with serialNumber 4"
 result=$(dcld query pki x509-cert --subject="$intermediate_cert_subject" --subject-key-id="$intermediate_cert_subject_key_id")
 echo $result | jq
 check_response "$result" "\"subject\": \"$intermediate_cert_subject\""
 check_response "$result" "\"subjectKeyId\": \"$intermediate_cert_subject_key_id\""
 check_response "$result" "\"serialNumber\": \"$intermediate_cert_2_serial_number\""
 response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_1_serial_number\""
 
-echo "Remove intermediate certificate with subject and subjectKeyId"
+echo "Revoke an intermediate certificate with serialNumber 3"
+result=$(echo "$passphrase" | dcld tx pki revoke-x509-cert --subject="$intermediate_cert_subject" --subject-key-id="$intermediate_cert_subject_key_id" --serial-number="$intermediate_cert_1_serial_number" --from=$trustee_account --yes)
+check_response "$result" "\"code\": 0"
+
+echo "Request all revoked certificates should contain only one intermediate certificate with serialNumber 3"
+result=$(dcld query pki all-revoked-x509-certs)
+echo $result | jq
+check_response "$result" "\"subject\": \"$intermediate_cert_subject\""
+check_response "$result" "\"subjectKeyId\": \"$intermediate_cert_subject_key_id\""
+check_response "$result" "\"serialNumber\": \"$intermediate_cert_1_serial_number\""
+response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_2_serial_number\""
+
+echo "Remove an intermediate certificate with subject and subjectKeyId"
 result=$(echo "$passphrase" | dcld tx pki remove-x509-cert --subject="$intermediate_cert_subject" --subject-key-id="$intermediate_cert_subject_key_id" --from=$trustee_account --yes)
 check_response "$result" "\"code\": 0"
 
-echo "Request approved intermediate certificates should be empty"
+echo "Request approved certificates by an intermediate certificate's subject and subjectKeyId should be empty"
 result=$(dcld query pki x509-cert --subject="$intermediate_cert_subject" --subject-key-id="$intermediate_cert_subject_key_id")
 echo $result | jq
 check_response "$result" "Not Found"
@@ -97,6 +109,15 @@ response_does_not_contain "$result" "\"subjectKeyId\": \"$intermediate_cert_subj
 response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_2_serial_number\""
 response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_1_serial_number\""
 
+echo "Request all revoked certificates should be empty"
+result=$(dcld query pki all-revoked-x509-certs)
+echo $result | jq
+check_response "$result" "Not Found"
+response_does_not_contain "$result" "\"subject\": \"$intermediate_cert_subject\""
+response_does_not_contain "$result" "\"subjectKeyId\": \"$intermediate_cert_subject_key_id\""
+response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_1_serial_number\""
+response_does_not_contain "$result" "\"serialNumber\": \"$intermediate_cert_2_serial_number\""
+
 echo "Request all certificates should contain only root and leaf certificates"
 result=$(dcld query pki all-x509-certs)
 echo $result | jq

integration_tests/grpc_rest/pki/helpers.go

@@ -1844,7 +1844,6 @@ func Demo(suite *utils.TestSuite) {
 	suite.AssertNotFound(err)
 
 	// Remove x509 by subject, subject key id and serial number
-
 	// Add intermediate certificates
 	msgAddX509Cert = pkitypes.MsgAddX509Cert{
 		Cert:   testconstants.IntermediateWithSameSubjectAndSKID1,

types/pki/errors.go

@@ -292,6 +292,16 @@ func NewErrMessageVidNotEqualAccountVid(msgVid int32, accountVid int32) error {
 	return sdkerrors.Wrapf(ErrMessageVidNotEqualAccountVid, "Message vid=%d is not equal to account vid=%d", msgVid, accountVid)
 }
 
+func NewErrMessageRemoveRoot(subject string, subjectKeyID string) error {
+	return sdkerrors.Wrapf(ErrInappropriateCertificateType, "Inappropriate Certificate Type: Certificate with subject=%s and subjectKeyID=%s "+
+		"is a root certificate.", subject, subjectKeyID,
+	)
+}
+
+func NewErrMessageOnlyOwnerCanExecute(command string) error {
+	return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Only owner can revoke certificate using %s", command)
+}
+
 func NewErrUnsupportedOperation(e interface{}) error {
 	return sdkerrors.Wrapf(ErrUnsupportedOperation, "%v", e)
 }

x/pki/handler_test.go

@@ -1676,7 +1676,7 @@ func TestHandler_RevokeX509Cert_BySerialNumber(t *testing.T) {
 	require.Equal(t, testconstants.IntermediateSubjectKeyID, revokedCerts.SubjectKeyId)
 }
 
-func TestHandler_RemoveX509Cert(t *testing.T) {
+func TestHandler_RemoveX509Cert_BySubjectAndSKID(t *testing.T) {
 	setup := Setup(t)
 	// propose and approve x509 root certificate
 	rootCertOptions := &rootCertOptions{
@@ -1726,22 +1726,40 @@ func TestHandler_RemoveX509Cert(t *testing.T) {
 	leafCerts, _ := queryApprovedCertificates(setup, testconstants.LeafCertWithSameSubjectAndSKIDSubject, testconstants.LeafCertWithSameSubjectAndSKIDSubjectKeyID)
 	require.Equal(t, 1, len(leafCerts.Certs))
 	require.Equal(t, testconstants.LeafCertWithSameSubjectAndSKIDSerialNumber, leafCerts.Certs[0].SerialNumber)
+}
+
+func TestHandler_RemoveX509Cert_BySerialNumber(t *testing.T) {
+	setup := Setup(t)
+	// propose and approve x509 root certificate
+	rootCertOptions := &rootCertOptions{
+		pemCert:      testconstants.RootCertWithSameSubjectAndSKID1,
+		subject:      testconstants.RootCertWithSameSubjectAndSKIDSubject,
+		subjectKeyID: testconstants.RootCertWithSameSubjectAndSKIDSubjectKeyID,
+		info:         testconstants.Info,
+		vid:          65521,
+	}
+	proposeAndApproveRootCertificate(setup, setup.Trustee1, rootCertOptions)
 
 	// Add two intermediate certificates again
-	addIntermediateX509Cert = types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateWithSameSubjectAndSKID1)
-	_, err = setup.Handler(setup.Ctx, addIntermediateX509Cert)
+	addIntermediateX509Cert := types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateWithSameSubjectAndSKID1)
+	_, err := setup.Handler(setup.Ctx, addIntermediateX509Cert)
 	require.NoError(t, err)
 	addIntermediateX509Cert = types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateWithSameSubjectAndSKID2)
 	_, err = setup.Handler(setup.Ctx, addIntermediateX509Cert)
 	require.NoError(t, err)
 
+	// Add a leaf certificate
+	addLeafX509Cert := types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.LeafCertWithSameSubjectAndSKID)
+	_, err = setup.Handler(setup.Ctx, addLeafX509Cert)
+	require.NoError(t, err)
+
 	intermediateCerts, _ := queryApprovedCertificates(setup, testconstants.IntermediateCertWithSameSubjectAndSKIDSubject, testconstants.IntermediateCertWithSameSubjectAndSKIDSubjectKeyID)
 	require.Equal(t, 2, len(intermediateCerts.Certs))
 	require.Equal(t, testconstants.IntermediateCertWithSameSubjectAndSKIDSubject, intermediateCerts.Certs[0].Subject)
 	require.Equal(t, testconstants.IntermediateCertWithSameSubjectAndSKIDSubjectKeyID, intermediateCerts.Certs[0].SubjectKeyId)
 
 	// remove  intermediate certificate by serial number
-	removeX509Cert = types.NewMsgRemoveX509Cert(
+	removeX509Cert := types.NewMsgRemoveX509Cert(
 		setup.Trustee1.String(),
 		testconstants.IntermediateCertWithSameSubjectAndSKIDSubject,
 		testconstants.IntermediateCertWithSameSubjectAndSKIDSubjectKeyID,
@@ -1751,10 +1769,10 @@ func TestHandler_RemoveX509Cert(t *testing.T) {
 	require.NoError(t, err)
 
 	// check that only root, intermediate(with serial number 3) and leaf certificates exists
-	allCerts, _ = queryAllApprovedCertificates(setup)
+	allCerts, _ := queryAllApprovedCertificates(setup)
 	require.Equal(t, 3, len(allCerts))
 	require.Equal(t, 3, len(allCerts[0].Certs)+len(allCerts[1].Certs)+len(allCerts[2].Certs))
-	leafCerts, _ = queryApprovedCertificates(setup, testconstants.LeafCertWithSameSubjectAndSKIDSubject, testconstants.LeafCertWithSameSubjectAndSKIDSubjectKeyID)
+	leafCerts, _ := queryApprovedCertificates(setup, testconstants.LeafCertWithSameSubjectAndSKIDSubject, testconstants.LeafCertWithSameSubjectAndSKIDSubjectKeyID)
 	require.Equal(t, 1, len(leafCerts.Certs))
 
 	intermediateCerts, _ = queryApprovedCertificates(setup, testconstants.IntermediateCertWithSameSubjectAndSKIDSubject, testconstants.IntermediateCertWithSameSubjectAndSKIDSubjectKeyID)
@@ -1782,6 +1800,147 @@ func TestHandler_RemoveX509Cert(t *testing.T) {
 	require.Equal(t, 1, len(leafCerts.Certs))
 }
 
+func TestHandler_RemoveX509Cert_RevokedCertificate(t *testing.T) {
+	setup := Setup(t)
+	// propose and approve x509 root certificate
+	rootCertOptions := &rootCertOptions{
+		pemCert:      testconstants.RootCertPem,
+		subject:      testconstants.RootSubject,
+		subjectKeyID: testconstants.RootSubjectKeyID,
+		info:         testconstants.Info,
+		vid:          65521,
+	}
+	proposeAndApproveRootCertificate(setup, setup.Trustee1, rootCertOptions)
+
+	// Add two intermediate certificates again
+	addIntermediateX509Cert := types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateCertPem)
+	_, err := setup.Handler(setup.Ctx, addIntermediateX509Cert)
+	require.NoError(t, err)
+
+	intermediateCerts, _ := queryApprovedCertificates(setup, testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID)
+	require.Equal(t, 1, len(intermediateCerts.Certs))
+	require.Equal(t, testconstants.IntermediateSubject, intermediateCerts.Certs[0].Subject)
+	require.Equal(t, testconstants.IntermediateSubjectKeyID, intermediateCerts.Certs[0].SubjectKeyId)
+
+	// revoke intermediate certificate by serial number
+	revokeX509Cert := types.NewMsgRevokeX509Cert(
+		setup.Trustee1.String(),
+		testconstants.IntermediateSubject,
+		testconstants.IntermediateSubjectKeyID,
+		testconstants.IntermediateSerialNumber,
+		testconstants.Info,
+	)
+	_, err = setup.Handler(setup.Ctx, revokeX509Cert)
+	require.NoError(t, err)
+
+	_, err = queryApprovedCertificates(setup, testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID)
+	require.Equal(t, codes.NotFound, status.Code(err))
+
+	revokedCerts, _ := queryRevokedCertificates(setup, testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID)
+	require.Equal(t, 1, len(revokedCerts.Certs))
+	require.Equal(t, testconstants.IntermediateSubject, revokedCerts.Certs[0].Subject)
+	require.Equal(t, testconstants.IntermediateSubjectKeyID, revokedCerts.Certs[0].SubjectKeyId)
+
+	// remove  intermediate certificate by serial number
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		setup.Trustee1.String(),
+		testconstants.IntermediateSubject,
+		testconstants.IntermediateSubjectKeyID,
+		testconstants.IntermediateSerialNumber,
+	)
+	_, err = setup.Handler(setup.Ctx, removeX509Cert)
+	require.NoError(t, err)
+
+	allCerts, _ := queryAllApprovedCertificates(setup)
+	require.Equal(t, 1, len(allCerts))
+	require.Equal(t, true, allCerts[0].Certs[0].IsRoot)
+
+	_, err = queryApprovedCertificates(setup, testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID)
+	require.Equal(t, codes.NotFound, status.Code(err))
+	_, err = queryRevokedCertificates(setup, testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID)
+	require.Equal(t, codes.NotFound, status.Code(err))
+}
+
+func TestHandler_RemoveX509Cert_CertificateDoesNotExist(t *testing.T) {
+	setup := Setup(t)
+
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		setup.Trustee1.String(), testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID, testconstants.IntermediateSerialNumber)
+	_, err := setup.Handler(setup.Ctx, removeX509Cert)
+	require.Error(t, err)
+	require.True(t, pkitypes.ErrCertificateDoesNotExist.Is(err))
+}
+
+func TestHandler_RemoveX509Cert_EmptyCertificatesList(t *testing.T) {
+	setup := Setup(t)
+
+	rootCertificate := rootCertificate(setup.Trustee1)
+	setup.Keeper.AddApprovedCertificate(setup.Ctx, rootCertificate)
+
+	setup.Keeper.SetApprovedCertificates(
+		setup.Ctx,
+		types.ApprovedCertificates{
+			Subject:      testconstants.IntermediateSubject,
+			SubjectKeyId: testconstants.IntermediateSubjectKeyID,
+		},
+	)
+
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		setup.Trustee1.String(), testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID, "")
+	_, err := setup.Handler(setup.Ctx, removeX509Cert)
+	require.Error(t, err)
+	require.True(t, pkitypes.ErrCertificateDoesNotExist.Is(err))
+}
+
+func TestHandler_RemoveX509Cert_ByNotOwner(t *testing.T) {
+	setup := Setup(t)
+
+	rootCertificate := rootCertificate(setup.Trustee1)
+	setup.Keeper.AddApprovedCertificate(setup.Ctx, rootCertificate)
+
+	addX509Cert := types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateCertPem)
+	_, err := setup.Handler(setup.Ctx, addX509Cert)
+	require.NoError(t, err)
+
+	anotherTrustee := GenerateAccAddress()
+	setup.AddAccount(anotherTrustee, []dclauthtypes.AccountRole{dclauthtypes.Trustee}, 1)
+
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		anotherTrustee.String(), testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID, "")
+	_, err = setup.Handler(setup.Ctx, removeX509Cert)
+	require.Error(t, err)
+	require.True(t, sdkerrors.ErrUnauthorized.Is(err))
+}
+
+func TestHandler_RemoveX509Cert_ForRootCertificate(t *testing.T) {
+	setup := Setup(t)
+
+	rootCertOptions := createTestRootCertOptions()
+	proposeAndApproveRootCertificate(setup, setup.Trustee1, rootCertOptions)
+
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		setup.Trustee1.String(), testconstants.RootSubject, testconstants.RootSubjectKeyID, testconstants.RootSerialNumber)
+	_, err := setup.Handler(setup.Ctx, removeX509Cert)
+	require.Error(t, err)
+	require.True(t, pkitypes.ErrInappropriateCertificateType.Is(err))
+}
+
+func TestHandler_RemoveX509Cert_InvalidSerialNumber(t *testing.T) {
+	setup := Setup(t)
+
+	rootCertOptions := createTestRootCertOptions()
+	proposeAndApproveRootCertificate(setup, setup.Trustee1, rootCertOptions)
+
+	addX509Cert := types.NewMsgAddX509Cert(setup.Trustee1.String(), testconstants.IntermediateCertPem)
+	_, err := setup.Handler(setup.Ctx, addX509Cert)
+	require.NoError(t, err)
+
+	removeX509Cert := types.NewMsgRemoveX509Cert(
+		setup.Trustee1.String(), testconstants.IntermediateSubject, testconstants.IntermediateSubjectKeyID, "invalid")
+	_, err = setup.Handler(setup.Ctx, removeX509Cert)
+	require.Error(t, err)
+	require.True(t, pkitypes.ErrCertificateDoesNotExist.Is(err))
+}
 func TestHandler_RevokeX509RootCertsBySubjectKeyId(t *testing.T) {
 	setup := Setup(t)