Update `CmsCommands` to use Store vs cert provider by mikeTWC1984 · Pull Request #11643 · PowerShell/PowerShell · GitHub

PR Summary

This PR replaces cert provider (windows only) with X509Store class to resolve certificate by name/thumbprint on CmsUtils.

PR Work items:

ResolveFromSubjectName and ResolveFromThumbprint - remove cert: provider call and merge into single method ( both lookups can utilize X509Certificate2Collection.Find )
Remove GetCertEKU (that uses native calls). Use X509Certificate2 Extensions property instead in CertHasOId method.
LocalMachine/My store - Windows only
replace List<509Certificate2> with X509Certificate2Collection on related methods.
Update tests
test on Mac
add wildcard for subjectname lookup

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
Breaking changes
- None
- OR
- Experimental feature(s) needed
  - Experimental feature name(s):
User-facing changes
- Not Applicable
- OR
- Documentation needed
  - Issue filed:
Testing - New and feature
- N/A or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
Tooling
- I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
- OR
- I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
  - Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
  - Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
  - Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
  - Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

All CLA requirements met.

mikeTWC1984 We should export the cmdlets on Unix-s:

PowerShell/src/Modules/Unix/Microsoft.PowerShell.Security/Microsoft.PowerShell.Security.psd1

Line 10 in bf85f3f

    
           CmdletsToExport="Get-Credential", "Get-ExecutionPolicy", "Set-ExecutionPolicy", "ConvertFrom-SecureString", "ConvertTo-SecureString", "Get-PfxCertificate"

https://github.com/PowerShell/PowerShell/blob/master/test/powershell/engine/Basic/DefaultCommands.Tests.ps1

Also please review tests https://github.com/PowerShell/PowerShell/tree/master/test/powershell/Modules/Microsoft.PowerShell.Security

GitHub
PowerShell/PowerShell
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.

I added WIP until the PR is ready to review.

OK. It's pretty much ready for review, but somehow Powershell-CI pester test is failing. I ran 3 tests today, all failed for linux, 2 for Mac and 1 for Windows. I also tried Start-PSPester locally for my branch and Powershell's master and got ~30 errors. A the moment it's not quite clear what's wrong. . Probably some temp issue, will keep experimenting. The build itself goes through, CMS commands seem to work, but I haven't done thorough testing yet.

mikeTWC1984 You should enable the cmdlets in DefaultCommands.Tests.ps1 too (See link above).

This PR does not appear to conflict with #11590

OK, tests finally pass on Mac. Turns out certs got added to the store automatically during creation (X509Store.add actually doesn't work ) . I'll work on the Doc today or tomorrow.

Ok, applied all the latest changes. I was back and forth between using CertificateCollection.Find and foreach loop to match SubjectName, and I'm finally sticking with foreach, so it's the same approach as original version had. I realized find will always do "contains" match, but I guess mostly user would need exact match.

LGTM
We still need to do a security review, but the code changes look good.

mikeTWC1984 If you ask about this IsMatch() it is ok to preserve the old behavior until we get a negative feedback.

This one is OK. It's bit counterintuitive to use wildcard (and directory) when expecting single item, but helps when typing those names manually. After going back and forth with this, I realized omitting CN= prefix in subject name would be very useful, so I added one extra IsMatch for simple name too

syncing fork

LGTM with two minor comments.

Travis Plunk (@TravisEz13) I think this is ready for security review. I also filed Documentation issue.

mikeTWC1984 Yes, I've already schedule the review. The reviews are Confidential and we have to decide how to communicate the results after the review.

mikeTWC1984 Security review summary: we don't think the security footing of the cmdlet has changed.

mikeTWC1984 Thanks for your contribution!

Cool, thanks all. There are few enhancements we can add to those commands, I'll probably submit an issue or PR sometime soon. I guess it's also the time to update documentation, will work on that.

mikeTWC1984 Open new issue(s) if there is a field for discussion.

🎉v7.1.0-preview.1 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

updated CmsCommands to use Store vs cert provider

8bd284e

ghost assigned Ilya (iSazonov) Jan 21, 2020

Aleksandrovs added 2 commits January 21, 2020 13:01

cmscommands fixed x509collection type

be31050

codacity fix 1

52e1679

mikeTWC1984 mentioned this pull request Jan 21, 2020

CmsCommands - simple fix for linux #11630

Closed

Ilya (iSazonov) requested review from Paul Higinbotham (PaulHigin), Travis Plunk (TravisEz13) and Dongbo Wang (daxian-dbw) January 22, 2020 03:38

Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Jan 22, 2020

Ilya (iSazonov) added this to the 7.1.0-preview.1 milestone Jan 22, 2020

Aleksandrovs added 2 commits January 23, 2020 10:18

removed GetCertEKU, getting EKUs from cert directly

164a0dd

added Cms commands to Unix modules

d020ca9

Ilya (iSazonov) changed the title updated CmsCommands to use Store vs cert provider WIP: Update CmsCommands to use Store vs cert provider Jan 23, 2020

removed break from CertHasOid

7124b18

mikeTWC1984 changed the title WIP: Update CmsCommands to use Store vs cert provider Update CmsCommands to use Store vs cert provider Jan 23, 2020

mikeTWC1984 changed the title Update CmsCommands to use Store vs cert provider WIP: Update CmsCommands to use Store vs cert provider Jan 23, 2020

checking if this is failing pester on linux

c9fe59d

mikeTWC1984 changed the title WIP: Update CmsCommands to use Store vs cert provider Update CmsCommands to use Store vs cert provider Jan 24, 2020

updated CertHasOid and DefaultCommandsTests

9da7f8e

Aleksandrovs and others added 3 commits January 28, 2020 16:20

added new test for CMS commands

931538b

codacity fix

f076dcc

removing test

a3a4b25

Ilya (iSazonov) reviewed Jan 29, 2020

View reviewed changes

Comment thread test/powershell/Modules/Microsoft.PowerShell.Security/CmsMessage2.Tests.ps1 Outdated

Ilya (iSazonov) reviewed Jan 29, 2020

View reviewed changes

Comment thread test/powershell/Modules/Microsoft.PowerShell.Security/CmsMessage2.Tests.ps1 Outdated

mikeTWC1984 added 2 commits February 3, 2020 23:52

add findBySubjectName search + codacity fix

514a9e8

codacity fix2

f00595f

Ilya (iSazonov) reviewed Feb 4, 2020

View reviewed changes

code review refactor1

9f3baa0

Ilya (iSazonov) reviewed Feb 4, 2020

View reviewed changes

Paul Higinbotham (PaulHigin) suggested changes Feb 4, 2020

View reviewed changes

Comment thread src/System.Management.Automation/security/SecuritySupport.cs Outdated

Comment thread src/System.Management.Automation/security/SecuritySupport.cs Outdated

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 4, 2020

review refact2 + go back to foreach

3b6ad33

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Feb 4, 2020

Paul Higinbotham (PaulHigin) approved these changes Feb 4, 2020

View reviewed changes

Ilya (iSazonov) reviewed Feb 5, 2020

View reviewed changes

Aleksandrovs added 2 commits February 5, 2020 10:55

merge remote-tracking branch 'upstream/master'

a7b19d9

syncing fork

test refactor + added simple name lookup

0c69438

Ilya (iSazonov) reviewed Feb 6, 2020

View reviewed changes

Comment thread src/System.Management.Automation/security/SecuritySupport.cs Outdated

Comment thread test/powershell/Modules/Microsoft.PowerShell.Security/CmsMessage2.Tests.ps1 Outdated

fixed minor format issues

3413394

mikeTWC1984 mentioned this pull request Feb 6, 2020

Document CMS cmdlets for Linux MicrosoftDocs/PowerShell-Docs#5422

Closed

11 tasks

Travis Plunk (TravisEz13) approved these changes Feb 6, 2020

View reviewed changes

Travis Plunk (TravisEz13) changed the title Update CmsCommands to use Store vs cert provider Update CmsCommands to use Store vs cert provider Feb 6, 2020

Travis Plunk (TravisEz13) merged commit 69bf704 into PowerShell:master Feb 6, 2020

microsoft-github-policy-service Bot added the In-PR Indicates that a PR is out for the issue label Feb 4, 2025

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026

Update CmsCommands to use Store vs cert provider (PowerShell#11643 )

acb9366

+                                          if (subjectNamePattern.IsMatch(cert.Subject))
+                                          {
+                                              certificatesToProcess.Add(cert);
+                                          }

Update `CmsCommands` to use Store vs cert provider by mikeTWC1984 · Pull Request #11643 · PowerShell/PowerShell · GitHub

Uh oh!

Update CmsCommands to use Store vs cert provider - #11643

Update CmsCommands to use Store vs cert provider#11643Travis Plunk (TravisEz13) merged 25 commits intoPowerShell:masterPowerShell/PowerShell:masterfrom mikeTWC1984:mastermikeTWC1984/PowerShell-1:masterCopy head branch name to clipboard

Conversation

mikeTWC1984 commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Work items:

PR Checklist

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Jan 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) commented Jan 22, 2020 • edited by unfurl-links Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ilya (iSazonov) commented Jan 23, 2020

Uh oh!

mikeTWC1984 commented Jan 23, 2020

Uh oh!

Ilya (iSazonov) commented Jan 24, 2020

Uh oh!

xtqqczze commented Jan 25, 2020

Uh oh!

Uh oh!

Uh oh!

mikeTWC1984 commented Feb 4, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikeTWC1984 commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Paul Higinbotham (PaulHigin) left a comment

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

mikeTWC1984 Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ilya (iSazonov) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mikeTWC1984 commented Feb 6, 2020

Uh oh!

Travis Plunk (TravisEz13) commented Feb 6, 2020

Uh oh!

Travis Plunk (TravisEz13) commented Feb 6, 2020

Uh oh!

Ilya (iSazonov) commented Feb 7, 2020

Uh oh!

mikeTWC1984 commented Feb 7, 2020

Uh oh!

Ilya (iSazonov) commented Feb 7, 2020

Uh oh!

Update CmsCommands to use Store vs cert provider#11643
Travis Plunk (TravisEz13) merged 25 commits into
PowerShell:masterfrom
mikeTWC1984:master

mikeTWC1984 commented Jan 21, 2020 •
edited
Loading

Microsoft Contribution License Agreements (msftclas) commented Jan 21, 2020 •
edited
Loading

Ilya (iSazonov) commented Jan 22, 2020 •
edited by unfurl-links Bot
Loading

mikeTWC1984 commented Feb 4, 2020 •
edited
Loading