Mark securesystemslib.gpg subpackage as internal #792
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
The `gpg` subpackage provides a vaguely defined API (`gpg.functions`) to create signatures, export public keys, and verify signatures. This API and the used formats are incompatible with the securesystemslib signer API. For the sake of a consistent API, the `gpg` subpackage is marked as internal (renamed to `_gpg`) and the above mentioned functionality is exposed via the new signer API. Replacement methods are: - `GPGSigner.import_` (replaces `export_pubkey`) - `GPGSigner.sign` - `GPGKey.verify_signature` Note that public key and signature formats also change, in order to match `Key` and `Signature` interfaces. This means: - signature field `signature` is renamed to `sig` - public key fields `type`, `method` and `hashes` are replaced by `keytype` and `scheme` fields, and - public keys no longer include `subkeys` or key expiration infos. This means that the signature verification function no longer needs to decide, if a key is authorized or valid to verify a given signature. See discussion for context: secure-systems-lab#488 (comment) secure-systems-lab#488 (comment) Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
|
The exception ideas seem reasonable to me |
* KeyNotFoundError, which is raised internally in multiple places, is caught and re-raised as ValueError in GPGSigner * CommandError, which was raised only in one place and close to the user boundary, is replaced directly with OSError. Using these builtin Error classes seems semantically correct, and consistent with other errors expected in GPGSigner. Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
|
Huh, mac tests seem to be running on arm now but python < 3.10 is not available... |
I think it's okay to not test on <3.10 on Mac. |
yeah macos-latest just changed from 12 to 14, and apparently 14 runs on arm now and does not have all python versions. |
Yeah the whole matrix might be a little overkill anyway: would likely be fine to test all pythons on linux and just latest on mac and windows... But the alternative is to pin macos-12 instead of macos-latest |
jku
approved these changes
Apr 23, 2024
Seems fine to me. I think I saw this somewhere recently. Is this what sigstore-python is doing? |
lukpueh
added a commit
to lukpueh/securesystemslib
that referenced
this pull request
Apr 24, 2024
CI started failing recently, because new macos-latest runs on arm, which does not include all older Python versions. As workaround and for the sake of a slimmer test matrix, we drop all but latest Python tests on macOS and Windows. The remaining matrix should still give us reasonable coverage. Related discussion in: secure-systems-lab#792 (comment) Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
lukpueh
added a commit
to lukpueh/securesystemslib
that referenced
this pull request
Apr 24, 2024
CI started failing recently, because new macos-latest runs on arm, which does not include all older Python versions. As workaround and for the sake of a slimmer test matrix, we drop all but latest Python tests on macOS and Windows. The remaining matrix should still give us reasonable coverage. Related discussion in: secure-systems-lab#792 (comment) Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
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.
TODO:CommandErrorandKeyNotFoundErrorraised in publicGPGSignermethods are now internal. This doesn't seem right, even though I doubt that anyone needs to handle those errors. I think we can re-raise the latter asValueError, because the key is not found for a passed keyid.CommandErrorcould maybe be anOSError?done
The
gpgsubpackage provides a vaguely defined API (gpg.functions) to create signatures, export public keys, and verify signatures. This API and the used formats are incompatible with the securesystemslib signer API.For the sake of a consistent API, the
gpgsubpackage is marked as internal (renamed to_gpg) and the above mentioned functionality is exposed via the new signer API. Replacement methods are:GPGSigner.import_(replacesexport_pubkey)GPGSigner.signGPGKey.verify_signatureNote that public key and signature formats also change, in order to match
KeyandSignatureinterfaces. This means:signatureis renamed tosigtype,methodandhashesare replaced bykeytypeandschemefields, andsubkeysor key expiration infos. This means that the signature verification function no longer needs to decide, if a key is authorized or valid to verify a given signature.See discussion for context: #488 (comment), #488 (comment)
Closes #270