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

Support for Cosign 2.0 #7248

Merged
merged 137 commits into from
Aug 15, 2023
Merged

Support for Cosign 2.0 #7248

merged 137 commits into from
Aug 15, 2023

Conversation

vishal-chdhry
Copy link
Member

@vishal-chdhry vishal-chdhry commented May 20, 2023

Signed-off-by: Vishal Choudhary sendtovishalchoudhary@gmail.com

Explanation

Related issue

Closes #6401

Upgrades to Cosign 2.0

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

@vishal-chdhry
Copy link
Member Author

vishal-chdhry commented May 20, 2023

After updating, the following test is failing with error expected a signed timestamp to verify an expired certificate

_, err = verifier.VerifySignature(context.TODO(), opts)
assert.NilError(t, err)

The error originates here: https://github.com/sigstore/cosign/blob/f21081a1820939cddc41c1c119632a6c489d57fd/pkg/cosign/verify.go#L744-L754C3

In Cosign 2.0:

By default, artifact signatures will be uploaded to Rekor, for both key-based and identity-based signing. To not upload to Rekor, include --tlog-upload=false.

Since, artifacts signatrues are uploaded to Rekor in cosign 2.0, if I don't set IgnoreTlog to true, this part will throw and error as Rekor is not provided: https://github.com/sigstore/cosign/blob/f21081a1820939cddc41c1c119632a6c489d57fd/pkg/cosign/verify.go#L982-L984

The error mentioned in the beginning only depends on IgnoreTlog as acceptableRFC3161Time will always be nil as we are not providing TSA Certificates: https://github.com/sigstore/cosign/blob/f21081a1820939cddc41c1c119632a6c489d57fd/pkg/cosign/verify.go#L623-L631

To fix this, I believe we will either have to provide a cert that is not expired or provide a rekor url

@vishal-chdhry vishal-chdhry changed the title Support for Cosign 2.0 [WIP] Support for Cosign 2.0 May 20, 2023
@vishal-chdhry vishal-chdhry marked this pull request as draft May 20, 2023 18:10
@JimBugwadia
Copy link
Member

@vishal-chdhry - the Rekor address is optional in Kyverno policy rules.

Can we set IgnoreTlog to true when rekor is set to nil?

@vishal-chdhry
Copy link
Member Author

Can we set IgnoreTlog to true when rekor is set to nil?

@JimBugwadia Yes, I have updated that in the buildCosignOptions

But in the new version they have added checks for expired certificates here: sigstore/cosign@f21081a/pkg/cosign/verify.go#L744-L754C3

This part throws an error in the tests, We will have to update the test image with a fresh signature

@JimBugwadia
Copy link
Member

But in the new version they have added checks for expired certificates here: sigstore/cosign@f21081a/pkg/cosign/verify.go#L744-L754C3
This part throws an error in the tests, We will have to update the test image with a fresh signature

Ok. Its hard to tell which test is failing due to that change, as there are other errors in your test run. Can you please fix this?

Error: ../../../go/pkg/mod/github.com/in-toto/in-toto-golang@v0.8.0/in_toto/model.go:1079:21: s.signer.Verify undefined (type *dsse.EnvelopeSigner has no field or method Verify)

@vishal-chdhry
Copy link
Member Author

vishal-chdhry commented May 22, 2023

@JimBugwadia there was some issue with the in-toto pkg which got fixed when i updated the version to 0.9.

Here is the error that i get now

--- FAIL: TestCosignKeyless (7.66s)
    kyverno/pkg/cosign/cosign_test.go:92: assertion failed: expected error to contain "subject mismatch: expected jim, received jim@nirmata.com", got "no matching signatures:\nexpected a signed timestamp to verify an expired certificate\n expected a signed timestamp to verify an expired certificate"
FAIL
FAIL	github.com/kyverno/kyverno/pkg/cosign	8.529s
FAIL

which says expected a signed timestamp to verify an expired certificate: Fixed

@JimBugwadia
Copy link
Member

So, for the TestCosignKeyless test are we using the public Rekor? You may need to set opts. RekorURL so it fetches the certs from the public tlog.

@vishal-chdhry vishal-chdhry changed the title [WIP] Support for Cosign 2.0 Support for Cosign 2.0 May 24, 2023
@vishal-chdhry vishal-chdhry marked this pull request as ready for review May 24, 2023 17:53
@JimBugwadia
Copy link
Member

@vishal-chdhry - please address the DCO errors.

vishal-chdhry and others added 17 commits May 25, 2023 00:42
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
* lazy evaluate vars in conditions

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* remove unnecessary conversion

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix test

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* Update test/conformance/kuttl/validate/clusterpolicy/standard/variables/lazyload/conditions/03-manifests.yaml

Signed-off-by: shuting <shutting06@gmail.com>

* Update test/conformance/kuttl/validate/clusterpolicy/standard/variables/lazyload/README.md

Signed-off-by: shuting <shutting06@gmail.com>

* added error check in test

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

---------

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: shuting <shutting06@gmail.com>
Co-authored-by: shuting <shutting06@gmail.com>
Co-authored-by: kyverno-bot <104836976+kyverno-bot@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: bakito <github@bakito.ch>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
…kyverno#7270)

Bumps [svenstaro/upload-release-action](https://github.com/svenstaro/upload-release-action) from 2.5.0 to 2.6.0.
- [Release notes](https://github.com/svenstaro/upload-release-action/releases)
- [Changelog](https://github.com/svenstaro/upload-release-action/blob/master/CHANGELOG.md)
- [Commits](svenstaro/upload-release-action@7319e47...58d5258)

---
updated-dependencies:
- dependency-name: svenstaro/upload-release-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
kyverno#7272)

Bumps [sigs.k8s.io/controller-runtime](https://github.com/kubernetes-sigs/controller-runtime) from 0.14.6 to 0.15.0.
- [Release notes](https://github.com/kubernetes-sigs/controller-runtime/releases)
- [Changelog](https://github.com/kubernetes-sigs/controller-runtime/blob/main/RELEASE.md)
- [Commits](kubernetes-sigs/controller-runtime@v0.14.6...v0.15.0)

---
updated-dependencies:
- dependency-name: sigs.k8s.io/controller-runtime
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Charles-Edouard Brétéché <charles.edouard@nirmata.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
…verno#7274)

Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.6.1 to 5.7.0.
- [Release notes](https://github.com/go-git/go-git/releases)
- [Commits](go-git/go-git@v5.6.1...v5.7.0)

---
updated-dependencies:
- dependency-name: github.com/go-git/go-git/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
…verno#7274)

Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.6.1 to 5.7.0.
- [Release notes](https://github.com/go-git/go-git/releases)
- [Commits](go-git/go-git@v5.6.1...v5.7.0)

---
updated-dependencies:
- dependency-name: github.com/go-git/go-git/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
…verno#7274)

Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.6.1 to 5.7.0.
- [Release notes](https://github.com/go-git/go-git/releases)
- [Commits](go-git/go-git@v5.6.1...v5.7.0)

---
updated-dependencies:
- dependency-name: github.com/go-git/go-git/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
Signed-off-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com>
@eddycharly eddycharly enabled auto-merge (squash) August 14, 2023 13:35
@realshuting realshuting enabled auto-merge (squash) August 15, 2023 10:48
auto-merge was automatically disabled August 15, 2023 10:49

Pull request was closed

@realshuting realshuting reopened this Aug 15, 2023
@realshuting realshuting enabled auto-merge (squash) August 15, 2023 10:49
@realshuting realshuting merged commit e9e4429 into kyverno:main Aug 15, 2023
395 of 400 checks passed
@realshuting
Copy link
Member

Great work folks🎉

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.

[Feature] Support for Cosign 2.0