Skip to content

chore: improve warning message when signing or verifying with tag#497

Merged
priteshbandi merged 1 commit intonotaryproject:mainfrom
priteshbandi:main
Feb 1, 2023
Merged

chore: improve warning message when signing or verifying with tag#497
priteshbandi merged 1 commit intonotaryproject:mainfrom
priteshbandi:main

Conversation

@priteshbandi
Copy link
Copy Markdown
Contributor

Old

➜ notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before signing.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before verification.
Warning: The resolved digest may not point to the same signed artifact, since tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

New

➜ notation git:(main) ✗ ./notation sign $IMAGE
Warning: Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before signing. 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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before verification. The resolved digest may not point to the same signed artifact, since tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Dec 27, 2022

Codecov Report

Merging #497 (629720d) into main (136ed18) will increase coverage by 0.03%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #497      +/-   ##
==========================================
+ Coverage   29.57%   29.61%   +0.03%     
==========================================
  Files          26       26              
  Lines        1515     1513       -2     
==========================================
  Hits          448      448              
+ Misses       1050     1048       -2     
  Partials       17       17              
Impacted Files Coverage Δ
cmd/notation/sign.go 43.67% <0.00%> (+0.49%) ⬆️
cmd/notation/verify.go 27.27% <0.00%> (+0.27%) ⬆️

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

Copy link
Copy Markdown
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.

Looks like there's a DCO failure. And should we remove cmd/notation/notation from this PR since it's a binary file?

@yizha1
Copy link
Copy Markdown
Contributor

yizha1 commented Jan 3, 2023

@priteshbandi any reason behind this change?

@shizhMSFT shizhMSFT changed the title Improve warning message when signing or verifying with tag chore: improve warning message when signing or verifying with tag Jan 4, 2023
@priteshbandi
Copy link
Copy Markdown
Contributor Author

@priteshbandi any reason behind this change?

Hi Yi, thanks for reviewing. Here are the reasons:

  1. Consistent warning message between sign and verify command (see old and new messages in PR description).
  2. Also consolidating warning message to one line instead of two, for better readability.

@patrickzheng200
Copy link
Copy Markdown
Contributor

patrickzheng200 commented Jan 31, 2023

IIRC, the warning message for sign and verification are different by design. Because for verification, it's quite nature to verify a tagged manifest, and the corresponding digest will be returned. User could then pull by digest. Therefore, Resolved artifact tag xxx to digest xxx before verification. was not a part of the warning in the original verify spec design. @yizha1 please correct me if I'm wrong.

@priteshbandi
Copy link
Copy Markdown
Contributor Author

IIRC, the warning message for sign and verification are different by design. Because for verification, it's quite nature to verify a tagged manifest, and the corresponding digest will be returned. User could then pull by digest. Therefore, Resolved artifact tag xxx to digest xxx before verification. was not a part of the warning in the original verify spec design. @yizha1 please correct me if I'm wrong.

In both sign and verify we are displaying what image digest was signed so that user can use digest for subsequent operations.

@yizha1
Copy link
Copy Markdown
Contributor

yizha1 commented Feb 1, 2023

It is a bit different between sign $tag and verify $tag.
For sign $tag, we want users to make sure the artifact that they are signing is the right one and using $digest is the only correct way to ensure it. However, we don't fail sign $tag, if users use $tag not $digest. So, there is a warning message first to warn the users before notation resolves the $tag to $digest. If we put the warning after resolving $tag, it looks like it is OK to use $tag, actually it is not.

New

➜ notation git:(main) ✗ ./notation sign $IMAGE
Warning: Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before signing. 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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

@priteshbandi
Copy link
Copy Markdown
Contributor Author

priteshbandi commented Feb 1, 2023

It is a bit different between sign $tag and verify $tag. For sign $tag, we want users to make sure the artifact that they are signing is the right one and using $digest is the only correct way to ensure it. However, we don't fail sign $tag, if users use $tag not $digest. So, there is a warning message first to warn the users before notation resolves the $tag to $digest. If we put the warning after resolving $tag, it looks like it is OK to use $tag, actually it is not.

New

➜ notation git:(main) ✗ ./notation sign $IMAGE
Warning: Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before signing. 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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Either Sign or Verify the gravity of warning should be same because in both the scenarios the impact is severe. Should we just omit line Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 as this information is duplicate?

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

@yizha1
Copy link
Copy Markdown
Contributor

yizha1 commented Feb 1, 2023

It is a bit different between sign $tag and verify $tag. For sign $tag, we want users to make sure the artifact that they are signing is the right one and using $digest is the only correct way to ensure it. However, we don't fail sign $tag, if users use $tag not $digest. So, there is a warning message first to warn the users before notation resolves the $tag to $digest. If we put the warning after resolving $tag, it looks like it is OK to use $tag, actually it is not.

New

➜ notation git:(main) ✗ ./notation sign $IMAGE
Warning: Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 before signing. 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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Either Sign or Verify the gravity of warning should be same because in both the scenarios the impact is severe. Should we just omit line Resolved artifact tag v1 to digest sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059 as this information is duplicate?

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

I think omitting the Resolving is OK, because we have the digest info in Successfully... message. @shizhMSFT do you have more comments? I remember you had some comments on the warning and Resolving message When I updated the spec for the first time.

… tag

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Copy link
Copy Markdown
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. It makes sense to write all warnings to stderr instead of stdout.

@shizhMSFT
Copy link
Copy Markdown
Contributor

shizhMSFT commented Feb 1, 2023

Definitely, we need to make sure that the users understand that they are signing a particular content addressed by a digest.
Verification is a bit tricky. Given that we don't support tag signing right now, I think we can output a warning message on resolving the tag for verification.

@shizhMSFT
Copy link
Copy Markdown
Contributor

One may argue the motivation to verify a content addressed by a known digest. It is about authenticity.

For example, Alice wants to pull an image with digest sha256:abcd..., which is an image of Bob's company. If the trust policy is set to only allow images from Alice's company, the pull should fail due to verification failure unless this image has a valid signature from Alice's company.

Copy link
Copy Markdown

@jondonas jondonas 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 375a701 into notaryproject:main Feb 1, 2023
priteshbandi added a commit to priteshbandi/notation that referenced this pull request Feb 2, 2023
…taryproject#497)

### Old
➜  notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before signing.
Successfully signed
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜  notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag `v1` to digest
`sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before verification.
Warning: The resolved digest may not point to the same signed artifact,
since tags are mutable.
Successfully verified signature for
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

### New

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
priteshbandi added a commit to priteshbandi/notation that referenced this pull request Feb 3, 2023
…taryproject#497)

### Old
➜  notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before signing.
Successfully signed
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜  notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag `v1` to digest
`sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before verification.
Warning: The resolved digest may not point to the same signed artifact,
since tags are mutable.
Successfully verified signature for
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

### New

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
priteshbandi added a commit to priteshbandi/notation that referenced this pull request Feb 3, 2023
…taryproject#497)

### Old
➜  notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before signing.
Successfully signed
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜  notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag `v1` to digest
`sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before verification.
Warning: The resolved digest may not point to the same signed artifact,
since tags are mutable.
Successfully verified signature for
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

### New

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
…taryproject#497)

### Old
➜  notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before signing.
Successfully signed
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜  notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag `v1` to digest
`sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before verification.
Warning: The resolved digest may not point to the same signed artifact,
since tags are mutable.
Successfully verified signature for
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

### New

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
FeynmanZhou pushed a commit to FeynmanZhou/notation that referenced this pull request May 15, 2025
…taryproject#497)

### Old
➜  notation git:(main) ✗ ./notation sign $IMAGE
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:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before signing.
Successfully signed
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜  notation git:(main) ✗ ./notation verify $IMAGE
Resolved artifact tag `v1` to digest
`sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059`
before verification.
Warning: The resolved digest may not point to the same signed artifact,
since tags are mutable.
Successfully verified signature for
localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

### New

➜ notation git:(main) ✗ ./notation sign $IMAGE
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.
Successfully signed localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

➜ notation git:(main) ✗ ./notation verify $IMAGE
Warning: Always verify the artifact using digest(@sha256:...) rather than a tag(:v1) because resolved digest may not point to the same signed artifact, as tags are mutable.
Successfully verified signature for localhost:6000/net-monitor@sha256:36ca4d6834ed680362327811238b97c687e77c5cf4a04a74d0853d3c0c17e059

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
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.

6 participants