feat: add cosign trust policies #1381

akashsinghal · 2024-04-10T19:03:08Z

Description

What this PR does / why we need it:

This is the fourth in a series of PRs to expand cosign support in Ratify.

updates charts
- adds new scopes field for trust policy
- switches cosign verifier template to use trust policies
- deprecates cosign.key value
- updates deployment template to not mount cosign secret if deprecated cosign.key is not provided
- updates values README with new updates
introduces trust policies and scoping with wildcards
- support for global * wild character
- support for absolute scopes: ghcr.io/deislabs/ratify:v1
- support for suffix wildcards: ghcr.io/deislabs*
- enforces strict overlap validation within a trust policy and across policies. Scopes cannot overlap.
introduces keys enumerations via matching to KMP
introduces file specification for specifying keys from file path. useful for CLI in future
introduces support for many new types of keys:
- RSA 2k, 3k, 4k
- EC: P256, p384, p521
introduces support for azure key vault keys
- includes extra hashing selection behavior based on key size provided (mirrors behavior of upstream cosign project)
- includes extra ASN.1 decoding required as workaround for bug in cosign akv signing plugin in upstream project
adds e2e tests
- moves e2e tests for cosign from plugin to base tests
- adds a new legacy cosign test to maintain backwards compatibility with key and rekorURL fields
- migrates azure e2e tests to create new RSA 2048 key and sign sample images with azurekms key
adds unit tests

Which issue(s) this PR fixes (optional, using `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when the PR gets merged):

Fixes #1191
Fixes #1190
Fixes #861
Fixes #1189

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Helm Chart Change (any edit/addition/update that is necessary for changes merged to the main branch)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Please also list any relevant details for your test configuration

Test A
Test B

Checklist:

Does the affected code have corresponding tests?
Are the changes documented, not just with inline documentation, but also with conceptual documentation such as an overview of a new feature, or task-based documentation like a tutorial? Consider if this change should be announced on your project blog.
Does this introduce breaking changes that would require an announcement or bumping the major version?
Do all new files have appropriate license header?

Post Merge Requirements

MAINTAINERS: manually trigger the "Publish Package" workflow after merging any PR that indicates Helm Chart Change

codecov · 2024-04-10T19:08:10Z

Codecov Report

Attention: Patch coverage is 77.43733% with 81 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (dev@6beff39). Click here to learn what that means.

❗ Current head fe94194 differs from pull request most recent head 63a4f93. Consider uploading reports for the commit 63a4f93 to get more accurate results

Files	Patch %	Lines
pkg/verifier/cosign/trustpolicy.go	68.86%	19 Missing and 14 partials ⚠️
pkg/verifier/cosign/cosign.go	78.23%	22 Missing and 10 partials ⚠️
...kg/keymanagementprovider/azurekeyvault/provider.go	57.14%	9 Missing ⚠️
pkg/verifier/cosign/trustpolicies.go	94.52%	2 Missing and 2 partials ⚠️
pkg/verifier/utils/utils.go	0.00%	2 Missing ⚠️
...kg/controllers/keymanagementprovider_controller.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##             dev    #1381   +/-   ##
======================================
  Coverage       ?   66.27%           
======================================
  Files          ?      112           
  Lines          ?     5901           
  Branches       ?        0           
======================================
  Hits           ?     3911           
  Misses         ?     1609           
  Partials       ?      381

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

akashsinghal · 2024-04-16T00:35:02Z

charts/ratify/templates/verifier.yaml

+          {{- range $i, $scope := .Values.cosign.scopes }}
+          - "{{$scope}}"
+          {{- end }}
+        keys:


record a demo and get experience feedback

pkg/verifier/cosign/cosign.go

susanshi · 2024-04-16T01:12:35Z

wondering do we plan to release the trust policy for Cosign under "experimental", do we anticipate changes in the cosign trust policy on user feedback?

akashsinghal · 2024-04-18T18:02:33Z

charts/ratify/README.md

 | cosign.enabled                                     | Enables/disables cosign tag-based signature lookup in ORAS store. MUST be set to true for cosign verification.                                                                                                                                                                                                                                                         | `true`                            |
-| cosign.key                                         | Public certificate used by cosign verifier                                                                                                                                                                                                                                                                                                                             | ``                                |
+| cosign.scopes                                      | An array of scopes relevant to the single trust policy configured in Cosign verifier                                                                                                                                                                                                                                                                                   | `["*"]`                           |


need to add link to docs to explain supported scopes syntax once docs are published on website

pkg/verifier/cosign/cosign.go

Makefile

susanshi

This is a impressive set of changes Akash. Wondering if you think we should do some buddy testing to have more eyes on different scenarios before we release. Maybe we could list out the main scenarios and have Yi, Binbin and I to cover a few each.

Makefile

charts/ratify/README.md

pkg/verifier/cosign/cosign.go

pkg/verifier/cosign/trustpolicy.go

charts/ratify/templates/verifier.yaml

pkg/keymanagementprovider/keymanagementprovider.go

pkg/verifier/cosign/trustpolicies.go

pkg/verifier/cosign/trustpolicy.go

pkg/verifier/cosign/cosign.go

binbin-li

lgtm, thanks for enhancing cosign experience!

akashsinghal self-assigned this Apr 10, 2024

akashsinghal commented Apr 16, 2024

View reviewed changes

pkg/verifier/cosign/cosign.go Outdated Show resolved Hide resolved

akashsinghal force-pushed the akashsinghal/trustpolicies branch from e30b51a to b43d46b Compare April 17, 2024 00:56

akashsinghal commented Apr 18, 2024

View reviewed changes

pkg/verifier/cosign/cosign.go Outdated Show resolved Hide resolved

feat: add cosign trust policies

50f9fdc

akashsinghal force-pushed the akashsinghal/trustpolicies branch from fb91db1 to 50f9fdc Compare April 18, 2024 18:52

akashsinghal marked this pull request as ready for review April 18, 2024 18:58

akashsinghal requested review from binbin-li, jimmyraywv, luisdlp, susanshi and toddysm as code owners April 18, 2024 18:58

susanshi reviewed Apr 19, 2024

View reviewed changes

Makefile Show resolved Hide resolved

susanshi reviewed Apr 19, 2024

View reviewed changes

Makefile Show resolved Hide resolved

charts/ratify/README.md Outdated Show resolved Hide resolved

pkg/verifier/cosign/cosign.go Outdated Show resolved Hide resolved

pkg/verifier/cosign/trustpolicy.go Show resolved Hide resolved

update extension config for verifier report

31db6c7

binbin-li reviewed Apr 22, 2024

View reviewed changes

akashsinghal added 7 commits April 22, 2024 19:30

address comments

ac7a0fc

Merge branch 'dev' into akashsinghal/trustpolicies

1f361bb

fix

812e0a6

add version

f8c365a

create akv helper

fe94194

comments

dcef905

Merge branch 'dev' into akashsinghal/trustpolicies

63a4f93

binbin-li approved these changes Apr 24, 2024

View reviewed changes

susanshi approved these changes Apr 24, 2024

View reviewed changes

akashsinghal merged commit 482ac34 into ratify-project:dev Apr 24, 2024
15 checks passed

akashsinghal mentioned this pull request May 13, 2024

The verifierReports did not include signature digest #1454

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add cosign trust policies #1381

feat: add cosign trust policies #1381

akashsinghal commented Apr 10, 2024 •

edited

Loading

codecov bot commented Apr 10, 2024 •

edited

Loading

akashsinghal Apr 16, 2024

susanshi commented Apr 16, 2024

akashsinghal Apr 18, 2024

susanshi left a comment

binbin-li left a comment

feat: add cosign trust policies #1381

feat: add cosign trust policies #1381

Conversation

akashsinghal commented Apr 10, 2024 • edited Loading

Description

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):

Type of change

How Has This Been Tested?

Checklist:

Post Merge Requirements

codecov bot commented Apr 10, 2024 • edited Loading

Codecov Report

akashsinghal Apr 16, 2024

Choose a reason for hiding this comment

susanshi commented Apr 16, 2024

akashsinghal Apr 18, 2024

Choose a reason for hiding this comment

susanshi left a comment

Choose a reason for hiding this comment

binbin-li left a comment

Choose a reason for hiding this comment

akashsinghal commented Apr 10, 2024 •

edited

Loading

Which issue(s) this PR fixes (optional, using `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when the PR gets merged):

codecov bot commented Apr 10, 2024 •

edited

Loading