-
Notifications
You must be signed in to change notification settings - Fork 44
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
Verification Plugin spec #165
Conversation
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
cc: @shizhMSFT @roywill @priteshbandi @rgnote for review |
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
84700cf
to
a3f6fda
Compare
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
specs/plugin-extensibility.md
Outdated
2. Validate that plugin version is greater than or equal to *Verification plugin minimum version* | ||
3. Fail signature verification if these validations fail | ||
3. Validate if plugin capabilities contains any `SIGNATURE_VERIFIER` capabilities | ||
1. Fail signature verification if a matching plugin with `SIGNATURE_VERIFIER` capability cannot be found. If signature envelope contains the *Verification Plugin Name* attribute, include it as a hint in error response. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If signature envelope contains the Verification Plugin Name attribute, include it as a hint in error response.
If plugin is invoked, doesnt that means signature envelope contains the Verification Plugin Name attribute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, updated the language Verification Plugin Name attribute will be always present.
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm all for extensibility and flexibility, enabling various scenarios that don't require notary maintainers to be involved. The remote signing extensibility has proven to work really well, enabling various cloud providers to meet their product goals for how they support remote signing/key management solutions. This was a key for Notary v2 scenarios and requirements
I have general concerns about a verification plugin that could limit the portability of signatures, as this could be counter to our usability and platform/vendor neutrality goals.
|
||
See [Extended attributes for *Notation* Plugins](./signature-specification.md#extended-attributes-for-notation-plugins) for detailed description og these headers. | ||
|
||
- **`io.cncf.notary.verificationPlugin`**(*string*)(critical): An OPTIONAL header that specifies the name of the verification plugin that MAY be used to verify the signature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, why do we think we need this?
The notary client should be able to verify any signature format supported. I haven't seen where we need plugins for verification.
signature-specification.md
Outdated
|
||
#### Extended attributes | ||
|
||
Implementations of Notary v2 signature spec MAY include additional signed attributes in the signature envelope. These attributes MAY be marked critical, i.e. the attribute MUST be understood and processed by a verifier, unknown critical attributes MUST cause signature verification to fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like it would impose instability and inconsistency.
A primary goal of Notary v2 is cloud/location independence. If a user copies an image from MAR to their ecr, they shouldn't need any special Microsoft stuff to validate it.
I'm guessing you have some ideas around new headers. Can we discuss those and find a platform neutral way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these the critical headers we are discussing here -
- JSW - https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.11
- COSE - https://datatracker.ietf.org/doc/html/rfc8152#appendix-C.1.4
Given the statement - basically it means that a signature with a critical header will put a requirement on the client environment to have the plugin available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the links point to how criticality of signed attributes is implemented in JWS and COSE. If an extended attribute is marked critical (not all extended attributes need to be critical) the verifier must process it either using a plugin or equivalent verification logic.
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
signature-envelope-jws.md
Outdated
- **[`crit`](https://datatracker.ietf.org/doc/html/rfc7515#section-4.1.11)**(*array of strings*): This OPTIONAL header lists the headers that implementation MUST understand and process. It MUST only contain headers apart from registered headers (e.g. `alg`, `cty`) in JWS specification, therefore this header is only present when the optional `io.cncf.notary.expiry` header is present in the protected headers collection. | ||
If present, the value MUST be `["io.cncf.notary.expiry"]`. | ||
- **`io.cncf.notary.signingScheme`**(*string*)(critical): This REQUIRED header specifies the [Notary v2 Signing Scheme](./signing-scheme.md) used by the signature. Supported values are `notary.default.x509` and `notary.signingAuthority.x509`. | ||
- **`io.cncf.notary.signingTime`**(*string*): This header specifies the time at which the signature was generated. This is an untrusted timestamp, and therefore not used in trust decisions. Its value is a [RFC 3339][rfc3339] formatted date time, the optional fractional second ([time-secfrac][rfc3339][[1](https://datatracker.ietf.org/doc/html/rfc3339#section-5.3)]) SHOULD NOT be used. This claim is REQUIRED and only valid when signing scheme is `notary.default.x509`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r/ .default
signature-specification.md
Outdated
@@ -126,9 +126,24 @@ Notary v2 requires the signature envelope to support the following signed attrib | |||
|
|||
#### Standard attributes | |||
|
|||
- **Signing time**: A REQUIRED claim that indicates the time at which the signature was generated. Though this claim is signed by the signing key, it’s considered unauthenticated as a signer can modify local time and manipulate this claim. More details [here](#signing-time). | |||
- **Signing Scheme** (critical): A REQUIRED claim that defines the [Notary v2 Signing Scheme](./signing-scheme.md) used by the signature. This attribute dictates the rest of signature schema - the set of signed and unsigned attributes to be included in the signature. Supported values are `notary.default.x509` and `notary.signingAuthority.x509`. | |||
- **Signing Time**: A claim that indicates the time at which the signature was generated. Though this claim is signed by the signing key, it’s considered unauthenticated as a signer can modify local time and manipulate this claim. More details [here](#signing-time). This claim is REQUIRED and only valid when signing scheme is `notary.default.x509` . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r/ .default
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Removed Signing scheme related spec updates in this PR which is now a separate PR (#175). |
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Signed-off-by: Milind Gokarn <gokarnm@amazon.com>
Resolved merge conflicts. |
Thanks @gokarnm for the latest updates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
verify-signature
plugin command.Signed-off-by: Milind Gokarn gokarnm@amazon.com