-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mark securesystemslib.gpg subpackage as internal #792
Conversation
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels right: all of this code is more or less an implementation detail of GPGSigner.
Seems fine to me. I think I saw this somewhere recently. Is this what sigstore-python is doing? |
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>
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>
TODO:CommandError
andKeyNotFoundError
raised in publicGPGSigner
methods 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.CommandError
could maybe be anOSError
?done
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_
(replacesexport_pubkey
)GPGSigner.sign
GPGKey.verify_signature
Note that public key and signature formats also change, in order to match
Key
andSignature
interfaces. This means:signature
is renamed tosig
type
,method
andhashes
are replaced bykeytype
andscheme
fields, andsubkeys
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: #488 (comment), #488 (comment)
Closes #270