Sign CLI: signature provider plugins #304

dtivel · 2023-11-09T19:10:33Z

Currently, Sign CLI only supports signing with Azure Key Vault. Sign CLI should support pluggable signature providers to enable other private key storage options. This is necessary to keep pace with recent CAB Forum changes to private key storage requirements for publicly trusted code signing certificates.

CC @clairernovotny, @richlander

agr · 2024-01-03T00:13:30Z

accepted/2023/sign-signature-provider-plugins.md

+
+The new interfaces assembly will be packaged and published to <https://nuget.org>, similar to [NuGetRecommender's contracts-only package](https://www.nuget.org/packages/Microsoft.DataAI.NuGetRecommender.Contracts).  The Sign CLI team will manage the source code repository for this package and publish the package to <https://nuget.org>.
+
+The interfaces package itself can have package dependencies; however, because Sign CLI and all plugins would inherit new interfaces package dependencies, we should exercise restraint and caution before adding new dependencies.  An example of a package dependency worth having is [`Microsoft.Extensions.Logging.Abstractions`](https://www.nuget.org/packages/Microsoft.Extensions.Logging.Abstractions).  Sign CLI uses [`ILogger`](https://learn.microsoft.com/dotnet/api/microsoft.extensions.logging.ilogger) ubiquitously, it makes sense for plugins to write to a shared logger.  From the provided `System.IServiceProvider` argument, plugins can request an `ILogger` instance for logging.


From the provided System.IServiceProvider argument

Example above uses concrete ServiceProvider. The text should be aligned with the code sample.

agr · 2024-01-03T00:26:49Z

accepted/2023/sign-signature-provider-plugins.md

+
+* Enable signature formats that require a certificate to sign without a certificate.
+* Enable signature formats to support asymmetric algorithms they do not already support.
+* Create a distribution channel for plugins.  Sign CLI is a .NET tool and is [available on https://nuget.org](https://www.nuget.org/packages/sign/).  Plugin packages can be published to any NuGet feed, including <https://nuget.org>.


If interaction with arbitrary NuGet feeds is expected to be supported shouldn't the spec cover how user would tell sign CLI which feed to use? What about authentication to feeds that require one? V2/V3 support?

I'm also wondering if NuGet-based plugins should be developed as a separate feature. Seems useful.

One other consideration - the current description seems to hint at the nuget package being just downloaded as a zip file and extracted onto disk - no real NuGet logic involved. This will mean that the host will have to implement some of the logic NuGet client already implements (like TFM resolution). Would it make more sense to resolve the package via nuget client instead? That would provide all of the resolution logic as well as authentication and so on.

@agr, @vitek-karas, good questions. We should use NuGet API's. This would enable TFM resolution, support alternate package feeds, authentication, etc. The spec should detail this.

@richlander, I agree a general model does seem like it might be useful. @clairernovotny and I were hard-pressed to find close precedents here, so what we're doing seemed novel. We're open to working with others on a generalized model if it would be useful.

agr · 2024-01-03T00:28:25Z

accepted/2023/sign-signature-provider-plugins.md

+* assembly loading details at `Debug` / `Trace` log levels
+* errors at the `Error` log level
+
+### Sign CLI commands


Should "list plugins" and "remove plugin" commands also exist?

The intention was this spec would represent v1 of the feature, which would not have those commands. Those commands could be added later.

Alternatively, we could spec them now and prioritize implementation after everything else. (The same could be said about other potential features, but we were trying to expedite v1 by not speccing too much.)

agr · 2024-01-03T00:29:33Z

accepted/2023/sign-signature-provider-plugins.md

+
+##### `sign plugin install <PluginPackageId> [--version <Version>]`
+
+This command will install the latest release version of the plugin package identified by `PluginPackageId` using existing package sources and NuGet's default NuGet.config lookup order.  If the latest version is already installed, the command will no-op.


What if a previous version of a plugin is installed? Will it fail or will it update? Or something else?

@agr, the intention was that it would install alongside the previous version without removing the previous version.

richlander · 2024-01-04T01:20:23Z

accepted/2023/sign-signature-provider-plugins.md

+A Sign CLI signature provider plugin:
+
+* extends Sign CLI functionality
+* contains a [`plugin.json`](#plugin-json-file) file in its root directory


I'm worried that this might lead to conflicts if people look at this work as a reference implementation. We might end up with a model where lots of .NET apps require a plugin.json for their plugins. I can imagine a case where some plugin can be a plugin for multiple apps.

I would agree - would it be better to base the plugin discovery on patterns?

Name of the file (Microsoft.Azure.KeyVault.SignPlugin.dll -> *.SignPlugin.dll) ?

Use System.Reflection.Metadata to load the assembly (but not for running) and inspect it - from there you could read the necessary metadata (name, description, ...) - one option would be assembly level attributes for example (just like version information is stored today).

@richlander, @vitek-karas, I understand your concerns. We drew inspiration from dotnet/templating's template.json file as a reference on how to declare assets and contributions. Here, plugin.json provides 2 things: a mechanism for locating a plugin's main assembly and CLI option metadata.

I'm not sure how to entirely eliminate your concerns without tackling a generic .NET tool plugin model at the same time, such that dotnet/sign follows the same pattern. A general .NET tool plugin model wasn't originally in scope, but we're open to working with others to find out how we can deliver this in alignment with a generic .NET tool plugin model. There is pressure for dotnet/sign to deliver something in the very near future.

We could use an assembly file name pattern instead and remove that section from plugin.json. plugin.json still specifies CLI option metadata, and we wanted to leave the door open for localization, even though it's out of scope now. @clairernovotny and I talked about using attributes in the assembly to provide CLI option metadata, potentially generating plugin.json at build time (similar to dotnet/templating), but that all felt nice-to-have not MVP.

Are you suggesting that we eliminate plugin.json entirely or that a plugin.json file should conform to a standard .NET tool plugin schema?

richlander · 2024-01-04T01:32:45Z

Looks good.

I'm seeing a lot over similarity / overlap with .NET tools. It would be good to talk with the tools folks to see if there can be sharing here. For example, tools are a specific package type.

On roll-forward, LatestMajor is probably the wrong setting. Context here: dotnet/sdk#37231

It seems like this project is 90%+ of the way to defining a generic plug-in model system for .NET. That's really interesting. We should talk to the app model folks about that.

@vitek-karas @baronfel

vitek-karas · 2024-01-04T15:24:18Z

accepted/2023/sign-signature-provider-plugins.md

+
+       <!-- Enable rolling forward to a later runtime. -->
+       <!-- See https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#rollforward -->
+       <RollForward>LatestMajor</RollForward>


This is unnecessary - the plugins will be loaded via managed host, so they have no say in runtime version selection.

vitek-karas · 2024-01-04T15:26:07Z

accepted/2023/sign-signature-provider-plugins.md

+
+       <!-- Copy runtime dependencies to the build output directory. -->
+       <!-- https://learn.microsoft.com/dotnet/core/project-sdk/msbuild-props#enabledynamicloading -->
+       <EnableDynamicLoading>true</EnableDynamicLoading>


This is needed, but the text doesn't describe it - the doc should probably mention this just like it mentions the specific attribute for the interface package reference. The idea is that it's likely we will extract info from this design doc into the official docs for the tool and this is an important bit (without this, the plugin will not get its dependencies and it won't get .deps.json which is required).

agocke · 2024-01-04T17:33:34Z

Sorry, I haven't had time to read the full proposal, but I see that it relies on an assembly load from disk as an extensibility point. This security model would devolve to arbitrary code execution, mediated by the security of the file system and the file loading code.

That seems like a very large security surface area. I'm not ruling out that assembly load may be necessary for this level of extensibility, but does pose a substantial risk. I think we should at minimum do a full security review of the plugin loading design. Alternatively, if we can build an extensibility model that doesn't require assembly load, that might be easier to secure.

vitek-karas · 2024-01-05T11:06:29Z

accepted/2023/sign-signature-provider-plugins.md

+
+* Create a plugin model that enables pluggable signature providers.  A signature provider plugin will offer an alternate implementation of [`System.Security.Cryptography.AsymmetricAlgorithm`](https://learn.microsoft.com/dotnet/api/system.security.cryptography.asymmetricalgorithm) and the [`System.Security.Cryptography.X509Certificates.X509Certificate2`](https://learn.microsoft.com/dotnet/api/system.security.cryptography.x509certificates.x509certificate2), if available, corresponding to the private key.  Note that while there may be valid scenarios for signing with a raw asymmetric key pair, some signing operations require a certificate and will fail if a certificate is not available.
+* Make Sign CLI plugin-neutral.  While Sign CLI may install some core plugins (TBD), most plugins should be installed separately from Sign CLI itself, and Sign CLI's only interactions with any plugin should be through this plugin model.
+* Enable Sign CLI and plugins to version and release independently.


What is the reason for this requirement?
Or rather, why do we think it's necessary to allow pretty much anybody to build a plugin? I understand that there are several providers which would want to add a plugin here, but is it really necessary to fully decouple them?
For example, why not build all of these in-repo and ask the providers to simply send a pull request?
At runtime the plugins could still be loaded dynamically, but you would avoid the complexity of acquisition, install and security.

What's the reason to restrict the ability for anyone to develop and distribute those on their own? Delivery mechanisms exist, it's not like whole new infrastructure is built just for distributing plugins.
What if someone doesn't want their code to be open source or disagrees with Sign CLI license? What if someone doesn't want a third party to control the release of their code?

What if someone doesn't want their code to be open source or disagrees with Sign CLI license? What if someone doesn't want a third party to control the release of their code?

They can fork the project and build their own distribution of the tool with their own modifications.

And then that fork would have to be maintained and we'll end up with several Sign CLI distributions with different signing capabilities. Why require extra gestures from third parties? The more friction is added to the process, the less desire there would be to participate.
Unless there are security concerns with plugin delivery and loading, efforts for adding plugins should be reduced.

Sorry if I wasn't clear - I'm not saying we should not do it. My point was about cost/complexity versus benefit. The added complexity and cost to support separate distribution of plugins looks substantial. I was mostly looking for a discussion about the needs and if there are other ways to solve the problem.

There's a somewhat similar concept in the .NET SDK - workloads. The requirements there are obviously different, but it was (and still is) a substantial effort to develop and maintain this (and I don't think we even allow 3rd part workloads).

Also, the entire "dotnet tool" functionality is similar in some ways as well.

Hi,

Trying to answer a few things here around the intents:

The binary separation of the plugins was in part to enable any service capable of providing a signature the ability to do so via this tool.

We wanted one tool, not forks, to keep it as simple as possible for end users. Forks are hard to maintain and there's also the possibility that multiple providers could be needed for more complex scenarios down the road.

We wanted to isolate a plugin's dependencies from the main tool. That could be parts of the Azure SDK for Key Vault, or could be the AWS SDK should AWS choose to write a plugin. Could be other things entirely.

We didn't want to be gatekeepers of who gets to play. That includes having to review PR's for additional providers. That doesn't really scale well.

We were looking at this similar to MSBuild Tasks and the way they are loaded

We wanted to isolate a plugin's dependencies from the main tool.

The proposed fork model does that even more naturally.

There is a significant downside to forking -- maintenance. It is anticipated that we'll have at least 15-20 signature providers. For each of those to fork, which puts a significant burden on them to stay up to date with any changes and create new releases each time. That is likely more effort than many/most of those organizations providing the signatures would like to sign up for--it's a much easier lift for them to create a plugin once and update it if the interface changes than to continually release updated versions of an app. It also creates support issues for each of the providers as now they're front-line support for the app and not just their plugin.

If an app gets out of date, then that creates significant confusion for end users as to what features or bugs exist in which version of the app. This has already happened with private forks of the codebase to support other providers, where their version contains bugs already fixed in the main code. The support cost has also deterred public release of that fork.

How often do you think that one of these signature providers would need to issue an update in the global tool model?

An out-of-process plugin model seems superior to this assembly load model. It would eliminate the need for a contract assembly and enable both Sign CLI and plugins to update independently.

@richlander, I don't know how frequently plugins would update. It seems reasonable that an Azure Key Vault plugin would periodically update for bug fixes, features, and whenever Azure SDK dependencies update.

agr · 2024-01-05T18:53:43Z

accepted/2023/sign-signature-provider-plugins.md

+Example:
+
+```JSON
+{


Do we need some kind of indicator that it is a plugin for Sign CLI specifically?

Alternatively, NuGet package type mechanism could be used for that.

@agr, if we move to a more generic .NET tool plugin pattern, then probably yes.

JeromySt · 2024-01-10T23:43:48Z

accepted/2023/sign-signature-provider-plugins.md

+    /// To support these signature formatters, the plugin should implement this interface.  Signature formatters may
+    /// fail if they require a certificate but a signature provider plugin does not implement this interface.
+    /// </remarks>
+    public interface ICertificateProvider


I'd imagine that it would be desirable for ICertificateProvider to be a special type of ISignatureProvider as well. The definition could be updated to public interface ICertificateProvider : ISignatureProvider such that those interfaces that only support ISignatureProvider can still use a certificate acquired from a ICertificateProvider?

JeromySt · 2024-01-10T23:46:21Z

accepted/2023/sign-signature-provider-plugins.md

+    /// </remarks>
+    public interface ICertificateProvider
+    {
+        Task<X509Certificate2> GetCertificateAsync(CancellationToken cancellationToken);


I can foresee a use case where this interface would also need to provide access to the certificate chain of the certificate (this is needed in the COSE Sign1 world). Would it be prudent to add that API surface to this interface?

baronfel · 2024-01-17T18:50:34Z

accepted/2023/sign-signature-provider-plugins.md

+1. Read a plugin's `plugin.json` file.
+1. Find the plugin's entry point that best matches the host (Sign CLI) not the runtime and deviate only if it's a sound fallback.  Examples:
+    1. If a plugin contains net6.0, net7.0, and net8.0 assemblies and Sign CLI's net7.0 assemblies are loaded by the .NET 8 runtime, Sign CLI should load the plugin's net7.0 assemblies.
+    1. If a plugin only had net6.0 and net8.0 assemblies, Sign CLI should load the net6.0 assemblies or report that plugin isn't available for the current runtime.
+1. Load the assembly at the entry point's `filePath` location.
+1. Create an instance of the type `implementationTypeName`.
+1. Cast the instance to an interface defined in the interfaces package with type name `interfaceTypeName`.
+
+As part of this process, Sign CLI will use [`System.Runtime.Loader.AssemblyLoadContext`](https://learn.microsoft.com/dotnet/api/system.runtime.loader.assemblyloadcontext) and [`System.Runtime.Loader.AssemblyDependencyResolver`](https://learn.microsoft.com/dotnet/api/system.runtime.loader.assemblydependencyresolver) to load a plugin assembly and its dependencies strictly from the directory cone of the plugin's entry point assembly.  For example, if the plugin's entry point is `lib/net6.0/Microsoft.Azure.KeyVault.Sign.dll`, then Sign CLI will attempt to resolve assemblies under `lib/net6.0`.


Out of curiosity, why is a load-based extensibility model vs a out-of-process, invocation-based model desirable? I thinking things like git extensions, docker credential providers, etc as prior art here that communicate using a specific IPC mechanism (JSON over stdout in the must crude case, but also protobufs or others).

@baronfel, we're open to an out-of-process design. We thought an in-process design was something we could deliver quickly. An out-of-process design does have benefits over an in-process design, as mentioned elsewhere in this review.

baronfel · 2024-02-21T22:02:06Z

I had a chat with @clairernovotny a bit ago and we covered a few points of interest to this design. There are a few major points that we covered around

Discoverability (both end-user knowledge of what plugins are available to be installed, and tool-plugin knowledge of what plugs are currently installed)
UX of the CLI interface to the resulting sign command: dotnet sign providerA may have drastically different arguments than dotnet sign providerB, and so each provider needs to be able to communicate its own set of arguments to the 'core' sign tool for areas like Help, Tab Completions, etc.
Hosting/Invocation of each loaded plugin - aka the protocol that each plugin would need to adhere to to actually sign things

To me the areas in which this tool differs from the extensibility model of systems like Git credential helpers or Docker credential helpers are these latter two areas. These credential-helper extensibility models have a very limited and defined protocol, with little in the way of CLI-based customization. They are also fairly stateless - each invocation is distinct. The proposed model for plugins implies more persistence - based on my conversation with @clairernovotny each plugin is called repeatedly in a single signing session in a way that would require a whole rework of the protocol to support with an out-of-proc model in order to not tank performance. That could be done (@dtivel has signaled above that such designs might be useful) but it would definitely cause more delay here.

One potential comparison point here would be the git CLI itself - it has convention-based lookups for commands of the form git-MY_COMMAND_HERE, but it does so at the cost of potentially losing consistency of help/tab completions/etc - is that a cost we'd be willing to pay for that tool?

I think these differences are unique enough to not stop work here while we figure out some unified plugin plan.

anangaur · 2024-02-22T17:57:54Z

@dtivel @baronfel @clairernovotny please take a look at prior art here: https://youtu.be/FRrslz_AHzE?list=PLdo4fOcmZ0oWiK8r9OkJM3MUUL7_bOT9z&t=460 This is Microsoft owned dotnet tool Upgrade Assistant

jkotas · 2024-02-22T19:25:49Z

@dtivel @baronfel @clairernovotny please take a look at prior art here: https://youtu.be/FRrslz_AHzE?list=PLdo4fOcmZ0oWiK8r9OkJM3MUUL7_bOT9z&t=460 This is Microsoft owned dotnet tool Upgrade Assistant

I do not see the extensions feature in the current release of the upgrade-assistant tool. Was it removed? (It would not surprise me given the versioning issues - like the ones discussed here - that comes with it.)

C:\>dotnet tool install -g upgrade-assistant
Skipping NuGet package signature verification.

C:\>upgrade-assistant extensions --help

Error: Unknown command 'extensions'.

       extensions --help
       ^^^^^^^^^^ No such command


C:\>upgrade-assistant --help
USAGE:
    upgrade-assistant [OPTIONS] <COMMAND>

EXAMPLES:
    upgrade-assistant upgrade <PROJECT_PATH>
    upgrade-assistant upgrade <PROJECT_PATH> --operation Inplace --targetFramework net6.0
    upgrade-assistant upgrade <PROJECT_PATH> --operation SideBySide --targetFramework LTS --destination
<NEW_PROJECT_NAME_OR_PATH>

OPTIONS:
    -h, --help       Prints help information
    -v, --version    Prints version information

COMMANDS:
    upgrade    Upgrade project or its features. This command lets you pick upgrade options and guides through upgrade
               process

Add spec

0603bb4

dtivel mentioned this pull request Nov 9, 2023

Design proposal for signature provider plugins dotnet/sign#640

Closed

Apply feedback

eae146c

agr reviewed Jan 3, 2024

View reviewed changes

richlander reviewed Jan 4, 2024

View reviewed changes

vitek-karas reviewed Jan 4, 2024

View reviewed changes

vitek-karas reviewed Jan 5, 2024

View reviewed changes

agr reviewed Jan 5, 2024

View reviewed changes

JeromySt reviewed Jan 10, 2024

View reviewed changes

baronfel reviewed Jan 17, 2024

View reviewed changes


		The new interfaces assembly will be packaged and published to <https://nuget.org>, similar to [NuGetRecommender's contracts-only package](https://www.nuget.org/packages/Microsoft.DataAI.NuGetRecommender.Contracts). The Sign CLI team will manage the source code repository for this package and publish the package to <https://nuget.org>.

		The interfaces package itself can have package dependencies; however, because Sign CLI and all plugins would inherit new interfaces package dependencies, we should exercise restraint and caution before adding new dependencies. An example of a package dependency worth having is [`Microsoft.Extensions.Logging.Abstractions`](https://www.nuget.org/packages/Microsoft.Extensions.Logging.Abstractions). Sign CLI uses [`ILogger`](https://learn.microsoft.com/dotnet/api/microsoft.extensions.logging.ilogger) ubiquitously, it makes sense for plugins to write to a shared logger. From the provided `System.IServiceProvider` argument, plugins can request an `ILogger` instance for logging.


		##### `sign plugin install <PluginPackageId> [--version <Version>]`

		This command will install the latest release version of the plugin package identified by `PluginPackageId` using existing package sources and NuGet's default NuGet.config lookup order. If the latest version is already installed, the command will no-op.

Sign CLI: signature provider plugins #304

Are you sure you want to change the base?

Sign CLI: signature provider plugins #304

Conversation

dtivel commented Nov 9, 2023 • edited by richlander 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agr Jan 3, 2024 • 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

Choose a reason for hiding this comment

richlander commented Jan 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agocke commented Jan 4, 2024

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

clairernovotny Jan 17, 2024 • 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

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

baronfel commented Feb 21, 2024

anangaur commented Feb 22, 2024 • edited Loading

jkotas commented Feb 22, 2024

dtivel commented Nov 9, 2023 •

edited by richlander

Loading

agr Jan 3, 2024 •

edited

Loading

clairernovotny Jan 17, 2024 •

edited

Loading

anangaur commented Feb 22, 2024 •

edited

Loading