Skip to content

Commit a0aa8a4

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 a0aa8a4

11 files changed

+83
-80
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.go

-26
Original file line numberDiff line numberDiff line change
@@ -155,29 +155,3 @@ func (k Keeper) verifyCertificate(ctx sdk.Context,
155155
fmt.Sprintf("Certificate verification failed for certificate with subject=%v and subjectKeyID=%v",
156156
x509Certificate.Subject, x509Certificate.SubjectKeyID))
157157
}
158-
159-
func (k Keeper) removeCertFromList(issuer string, serialNumber string, certs *types.ApprovedCertificates) {
160-
certIndex := -1
161-
162-
for i, cert := range certs.Certs {
163-
if cert.SerialNumber == serialNumber && cert.Issuer == issuer {
164-
certIndex = i
165-
166-
break
167-
}
168-
}
169-
if certIndex == -1 {
170-
return
171-
}
172-
certs.Certs = append(certs.Certs[:certIndex], certs.Certs[certIndex+1:]...)
173-
}
174-
175-
func findCertificate(serialNumber string, certificates *[]*types.Certificate) (*types.Certificate, bool) {
176-
for _, cert := range *certificates {
177-
if cert.SerialNumber == serialNumber {
178-
return cert, true
179-
}
180-
}
181-
182-
return nil, false
183-
}

‎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/keeper.go

+26
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,29 @@ func (k Keeper) EnsureVidMatches(ctx sdk.Context, owner string, signer string) e
7676

7777
return nil
7878
}
79+
80+
func removeCertFromList(issuer string, serialNumber string, certs *[]*types.Certificate) {
81+
certIndex := -1
82+
83+
for i, cert := range *certs {
84+
if cert.SerialNumber == serialNumber && cert.Issuer == issuer {
85+
certIndex = i
86+
87+
break
88+
}
89+
}
90+
if certIndex == -1 {
91+
return
92+
}
93+
*certs = append((*certs)[:certIndex], (*certs)[certIndex+1:]...)
94+
}
95+
96+
func findCertificate(serialNumber string, certificates *[]*types.Certificate) (*types.Certificate, bool) {
97+
for _, cert := range *certificates {
98+
if cert.SerialNumber == serialNumber {
99+
return cert, true
100+
}
101+
}
102+
103+
return nil, false
104+
}

‎x/pki/keeper/msg_server_approve_revoke_x_509_root_cert.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ func (k msgServer) _revokeRootCertificate(
117117
SubjectKeyId: cert.SubjectKeyId,
118118
Certs: []*types.Certificate{cert},
119119
})
120-
k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates)
120+
removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs)
121121

122122
if len(certificates.Certs) == 0 {
123123
k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId)
@@ -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

+15-21
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,19 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50
5353
// remove from subject with serialNumber map
5454
k.RemoveUniqueCertificate(ctx, certBySerialNumber.Issuer, certBySerialNumber.SerialNumber)
5555

56-
certs := types.ApprovedCertificates{
57-
Subject: msg.Subject,
58-
SubjectKeyId: msg.SubjectKeyId,
59-
Certs: certificates,
60-
}
61-
k.removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &certs)
62-
6356
if foundApproved {
64-
k._removeApprovedX509Cert(ctx, certID, certs)
57+
removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &aprCerts.Certs)
58+
k._removeApprovedX509Cert(ctx, certID, &aprCerts, msg.SerialNumber)
6559
}
6660
if foundRevoked {
67-
k._removeRevokedX509Cert(ctx, certID, certs)
61+
certs := types.ApprovedCertificates{
62+
Subject: revCerts.Subject,
63+
SubjectKeyId: revCerts.SubjectKeyId,
64+
Certs: revCerts.Certs,
65+
}
66+
removeCertFromList(certBySerialNumber.Issuer, certBySerialNumber.SerialNumber, &certs.Certs)
67+
revCerts.Certs = certs.Certs
68+
k._removeRevokedX509Cert(ctx, certID, &revCerts)
6869
}
6970
} else {
7071
k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId)
@@ -83,31 +84,24 @@ func (k msgServer) RemoveX509Cert(goCtx context.Context, msg *types.MsgRemoveX50
8384
return &types.MsgRemoveX509CertResponse{}, nil
8485
}
8586

86-
func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) {
87+
func (k msgServer) _removeApprovedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates *types.ApprovedCertificates, serialNumber string) {
8788
if len(certificates.Certs) == 0 {
8889
k.RemoveApprovedCertificates(ctx, certID.Subject, certID.SubjectKeyId)
8990
k.RemoveApprovedCertificateBySubject(ctx, certID.Subject, certID.SubjectKeyId)
9091
k.RemoveApprovedCertificatesBySubjectKeyID(ctx, certID.Subject, certID.SubjectKeyId)
9192
} else {
92-
k.SetApprovedCertificates(ctx, certificates)
93-
k.SetApprovedCertificatesBySubjectKeyID(
94-
ctx,
95-
types.ApprovedCertificatesBySubjectKeyId{SubjectKeyId: certID.SubjectKeyId, Certs: certificates.Certs},
96-
)
93+
k.SetApprovedCertificates(ctx, *certificates)
94+
k.RemoveApprovedCertificatesBySubjectKeyIDAndSerialNumber(ctx, certID.Subject, certID.SubjectKeyId, serialNumber)
9795
}
9896
}
9997

100-
func (k msgServer) _removeRevokedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates types.ApprovedCertificates) {
98+
func (k msgServer) _removeRevokedX509Cert(ctx sdk.Context, certID types.CertificateIdentifier, certificates *types.RevokedCertificates) {
10199
if len(certificates.Certs) == 0 {
102100
k.RemoveRevokedCertificates(ctx, certID.Subject, certID.SubjectKeyId)
103101
} else {
104102
k.SetRevokedCertificates(
105103
ctx,
106-
types.RevokedCertificates{
107-
Subject: certID.Subject,
108-
SubjectKeyId: certID.SubjectKeyId,
109-
Certs: certificates.Certs,
110-
},
104+
*certificates,
111105
)
112106
}
113107
}

‎x/pki/keeper/msg_server_revoke_noc_root_x_509_cert.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (k msgServer) _revokeNocRootCertificate(
105105
}
106106
k.AddRevokedNocRootCertificates(ctx, revNocCerts)
107107

108-
k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates)
108+
removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs)
109109

110110
if len(certificates.Certs) == 0 {
111111
k.RemoveNocRootCertificate(ctx, vid, certificates.Subject, certificates.SubjectKeyId)
@@ -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

+2-5
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,15 @@ func (k msgServer) _revokeNocCertificate(
8080
}
8181
k.AddRevokedCertificates(ctx, revCerts)
8282

83-
k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates)
83+
removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs)
8484
if len(certificates.Certs) == 0 {
8585
k.RemoveNocCertificate(ctx, certificates.Subject, certificates.SubjectKeyId, vid)
8686
k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId)
8787
k.RemoveApprovedCertificateBySubject(ctx, cert.Subject, cert.SubjectKeyId)
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

+2-5
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func (k msgServer) _revokeX509Certificate(ctx sdk.Context, cert *types.Certifica
7777
SubjectKeyId: cert.SubjectKeyId,
7878
Certs: []*types.Certificate{cert},
7979
})
80-
k.removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates)
80+
removeCertFromList(cert.Issuer, cert.SerialNumber, &certificates.Certs)
8181
if len(certificates.Certs) == 0 {
8282
k.RemoveApprovedCertificates(ctx, cert.Subject, cert.SubjectKeyId)
8383
// Remove certificate identifier from issuer's ChildCertificates record
@@ -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)