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

Support for Signing scheme #175

Merged
merged 6 commits into from
Jul 20, 2022
Merged

Conversation

gokarnm
Copy link
Contributor

@gokarnm gokarnm commented Jul 14, 2022

Signed-off-by: Milind Gokarn gokarnm@amazon.com

cc: @shizhMSFT @roywill @SteveLasker @sajayantony @priteshbandi @rgnote for review

Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
@gokarnm
Copy link
Contributor Author

gokarnm commented Jul 14, 2022

Signing scheme was previously discussed #147 , in Signature spec.

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.

Lots of interesting ideas here.

A few nits, and the discussion to remove .default from the x.509 scheme as default would be a configuration option.

signature-envelope-jws.md Outdated Show resolved Hide resolved
signature-envelope-jws.md Outdated Show resolved Hide resolved
signature-specification.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
*Q:* What is the relationship of Signing Scheme with Signature Envelope format?
*A:* Signing Scheme aims to be agnostic of the Signature Envelope format. A given signing scheme can be implemented through any signature envelope format (such as JWS or COSE) as long as it can support the required signature schema used by the signing scheme.

*Q:* Why trust store for Signing Authority (`x509/signingAuthority`) is distinct from trust store for Certificate Authority (`x509/ca`), why can’t they share the same trust store?
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Why trust store for Signing Authority
r/Why is the trust store used for the signing authority

Copy link
Contributor

Choose a reason for hiding this comment

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

@gokarnm, I'm not seeing the change here.

Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
@gokarnm
Copy link
Contributor Author

gokarnm commented Jul 14, 2022

Added line breaks on sentences in signing-scheme.md, will format other docs as a separate PR, and addressed some of review feedback from @SteveLasker . Still to rename signing scheme values and address feedback from Roy about auditability.

Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
@gokarnm
Copy link
Contributor Author

gokarnm commented Jul 15, 2022

  • I've renamed the signing schemes to notary.x509 and notary.x509.signingAuthority
  • Added content for authority audits

cc: @roywill @SteveLasker for review.

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

signature-envelope-jws.md Outdated Show resolved Hide resolved
signing-scheme.md Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
signing-scheme.md Outdated Show resolved Hide resolved
@gokarnm gokarnm mentioned this pull request Jul 18, 2022
Signed-off-by: Milind Gokarn <gokarnm@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

Signed-off-by: Milind Gokarn <gokarnm@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

@NiazFK NiazFK requested review from NiazFK and removed request for SteveLasker July 20, 2022 17:20
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

@NiazFK NiazFK merged commit 814a8cb into notaryproject:main Jul 20, 2022
@gokarnm gokarnm deleted the signingScheme branch July 20, 2022 17:27
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.

4 participants