Change default keytype to RSA #154
Merged
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
* Add a test for simple targets rollback * Fix the test for targets rollback when metafile hashes are used This now leads to the latter test failing because of theupdateframework#128: This PR can be used to test the fix for theupdateframework#128. This PR should not be merged before that. Signed-off-by: Jussi Kukkonen <[email protected]>
We can parametrize these tests so that they work both with and without hashes: * Replace two targets rollback tests with a parametrized test * Replace on snapshot rollback test with a parametrized test (the "with hashes" case was not tested before) Signed-off-by: Jussi Kukkonen <[email protected]>
This is to get deterministic signatures: this is a workaround to get the hash tests fixed now. Fix the hash test in test_basic: it was incorrect. Signed-off-by: Jussi Kukkonen <[email protected]>
|
I'm leaving this a draft until we know if this is acceptable to known external users... |
This comment was marked as outdated.
This comment was marked as outdated.
This is to get deterministic signatures: this is a workaround to get the hash tests fixed now. * Use RSA as the default keytype * Fix the hash test in test_basic: it was incorrect. * Create a list of signing keys at repository_simulator.py import time: Use these keys in all tests to avoid hundreds of keys being generated (as RSA key generation is slow) Signed-off-by: Jussi Kukkonen <[email protected]>
segiddins
approved these changes
Aug 19, 2024
This was
linked to
issues
Aug 19, 2024
segiddins
approved these changes
Aug 19, 2024
This wraps popping the next signing key from the pregenerated list making it: * more user friendly if we run out of keys * better for the likely near future where we want to generate many more keytypes This commit only supports rsa-pkcs1v15-sha256 but others should be trivial to add now. Signed-off-by: Jussi Kukkonen <[email protected]>
|
last force push fixes a naming mixup in exception message |
AdamKorcz
approved these changes
Aug 20, 2024
|
I just found out there is a PR to remove support for this keytype in go-tuf: This seems a bit reckless to me but let's make sure that's not the plan ... |
|
No, reading the PR code, it does keep the compatibility. We are good to go |
segiddins
reviewed
Aug 20, 2024
| assert client.version(Snapshot.type) == 1 | ||
| # Verify client refuses targets that does not match hashes | ||
| assert client.refresh(init_data) == 1 | ||
| assert client.version(Snapshot.type) == 3 |
There was a problem hiding this comment.
shouldn't this be v2, since the refresh fails due to the snapshot hash mismatch?
There was a problem hiding this comment.
The mismatch is in targets hash (that is stored in snapshot) -- the docstring could make this clearer:
- snapshot v3 is a valid snapshot (with meta that contains targets v2).
- Updating to targets v2 fails because it does not match the hash/length in snapshot meta.
There was a problem hiding this comment.
ah i think this is actually surfacing a bug in my implementation!
There was a problem hiding this comment.
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.
This is to get deterministic signatures: this is a workaround to get the hash tests fixed now (#128). The change of default might be temporary but all other changes are still useful if we switch back to default ecdsa later (like only generating 8 keys instead of 600 keys):
* Use RSA as the default keytype
* Create a list of RSA signers at repository_simulator.py import time: Use these in all tests to avoid generating keys in every test
* Fix the hash test in test_basic.py: it was incorrect.
* Fix and parametrize the rollback tests (these are the commits from #151)
The last bullet point verifies that this works as I wrote the tests in #151 so they fail with main: now they succeed with this keytype change