Bring back Certificate provider parameters by iSazonov · Pull Request #10622 · PowerShell/PowerShell

Ilya (iSazonov) · 2019-09-25T10:16:40Z

PR Summary

Bring back Certificate provider parameters:
- DNSName
- DocumentEncryptionCert
- EKU
- ExpiringInDays
- SSLServerAuthentication
Add new tests and update existing tests. (There are many style issues - will fix in follow PR.)
Update test certificate for EKU and DNSName tests.
Remove old unneeded code.

PR Context

Related #3847

After removing undocumented certificate API in PR #3818 we lost some parameters in Certificate provider.

We need to review documentation because the parameters is not all documented and it seems they do not always documented correctly.
(For reference https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.security/about/about_certificate_provider?view=powershell-6)

PR Checklist

Ilya (iSazonov) · 2019-09-25T13:59:28Z

It seems Get-PfxCertificate crush the pwsh process if open "GoodCertificate" which I updated. All works well on Windows and Linux but fail on MacOs. Looks like a bug on MacOs. At least pwsh should return an error and doesn't crush.
I haven't MacOs and can not investigate in depth. I need help to resolve the issue.

Travis Plunk (TravisEz13)

This change requires a security review

Travis Plunk (TravisEz13) · 2019-09-25T21:37:59Z

Ilya (@iSazonov) I don't have time to review now. Blocking this PR, until I can organize a security review.

Ilya (iSazonov) · 2019-09-26T03:10:51Z

Add WIP while waiting security review.

Travis Plunk (TravisEz13) · 2019-10-04T17:27:21Z

Have you verified this will work with punycode?

Do you mean that Windows PowerShell support also punycode in dnsname filter? I did not find this in docs https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.security/about/about_certificate_provider?view=powershell-6

What is a scenario where it could be used?

When the name in the cert is in punycode. This is a compliance requirement. you deleted the code to do this.

Clear about compliance. Not clear what code do you mean.
Also docs say:

DnsName <Microsoft.PowerShell.Commands.DnsNameRepresentation>
This parameter gets certificates that have the specified domain name or name pattern in the DNSNameList property of the certificate. The value of this parameter can either be "Unicode" or "ASCII". Punycode values are converted to Unicode. Wildcard characters (*) are permitted.

It is not clear how punycode converted to Unicode. It seems this did unpublic code (in P/Invoke) which we removed long ago.

I looked original code which was removed and see only one difference - DNSName parameter was DnsNameRepresentation type, now string.
In both cases DnsNameRepresentation is initialized by one constructor with string parameter which assigned to DNSname and punycode - no conversion. So new code works for punycode.
Question is should we change type from string to DnsNameRepresentation?

Paul Higinbotham (PaulHigin)

LGTM

Travis Plunk (TravisEz13) · 2019-10-07T22:32:06Z

macOS tests are hanging

  Describing CmsMessage cmdlets and Get-PfxCertificate basic tests
Certificate written to /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/protectedCert.pfx
Enter password:                                                                                                                                                                                                                                                                                                                                 
Execution of { & $powershell $PSFlags -c $command } by build.psm1: line 1231 failed with exit code 1
At /Users/vsts/agent/2.158.0/work/1/s/build.psm1:2074 char:17
+                 throw $errorMessage
+                 ~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : OperationStopped: (Execution of { & $p\u2026ed with exit code 1:String) [], RuntimeException
+ FullyQualifiedErrorId : Execution of { & $powershell $PSFlags -c $command } by build.psm1: line 1231 failed with exit code 1

Ilya (iSazonov) · 2019-10-08T03:07:36Z

Travis Plunk (@TravisEz13)

macOS tests are hanging

I did not change anything except the certificate. I tend to think it's a bug outside PowerShell :-(
I haven't MacOs and I only can revert to old certificate and generate new for new tests to
minimize area.

Travis Plunk (TravisEz13) · 2020-05-21T22:58:01Z

PoshChan-Bot (@PoshChan) Please remind me in 24 hours

PoshChan-Bot (PoshChan) · 2020-05-22T22:58:48Z

Travis Plunk (@TravisEz13), this is the reminder you requested 24 hours ago

Deleted user (ghost) · 2020-05-30T02:00:14Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

Travis Plunk (TravisEz13) · 2020-05-30T17:35:51Z

Can you resolve the conflicts?

Travis Plunk (TravisEz13) · 2020-06-01T19:57:52Z

PoshChan-Bot (@PoshChan) Please remind me in 1 hour

PoshChan-Bot (PoshChan) · 2020-06-01T20:58:05Z

Travis Plunk (@TravisEz13), this is the reminder you requested 1 hour ago

Deleted user (ghost) · 2020-06-25T19:06:53Z

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

Handy links:

Release Notes

Ilya (iSazonov) added Documentation Needed in this repo Documentation is needed in this repo CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Sep 25, 2019

Ilya (iSazonov) added this to the 7.0.0-preview.5 milestone Sep 25, 2019

Ilya (iSazonov) requested review from Steve Lee (SteveL-MSFT), Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan) and Dongbo Wang (daxian-dbw) September 25, 2019 10:16

Ilya (iSazonov) assigned Travis Plunk (TravisEz13) Sep 25, 2019

Ilya (iSazonov) force-pushed the add-expiringindays branch from edb9a44 to 4e915bc Compare September 25, 2019 12:11

PoshChan-Bot (PoshChan) reviewed Sep 25, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

PoshChan-Bot (PoshChan) reviewed Sep 25, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

PoshChan-Bot (PoshChan) reviewed Sep 25, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

PoshChan-Bot (PoshChan) reviewed Sep 25, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

Ilya (iSazonov) force-pushed the add-expiringindays branch from 79ea18d to a24b03b Compare September 25, 2019 13:52

Ilya (iSazonov) requested a review from Rob Holt (rjmholt) September 25, 2019 14:00

Travis Plunk (TravisEz13) requested changes Sep 25, 2019

View reviewed changes

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 25, 2019

Ilya (iSazonov) changed the title ~~Bring back Certificate provider parameters~~ WIP: Bring back Certificate provider parameters Sep 26, 2019

ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Sep 26, 2019

Travis Plunk (TravisEz13) reviewed Oct 4, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

Paul Higinbotham (PaulHigin) suggested changes Oct 4, 2019

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs Outdated

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

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

ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Oct 4, 2019

Paul Higinbotham (PaulHigin) approved these changes Oct 7, 2019

View reviewed changes

Add ExpiringInDays filter in Certificate provider

c4f735e

Ilya (iSazonov) force-pushed the add-expiringindays branch from 30c1b06 to c4f735e Compare March 29, 2020 18:42

PoshChan-Bot (PoshChan) reviewed Mar 29, 2020

View reviewed changes

Comment thread src/Microsoft.PowerShell.Security/security/CertificateProvider.cs

Fix test

ee6f752

Ilya (iSazonov) modified the milestones: 7.1.0-preview.1, 7.1.0-preview.2 Mar 29, 2020

Travis Plunk (TravisEz13) modified the milestones: 7.1.0-preview.2, 7.1.0-preview.3 Apr 24, 2020

Travis Plunk (TravisEz13) modified the milestones: 7.1.0-preview.3, 7.1.0-preview.4 May 14, 2020

ghost added the Review - Needed The PR is being reviewed label May 30, 2020

ghost removed the Review - Needed The PR is being reviewed label May 30, 2020

Travis Plunk (TravisEz13) added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label May 30, 2020

Merge branch 'master' into add-expiringindays

430fa2a

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

Travis Plunk (TravisEz13) closed this Jun 1, 2020

Travis Plunk (TravisEz13) reopened this Jun 1, 2020

Travis Plunk (TravisEz13) approved these changes Jun 2, 2020

View reviewed changes

Travis Plunk (TravisEz13) merged commit 73e8427 into PowerShell:master Jun 2, 2020

Ilya (iSazonov) deleted the add-expiringindays branch June 3, 2020 02:57

Ilya (iSazonov) mentioned this pull request Jun 3, 2020

Need to document some Certificate provider parameters MicrosoftDocs/PowerShell-Docs#6059

Closed

12 tasks

Ilya (iSazonov) removed the Documentation Needed in this repo Documentation is needed in this repo label Jun 3, 2020

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

Bring back Certificate provider parameters (PowerShell#10622)

2153ed8

Uh oh!

Conversation

Ilya (iSazonov) commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ilya (iSazonov) commented Sep 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Travis Plunk (TravisEz13) left a comment

Choose a reason for hiding this comment

Uh oh!

Travis Plunk (TravisEz13) commented Sep 25, 2019

Uh oh!

Ilya (iSazonov) commented Sep 26, 2019

Uh oh!

Travis Plunk (TravisEz13) Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Oct 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Travis Plunk (TravisEz13) Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Oct 4, 2019

Choose a reason for hiding this comment

Uh oh!

Ilya (iSazonov) Oct 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Paul Higinbotham (PaulHigin) left a comment

Choose a reason for hiding this comment

Uh oh!

Travis Plunk (TravisEz13) commented Oct 7, 2019

Uh oh!

Ilya (iSazonov) commented Oct 8, 2019

Uh oh!

Uh oh!

Travis Plunk (TravisEz13) commented May 21, 2020

Uh oh!

PoshChan-Bot (PoshChan) commented May 22, 2020

Uh oh!

Deleted user (ghost) commented May 30, 2020

Uh oh!

Travis Plunk (TravisEz13) commented May 30, 2020

Uh oh!

Travis Plunk (TravisEz13) commented Jun 1, 2020

Uh oh!

PoshChan-Bot (PoshChan) commented Jun 1, 2020

Uh oh!

Deleted user (ghost) commented Jun 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Ilya (iSazonov) commented Sep 25, 2019 •

edited

Loading

Ilya (iSazonov) commented Sep 25, 2019 •

edited

Loading

Ilya (iSazonov) Oct 4, 2019 •

edited

Loading