Plugin interface for signing. #124

gokarnm · 2022-01-27T05:56:17Z

Related issue notaryproject/roadmap/issues/26.

Includes plugin interfaces for signing workflow
Addresses feedback on plugin mechanism and versioning
Added validations performed by Notation for plugin responses
Added error contract and error codes
hackmd doc

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

priteshbandi · 2022-02-04T07:05:23Z

Can you please run markdown linter on the change.

rchincha · 2022-02-04T23:54:42Z

specs/plugin-extensibility.md

+* Each plugin executable and dependencies are installed under directory `~/.notation/plugins/{plugin-name}` with an executable under that directory `~/.notation/plugins/{plugin-name}/notation-{plugin-name}`. The executable can be a shim which calls plugin dependecies installed elsewhere on the file system.
+* To use a plugin for signing, the user associates the plugin as part of registering a signing key. E.g.
+  * `notation key add --name "mysigningkey" --id "keyid" --plugin "com.amazonaws.signer.nv2plugin"`
+  * In the example, the command registers a signing key in `/notation/config.json`, where `mysigningkey` is a friendly key name to refer during signing operation, `id` is an AWS resource identifier that is used for signing, and the value of `plugin` specifies it's using a plugin located at `~/.notation/plugins/com.amazonaws.signer.nv2plugin/notation-com.amazonaws.signer.nv2plugin`.


Nitpicking.

com.amazonaws.signer.nv2plugin repetitive in `~/.notation/plugins/com.amazonaws.signer.nv2plugin/notation-com.amazonaws.signer.nv2plugin

That was the intent, /plugin has a child dir called {plugin-name}, which contains an executable called notation-{plugin-name}.

~/.notation/plugins/com.amazonaws.signer.nv2plugin/notation-com.amazonaws.signer.nv2plugin => ~/.notation/plugins/com.amazonaws.signer/nv2

Your call though.

I'd prefer to keep the existing convention, I agree it's repetitive, though it does not directly impact the end user experience. Initially the executable name was {plugin-name}, then we prefixed notation following docker's convention for CLI plugins. @sajayantony let me know if you have any feedback to simplify this.

specs/plugin-extensibility.md

SteveLasker · 2022-02-05T00:49:03Z

@aramase, can you help here?

specs/plugin-extensibility.md

priteshbandi · 2022-02-06T07:26:01Z

specs/plugin-extensibility.md

+### Plugin installation and config
+
+* Plugin publisher will provide instructions to download and install the plugin. Plugins intended for public distribution should also include instructions for users to verify the authenticity of the plugin.
+* Each plugin executable and dependencies are installed under directory `~/.notation/plugins/{plugin-name}` with an executable under that directory `~/.notation/plugins/{plugin-name}/notation-{plugin-name}`. The executable can be a shim which calls plugin dependecies installed elsewhere on the file system.


nit: The notation in notation-{plugin-name} of ~/.notation/plugins/{plugin-name}/notation-{plugin-name} is redundant.

Similar pattern for executables can be found in application bundles.

specs/plugin-extensibility.md

priteshbandi · 2022-02-06T08:52:59Z

specs/plugin-extensibility.md

+* ACCESS_DENIED - Authentication/authorization error to use given key.
+* TIMEOUT - The operation to generate signature timed out and can be retried by Notation.
+* THROTTLED - The operation to generate signature was throttles and can be retried by Notation.
+* ERROR - Any general error that does not fall into previous error categories.


s/ERROR/INTERNAL_ERROR ?

This is supposed to be a catch all error that would cover any known, and unhandled plugin errors that do not fall into other categories.

specs/plugin-extensibility.md

priteshbandi · 2022-02-06T09:05:32Z

specs/plugin-extensibility.md

+1. Determine if the registered key uses a plugin
+1. Execute the plugin with `discover` command
+    1. If plugin supports capability `SIGNATURE_ENVELOPE_GENERATOR`
+        1. Execute the plugin with `generate-envelope` command, set `request.key` to key definition (from `/notation/config.json`), `request.payload` to base64 encoded descriptor, `request.payloadType` to `application/vnd.oci.descriptor.v1+json` and `request.signatureEnvelopeType` to `application/vnd.cncf.notary.v2.jws.v1`.


Will Notation generate timestamp signature for Signature Envelope Generator plugin or its responsibility of plugin publisher?

Good point, will address in next rev.

specs/plugin-extensibility.md

sajayantony

Good for initial push with issues created.

specs/plugin-extensibility.md

aramase · 2022-02-08T06:57:35Z

specs/plugin-extensibility.md

+
+*plugin-name* - Plugin name uses reverse domain name notation to avoid plugin name collisions.
+
+*supported-contract-versions* - The list of contract versions supported by the plugin. Currently this list must include only one version, per major version. Post initial release, Notation may add new features through plugins, in the form of new commands (e.g. tsa-sign for timestamping), or additional request and response parameters. Notation will publish updates to plugin interface along with appropriate contract version update. Backwards compatible changes (changes for which older version of plugin continue to work with versions of Notation using newer contract version) like new optional parameters on existing contracts, and new commands will be supported through minor version contract updates, breaking changes through major version updates. Plugin `discover` command returns the contract version a plugin supports. Notation will evaluate the minimum plugin version required to satisfy a user's request, and reject the request if the plugin does not support the required version.


Do we also need to define the supportability contract for major and minor versions? How long will a particular minor version be supported in notation when there are multiple minor versions?

@priteshbandi @aramase, plugin publishers specify the major.minor version of contract interface, for each major version. We are only putting in place mechanisms to support breaking and non breaking changes to plugin interface. These will not rev at the same rate as Notation updates. Notation should ideally support all minor versions of contract. The intent is plugin publishers can choose to implement the latest version of contract, but we don't want to break support for plugins implementing older minor versions of contract (within the major version).

specs/plugin-extensibility.md

aramase · 2022-02-08T07:07:14Z

specs/plugin-extensibility.md

+* VALIDATION_ERROR - Any of the required request fields was empty, or a value was malformed/invalid. Includes condition where the key referenced by `keyId` was not found.
+* UNSUPPORTED_CONTRACT_VERSION - The contract version used in the request is unsupported.
+* ACCESS_DENIED - Authentication/authorization error to use given key.
+* TIMEOUT - The operation to generate signature timed out and can be retried by Notation.
+* THROTTLED - The operation to generate signature was throttles and can be retried by Notation.
+* ERROR - Any general error that does not fall into previous error categories.


These error codes are good to be surfaced out but IMO notation should not perform any retry action. In case of throttled requests, it's possible request can only be retried after a certain time defined in retry-after header. The plugin can decide to (1) retry the request internally before giving up and returning an error or (2) return the retry-after header info as part of error message, so the user can retry the request after the time has elapsed.

This also makes me think there should be a context defined with timeout as part of the request to the plugin. We can use CommandContext to ensure the request doesn't hang indefinitely

Agreed, Notation should not retry. The errors would be surfaced to caller invoking Notation, and it can decide to retry if needed.

Can you provide more details about the context with timeout as part of the request, maybe with an example, I didn't understand that.

aramase · 2022-02-08T07:12:11Z

specs/plugin-extensibility.md

+  // SIGNATURE_GENERATOR or
+  // SIGNATURE_ENVELOPE_GENERATOR


If SIGNATURE_GENERATOR and SIGNATURE_ENVELOPE_GENERATOR are supported by the plugin which one takes precedence when notation sign is invoked?

Currently we expect a signing plugin to implement one of the signing interfaces not both. Let me know if this does not work for a specific case.

specs/plugin-extensibility.md

shizhMSFT · 2022-02-08T07:10:26Z

specs/plugin-extensibility.md

+### Plugin installation and config
+
+* Plugin publisher will provide instructions to download and install the plugin. Plugins intended for public distribution should also include instructions for users to verify the authenticity of the plugin.
+* Each plugin executable and dependencies are installed under directory `~/.notation/plugins/{plugin-name}` with an executable under that directory `~/.notation/plugins/{plugin-name}/notation-{plugin-name}`. The executable can be a shim which calls plugin dependecies installed elsewhere on the file system.


Similar pattern for executables can be found in application bundles.

specs/plugin-extensibility.md

shizhMSFT

LGTM on this initial spec. We can add the specs for timestamping and verification in future PRs.

Signed-off-by: Milind Gokarn <gokarnm@amazon.com> Co-authored-by: Pritesh Bandi <priteshbandi@gmail.com> Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com> Co-authored-by: Shiwei Zhang <shizh@microsoft.com>

gokarnm · 2022-02-17T06:03:09Z

Addressed review feedback, linked open issues for verification plugin and threat model, and squashed commits.

priteshbandi

LGTM

NiazFK

LGTM

aramase

lgtm

for some of the outstanding questions, we can follow up during implementation.

SteveLasker

LGTM

gokarnm mentioned this pull request Feb 1, 2022

support "attach"ing signatures generated by a separate crypto mechanism notaryproject/notation#151

Closed

iamsamirzon mentioned this pull request Feb 4, 2022

Signing plugin interface API Spec notaryproject/roadmap#26

Closed

gokarnm changed the title ~~Initial version of plugin interface for signing.~~ Plugin interface for signing. Feb 4, 2022

rchincha reviewed Feb 4, 2022

View reviewed changes

specs/plugin-extensibility.md Outdated Show resolved Hide resolved

priteshbandi reviewed Feb 6, 2022

View reviewed changes

gokarnm commented Feb 7, 2022

View reviewed changes

specs/plugin-extensibility.md Outdated Show resolved Hide resolved

sajayantony approved these changes Feb 7, 2022

View reviewed changes

specs/plugin-extensibility.md Outdated Show resolved Hide resolved

aramase reviewed Feb 8, 2022

View reviewed changes

shizhMSFT reviewed Feb 8, 2022

View reviewed changes

shizhMSFT approved these changes Feb 17, 2022

View reviewed changes

gokarnm mentioned this pull request Feb 17, 2022

Add threat model for plugin extensibility #135

Open

Initial version of plugin interface for signing.

14260e0

Signed-off-by: Milind Gokarn <gokarnm@amazon.com> Co-authored-by: Pritesh Bandi <priteshbandi@gmail.com> Co-authored-by: Anish Ramasekar <anish.ramasekar@gmail.com> Co-authored-by: Shiwei Zhang <shizh@microsoft.com>

priteshbandi approved these changes Feb 18, 2022

View reviewed changes

NiazFK reviewed Feb 18, 2022

View reviewed changes

NiazFK approved these changes Feb 18, 2022

View reviewed changes

aramase approved these changes Feb 18, 2022

View reviewed changes

SteveLasker approved these changes Feb 18, 2022

View reviewed changes

SteveLasker merged commit ac47993 into notaryproject:main Feb 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin interface for signing. #124

Plugin interface for signing. #124

gokarnm commented Jan 27, 2022 •

edited

priteshbandi commented Feb 4, 2022

rchincha Feb 4, 2022 •

edited

gokarnm Feb 5, 2022

rchincha Feb 5, 2022 •

edited

gokarnm Feb 17, 2022

SteveLasker commented Feb 5, 2022

priteshbandi Feb 6, 2022

shizhMSFT Feb 8, 2022

priteshbandi Feb 6, 2022

gokarnm Feb 17, 2022

priteshbandi Feb 6, 2022

gokarnm Feb 17, 2022

sajayantony left a comment

aramase Feb 8, 2022

gokarnm Feb 17, 2022

aramase Feb 8, 2022

aramase Feb 8, 2022

gokarnm Feb 17, 2022

gokarnm Feb 17, 2022

aramase Feb 8, 2022

gokarnm Feb 17, 2022

shizhMSFT Feb 8, 2022

shizhMSFT left a comment

gokarnm commented Feb 17, 2022

priteshbandi left a comment

NiazFK left a comment

aramase left a comment

SteveLasker left a comment


		plugin-name - Plugin name uses reverse domain name notation to avoid plugin name collisions.

		supported-contract-versions - The list of contract versions supported by the plugin. Currently this list must include only one version, per major version. Post initial release, Notation may add new features through plugins, in the form of new commands (e.g. tsa-sign for timestamping), or additional request and response parameters. Notation will publish updates to plugin interface along with appropriate contract version update. Backwards compatible changes (changes for which older version of plugin continue to work with versions of Notation using newer contract version) like new optional parameters on existing contracts, and new commands will be supported through minor version contract updates, breaking changes through major version updates. Plugin `discover` command returns the contract version a plugin supports. Notation will evaluate the minimum plugin version required to satisfy a user's request, and reject the request if the plugin does not support the required version.

Plugin interface for signing. #124

Plugin interface for signing. #124

Conversation

gokarnm commented Jan 27, 2022 • edited

priteshbandi commented Feb 4, 2022

rchincha Feb 4, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rchincha Feb 5, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveLasker commented Feb 5, 2022

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

sajayantony 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

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

shizhMSFT left a comment

Choose a reason for hiding this comment

gokarnm commented Feb 17, 2022

priteshbandi left a comment

Choose a reason for hiding this comment

NiazFK left a comment

Choose a reason for hiding this comment

aramase left a comment

Choose a reason for hiding this comment

SteveLasker left a comment

Choose a reason for hiding this comment

gokarnm commented Jan 27, 2022 •

edited

rchincha Feb 4, 2022 •

edited

rchincha Feb 5, 2022 •

edited