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: support for KMS keys in cosign signature #384

Closed

Conversation

brennoo
Copy link
Contributor

@brennoo brennoo commented Jan 20, 2024

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

/kind flaky-test

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area library

/area cli

/area tests

/area examples

What this PR does / why we need it:
This PR adds support for KMS providers in the cosign signature verify process, making it possible to use cloud providers KMS, hashicorp vault and kubernetes.

Which issue(s) this PR fixes:

Fixes #380

Special notes for your reviewer:
I've tested key parameter with different values including http://, awskms:// and local file pubkey.pem - it was needed to add the private-infrastructure flag for cases where the transaction is not sent to the transparency log

loresuso and others added 30 commits January 20, 2023 11:09
Co-authored-by: Leonardo Grasso <[email protected]>
Signed-off-by: Lorenzo Susini <[email protected]>
…ecify artifact follow intervals.

Signed-off-by: Federico Di Pierro <[email protected]>
Moreover, avoid shadowing `equal` method.

Signed-off-by: Federico Di Pierro <[email protected]>
… endpoint

The follower retrievs the api versions from falco at start up. The api versions
are used to check if an artifact is compatible with the running instance of falco.
Here we expect falco to sent generic data, and we do not make assumptions on the
data type when unmarshalling the received data.

Signed-off-by: Aldo Lacuku <[email protected]>
Each time a falcoctl artifact (binary/OCI image) is created we make sure that the falcoctl binary
has the version fields set.

Signed-off-by: Aldo Lacuku <[email protected]>
Signed-off-by: Roberto Scolaro <[email protected]>
When setting the build date in the version info, use the ISO 8601
format, same as the goreleaser. This fix affects only the docker image.

Signed-off-by: Aldo Lacuku <[email protected]>
Co-authored-by: Roberto Scolaro <[email protected]>
Signed-off-by: Leonardo Grasso <[email protected]>
Signed-off-by: Roberto Scolaro <[email protected]>
Signed-off-by: Aldo Lacuku <[email protected]>
Co-authored-by: Lorenzo Susini <[email protected]>
Since index files can be overwritten using ENV variables we needed a
new solution to handle them on a per-command basis. Added an in-memory
caching system that handles the index files during the command execution.
Furthermore, it handles loading and saving the index cache from/to filesystem.
The files that tracks the indexes are saved to ~/.config/falcoctl.

Signed-off-by: Aldo Lacuku <[email protected]>
Co-authored-by: Lorenzo Susini <[email protected]>
dependabot bot and others added 4 commits January 24, 2024 21:05
Bumps [github.com/docker/docker](https://github.com/docker/docker) from 24.0.7+incompatible to 25.0.1+incompatible.
- [Release notes](https://github.com/docker/docker/releases)
- [Commits](moby/moby@v24.0.7...v25.0.1)

---
updated-dependencies:
- dependency-name: github.com/docker/docker
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github.com/spf13/viper](https://github.com/spf13/viper) from 1.17.0 to 1.18.2.
- [Release notes](https://github.com/spf13/viper/releases)
- [Commits](spf13/viper@v1.17.0...v1.18.2)

---
updated-dependencies:
- dependency-name: github.com/spf13/viper
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github.com/onsi/gomega](https://github.com/onsi/gomega) from 1.27.8 to 1.31.1.
- [Release notes](https://github.com/onsi/gomega/releases)
- [Changelog](https://github.com/onsi/gomega/blob/master/CHANGELOG.md)
- [Commits](onsi/gomega@v1.27.8...v1.31.1)

---
updated-dependencies:
- dependency-name: github.com/onsi/gomega
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Bumps the gomod group with 5 updates:

| Package | From | To |
| --- | --- | --- |
| [github.com/falcosecurity/driverkit](https://github.com/falcosecurity/driverkit) | `0.16.0` | `0.16.3` |
| [github.com/oras-project/oras-credentials-go](https://github.com/oras-project/oras-credentials-go) | `0.3.0` | `0.3.1` |
| [github.com/pterm/pterm](https://github.com/pterm/pterm) | `0.12.67` | `0.12.76` |
| [github.com/sigstore/cosign/v2](https://github.com/sigstore/cosign) | `2.2.1` | `2.2.2` |
| [oras.land/oras-go/v2](https://github.com/oras-project/oras-go) | `2.3.0` | `2.3.1` |


Updates `github.com/falcosecurity/driverkit` from 0.16.0 to 0.16.3
- [Release notes](https://github.com/falcosecurity/driverkit/releases)
- [Changelog](https://github.com/falcosecurity/driverkit/blob/master/RELEASE.md)
- [Commits](falcosecurity/driverkit@v0.16.0...v0.16.3)

Updates `github.com/oras-project/oras-credentials-go` from 0.3.0 to 0.3.1
- [Release notes](https://github.com/oras-project/oras-credentials-go/releases)
- [Commits](oras-project/oras-credentials-go@v0.3.0...v0.3.1)

Updates `github.com/pterm/pterm` from 0.12.67 to 0.12.76
- [Release notes](https://github.com/pterm/pterm/releases)
- [Commits](pterm/pterm@v0.12.67...v0.12.76)

Updates `github.com/sigstore/cosign/v2` from 2.2.1 to 2.2.2
- [Release notes](https://github.com/sigstore/cosign/releases)
- [Changelog](https://github.com/sigstore/cosign/blob/main/CHANGELOG.md)
- [Commits](sigstore/cosign@v2.2.1...v2.2.2)

Updates `oras.land/oras-go/v2` from 2.3.0 to 2.3.1
- [Release notes](https://github.com/oras-project/oras-go/releases)
- [Commits](oras-project/oras-go@v2.3.0...v2.3.1)

---
updated-dependencies:
- dependency-name: github.com/falcosecurity/driverkit
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: gomod
- dependency-name: github.com/oras-project/oras-credentials-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: gomod
- dependency-name: github.com/pterm/pterm
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: gomod
- dependency-name: github.com/sigstore/cosign/v2
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: gomod
- dependency-name: oras.land/oras-go/v2
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: gomod
...

Signed-off-by: dependabot[bot] <[email protected]>
@brennoo brennoo force-pushed the feat/kms_keys_signatures branch from ab26df1 to 1e7a0ee Compare January 26, 2024 08:29
dependabot bot added 4 commits January 29, 2024 07:32
Bumps [github.com/docker/cli](https://github.com/docker/cli) from 24.0.7+incompatible to 25.0.1+incompatible.
- [Commits](docker/cli@v24.0.7...v25.0.1)

---
updated-dependencies:
- dependency-name: github.com/docker/cli
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.153.0 to 0.159.0.
- [Release notes](https://github.com/googleapis/google-api-go-client/releases)
- [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md)
- [Commits](googleapis/google-api-go-client@v0.153.0...v0.159.0)

---
updated-dependencies:
- dependency-name: google.golang.org/api
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.28.3 to 0.29.1.
- [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md)
- [Commits](kubernetes/client-go@v0.28.3...v0.29.1)

---
updated-dependencies:
- dependency-name: k8s.io/client-go
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [github.com/sigstore/sigstore](https://github.com/sigstore/sigstore) from 1.7.6 to 1.8.1.
- [Release notes](https://github.com/sigstore/sigstore/releases)
- [Commits](sigstore/sigstore@v1.7.6...v1.8.1)

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

Signed-off-by: dependabot[bot] <[email protected]>
@LucaGuerra
Copy link
Contributor

cc @cpanato , as mentioned in the linked issue ;) would be very appreciated if you could look into the PR, you're the expert here!

@brennoo brennoo force-pushed the feat/kms_keys_signatures branch from 1e7a0ee to be4832a Compare January 29, 2024 14:36
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

A few nits and also please squash the commits

go.mod Outdated
github.com/sigstore/sigstore/pkg/signature/kms/aws v1.7.6
github.com/sigstore/sigstore/pkg/signature/kms/azure v1.7.6
github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.7.6
github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.7.6
Copy link
Member

Choose a reason for hiding this comment

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

can you please update to v1.8.1 to match the dependency in line?

CertOidcIssuer: signature.Cosign.CertificateOidcIssuer,
CertOidcIssuerRegexp: signature.Cosign.CertificateOidcIssuerRegexp,
},
if signature.Cosign.KeyRef == "" {
Copy link
Member

Choose a reason for hiding this comment

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

I think this does not need if and elses can be just the entire thing as was before, because in the case before we can only have either CertificateIdentity or CertificateIdentityRegexp

so then it will be

v := cosign.VerifyCommand{
		CertVerifyOptions: options.CertVerifyOptions{
			CertIdentity:         signature.Cosign.CertificateIdentity,
			CertIdentityRegexp:   signature.Cosign.CertificateIdentityRegexp,
			CertOidcIssuer:       signature.Cosign.CertificateOidcIssuer,
			CertOidcIssuerRegexp: signature.Cosign.CertificateOidcIssuerRegexp,
                         // used when using the kms verify
			KeyRef:     signature.Cosign.KeyRef,
			IgnoreTlog: signature.Cosign.PrivateInfrastructure,
		},
}

I don't have a KMS to test, but is it possible for you to check for both cases? I think this will work, and it simplifies a lot.

Thanks for this PR

@poiana
Copy link
Contributor

poiana commented Feb 3, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: brennoo
Once this PR has been reviewed and has the lgtm label, please ask for approval from cpanato. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@brennoo brennoo force-pushed the feat/kms_keys_signatures branch from d78dcc8 to a6a8657 Compare February 3, 2024 10:40
@brennoo brennoo changed the base branch from main to audit-control February 3, 2024 10:42
@brennoo brennoo closed this Feb 3, 2024
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.

Support KMS keys in cosign signature