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: add support for signed user metadata #242

Merged
merged 12 commits into from Feb 8, 2023

Conversation

byronchien
Copy link
Contributor

@byronchien byronchien commented Jan 13, 2023

Adds support for signed user metadata in notation sign and notation verify. Relevant spec

example sign usage:

chienb@a07817b52895 notation % notation sign $IMAGE --user-metadata io.wabbit-networks.buildId=123 --user-metadata io.wabbit-networks.buildTime=123
Warning: Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:v1`) because tags are mutable and a tag reference can point to a different artifact than the one signed.
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before signing.
Successfully signed localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b

example metadata displayed on verification (without metadata flag)

chienb@a07817b52895 notation % notation verify $IMAGE
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Successfully verified signature for localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b

The artifact was signed with the following user metadata.

KEY                          VALUE
io.wabbit-networks.buildTime   123
io.wabbit-networks.buildId     123

example verification:

chienb@a07817b52895 notation % notation verify $IMAGE --user-metadata io.wabbit-networks.buildTime=123
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Successfully verified signature for localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b

The artifact was signed with the following user metadata.

KEY                            VALUE
io.wabbit-networks.buildTime   123
io.wabbit-networks.buildId     123

example verification failure

chienb@a07817b52895 notation % notation verify $IMAGE --user-metadata foo=bar
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Error: signature verification failed: signature verification failed

Signed-off-by: Byron Chien chienb@amazon.com

Signed-off-by: Byron Chien <chienb@amazon.com>
@byronchien
Copy link
Contributor Author

related: notaryproject/notation#507

@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2023

Codecov Report

Merging #242 (ffaf954) into main (e11bfd8) will increase coverage by 0.03%.
The diff coverage is 57.74%.

@@            Coverage Diff             @@
##             main     #242      +/-   ##
==========================================
+ Coverage   73.31%   73.34%   +0.03%     
==========================================
  Files          23       23              
  Lines        1746     1827      +81     
==========================================
+ Hits         1280     1340      +60     
- Misses        372      390      +18     
- Partials       94       97       +3     
Impacted Files Coverage Δ
errors.go 0.00% <0.00%> (ø)
notation.go 58.03% <50.98%> (-1.84%) ⬇️
verifier/verifier.go 76.75% <100.00%> (+3.28%) ⬆️
registry/repository.go 39.18% <0.00%> (+6.98%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

signer/plugin.go Outdated Show resolved Hide resolved
internal/mock/testdata/sig_env_with_metadata.json Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
signer/signer.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
@priteshbandi
Copy link
Contributor

chienb@a07817b52895 notation % notation verify $IMAGE --user-metadata foo=bar
Resolved artifact tag `v1` to digest `sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b` before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Error: signature verification failed: signature verification failed

Shouldn't the error message be something like, missing matching user-metadata in the signature. ?

verifier/verifier.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
notation.go Show resolved Hide resolved
notation.go Show resolved Hide resolved
signer/signer.go Outdated Show resolved Hide resolved
@shizhMSFT shizhMSFT changed the title Adds support for signed user metadata feat: add support for signed user metadata Jan 17, 2023
Signed-off-by: Byron Chien <chienb@amazon.com>
signer/signer.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
signer/signer.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
notation.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

errors.go Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
notation.go Outdated Show resolved Hide resolved
verifier/verifier.go Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
@yizha1 yizha1 added this to the RC-2 milestone Feb 1, 2023
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM with one comment

notation.go Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
Byron Chien added 2 commits February 6, 2023 16:10
Signed-off-by: Byron Chien <chienb@amazon.com>
Signed-off-by: Byron Chien <chienb@amazon.com>
@yizha1
Copy link
Contributor

yizha1 commented Feb 7, 2023

@byronchien could you fix the conflicts?

@byronchien
Copy link
Contributor Author

resolved conflict

@yizha1 yizha1 requested a review from shizhMSFT February 8, 2023 00:15
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@priteshbandi priteshbandi merged commit 6ef3544 into notaryproject:main Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants