update: check SignatureMediaType in notation.Verify

[patrickzheng200]

In this PR:

1. Created RemoteVerifyOptions for notation.Verify(), and therefore, VerifyOptions is now only for verifier.Verify().
2. In notation.Verify(), using signature media type fetched from registry.

Signed-off-by: Patrick Zheng patrickzheng@microsoft.com

[codecov-commenter]

Codecov Report

Merging #208 (ccd6de7) into main (7e391f7) will increase coverage by 1.10%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##             main     #208      +/-   ##
==========================================
+ Coverage   66.64%   67.75%   +1.10%     
==========================================
  Files          23       23              
  Lines        1559     1569      +10     
==========================================
+ Hits         1039     1063      +24     
+ Misses        455      441      -14     
  Partials       65       65              

Impacted Files	Coverage Δ
notation.go	53.53% <89.47%> (+20.95%)	⬆️

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

[priteshbandi]

Why are we even taking opts.SignatureMediaType as input? Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

[patrickzheng200]

Why are we even taking opts.SignatureMediaType as input?

This enables users to pick their trusted signature format. Signature format that they don't trust will not be processed even if it might pass the verification process.

Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

No, it won't hurt multi-signature format support.
Basically, there are 3 scenarios:

1. VerifyOptions.SignatureMediaType is empty. In this case, MediaType fetched from repo will be used.
2. VerifyOptions.SignatureMediaType is not empty but does not match MediaType fetched from repo. In this case, this certain signature will be skipped, and the program will continue to verify the next signature.
3. VerifyOptions.SignatureMediaType equals to MediaType fetched from repo. This MediaType will be used to verify the signature.

To enable both JWS and COSE, user could just leave VerifyOptions.SignatureMediaType as empty. (I updated the code to reflect this.)

[shizhMSFT]

LGTM

[JeyJeyGao]

LGTM

[priteshbandi]

Why are we even taking opts.SignatureMediaType as input?

This enables users to pick their trusted signature format. Signature format that they don't trust will not be processed even if it might pass the verification process.

The question here is, if a user trust notary v2 for signing and verifying, why wont they trust a signature format that has been vetted by notary v2 ?

Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

No, it won't hurt multi-signature format support. Basically, there are 3 scenarios:

1. VerifyOptions.SignatureMediaType is empty. In this case, MediaType fetched from repo will be used.
2. VerifyOptions.SignatureMediaType is not empty but does not match MediaType fetched from repo. In this case, this certain signature will be skipped, and the program will continue to verify the next signature.
3. VerifyOptions.SignatureMediaType equals to MediaType fetched from repo. This MediaType will be used to verify the signature.

To enable both JWS and COSE, user could just leave VerifyOptions.SignatureMediaType as empty. (I updated the code to reflect this.)

Yes, that's an implementation detail. I am wondering why will a user want to trust one signature format but not the other. Also, if an implementation does that then it's not abiding by notartv2 spec.

[patrickzheng200]

Why are we even taking opts.SignatureMediaType as input?

This enables users to pick their trusted signature format. Signature format that they don't trust will not be processed even if it might pass the verification process.

The question here is, if a user trust notary v2 for signing and verifying, why wont they trust a signature format that has been vetted by notary v2 ?

Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

No, it won't hurt multi-signature format support. Basically, there are 3 scenarios:

1. VerifyOptions.SignatureMediaType is empty. In this case, MediaType fetched from repo will be used.
2. VerifyOptions.SignatureMediaType is not empty but does not match MediaType fetched from repo. In this case, this certain signature will be skipped, and the program will continue to verify the next signature.
3. VerifyOptions.SignatureMediaType equals to MediaType fetched from repo. This MediaType will be used to verify the signature.

To enable both JWS and COSE, user could just leave VerifyOptions.SignatureMediaType as empty. (I updated the code to reflect this.)

Yes, that's an implementation detail. I am wondering why will a user want to trust one signature format but not the other. Also, if an implementation does that then it's not abiding by notartv2 spec.

Please don't get me wrong, I don't mean users should or need to pick one specific signature format here. Leaving the field empty is the default behavior, i.e. taking both signature formats is our default. The purpose of this PR is not enabling users to pick a signature format. On the contrary, previous to this PR, the logic is only using opts.SignatureMediaType. This PR actually fixes this issue and takes SignatureMediaType from registry.

[priteshbandi]

Why are we even taking opts.SignatureMediaType as input?

This enables users to pick their trusted signature format. Signature format that they don't trust will not be processed even if it might pass the verification process.

The question here is, if a user trust notary v2 for signing and verifying, why wont they trust a signature format that has been vetted by notary v2 ?

Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

No, it won't hurt multi-signature format support. Basically, there are 3 scenarios:

1. VerifyOptions.SignatureMediaType is empty. In this case, MediaType fetched from repo will be used.
2. VerifyOptions.SignatureMediaType is not empty but does not match MediaType fetched from repo. In this case, this certain signature will be skipped, and the program will continue to verify the next signature.
3. VerifyOptions.SignatureMediaType equals to MediaType fetched from repo. This MediaType will be used to verify the signature.

To enable both JWS and COSE, user could just leave VerifyOptions.SignatureMediaType as empty. (I updated the code to reflect this.)

Yes, that's an implementation detail. I am wondering why will a user want to trust one signature format but not the other. Also, if an implementation does that then it's not abiding by notartv2 spec.

Please don't get me wrong, I don't mean users should or need to pick one specific signature format here. Leaving the field empty is the default behavior, i.e. taking both signature formats is our default. The purpose of this PR is not enabling users to pick a signature format. On the contrary, previous to this PR, the logic is only using opts.SignatureMediaType. This PR actually fixes this issue and takes SignatureMediaType from registry.

if the purpose of this PR is not enabling users to pick a signature format, can we always use mediatype returned by signature manifest i.e. user shouldn't be able to choose?

[patrickzheng200]

Why are we even taking opts.SignatureMediaType as input?

This enables users to pick their trusted signature format. Signature format that they don't trust will not be processed even if it might pass the verification process.

The question here is, if a user trust notary v2 for signing and verifying, why wont they trust a signature format that has been vetted by notary v2 ?

Also, will checking check SignatureMediaType in notation.Verify hurt multi-signature envelope support?

No, it won't hurt multi-signature format support. Basically, there are 3 scenarios:

1. VerifyOptions.SignatureMediaType is empty. In this case, MediaType fetched from repo will be used.
2. VerifyOptions.SignatureMediaType is not empty but does not match MediaType fetched from repo. In this case, this certain signature will be skipped, and the program will continue to verify the next signature.
3. VerifyOptions.SignatureMediaType equals to MediaType fetched from repo. This MediaType will be used to verify the signature.

To enable both JWS and COSE, user could just leave VerifyOptions.SignatureMediaType as empty. (I updated the code to reflect this.)

Yes, that's an implementation detail. I am wondering why will a user want to trust one signature format but not the other. Also, if an implementation does that then it's not abiding by notartv2 spec.

Please don't get me wrong, I don't mean users should or need to pick one specific signature format here. Leaving the field empty is the default behavior, i.e. taking both signature formats is our default. The purpose of this PR is not enabling users to pick a signature format. On the contrary, previous to this PR, the logic is only using opts.SignatureMediaType. This PR actually fixes this issue and takes SignatureMediaType from registry.

if the purpose of this PR is not enabling users to pick a signature format, can we always use mediatype returned by signature manifest i.e. user shouldn't be able to choose?

Code updated. Changed the field to an array, and separated verify options for notation.Verify() and verifier.Verify().

[shizhMSFT]

LGTM

[priteshbandi]

if the purpose of this PR is not enabling users to pick a signature format, can we always use mediatype returned by signature manifest i.e. user shouldn't be able to choose?

@patrickzheng200 Can we do this?

[patrickzheng200]

@priteshbandi Oh yeah, I've updated the code. PTAL.

[shizhMSFT]

LGTM

[priteshbandi]

LGTM

[shizhMSFT]

LGTM