Skip to content

Commit 8bb374c

Browse files
AbdulboisAbdulbois
committed
#524 Enable revocation of NOC non-root certificates
Fix bug related to removing certs from subject-key-id map Signed-off-by: Abdulbois <abdulbois.tursunov@dsr-corporation.com> Signed-off-by: Abdulbois <abdulbois123@gmail.com>
1 parent f4d7477 commit 8bb374c

9 files changed

+41
-35
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ response_does_not_contain "$result" "\"serialNumber\": \"$noc_cert_1_serial_numb
424424
response_does_not_contain "$result" "\"serialNumber\": \"$noc_cert_1_copy_serial_number\""
425425
echo $result | jq
426426

427-
echo "Request NOC certificate by VID = $vid should contain ony leaf certificate"
427+
echo "Request NOC certificate by VID = $vid should contain one leaf certificate"
428428
result=$(dcld query pki noc-x509-certs --vid="$vid")
429429
echo $result | jq
430430
check_response "$result" "\"subject\": \"$noc_leaf_cert_1_subject\""

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ check_response "$result" "\"serialNumber\": \"$noc_cert_2_serial_number\""
204204
check_response "$result" "\"serialNumber\": \"$noc_cert_2_copy_serial_number\""
205205
check_response "$result" "\"serialNumber\": \"$noc_leaf_cert_2_serial_number\""
206206

207-
echo "$vendor_account Vendor revokes root NOC certificate by setting \"revoke-child\" flag to true, it should revoke child certificates too"
207+
echo "$vendor_account Vendor revokes non-root NOC certificate by setting \"revoke-child\" flag to true, it should revoke child certificates too"
208208
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)
209209
check_response "$result" "\"code\": 0"
210210

‎x/pki/handler_revoke_noc_cert_test.go

-11
Original file line numberDiff line numberDiff line change
@@ -202,12 +202,6 @@ func TestHandler_RevokeNocX509Cert_RevokeDefault(t *testing.T) {
202202
require.Equal(t, testconstants.NocCert1Subject, revokedNocCerts.Subject)
203203
require.Equal(t, testconstants.NocCert1SubjectKeyID, revokedNocCerts.SubjectKeyId)
204204

205-
revokedCerts, err := queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID)
206-
require.NoError(t, err)
207-
require.Equal(t, 2, len(revokedCerts.Certs))
208-
require.Equal(t, testconstants.NocCert1Subject, revokedCerts.Subject)
209-
require.Equal(t, testconstants.NocCert1SubjectKeyID, revokedCerts.SubjectKeyId)
210-
211205
// query noc certificate by Subject
212206
_, err = queryApprovedCertificatesBySubject(setup, testconstants.NocCert1Subject)
213207
require.Error(t, err)
@@ -396,11 +390,6 @@ func TestHandler_RevokeNocX509Cert_RevokeBySerialNumber(t *testing.T) {
396390
require.Equal(t, 1, len(revokedNocCerts.Certs))
397391
require.Equal(t, testconstants.NocCert1SerialNumber, revokedNocCerts.Certs[0].SerialNumber)
398392

399-
revokedCerts, err := queryRevokedCertificates(setup, testconstants.NocCert1Subject, testconstants.NocCert1SubjectKeyID)
400-
require.NoError(t, err)
401-
require.Equal(t, 1, len(revokedCerts.Certs))
402-
require.Equal(t, testconstants.NocCert1SerialNumber, revokedCerts.Certs[0].SerialNumber)
403-
404393
// Child certificate should not be revoked
405394
_, err = queryRevokedCertificates(setup, testconstants.NocLeafCert1Subject, testconstants.NocLeafCert1SubjectKeyID)
406395
require.Equal(t, codes.NotFound, status.Code(err))

‎x/pki/keeper/approved_certificates_by_subject_key_id.go

+32
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ func (k Keeper) RemoveApprovedCertificatesBySubjectKeyID(
9898
}
9999
}
100100

101+
func (k Keeper) RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx sdk.Context, subject, subjectKeyID, serialNumber string) {
102+
k._removeCertificatesFromSubjectKeyIDState(ctx, subjectKeyID, func(cert *types.Certificate) bool {
103+
return cert.Subject == subject && cert.SubjectKeyId == subjectKeyID && cert.SerialNumber == serialNumber
104+
})
105+
}
106+
101107
// GetAllApprovedCertificatesBySubjectKeyID returns all approvedCertificatesBySubjectKeyId.
102108
func (k Keeper) GetAllApprovedCertificatesBySubjectKeyID(ctx sdk.Context) (list []types.ApprovedCertificatesBySubjectKeyId) {
103109
store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.ApprovedCertificatesBySubjectKeyIDKeyPrefix))
@@ -113,3 +119,29 @@ func (k Keeper) GetAllApprovedCertificatesBySubjectKeyID(ctx sdk.Context) (list
113119

114120
return
115121
}
122+
123+
func (k Keeper) _removeCertificatesFromSubjectKeyIDState(ctx sdk.Context, subjectKeyID string, filter func(cert *types.Certificate) bool) {
124+
certs, found := k.GetApprovedCertificatesBySubjectKeyID(ctx, subjectKeyID)
125+
if !found {
126+
return
127+
}
128+
129+
numCertsBefore := len(certs.Certs)
130+
for i := 0; i < len(certs.Certs); {
131+
cert := certs.Certs[i]
132+
if filter(cert) {
133+
certs.Certs = append(certs.Certs[:i], certs.Certs[i+1:]...)
134+
} else {
135+
i++
136+
}
137+
}
138+
139+
if len(certs.Certs) == 0 {
140+
store := prefix.NewStore(ctx.KVStore(k.storeKey), pkitypes.KeyPrefix(types.ApprovedCertificatesBySubjectKeyIDKeyPrefix))
141+
store.Delete(types.ApprovedCertificatesBySubjectKeyIDKey(
142+
subjectKeyID,
143+
))
144+
} else if numCertsBefore > len(certs.Certs) { // Update state only if any certificate is removed
145+
k.SetApprovedCertificatesBySubjectKeyID(ctx, certs)
146+
}
147+
}

‎x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,6 @@ func (k msgServer) _revokeRootCertificate(
131131
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId)
132132
} else {
133133
k.SetApprovedCertificates(ctx, certificates)
134-
k.SetApprovedCertificatesBySubjectKeyID(
135-
ctx,
136-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs},
137-
)
134+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber)
138135
}
139136
}

‎x/pki/keeper/msg_server_remove_x_509_cert.go

+3-6
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50
6161
k.removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &certs)
6262

6363
if foundApproved {
64-
k._removeApprovedX509Cert(ctx, certID, certs)
64+
k._removeApprovedX509Cert(ctx, certID, certs, msg.SerialNumber)
6565
}
6666
if foundRevoked {
6767
k._removeRevokedX509Cert(ctx, certID, certs)
@@ -83,17 +83,14 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50
8383
return &types.MsgRemoveX509CertResponse{}, nil
8484
}
8585

86-
func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) {
86+
func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates, serialNumber string) {
8787
if len(certificates.Certs) == 0 {
8888
k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId)
8989
k.RemoveApprovedCertificateBySubject(ctx, certID.Subject, certID.SubjectKeyId)
9090
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, certID.Subject, certID.SubjectKeyId)
9191
} else {
9292
k.SetApprovedCertificates(ctx, certificates)
93-
k.SetApprovedCertificatesBySubjectKeyID(
94-
ctx,
95-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: certID.SubjectKeyId, Certs: certificates.Certs},
96-
)
93+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, certID.Subject, certID.SubjectKeyId, serialNumber)
9794
}
9895
}
9996

‎x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,7 @@ func (k msgServer) _revokeNocRootCertificate(
115115
} else {
116116
k.RemoveNocRootCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber)
117117
k.SetApprovedCertificates(ctx, certificates)
118-
k.SetApprovedCertificatesBySubjectKeyID(
119-
ctx,
120-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs},
121-
)
118+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber)
122119
}
123120

124121
return nil

‎x/pki/keeper/msg_server_revoke_noc_x_509_cert.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,7 @@ func (k msgServer) _revokeNocCertificate(
8888
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId)
8989
} else {
9090
k.RemoveNocCertificateBySerialNumber(ctx, vid, cert.Subject, cert.SubjectKeyId, serialNumber)
91-
k.SetApprovedCertificatesBySubjectKeyID(
92-
ctx,
93-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs},
94-
)
91+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, serialNumber)
9592
k.SetApprovedCertificates(ctx, certificates)
9693
}
9794

‎x/pki/keeper/msg_server_revoke_x_509_cert.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,6 @@ func (k msgServer) _revokeX509Certificate(ctx sdk.Context, cert *types.Certifica
8787
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, cert.Subject, cert.SubjectKeyId)
8888
} else {
8989
k.SetApprovedCertificates(ctx, certificates)
90-
k.SetApprovedCertificatesBySubjectKeyID(
91-
ctx,
92-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: cert.SubjectKeyId, Certs: certificates.Certs},
93-
)
90+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, cert.Subject, cert.SubjectKeyId, cert.SerialNumber)
9491
}
9592
}

0 commit comments

Comments
 (0)