Skip to content
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

Merged

Conversation

cgwalters
Copy link
Member

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.

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.
Copy link
Member

@d4s d4s left a 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

@openshift-ci-robot
Copy link
Collaborator

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@d4s
Copy link
Member

d4s commented May 23, 2020

/test sanity

@cgwalters
Copy link
Member Author

/override continuous-integration/jenkins/pr-merge
xref coreos/coreos-assembler#1478

@openshift-ci-robot
Copy link
Collaborator

@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge

In response to this:

/override continuous-integration/jenkins/pr-merge
xref coreos/coreos-assembler#1478

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.

@openshift-merge-robot openshift-merge-robot merged commit 8e02597 into ostreedev:master May 24, 2020
cgwalters added a commit to cgwalters/ostree that referenced this pull request May 28, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants