Skip to content

Commit 1981ae6

Browse files
AbdulboisAbdulbois
committed
#524 Enable revocation of NOC certificates
Minor refactoring due to comments of PR Signed-off-by: Abdulbois <abdulbois.tursunov@dsr-corporation.com> Signed-off-by: Abdulbois <abdulbois123@gmail.com>
1 parent bd7df90 commit 1981ae6

9 files changed

+96
-21
lines changed

‎integration_tests/cli/pki-noc-certs.sh

+4-2
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,11 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial
315315
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\""
316316
echo $result | jq
317317

318-
echo "Request NOC certificate by VID must not contain revoked root certificates"
318+
echo "Request NOC root certificate by VID = $vid must not contain revoked root certificates"
319319
result=$(dcld query pki noc-x509-root-certs --vid="$vid")
320-
check_response "$result" "Not Found"
320+
check_response "$result" "\"subject\": \"$noc_root_cert_2_subject\""
321+
check_response "$result" "\"subjectKeyId\": \"$noc_root_cert_2_subject_key_id\""
322+
check_response "$result" "\"serialNumber\": \"$noc_root_cert_2_serial_number\""
321323
response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\""
322324
response_does_not_contain "$result" "\"subjectKeyId\": \"$noc_root_cert_1_subject_key_id\""
323325
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial_number\""

‎integration_tests/cli/pki-noc-revocation-with-revoking-child.sh

+3-3
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial
100100
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\""
101101
echo $result | jq
102102

103-
echo "Request NOC certificate by VID should be empty"
103+
echo "Request NOC root certificate by VID = $vid should be empty"
104104
result=$(dcld query pki noc-x509-root-certs --vid="$vid")
105105
check_response "$result" "Not Found"
106106
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
109109
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\""
110110
echo $result | jq
111111

112-
echo "Request all certificates by subject should be empty"
112+
echo "Request all certificates by NOC root certificate's subject should be empty"
113113
result=$(dcld query pki all-subject-x509-certs --subject="$noc_root_cert_1_subject")
114114
check_response "$result" "Not Found"
115115
response_does_not_contain "$result" "\"$noc_root_cert_1_subject\""
116116
response_does_not_contain "$result" "\"$noc_root_cert_1_subject_key_id\""
117117
echo $result | jq
118118

119-
echo "Request all certificates by subjectKeyId should be empty"
119+
echo "Request all certificates by NOC root certificate's subjectKeyId should be empty"
120120
result=$(dcld query pki x509-cert --subject-key-id="$noc_root_cert_1_subject_key_id")
121121
check_response "$result" "Not Found"
122122
response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\""

‎integration_tests/cli/pki-noc-revocation-with-serial-number.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ response_does_not_contain "$result" "\"subjectKeyId\": \"$noc_root_cert_1_subjec
101101
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial_number\""
102102
echo $result | jq
103103

104-
echo "Request NOC certificate by VID should contain only one root certificate with serialNumber=$noc_root_cert_1_copy_serial_number"
104+
echo "Request NOC root certificate by VID = $vid should contain only one root certificate with serialNumber=$noc_root_cert_1_copy_serial_number"
105105
result=$(dcld query pki noc-x509-root-certs --vid="$vid")
106106
check_response "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\""
107107
check_response "$result" "\"subject\": \"$noc_root_cert_1_subject\""
@@ -192,7 +192,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_serial
192192
response_does_not_contain "$result" "\"serialNumber\": \"$noc_root_cert_1_copy_serial_number\""
193193
echo $result | jq
194194

195-
echo "Request NOC certificate by VID should be empty"
195+
echo "Request NOC root certificate by VID = $vid should be empty"
196196
result=$(dcld query pki noc-x509-root-certs --vid="$vid")
197197
check_response "$result" "Not Found"
198198
response_does_not_contain "$result" "\"subject\": \"$noc_root_cert_1_subject\""

‎types/pki/errors.go

+7
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,13 @@ func NewErrMessageRemoveRoot(subject string, subjectKeyID string) error {
357357
)
358358
}
359359

360+
func NewErrMessageExistingCertIsNotRoot(subject string, subjectKeyID string) error {
361+
return sdkerrors.Wrapf(ErrInappropriateCertificateType,
362+
"The existing certificate with the same combination of subject (%v) and subjectKeyID (%v) is not a root certificate",
363+
subject, subjectKeyID,
364+
)
365+
}
366+
360367
func NewErrUnsupportedOperation(e interface{}) error {
361368
return sdkerrors.Wrapf(ErrUnsupportedOperation, "%v", e)
362369
}

‎x/pki/handler_revoke_noc_root_cert_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func TestHandler_RevokeNocX509RootCert_CertificateExists(t *testing.T) {
8181
Vid: testconstants.Vid,
8282
},
8383
nocRoorCert: testconstants.RootCertPem,
84-
err: sdkerrors.ErrUnauthorized,
84+
err: pkitypes.ErrInappropriateCertificateType,
8585
},
8686
{
8787
name: "ExistingNotNocCert",
@@ -235,9 +235,10 @@ func TestHandler_RevokeNocX509RootCert_RevokeDefault(t *testing.T) {
235235
require.Equal(t, 0, len(aprCertsBySubjectKeyID))
236236

237237
// query noc root certificate by VID
238-
_, err = queryNocRootCertificates(setup, testconstants.Vid)
239-
require.Error(t, err)
240-
require.Equal(t, codes.NotFound, status.Code(err))
238+
nocRootCerts, err := queryNocRootCertificates(setup, testconstants.Vid)
239+
require.NoError(t, err)
240+
require.Equal(t, 1, len(nocRootCerts.Certs))
241+
require.Equal(t, testconstants.NocRootCert2SubjectKeyID, nocRootCerts.Certs[0].SubjectKeyId)
241242

242243
// Child certificate should not be revoked
243244
_, err = queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID)

‎x/pki/keeper/child_certificates.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,10 @@ func (k msgServer) RevokeChildCertificates(ctx sdk.Context, issuer string, autho
111111
// Revoke certificates with this subject/subjectKeyID combination
112112
certificates, _ := k.GetApprovedCertificates(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId)
113113
k.AddRevokedCertificates(ctx, certificates)
114-
// FIXME: Should be replaced
114+
// FIXME: Below two lines is not in the context of RevokeChildCertificates method. In future current implementation must be refactored
115115
if len(certificates.Certs) > 0 {
116116
// If cert is NOC then remove it from NOC certificates list
117-
k.RemoveNocCertificates(ctx, certificates.Certs[0].Vid)
117+
k.RemoveNocCertificate(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId, certificates.Certs[0].Vid)
118118
}
119119
k.RemoveApprovedCertificates(ctx, certIdentifier.Subject, certIdentifier.SubjectKeyId)
120120

‎x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go

+4-8
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (k msgServer) RevokeNocRootX509Cert(goCtx context.Context, msg *types.MsgRe
3030

3131
cert := certificates.Certs[0]
3232
if !cert.IsRoot {
33-
return nil, pkitypes.NewErrUnauthorizedCertIssuer(cert.Subject, cert.SubjectKeyId)
33+
return nil, pkitypes.NewErrMessageExistingCertIsNotRoot(cert.Subject, cert.SubjectKeyId)
3434
}
3535
// Existing certificate must be NOC certificate
3636
if !cert.IsNoc {
@@ -71,7 +71,7 @@ func (k msgServer) _revokeNocRootCertificates(ctx sdk.Context, certificates type
7171
k.AddRevokedNocRootCertificates(ctx, types.RevokedNocRootCertificates(certificates))
7272

7373
// Remove certs from NOC and approved lists
74-
k.RemoveNocRootCertificates(ctx, vid)
74+
k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId)
7575
k.RemoveApprovedCertificates(ctx, certificates.Subject, certificates.SubjectKeyId)
7676
// remove from subject -> subject key ID map
7777
k.RemoveApprovedCertificateBySubject(ctx, certificates.Subject, certificates.SubjectKeyId)
@@ -108,16 +108,12 @@ func (k msgServer) _revokeNocRootCertificate(
108108
k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates)
109109

110110
if len(certificates.Certs) == 0 {
111+
k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId)
111112
k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId)
112-
k.RemoveNocRootCertificates(ctx, vid)
113113
k.RemoveApprovedCertificateBySubject(ctx, cert.Subject, cert.SubjectKeyId)
114114
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId)
115115
} else {
116-
certs := types.NocRootCertificates{
117-
Vid: vid,
118-
Certs: certificates.Certs,
119-
}
120-
k.SetNocRootCertificates(ctx, certs)
116+
k.RemoveNocRootCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber)
121117
k.SetApprovedCertificates(ctx, certificates)
122118
k.SetApprovedCertificatesBySubjectKeyID(
123119
ctx,

‎x/pki/keeper/noc_certificates.go

+21
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,27 @@ func (k Keeper) RemoveNocCertificates(
7171
))
7272
}
7373

74+
func (k Keeper) RemoveNocCertificate(ctx sdk.Context, subject, subjectKeyID string, vid int32) {
75+
certs, found := k.GetNocCertificates(ctx, vid)
76+
if !found {
77+
return
78+
}
79+
80+
for i := 0; i < len(certs.Certs); {
81+
if certs.Certs[i].Subject == subject && certs.Certs[i].SubjectKeyId == subjectKeyID {
82+
certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...)
83+
} else {
84+
i++
85+
}
86+
}
87+
88+
if len(certs.Certs) == 0 {
89+
k.RemoveNocCertificates(ctx, vid)
90+
} else {
91+
k.SetNocCertificates(ctx, certs)
92+
}
93+
}
94+
7495
// GetAllNocCertificates returns all nocCertificates.
7596
func (k Keeper) GetAllNocCertificates(ctx sdk.Context) (list []types.NocCertificates) {
7697
store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.NocCertificatesKeyPrefix))

‎x/pki/keeper/noc_root_certificates.go

+48
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,54 @@ func (k Keeper) RemoveNocRootCertificates(
7070
))
7171
}
7272

73+
func (k Keeper) RemoveNocRootCertificate(ctx sdk.Context, vid int32, subject, subjectKeyID string) {
74+
certs, found := k.GetNocRootCertificates(ctx, vid)
75+
if !found {
76+
return
77+
}
78+
79+
certsBefore := len(certs.Certs)
80+
for i := 0; i < len(certs.Certs); {
81+
cert := certs.Certs[i]
82+
if cert.Subject == subject && cert.SubjectKeyId == subjectKeyID {
83+
certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...)
84+
} else {
85+
i++
86+
}
87+
}
88+
89+
if len(certs.Certs) == 0 {
90+
k.RemoveNocRootCertificates(ctx, vid)
91+
} else if certsBefore > len(certs.Certs) { // Update state only if any certificate is removed
92+
k.SetNocRootCertificates(ctx, certs)
93+
}
94+
}
95+
96+
func (k Keeper) RemoveNocRootCertificateBySerialNumber(ctx sdk.Context, vid int32, subject, subjectKeyID, serialNumber string) {
97+
certs, found := k.GetNocRootCertificates(ctx, vid)
98+
if !found {
99+
return
100+
}
101+
102+
certsBefore := len(certs.Certs)
103+
for i := 0; i < len(certs.Certs); {
104+
cert := certs.Certs[i]
105+
if cert.Subject == subject &&
106+
cert.SubjectKeyId == subjectKeyID &&
107+
cert.SerialNumber == serialNumber {
108+
certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...)
109+
} else {
110+
i++
111+
}
112+
}
113+
114+
if len(certs.Certs) == 0 {
115+
k.RemoveNocRootCertificates(ctx, vid)
116+
} else if certsBefore > len(certs.Certs) { // Update state only if any certificate is removed
117+
k.SetNocRootCertificates(ctx, certs)
118+
}
119+
}
120+
73121
// GetAllNocRootCertificates returns all nocRootCertificates.
74122
func (k Keeper) GetAllNocRootCertificates(ctx sdk.Context) (list []types.NocRootCertificates) {
75123
store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.NocRootCertificatesKeyPrefix))

0 commit comments

Comments
 (0)