Add FindAllCertificates method - #71
Closed
mekpavit wants to merge 5 commits into
Closed
Conversation
Add FindAllCertificates method
solcates
reviewed
Sep 14, 2020
solcates
left a comment
solcates
left a comment
Contributor
There was a problem hiding this comment.
Sorry for the delayed review. LGTM, merging. Thanks for the handy contrib.
solcates
approved these changes
Sep 14, 2020
Nicolas-Peiffer
added a commit
that referenced
this pull request
Aug 4, 2026
FindCertificate needs an id, label or serial, and FindAllPairedCertificates only returns certificates that have a matching private key. A caller that knows nothing about a token — the case PR #71 was opened for in August 2020 — had no way to list what is on it. Add the plain enumerator, symmetric with FindAllKeys and FindAllKeyPairs. Take the API from #71, with three changes to the implementation: - Page C_FindObjects instead of taking a single call's worth. The original patch stopped at maxHandlePerFind, silently returning 20 certificates on a token holding more. Enumeration now goes through findKeysWithAttributes, which already pages and is not key-specific, so the batching is right in one place rather than two. - Match only CKC_X_509 objects. A WTLS or attribute certificate is not an X.509 certificate and would fail x509.ParseCertificate, so a single one would hide every real certificate on the token — the shape of #68. The token filters them out instead. - Parse via parseCertificateValue, not x509.ParseCertificate directly, so the null-padded CKA_VALUE handling from d5490a7 applies here too. The original patch would have regressed on the tokens #106 reported. A CKA_VALUE that claims to be X.509 but does not parse is still an error rather than a skip: that is corruption, not a kind of certificate this package cannot represent. The error now names the object by CKA_ID and CKA_LABEL, since failing anonymously part-way through a token full of certificates says nothing about which one is at fault. The label is quoted, not interpolated raw — it is arbitrary bytes chosen by whoever wrote the object. Document that enumeration is not trust. This API's whole audience is the caller who knows nothing about the token, which is exactly the caller most likely to feed the result into a trust store; anyone able to write to the token can add a certificate. Tests import a few certificates and then one more than a batch, matching on full DER so the finder is shown to return the right bytes and not merely the right count. They delete only what they imported: #71 proposed a removeAllCertificates helper that destroys every certificate object on the token, which is not something a test should do to a shared or production HSM. #71 Co-Authored-By: mekpavit <21259908+mekpavit@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Hello @mekpavit , thank you 🙏 for your (old but still relevant) contribution. I am part of the new maintainers of crypto11. In 2026 the big project for crypto11 is to add the support of PKCS#11 v3.2 and the support of the ML-KEM PQC algorithm:
The other big project for 2026 is giving + moving the crypto11 project under the Eclipse Foundation. Closing this PR as it has been implemented by 76aae40 . Original author @mekpavit is mentioned in the commit 76aae40. Targets |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This pull request add new method
FindAllCertificateson*crypto11.Contextclass. The use case of this method is when the client doesn't know any details about certificates stored in HSM token but want to retrieve/lookup for the certificate.Changes:
FindAllCertificatesmethod. The code is based onFindCertificateand was edited for the new functionality. The code duplication betweenFindCertificateandFindAllCertificatescan be removed but I think it's fine to leave it like this for now.TestFindAllCertificatesfunction incertificates_test.goto testFindAllCertificatesmethodremoveAllCertificatesincertificates_test.gohelper function that find and remove all certificates in the tokenremoveAllCertificates()at the end of each test cases incertificates_test.goto make each case independent of each other