da_revocation: sample revoked DACs and PAI certs for VID:0xFFF1 PID:0x8001 #36838
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
created the PAI using chip-cert by using
credentials/test/attestation/Chip-Test-PAA-FFF1-{Cert,Key}.pem as
the signing CA.
created the DAC (vid: 0xFFF1, pid: 0x8001) using the generated PAI
Revoked the PAI and created the CRL for the same.
0xFFF1 and PID 0x8001 created the DAC using chip-cert by using credentials/development/attestation/Matter-Development-PAI-FFF1-noPID-Cert.pem as the signing CA. Revoked the DAC and created CRL for the same.
|
Review changes with |
|
PR #36838: Size comparison from 9e203e2 to c24a766 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
credentials/test/attestation/Chip-Test-DAC-FFF1-8001-Singed-By-Revoked-PAI-Cert.pem
Outdated
Show resolved
Hide resolved
tcarmelveilleux
approved these changes
Dec 13, 2024
credentials/development/attestation/Matter-Development-DAC-FFF1-8001-Revoked-01-Cert.pem
Show resolved
Hide resolved
bh3000
reviewed
Dec 13, 2024
There was a problem hiding this comment.
Could you add a set of CRLs and certs for the indirect CRL signing use case as well?
For complete test/example coverage there needs to be versions that includes the CRL entry extension Certificate Issuer as well. Note that there are two approaches here that should be included, the case where each entry has the Certificate Issuer, and the case where the entry's are grouped by Certificate Issuer (multiple entries with the same Certificate Issuer where only the first in the group contain the extension). See section 6.2.4.1. Step 10.a. and RFC 5280 section 5.3.3 for detail.
Can you add those?
credentials/development/attestation/Matter-Development-PAI-FFF1-noPID-CRL.pem
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The CRLs have to be regenerated with the Authority Key Identifier extension matching the CRLSignerCertificates Subject Key Identifier.
There was a problem hiding this comment.
To add this you can update the ca.conf you use to include:
[ crl_ext ]
authorityKeyIdentifier=keyid:always
and within the [ ca ], or where the default_ca points to add:
crl_extensions = crl_ext
There was a problem hiding this comment.
Thanks @bh3000 for the helping out with it. Added the AKID extension to the CRL.
This can be done in a separate PR. No need to do here. |
andy31415
reviewed
Dec 13, 2024
There was a problem hiding this comment.
@shubhamdp please add a Testing entry in the summary of this PR and explain how this change was tested.
Since I see no unit test changes, I assume this will be "manually tested via..." and in that case we would like to intentionally set the bar higher on manual tests (to avoid all PRs setting manual because it is easier) so please spend time to add details on how this was tested as well as a justification why it is impossible for this PR to be tested in an automated fashion.
Added a comment on the original issue to keep track of it #26582 (comment) |
|
PR #36838: Size comparison from 9e203e2 to 9d32b2d Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
@andy31415 Sorry, I wasn't aware about the decision in the TT call. I have updated the Testing section in the PR description. |
There was a problem hiding this comment.
I thought the contents of this dir were auto-generated by running src/tools/chip-cert/dacs.py. Is that not the case? Or if it is, will running that script not affect the new files added here?
There was a problem hiding this comment.
Yeah, I see the certs in this directory are being generated by running src/tools/chip-cert/dacs.py. If present it skips that particular entry.
I think, we can move the revocation certs to separate directory, @tcarmelveilleux @bh3000 ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #26582.
Generated the certificates with the help of
chip-cert(please check the commands used below).Generated and updated CRL using
openssl cafunctionality.Next update time is set to 99 years.
chip-cert commands used for generating DACs
chip-cert commands used for generating PAI and DAC
Testing
This PR is the building block for automating the DA revocation implementation.
It adds the test data for generating the device attestation revocation set.
Newly added test data is manually tested by dumping the certs/kets/crl using
openssl [x509 | ec | crl]command. Also verified if both.derand.pempairs outputs the same data.