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

Signature specification #102

Merged
merged 11 commits into from Oct 15, 2021
Merged

Signature specification #102

merged 11 commits into from Oct 15, 2021

Conversation

priteshbandi
Copy link
Contributor

@priteshbandi priteshbandi commented Oct 8, 2021

Summary
First draft of signature specification.

  • Added support for multiple signature format.
  • Added specs for JWS JSON serialization signature format. This is intended to be default format for alpha releases. The signature format for RC is TBD.

issues for future work

How is this PR different from #93?

@iamsamirzon iamsamirzon added this to In progress in Notary Project Board via automation Oct 12, 2021
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
@SteveLasker
Copy link
Contributor

Incorporates #93

@iamsamirzon iamsamirzon added this to the alpha-1 milestone Oct 12, 2021
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
@sudo-bmitch
Copy link
Contributor

Can we clarify what is being decided in this PR vs #93

signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
Pritesh Bandi and others added 10 commits October 14, 2021 18:04
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Co-authored-by: Milind Gokarn <milind81@gmail.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Copy link
Contributor

@NiazFK NiazFK left a comment

Choose a reason for hiding this comment

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

LGTM

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.

General comments on formating. We should always do backquotes for words like alg, iat, and exp... even if they are in the description.

signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
3. `alg` header values for various signature algorithms:
| Signature Algorithm | "alg" Param Value |
| -------- | -------- |
| ECDSA on secp256r1 with SHA-256 | ES256 |
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a more friendly name for secp256r1 from SECG? like P-256 from NIST?

Also, we need to provide a reference for readers. RFC 8422 Appendix A is a good one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all algos and curves are in NIST and that's the reason refrained from using NIST curves.
Added reference to RFC 8422 Appendix A for discovery.

signature-specification.md Outdated Show resolved Hide resolved
@SteveLasker
Copy link
Contributor

@priteshbandi, looks like a few minor clarification items are still open.

Signed-off-by: Pritesh Bandi <pritesb@amazon.com>
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

LGTM

Notary Project Board automation moved this from In progress to Reviewer approved Oct 15, 2021
@SteveLasker SteveLasker merged commit 382f52c into notaryproject:main Oct 15, 2021
Notary Project Board automation moved this from Reviewer approved to Done Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants