[SecretsManagement] [Design] Improved Script Extension Provider Model

[JustinGrote]

Summary of the new feature/enhancement

As a Powershell Developer, I want the highest performance and least friction method to add secrets vault support to my Powershell Script modules.

The current binary module implementation seems fine, but the PS script method could be better. The existing method of importing a "sub module" has lots of limitations around performance and debugging https://twitter.com/Jaykul/status/1228098209999925248 https://twitter.com/JeremyMcgee73/status/1228135629361270784

There are some other more standard "tried-and-true" methods to consider

Proposed technical implementation details (optional)

Powershell Class (DSC Style)

Powershell Classes were purpose-built for this very purpose with DSC, the same concepts can be leveraged for SecretsManagement https://docs.microsoft.com/en-us/powershell/scripting/dsc/resources/authoringresourceclass?view=powershell-7

Module Private Functions

Some well-defined non-exported functions such as Get-MicrosoftSecretsManagementSecret can be defined within a module scope, and SecretsManagement can call those functions within the module scope. This has the advantage of the function being able to use any state that the module currently has, such as existing connections to applications, so that in several cases the authentication aspect can be abstracted out and secret keys don't have to be kept in the metastore which will help speed the cross-platform implementation

The reason to not export them is to both avoid clobber, and cluttering the command space with e.g. Get-AzureKeyVaultMicrosoftSecretsManagementSecret. The implementation could also allow the provider to specify what functions to use for each purpose, but that makes implementation more difficult IMHO. @jaykul to weigh in.

Processing Order

All 3 methods can be enabled, and developers can choose their favorite, priority discovery order should be:

1. Class
2. Private Module functions
3. Nested Module

[Jaykul]

First: The private module functions seem like a bad idea to me. It introduces a new concept (the hidden module) and new magic (the special folder name), but to what purpose? What is gained from this, and why do these commands need to be hidden?

Second: The current implementation is ... bad.

1. It's bad for debugging. The module gets reimported each time in a new session, break points don't work, etc.
2. It's bad for performance. Every command is called after a fresh import of the module in a fresh PowerShell instance.
3. It's bad for design. Recreating PowerShell every time means there's no persistent variable scope -- both module ("script") and "global" scope are reset between calls. There's no way to persist a connection to a service or the decrypting of a crypto vault on disk, etc. This might be fine for stateless web services, but it's going to mean re-authenticating and re-decrypting on every call! Just look at the examples: they are reimporting all their dependencies and re-connecting their credentials each time. Worse: this was not documented.
4. It's bad for the pipeline. Actually, in this implementation there is no way to use these commands in a pipeline, so it might be more accurate to say it completely breaks the pipeline. Worse: this was not documented.
5. It's bad for standards. If I already have a module like SecretServer, I can't just add a few commands to support SecretsManagement. Instead, I have to create weird nested module to put the functions in, and change my build/test/deploy process to accomodate this weird magic folder.

Frankly, to sum up these points: the whole thing seems designed in C# with PowerShell shoehorned in poorly. Nobody should implement vault extensions in PowerShell script, given this model.

As a side note: the C# examples like AKVaultExtension are bad examples ... because they're not actually C# examples, they're wrappers around PowerShell!

Thoughts on improving the PowerShell implementation

As @JustinGrote said, PowerShell developers can derive classes from interfaces (or abstract base classes) the same as .Net develowers. If it's too hard to leverage PowerShell classes from C#, you know what's not hard? Leveraging them from PowerShell. Maybe the glue cmdlets need to be written in PowerShell in order to make this work properly without making PowerShell feel like a second-class citizen in it's own ecosystem. To me, having the PowerShell implementation be first-class is more important than performance.

Most importantly, I think, we have to stop creating new PowerShell instances for each command execution. If we really need to implement the manager in C# and use nested PowerShell instances, we need to create one instance (for each registered vault?) and keep it around, so that scope variables and debugging are possible (debugging will still be hard, but at least we could attach to the PowerShell instance and debug)...

Personally ... I think using an internal PowerShell is just not the right idea:

• Let the commands be public commands in a public module.
• Use them from the global scope.

If you want to avoid name collisions (you don't need to), you can just (re)import the module with a prefix. That is, given I adapt the SecretServer module, and someone writes:

Register-SecretsVault -Module SecretServer -Name Work -VaultParameters @{ Uri = "https://ss.poshcode.org/winauthwebservices/sswinauthwebservice.asmx" }

The SecretsManagement module could import the SecretServer module like this:

Import-Module SecredServert -Prefix PSSMWork -Scope Global

And to look up a secret, call Get-PSSMWorkSecret instead of Get-Secret passing in the VaultParameters the way it currently does ...

[JustinGrote]

@Jaykul Re: Private Functions (assuming you were referring to my recommendation and not the current implementation)

Per, my description, the intent of the private functions was so you could have SecretsManagement functionality integrated within the same module, so for instance a keepass module could expose a secretsmanagement implementation without the user having to download a completely separate module. Since that would be imported as a normal course of action, having the functions private would prevent clobber, and would also prevent a separate module import per vault if you set up, say, 5 azure vaults, they all can use the same module for efficiency and infer state from the module if present.

It's not a "hidden module" nor does it require a "secret folder", you would just add certain non-exported functions to your module to do the get/set/remove, very similar to a DSC script resource.

I think your method of public commands as mentioned would work, but I'm concerned when you have a lot of vaults registered, your command workspace gets very cluttered with a bunch of what are effectively "internal" commands that one wouldn't normally use in day-to-day use, hence why I recommended them to be unexported. They are also commonly discoverable with the same function name.

However I think we are both in agreement that a DSC style powershell class makes the most sense for at least one implementation method, and since the module was developed in C#, just exposing an interface that the powershell class can inherit (not that there'd be much to inherit, can act as a bit of a validation).

[vexx32]

Honestly, every time I take the slightest peek at SecretsManagement and the discussions around it, I can only think: "why weren't all of these details discussed and laid out in the original RFC so we could talk about design ahead of time?"

I feel that the module currently needs a significant amount of work to get it to a point where I'd recommend anyone actually use it. I think we may be better off handling individual "vaults" in completely separate modules, at least until the open questions around this module are resolved properly.

I really want to see this effort succeed, I want there to be a really solid module like this that we can point later authors to as a gold standard for how to implement an extensible module -- or at least a best effort. I think what's really needed is a more complete design document to be drawn up and published, before we go any further in development. We're already seeing things start to be built in that probably will be very hard to change further down the line. The community needs and in my opinion deserves to be fully involved in the design of this module if we're going to be expected to use it and its various extensions.

If we settle for an incomplete design, we'll only ever get an incomplete implementation -- and the same can be said for extensions that end up being created to work with it. If the core of it is hard to use, all of the modules will end up being hard to use, and most likely quite difficult to maintain properly as well.

I recognise this module is supposed to be in preview, but the current direction is not encouraging at the moment, and the overall communication around the design of the module itself is worryingly absent. 😕

[JustinGrote]

@vexx32 I agree it feels like an echo chamber at the moment. I optimistically assume @PaulHigin and @SydneyhSmith are taking well deserved vacations before diving back in :)

[PaulHigin]

Thanks for your input. There is a lot of information here, and frankly I don't fully understand the point of some of the feedback. But I'll respond as best I can.

Extension vaults are PowerShell modules, either a binary module or a script module. We could also have a PowerShell class based extension vault, but it seems more of a nice to have feature. We decided on the these two extension module types because we thought it would be familiar to most developers. PowerShell classes are less well known.

A binary module requires implementing an abstract class and four simple methods. There doesn't seem to be much feedback on this.

A script module implement four simple functions, and resides in a subfolder (like DSC) so that its functions do not pollute the user command space. I experimented with module scope but found it to be unreliable so opted for the DSC structure.

Regarding script module debug-ability ... well its a PowerShell module so you debug it just like any other PowerShell module. I feel that the majority of problems can be found and fixed that way. Once the extension module is registered, the author can use the 'ValidateExtension' function (which we discussed in another issue) to validate the vault. Finally an author can always use 'Wait-Debugger' and 'Debug-Runspace' to debug complex issues while the module is registered as a vault. These are all normal (if advanced) debugging techniques. Yes, they are involved and require some expertise, but we assume vault authors will be more advanced developers. We want using secrets to be easy, but developing good extension vaults requires advanced skills.

Regarding performance of the script module extension vault, I decided to make resource usage and security a priority. I envisage users of Secrets Management having large scripts where they may be fanning out to hundreds of machines, and a very small (but important) part is to retrieve secrets. I didn't want the working session to include persistently loaded vault modules. In addition I worried about information leakage with multiple third part extension vaults loaded in the same session. A malicious extension vault could harvest secrets from other vaults. I did think about caching sessions (runspaces) for each loaded module, but again it seemed like a poor use of resources for retrieving some secrets as a small part of the workload. But this is something that can be easily changed if needed. It is difficult to predict performance problems without specific scenarios. Spinning up a runspace to run a one-off script is a common pattern if performance is not a concern, and it is not yet clear to me that it is.

Regarding the RFC and discussion of design trade offs, I feel the document had (more than) enough discussion on how this should work. At some point you need to put a stake in the ground, and that is what this alpha preview is.

[JustinGrote]

@PaulHigin Thanks for the feedback. Let's see how the next round of extension authoring goes with the changes you've made, I'll leave this issue open for now. It still seems to me defining an interface that can be extended by a Powershell Class seems the best way to go (assuming we are targeting PS5+ compatibility which I assume we are) and a good way to get a lot of authors used to doing DSC modules into the fold.

There can always be both! I assume the way you are loading in the methods from extensions could be easily re-exposed as a class (I haven't looked at the codebase that deeply)