Adding tolerance in a few tests for existing objects on token - #79
Closed
ProsaicSatsuma wants to merge 1 commit into
Closed
Adding tolerance in a few tests for existing objects on token#79ProsaicSatsuma wants to merge 1 commit into
ProsaicSatsuma wants to merge 1 commit into
Conversation
Contributor
|
Hello @ProsaicSatsuma , TY 🙏 for your contribution. 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. Maybe For testing we now rely on this project https://github.com/pqctoday-org/pqctoday-hsm A couple of example of our tests improvements: d7ef862, 64bd799, da787f0 |
Nicolas-Peiffer
added a commit
that referenced
this pull request
Aug 4, 2026
Five assertions in keys_test.go counted every matching object on the token rather than the ones the test created, so they only passed against a token that started empty. On a persistent token — CloudHSM, a shared dev HSM, or a rerun after a crash left keys behind — they failed: - TestFindingKeysWithAttributes, searching on CkaValueLen 16 and 32 - TestFindingKeyPairsWithAttributes, searching on CkaKeyType CKK_RSA - TestFindingPrivateKeysWithAttributes, the same search - TestFindingAllKeys and TestFindingAllKeyPairs Take the fix proposed in PR #79, by way of the idiom this branch already uses in TestFindAllCertificates: identify the objects by an attribute instead of counting them. #79 snapshotted a baseline count and asserted n + len(baseline), which still races against anything created concurrently and, more importantly, passes even when the finder returns the wrong keys. Matching on CKA_ID does not. Add two generic helpers, requireKeysFound and requireKeysAbsent, and convert the five sites. Keys whose CKA_ID cannot be read are skipped rather than failing the test, since they are by definition not ours. Searches by label keep their exact counts: the labels are randomBytes(), so those searches are already isolated and the count is the stronger assertion. The CkaValueLen searches also gain an exclusion check. require.Len was the only thing asserting that a 256-bit key does not answer a CkaValueLen=16 query; containment alone would have dropped that, so requireKeysAbsent now states it directly. TestFindingPrivateKeysWithAttributes is not in #79 — it landed in 62bbca8 and reproduced the same pattern. #79 Co-Authored-By: Rob Fitzpatrick <19215121+ProsaicSatsuma@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Closing this PR as implemented for crypto11@v2 by 68c3cbb. Original PR author @ProsaicSatsuma is mentioned in the commit 68c3cbb as his contribution was used as the basis. |
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
2 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.
Adds tolerance in a few tests for existing objects on the token