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

Enable user protected keys in certificate store #757

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

mcurland
Copy link
Contributor

Sign needs to be able to use user-protected keys, such as keys stored in a USB eToken device. These generally require user interaction when the key is used and signing will fail when dialogs are blocked.

User interaction requires the 'Silent' flag to not be set in CertificateStoreService.GetRsaAsync method. The rest of this is plumbing to make a user option available to this code.

The new option is --user-protected-key/-upk to turn off the silent flag. The flag name corresponds to the enum value
CspProviderFlags.UseUserProtectedKey in
System.Security.Cryptography. I preferred this to not-silent or -silent false (with a true default).

I'm sure there is a process for translation, but I do not know if this is done with the initial pull request.

@mcurland mcurland requested a review from a team as a code owner August 15, 2024 21:09
@mcurland
Copy link
Contributor Author

@mcurland please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree company="ORM Solutions, LLC"

@dlemstra
Copy link
Contributor

dlemstra commented Aug 16, 2024

I wonder if --suppress-ui-prompting would be a better name for the option?

@mcurland
Copy link
Contributor Author

I did not want to change the current default behavior (using the .Silent flag), so suppress-ui-prompting (or suppress-ui-prompts or just suppress-ui) is what it is doing already. So, you could do this, but you would need a false value, not just a flag name, because true is the default state..

I think a rename to -ui --allow-ui-prompts would be more intuitive. I have no strong affinity for the name I chose, I just took one that was already part of the Cryptography API set, and I don't like adding negated flag names (like --not-silent).

Are you ok with --allow-ui-prompts and -ui? These would keep the same behavior (default false, unspecified adds the .Silent flag).

@dlemstra
Copy link
Contributor

dlemstra commented Aug 17, 2024

I am contributor to this project but not one of the people that will make the decision which name will be used. You can use --suppress-ui-prompting=false and have true as the default value.

@mcurland
Copy link
Contributor Author

Did we cross messages? I just pushed --allow-ui-prompts with -ui as the short form.

If you're not deciding on this we'll let the big bosses decide. I'm much more worried about being able to use my hardware signing token than the flag name.

We shouldn't need to provide a Boolean value to a flag. Let the flag speak for itself.

I've been doing tool development a really long time with significant UI work (I'll date myself by claiming the ui design and implementation of the initial VB/VBA IntelliSense implementation circa '96). I have never heard 'prompting' used as a noun in computer jargon. It is always a 'prompt'. You'd get a good laugh here in Utah if you said "I received a prompting from my computer" because 'prompting' is generally used in a religious context. In the months of work time I've spent in the Netherlands I know your English is amazing, but I'm going to pull native speaker rank on this one.

It isn't my tool and I'm less invested now that I can actually use the $800 3-year signing token I just bought from a discount reseller (I paid $180 in 2021 for the same capability, which already felt extortionary). The article on this tool (and deprecating VsixSignTool) didn't really come close to making this a seamless experience (it is not obvious that the extracted public-key-only .cer file provides the magic hash you need for the cfp input), and then it didn't work anyway. It was nice to have the source so I could at least move forward. I figured I'd share the work as I'm sure I won't be the only one trying to sign locally with a hardware token.

Note that there is a one-word option here: change "CngKeyOpenOptions.Silent" to "CngKeyOpenOptions.None", but I did not want to change the default behavior. However, this is by far the easiest fix.

@dtivel
Copy link
Collaborator

dtivel commented Sep 18, 2024

@mcurland, thank you for making this change. We appreciate your contribution!

After some discussion, we think --interactive / -i is more familiar for .NET users. .NET already has a strong pattern of using --interactive for scenarios like this. The default remains the same (not interactive).

I've pushed a branch with a commit for the rename.

If you want to cherry-pick the commit and update this PR, great. Otherwise, if we don't hear back from you, I'll close this PR, open a new PR based on my branch, and merge that.

mcurland and others added 3 commits September 18, 2024 13:12
Sign needs to be able to use user-protected keys, such as keys stored
in a USB eToken device. These generally require user interaction
when the key is used and signing will fail when dialogs are blocked.

User interaction requires the 'Silent' flag to not be set in
CertificateStoreService.GetRsaAsync method. The rest of this is
plumbing to make a user option available to this code.

The new option is --user-protected-key/-upk to turn off the silent
flag. The flag name corresponds to the enum value
CspProviderFlags.UseUserProtectedKey in
System.Security.Cryptography. I preferred this to not-silent or
-silent false (with a true default).

I'm sure there is a process for translation, but I do not know if this
is done with the initial pull request.
Use a more intuitive name for the user-protected-key option.

--allow-ui-prompts has a clear meaning and shortens nicely to -ui.
@mcurland
Copy link
Contributor Author

I cherry-picked into the PR branch. I'm glad you settled on a name.

I might crack this again one of these days I'll figure out why it is so incredibly slow to sign my VSIX files. I don't know if it is the app, the device or the timestamp server. My release break down into {free|pro, admin|peruser, VS20[17|19|22]} VSIX files, so 12 signs per build. The signing used to be a few seconds, now it is closer to 2 minutes/VSIX, so build-and-post in 10 minutes is no longer my reality.

@dtivel
Copy link
Collaborator

dtivel commented Sep 18, 2024

Thanks, @mcurland!

If you have additional details, especially repro steps or the last build where it was fast, please create a new issue. We'd love to make signing time faster.

@dtivel dtivel merged commit ea23724 into dotnet:main Sep 18, 2024
3 checks passed
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.

3 participants