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

feat: Keyless verification for kwctl {verify,pull,run} #169

Merged
merged 8 commits into from Mar 14, 2022
Merged

feat: Keyless verification for kwctl {verify,pull,run} #169

merged 8 commits into from Mar 14, 2022

Conversation

viccuad
Copy link
Member

@viccuad viccuad commented Feb 25, 2022

Description

Part of kubewarden/policy-server#142

Implement Sigstore keyless verification forkwctl {verify,pull,run}.
Now, accept new --verification-config-path that contains the config from the Sigstore verification RFC.
Previous flags are still supported: they get translated into a VerificationSettings config, to be used with the verification.

Test

Run make test e2e-test.
Added e2e-tests that exercise --verification-config-path.

Additional Information

Depends on kubewarden/policy-fetcher#55.

Tradeoff

Potential improvement

Create verification-config:
Provide better info on missing signatures: opened kubewarden/policy-fetcher#57
Provide default verification-config: opened #170

@@ -21,7 +21,7 @@ kube = { version = "0.68.0", default-features = false, features = ["client", "ru
lazy_static = "1.4.0"
mdcat = "0.25.1"
policy-evaluator = { git = "https://github.com/kubewarden/policy-evaluator", tag = "v0.2.12" }
policy-fetcher = { git = "https://github.com/kubewarden/policy-fetcher", tag = "v0.4.3" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be updated when kubewarden/policy-fetcher#55 is in.

@viccuad viccuad marked this pull request as ready for review March 1, 2022 11:47
@viccuad viccuad requested a review from a team as a code owner March 1, 2022 11:47
@viccuad viccuad moved this from In progress to Pending review in Development Mar 1, 2022
@viccuad
Copy link
Member Author

viccuad commented Mar 1, 2022

I'm puzzled by the GHA errors, GH is trying to pull a commit of policy-fetcher that doesn't exist anymore. It seems like a cache problem, but there's no way to clean the GHA cache manually.
All tests, clippy, etc pass fine locally.

e2e-tests/e2e.bats Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
src/main.rs Outdated Show resolved Hide resolved
@viccuad
Copy link
Member Author

viccuad commented Mar 9, 2022

Rebased on top of main and fixed conflicts, ready for review. Still pointing to my fork of policy-fetcher instead of a release.

@viccuad viccuad moved this from Blocked to Pending review in Development Mar 9, 2022
@viccuad viccuad requested review from a team and flavio March 9, 2022 12:04
Copy link
Member

@flavio flavio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

@viccuad
Copy link
Member Author

viccuad commented Mar 10, 2022

Consumed LatestVerificationConfig, fixed conflicts with main.

Bump needed dependencies, particularly cyclic dependency between rustls,
tracing and policy-fetcher wrt policy-evaluator:

cargo update -p tracing --precise 0.1.31
cargo update -p rustls --precise 0.20.3
Now we accept:

```console
kwctl verify --verification-config-path
kwctl pull --verification-config-path
kwctl run --verification-config-path
```

Still, specific flags have precedence over
`--verification-config-path`, respectively:

```console
kwctl verify -v foo -k bar=baz
kwctl pull -v foo -k bar=baz
kwctl run -v foo -k bar=baz
```
`build_verification_config_from_flags()` creates a config from flags.
`verification_options()` now tries to:
1. create config from flags.
2. If there's no flags, tries to use `--verification-config-path`.
3. If there's no `--verification-config-path`, looks at default path.
4. If there's no config in default path, it returns Ok(None).

Remove `verify::read_key_file()`, and use
`policy-fetcher::verify::config::read_verification_file()` instead.
Now `kwctl::verify::verify()` expects a `VerificationSettings`, instead
of a key and its annotations.
Since `verification_options()` now returns an `Option` as do
`sources_options()` and `sigstore_options()`, we can use it to enable
verification.

For verify, expect verification_options to be `Some`.
For pull, run, verify if verification_options is `Some`.
@viccuad
Copy link
Member Author

viccuad commented Mar 10, 2022

we need to release a new policy-fetcher and consume it in this pr. We either release policy-fetcher as it is, or we get kubewarden/policy-fetcher#57 in, too.

From:

```
Error: Policy registry://ghcr.io/kubewarden/tests/pod-privileged:v0.1.9 cannot be validated: Image verification failed: missing signatures
```

To:

```
Error: Policy registry://ghcr.io/kubewarden/tests/pod-privileged:v0.1.9 cannot be validated
Image verification failed: missing signatures
```
@viccuad viccuad linked an issue Mar 11, 2022 that may be closed by this pull request
@viccuad
Copy link
Member Author

viccuad commented Mar 14, 2022

Merging and consuming policy-fetcher from main, as we foresee that we will have several policy-fetcher releases shortly.

@viccuad viccuad merged commit 481bd1b into kubewarden:main Mar 14, 2022
Development automation moved this from Pending review to Done (weekly) Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development
Done (weekly)
Development

Successfully merging this pull request may close these issues.

Handle keyless verification
2 participants