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

doc: update notation sign and verify spec for metadata #498

Merged
merged 5 commits into from
Jan 12, 2023

Conversation

byronchien
Copy link
Contributor

Spec update to support notaryproject/roadmap#67

notation sign:

  • user will be able to specify additional key value pairs with the --user-metadata flag (-um short) that will be signed as part of the payload.

notation verify:

  • user will be able to specify additional key value pairs with the --user-metadata flag (-um short) that must be present in the signature to pass verification.
  • user will be able to configure output format as either plaintext (default) or json with the --output flag (-o short)

Signed-off-by: Byron Chien <chienb@amazon.com>
@byronchien byronchien changed the title Upate notation sign and verify spec for metadata Update notation sign and verify spec for metadata Jan 3, 2023
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 4, 2023

Codecov Report

Merging #498 (765ca1d) into main (f83a48b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #498   +/-   ##
=======================================
  Coverage   29.57%   29.57%           
=======================================
  Files          26       26           
  Lines        1515     1515           
=======================================
  Hits          448      448           
  Misses       1050     1050           
  Partials       17       17           

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

@shizhMSFT shizhMSFT changed the title Update notation sign and verify spec for metadata doc: update notation sign and verify spec for metadata Jan 4, 2023
Copy link
Contributor

@yizha1 yizha1 left a comment

Choose a reason for hiding this comment

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

Thanks @byronchien for the contribution. I have provided my comments.

specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/sign.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
specs/commandline/verify.md Outdated Show resolved Hide resolved
Signed-off-by: Byron Chien <chienb@amazon.com>
Copy link
Contributor

@yizha1 yizha1 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 some comments

An example of output messages for an unsuccessful verification:

```text
Error: signature verification failed for all the signatures associated with localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9
Copy link
Contributor

Choose a reason for hiding this comment

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

If the verification failure is due to metadata, there should be error log like you shared under the debug logging.

"Error: specified metadata is not present in the signature."

Signed-off-by: Byron Chien <chienb@amazon.com>
@@ -16,6 +16,17 @@ Warning: The resolved digest may not point to the same signed artifact, since ta
Successfully verified signature for <registry>/<repository>@<digest>
```

The signed descriptor may have user defined metadata attached. If the signature for the OCI artifact contains any metadata, the output message is as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The signed descriptor may have user defined metadata attached. If the signature for the OCI artifact contains any metadata, the output message is as follows:
A signature can have user defined metadata. If the signature for the OCI artifact contains any metadata, the output message is as follows:

```text
Successfully verified signature for <registry>/<repository>@<digest>

The artifact is signed with the following user metadata.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The artifact is signed with the following user metadata.
The artifact was signed with the following user metadata:

Comment on lines 155 to 176
An example of output messages for an unsuccessful verification with verbose logging enabled:

```text
INFO Checking whether signature verification should be skipped or not
INFO Check over. Trust policy is not configured to skip signature verification
INFO Processing signature with digest: sha256:dbb22c0686b714ccbb53e4579771ee0f9ab9d37cd77cadb767549322742979f3
INFO User Metadata flag is present. Checking signature metadata for specified values.
Error: unable to find specified metadata in any signatures associated with localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9
Error: signature verification failed for all the signatures associated with localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9
```

An example of output messages for an unsuccessful verification with debug logging enabled:

```text
...
INFO User Metadata flag is present. Checking signature metadata for specified values.
DEBU[2023-01-05T11:35:07-08:00] Verifying that metadata { "io.wabbit-networks.buildId":"123" } is present in signature metadata.
DEBU[2023-01-05T11:35:07-08:00] Signature metadata: { "io.wabbit-networks.buildId":"321" }
DEBU[2023-01-05T11:35:07-08:00] Error: specified metadata is not present in the signature.
Error: unable to find specified metadata in any signatures associated with localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9
Error: signature verification failed for all the signatures associated with localhost:5000/net-monitor@sha256:b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't add verbose and debug usecases in the spec as this can change during implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, will remove, also matches existing spec that doesn't have verbose/debug examples

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

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

@priteshbandi priteshbandi merged commit 6fb9eef into notaryproject:main Jan 12, 2023
priteshbandi pushed a commit to priteshbandi/notation that referenced this pull request Feb 1, 2023
…#498)

Spec update to support notaryproject/roadmap#67

`notation sign`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that will be signed as part of the payload.

`notation verify`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that must be present in the signature to pass verification.

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit to priteshbandi/notation that referenced this pull request Feb 1, 2023
…#498)

Spec update to support notaryproject/roadmap#67

`notation sign`: 
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that will be signed as part of the payload.

`notation verify`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that must be present in the signature to pass verification.

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit to priteshbandi/notation that referenced this pull request Feb 1, 2023
…#498)

Spec update to support notaryproject/roadmap#67

`notation sign`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that will be signed as part of the payload.

`notation verify`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that must be present in the signature to pass verification.

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit to priteshbandi/notation that referenced this pull request Feb 1, 2023
…#498)

Spec update to support notaryproject/roadmap#67

`notation sign`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that will be signed as part of the payload.

`notation verify`:
- user will be able to specify additional key value pairs with the `--user-metadata` flag (`-um` short) that must be present in the signature to pass verification.

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit to notaryproject/notation-go that referenced this pull request Feb 8, 2023
Adds support for signed user metadata in `notation sign` and `notation verify`. [Relevant spec](notaryproject/notation#498)

example sign usage:
notation % notation sign $IMAGE --user-metadata io.wabbit-networks.buildId=123 --user-metadata io.wabbit-networks.buildTime=123
Successfully signed localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b

example verification:
```
notation % notation verify $IMAGE --user-metadata io.wabbit-networks.buildTime=123
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
```

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit that referenced this pull request Feb 8, 2023
…y cmds (#507)

Adds support for signed user metadata in `notation sign` and `notation verify`. [Relevant spec](#498)

example sign usage:
chienb@a07817b52895 notation % notation sign $IMAGE --user-metadata io.wabbit-networks.buildId=123 --user-metadata io.wabbit-networks.buildTime=123
Successfully signed localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b
---------------
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
-----

Signed-off-by: Byron Chien <chienb@amazon.com>
priteshbandi pushed a commit that referenced this pull request Feb 10, 2023
allows json output for `notation verify`. Fixes notaryproject/roadmap#67 and #498

chienb@a07817b52895 notation % ./bin/notation verify $IMAGE --output json
{
    "reference": "localhost:5000/net-monitor@sha256:5a07385af4e6b6af81b0ebfd435aedccdfa3507f0609c658209e1aba57159b2b",
    "userMetadata": {
        "foo": "bar"
    },
    "result": "Success"
}

Signed-off-by: Byron Chien <chienb@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.

None yet

5 participants