COSE envelope signature spec updates #148

SteveLasker · 2022-04-20T20:43:17Z

Tracking the merger of the cose-envelope branch into main.

* cbor payload * cose envelope * finalize payload * remove JWS * refine notations * refine datetime Signed-off-by: Shiwei Zhang <[email protected]> Merging into the remote-siging branch for further iteration, addressing the PR feedback with additional PRs into this branch.

Signed-off-by: Steve Lasker <[email protected]>

Updates to signature specification * Moves signed attributes out of payload * Defines new signed attributes and marks them as critical/informational * Introduces signature scheme to support different trust models Signed-off-by: Milind Gokarn <[email protected]>

signature-envelope-cose.md

signature-envelope-jws.md

signature-envelope-cose.md

signature-envelope-jws.md

signature-envelope-cose.md

SteveLasker · 2022-07-14T15:41:29Z

With the go-cose v1.0.0-rc.1 release, with a security review completed, we'll be prioritizing this work.

There are a few updates that we'll incorporate that include multiple content types that support registry descriptors, hashes of files that may not be store in a registry and direct wrapping of content.

* Update signature-envelope-cose.md (#153) The `crit` header should be optional, not required. The spec already defines `cty` and `signingtime` as required, so a compliant verifier should reject any signature that does not include them, irrespective of whether or not the envelope also declares them as critical. An appropriate use of `crit` would be if a signer intends to include a new header not defined in the spec, to instruct verifiers that they must understand and process that header in order for the signature to be successfully validated. Another scenario is if a signer intends to mandate processing of an otherwise optional header (e.g., `expiry`). In other words, `crit` can be used by a signer to add requirements to the verification flow, but it is unnecessary and redundant if the verifier already has the same requirements. Signed-off-by: Brian Krell <[email protected]> * `cose-envelope`: merge changes from `main` (#177) * Align COSE spec with JWS Signed-off-by: letmaik <[email protected]> Co-authored-by: Steve Lasker <[email protected]> Co-authored-by: Quim Muntal <[email protected]> Co-authored-by: Quim Muntal <[email protected]> Co-authored-by: Milind Gokarn <[email protected]> Co-authored-by: Milind Gokarn <[email protected]> Co-authored-by: Shiwei Zhang <[email protected]> Co-authored-by: Pritesh Bandi <[email protected]> Co-authored-by: Sajay Antony <[email protected]> * update CBOR spec Co-authored-by: Steve Lasker <[email protected]> Co-authored-by: qmuntal <[email protected]> Co-authored-by: Milind Gokarn <[email protected]> Co-authored-by: Pritesh Bandi <[email protected]> Co-authored-by: Sajay Antony <[email protected]> Signed-off-by: Shiwei Zhang <[email protected]> Co-authored-by: Brian Krell <[email protected]> Co-authored-by: Maik Riechert <[email protected]> Co-authored-by: Steve Lasker <[email protected]> Co-authored-by: Quim Muntal <[email protected]> Co-authored-by: Quim Muntal <[email protected]> Co-authored-by: Milind Gokarn <[email protected]> Co-authored-by: Milind Gokarn <[email protected]> Co-authored-by: Pritesh Bandi <[email protected]> Co-authored-by: Sajay Antony <[email protected]> Co-authored-by: Pritesh Bandi <[email protected]> Co-authored-by: Sajay Antony <[email protected]>

…t into cose-envelope

thomas-fossati

A really nice work. I have left a few flyby comments for your consideration.

signature-envelope-cose.md

signature-specification.md

thomas-fossati · 2022-09-09T16:31:26Z

signature-specification.md

+
+**Implementation Constraints**: Notary v2 implementation MUST enforce the following constraints on signature generation and verification:
+
+1. `alg` header value MUST NOT be `none` or any symmetric-key algorithm such as `HMAC`.


re: prohibition on HMAC. This may be redundant since I don't think one can use symmetric key algorithms with COSE Sign/Sign1. Those should be only available for Mac or Mac0 COSEs.

Co-authored-by: Thomas Fossati <[email protected]> Signed-off-by: David Tesar <[email protected]>

gokarnm

Overall looks good, except for few specific details about COSE envelope and outdated content from merge, which should be resolved.

signature-envelope-cose.md

signature-specification.md

Signed-off-by: David Tesar <[email protected]>

specs/signature-specification.md

OR13 · 2022-11-16T18:02:24Z

signature-envelope-cose.md

+    'io.cncf.notary.signingScheme',
+    'io.cncf.notary.expiry'
+  ],
+  / cty / 3: 'application/vnd.cncf.notary.payload.v1+json',


this header struct looks good to me.

OR13 · 2022-11-16T18:03:06Z

signature-envelope-cose.md

+{
+  / x5chain / 33: [
+    << DER(leafCert) >>,
+    << DER(intermediate CACert) >>,


I am still a bit nervous about exactly what this means, but the approach seems reasonable... I would just want to see a complete example.

OR13 · 2022-11-16T18:04:16Z

signature-envelope-cose.md

+  | RSASSA-PSS with SHA-256         | -37               |
+  | RSASSA-PSS with SHA-384         | -38               |
+  | RSASSA-PSS with SHA-512         | -39               |
+  | ECDSA on secp256r1 with SHA-256 | -7                |


I assume lack of EdDSA is intentional here.

OR13 · 2022-11-16T18:05:24Z

specs/signature-specification.md

@@ -43,6 +43,7 @@ Signature Manifest Example
 - **`blobs`** (*array of objects*): This REQUIRED property contains collection of only one [artifact descriptor][artifact-descriptor] referencing signature envelope.
  - **`mediaType`** (*string*): This REQUIRED property contains media type of signature envelope blob. Following values are supported
    - `application/jose+json`


This seems like maybe not correct? are you saying that compact representation is NOT allowed? thats how i read them.

OR13 · 2022-11-16T18:06:33Z

specs/signature-specification.md

+
+```json
+{
+   "subject": {


good call, since sub is a string... maybe you want to make it clear that this is a "private claim", that is not part of the original RFC.

yizha1 · 2022-11-17T05:06:18Z

@gokarnm All your comments are resolved now. Could you give a final check and approve it if there are no further issues?

gokarnm

LGTM! @priteshbandi could you review and approve as well.

* spec: update according to review comments * update to RFC for the fields of the sig_structure * update description for field context Signed-off-by: Yi Zha <[email protected]>

SteveLasker · 2022-11-17T18:54:00Z

+1 to merging, with incremental improvements.

priteshbandi · 2022-11-17T19:55:39Z

specs/signature-envelope-cose.md

+Note: `<<` and `>>` are used to notate the CBOR byte string resulting from encoding the data item.
+
+- **[`x5chain`](https://datatracker.ietf.org/doc/html/draft-ietf-cose-x509-08#section-2)** (*array of bstr*): This REQUIRED parameter (label `33` by [IANA](https://www.iana.org/assignments/cose/cose.xhtml#header-parameters)) contains the ordered list of X.509 certificate or certificate chain ([RFC5280](https://datatracker.ietf.org/doc/html/rfc5280)) corresponding to the key used to digitally sign the COSE. The certificate chain is represented as an array of certificate, each certificate in the array is DER encoded and then wrapped in a CBOR byte string. The certificate containing the public key corresponding to the key used to digitally sign the COSE MUST be the first certificate, followed by the intermediate and root certificates in the correct order. Refer [*Certificate Chain* unsigned attribute](signature-specification.md#unsigned-attributes) for more details. Optionally, this header can be presented in the protected header.
+  - **TODO** update signature specification to allow chains in protected header


What's the motivation behind this? can we remove it from spec and open it as issue for discussion ?

@letmaik, @shizhMSFT can you comment?

The motivation is that the signing key of a signer may have multiple organizations associated. How to determine which organization that the signer represents at the signing time is an issue.

Here's an example, the signer has two cert chains Company A -> Signer and Company B -> Signer.

graph TD; a["CA (Company A)"] --> s["Signer Cert"] b["CA (Company B)"] --> s

Loading

Consider the following scenario:

Signer signs the artifact and associates cert chain Company A -> Signer to the resulted signature. In other words, the signer signs the artifact representing the company A.

Signer publishes the artifact and the signature.

Adversary sees the signature and replaces the cert chain in the unprotected header with a cert chain Company B -> Signer.

Now, from the view of verifiers, the signature is signed by the Signer, representing the company B.

This kind of attack can be mitigated by never reusing the signing key but it is better if we can sign the signing identity in the signature itself.

@yizha1 Let's remove it from spec and open it as issue for discussion.

@ianjmcm, can you provide some 👀 ?

priteshbandi

LGTM with one comment

* move certificate chain discussion to a new issue Signed-off-by: Yi Zha <[email protected]>

shizhMSFT

LGTM

SteveLasker · 2022-11-22T15:20:05Z

Thanks @shizhMSFT, @qmuntal, @letmaik, @yogeshbdeshpande, @thomas-fossati and everyone else that completed this baseline, including all the updates to https://github.com/veraison/go-cose

Great work!

shizhMSFT and others added 3 commits March 25, 2022 11:36

Support for JWT and COSE envelopes (#145)

33d1ee9

Signed-off-by: Steve Lasker <[email protected]>

SteveLasker added this to the RC-1 milestone Apr 20, 2022

SteveLasker requested review from gokarnm, shizhMSFT and iamsamirzon April 20, 2022 20:43

iamsamirzon reviewed Apr 20, 2022

View reviewed changes

signature-envelope-cose.md Outdated Show resolved Hide resolved

This was referenced Apr 22, 2022

Add COSE support notaryproject/notation#163

Closed

Support COSE/CBOR Azure/notation-azure-kv#11

Closed

letmaik reviewed Apr 23, 2022

View reviewed changes

letmaik added a commit to letmaik/notaryproject that referenced this pull request May 4, 2022

address some issues on JWS (notaryproject#148)

c078c9e

letmaik mentioned this pull request May 4, 2022

Address review comments on #148 for JWS #156

Closed

iamsamirzon modified the milestones: RC-1, RC-2 May 26, 2022

SteveLasker modified the milestones: RC-2, RC-1 Jul 14, 2022

SteveLasker mentioned this pull request Jul 22, 2022

cose-envelope: merge changes from main #177

Merged

SteveLasker force-pushed the cose-envelope branch 2 times, most recently from d18dc4c to 3e93835 Compare August 2, 2022 18:18

shizhMSFT and others added 2 commits August 2, 2022 15:19

Merge branch 'main' into cose-envelope

e1183b8

dtzar force-pushed the cose-envelope branch from 07a28b2 to e1183b8 Compare September 8, 2022 21:26

Merge branch 'cose-envelope' of github.com:notaryproject/notaryprojec…

68e31b4

…t into cose-envelope

dtzar mentioned this pull request Sep 8, 2022

cose-envelope branch: fix conflict with main branch caused by squash merge #181

Closed

thomas-fossati reviewed Sep 9, 2022

View reviewed changes

dtzar and others added 2 commits September 22, 2022 16:15

Update signature-envelope-cose.md

379e642

Co-authored-by: Thomas Fossati <[email protected]> Signed-off-by: David Tesar <[email protected]>

Update signature-envelope-cose.md

32507a5

Co-authored-by: Thomas Fossati <[email protected]> Signed-off-by: David Tesar <[email protected]>

gokarnm requested changes Nov 10, 2022

View reviewed changes

spec: update data type of signingTime to date/time (#199)

e7c8b55

Signed-off-by: David Tesar <[email protected]>

dtzar force-pushed the cose-envelope branch from 66c6a05 to e7c8b55 Compare November 11, 2022 00:12

Merge branch 'main' into cose-envelope

5e2f76a

priteshbandi reviewed Nov 14, 2022

View reviewed changes

specs/signature-specification.md Outdated Show resolved Hide resolved

specs/signature-specification.md Outdated Show resolved Hide resolved

specs/signature-specification.md Show resolved Hide resolved

specs/signature-specification.md Show resolved Hide resolved

OR13 reviewed Nov 16, 2022

View reviewed changes

gokarnm approved these changes Nov 17, 2022

View reviewed changes

gokarnm requested a review from priteshbandi November 17, 2022 06:46

spec: update cose and signature spec according to review comments (#204)

cf533b8

* spec: update according to review comments * update to RFC for the fields of the sig_structure * update description for field context Signed-off-by: Yi Zha <[email protected]>

yizha1 force-pushed the cose-envelope branch from 6fc8e1d to cf533b8 Compare November 17, 2022 11:34

priteshbandi reviewed Nov 17, 2022

View reviewed changes

priteshbandi approved these changes Nov 17, 2022

View reviewed changes

yizha1 mentioned this pull request Nov 18, 2022

Determine the organization that the signer represents at the signing time #208

Closed

yizha1 added 2 commits November 22, 2022 10:40

spec: move certificate chain discussion to a new issue (#209)

fa0d6aa

* move certificate chain discussion to a new issue Signed-off-by: Yi Zha <[email protected]>

Merge branch 'main' into cose-envelope

d65a512

shizhMSFT approved these changes Nov 22, 2022

View reviewed changes

SteveLasker merged commit efc8282 into main Nov 22, 2022

yizha1 deleted the cose-envelope branch November 23, 2022 02:08

yizha1 restored the cose-envelope branch November 23, 2022 02:09

yizha1 deleted the cose-envelope branch April 2, 2024 00:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

COSE envelope signature spec updates #148

COSE envelope signature spec updates #148

SteveLasker commented Apr 20, 2022

SteveLasker commented Jul 14, 2022

thomas-fossati left a comment

thomas-fossati Sep 9, 2022

gokarnm left a comment

OR13 Nov 16, 2022

OR13 Nov 16, 2022

OR13 Nov 16, 2022

OR13 Nov 16, 2022

OR13 Nov 16, 2022

yizha1 commented Nov 17, 2022

gokarnm left a comment

SteveLasker commented Nov 17, 2022

priteshbandi Nov 17, 2022 •

edited

Loading

SteveLasker Nov 17, 2022

shizhMSFT Nov 18, 2022

SteveLasker Nov 18, 2022

priteshbandi left a comment

shizhMSFT left a comment

SteveLasker commented Nov 22, 2022 •

edited

Loading


		Implementation Constraints: Notary v2 implementation MUST enforce the following constraints on signature generation and verification:

		1. `alg` header value MUST NOT be `none` or any symmetric-key algorithm such as `HMAC`.

COSE envelope signature spec updates #148

COSE envelope signature spec updates #148

Conversation

SteveLasker commented Apr 20, 2022

SteveLasker commented Jul 14, 2022

thomas-fossati left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gokarnm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yizha1 commented Nov 17, 2022

gokarnm left a comment

Choose a reason for hiding this comment

SteveLasker commented Nov 17, 2022

priteshbandi Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priteshbandi left a comment

Choose a reason for hiding this comment

shizhMSFT left a comment

Choose a reason for hiding this comment

SteveLasker commented Nov 22, 2022 • edited Loading

priteshbandi Nov 17, 2022 •

edited

Loading

SteveLasker commented Nov 22, 2022 •

edited

Loading