-
Notifications
You must be signed in to change notification settings - Fork 307
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
pull: Add support for sign-verify=<list> #2105
pull: Add support for sign-verify=<list> #2105
Conversation
3e87f15
to
95fe404
Compare
The goal here is to move the code towards a model where the *client* can explicitly specify which signature types are acceptable. We retain support for `sign-verify=true` for backwards compatibility. But in that configuration, a missing public key is just "no signatures found". With `sign-verify=ed25519` and no key configured, we can explicitly say `No keys found for required signapi type ed25519` which is much, much clearer. Implementation side, rather than maintaining `gboolean sign_verify` *and* `GPtrArray sign_verifiers`, just have the array. If it's `NULL` that means not to verify. Note that currently, an explicit list is an OR of signatures, not AND. In practice...I think most people are going to be using a single entry anyways.
95fe404
to
5cb9d0d
Compare
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.
@cgwalters I really like the way you did in this MR. Thanks!
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, d4s The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test sanity |
/override continuous-integration/jenkins/pr-merge |
@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
In ostreedev@588f42e we added a way to add keys for sign types when doing a `remote add`, and in ostreedev#2105 we extended `sign-verify` to support *limiting* to an explicit set. This PR changes the *default* for `remote add` to combine the two - when providing an explicit `--sign-verify=type`, we now limit the accepted types to only those.
The goal here is to move the code towards a model
where the client can explicitly specify which signature types
are acceptable.
We retain support for
sign-verify=true
for backwards compatibility.But in that configuration, a missing public key is just "no signatures found".
With
sign-verify=ed25519
and no key configured, we canexplicitly say
No keys found for required signapi type ed25519
which is much, much clearer.
Implementation side, rather than maintaining
gboolean sign_verify
andGPtrArray sign_verifiers
, just have the array. If it'sNULL
that meansnot to verify.
Note that currently, an explicit list is an OR of signatures, not AND.
In practice...I think most people are going to be using a single entry
anyways.