Replace test certificates with self-signed certificate generating command #7875

rjmholt · 2018-09-26T22:07:54Z

PR Summary

Adds a dependency on the newly published SelfSignedCertificate module so that we can construct our own self-signed certificates at test time rather than checking them in.

To do this, the following happens:

Publish-PSTestTools will Save-Module the SelfSignedCertificate from the gallery to
$RepoRoot/test/tools/Modules/
In WebListener.psm1 we import the SelfSignedCertificate module
- I tried both using module at the top of the psm1 and NestedModules in the psd1 here but neither seemed to work
Start-WebListener now builds both client and server certificates when it is called

PR Checklist

SteveL-MSFT · 2018-09-26T23:10:45Z

.gitignore

 /staging/
 /Packages/
 *.nuget.props
+/test/tools/Modules/SelfSignedCertificate/


Even though this module is for test, perhaps it's ok to just use install-module -scope currentuser? Adding it to a repo path breaks our work towards separating tests from dependencies on the repo.

I had that exact discussion with @JamesWTruher and argued that we shouldn't pollute the installed modules because PowerShell was build on a machine. But I will make the change

we should not be polluting the user's space with anything that isn't explicitly testing our behavior for the current user. These tests are very likely to be run on a dev box and we should not be adding stuff to the current user's scope. Further, this is a tool only for our tests. If the user happens to have this already, it still shouldn't use it, we should be using the module that we know we should be using in the location that we control without molesting their environment.

I'd rather put it into a temp folder then

SteveL-MSFT · 2018-09-26T23:11:47Z

.gitignore

 *.AppImage

+# Ignore certificates
+*.pfx


Similarly, the pfx files should be generated outside of the repo structure

The question is where. We don't have a cross-platform temp dir and I would very much prefer not to put it in $HOME. Ideally it would be on the $TestDrive, but anything that tries to operate across Describe blocks with a generated certificate will fail.

Arguably we should refactor things to do exactly that, but I think that is out of scope of this PR.

For now, Path::GetTempPath() is good enough I think

SteveL-MSFT · 2018-09-26T23:15:11Z

test/tools/Modules/WebListener/WebListener.psm1

+{
+    param([string]$CertificatePath)
+
+    $password = ConvertTo-SecureString -Force -AsPlainText 'password'


We shouldn't have a hardcoded password. Looks like the GeneratePassword() method isn't available in corefx. I think it's fine to generate a random alphanumeric password.

SteveL-MSFT · 2018-09-26T23:16:27Z

test/tools/Modules/WebListener/WebListener.psm1

        $initTimeoutSeconds  = 15
        $appDll              = 'WebListener.dll'
        $serverPfx           = 'ServerCert.pfx'
        $serverPfxPassword   = 'password'


Maybe be better to store in env var the generated password rather than hardcode

…emove-web-cmdlet-test-certs

…holt/PowerShell into remove-web-cmdlet-test-certs

iSazonov · 2018-09-27T11:38:26Z

We slow down CIs. Why we need the change?

SteveL-MSFT · 2018-09-27T17:20:36Z

@iSazonov as a Microsoft project, we can't have secrets (even test secrets) stored statically in the repo

SteveL-MSFT · 2018-09-27T17:23:07Z

@rjmholt the test failures are related to this PR

rjmholt · 2018-09-27T17:23:42Z

@SteveL-MSFT Yes I just fixed them 🤞

iSazonov

@SteveL-MSFT I found some samples of hard coded passwords in test files by key "password". (And in New-TestHost too).

iSazonov · 2018-09-28T03:22:56Z

build.psm1


+# Path to a directory to store modules we temporarily need to test PowerShell
+$script:TestModuleDirPath = Join-Path ([System.IO.Path]::GetTempPath()) 'PwshTestModules'
+New-Item -Force -ItemType Directory -Path $script:TestModuleDirPath


Do we need output to console? If no we should New-Item > $null.

iSazonov · 2018-09-28T03:27:41Z

build.psm1


    # Autoload (in subprocess) temporary modules used in our tests
-    $command += '$env:PSModulePath = '+"'$TestModulePath$TestModulePathSeparator'" + '+$($env:PSModulePath);'
+    $command += '$env:PSModulePath = '+"'$TestModulePath$TestModulePathSeparator$script:TestModuleDirPath$TestModulePathSeparator'" + '+$env:PSModulePath;'


I'm afraid of such expressions. Maybe use Join-Path before the line?

Join-Path won't work since we want ; and not \ (or : and not / on *nix). But I'll break this expression up a bit

build.psm1

daxian-dbw · 2018-10-01T17:40:19Z

test/powershell/Modules/Microsoft.PowerShell.Security/certificateCommon.psm1

+
+    # Hackery to create a new 10 digit random hex string as a password
+    $rand = [Random]::new()
+    $global:protectedCertPassword = ConvertTo-SecureString -Force -AsPlainText (((1..10).ForEach{ '{0:x}' -f $rand.Next(0xf) }) -join '')


Better expose it through a function, instead of a global variable. The global variable will linger around even after you remove the module from a session.

test/powershell/Modules/Microsoft.PowerShell.Security/certificateCommon.psm1

adityapatwardhan · 2018-10-01T18:17:11Z

test/tools/Modules/WebListener/WebListener.psm1

+
+    $random = [Random]::new()
+
+    return ((1..$Length).ForEach{ '{0:x}' -f $random.Next(0xf) }) -join ''


This code is the same as New-CertificatePassword can we re-use the code?

Is there a common module I could put the function to generate a password in that is imported in both the web cmdlet tests and the CmdMessage tests?

Right now the only common module I can think of is the SelfSignedCertificate module, but that's not part of this codebase

@rjmholt How about in here: https://github.com/PowerShell/PowerShell/blob/master/test/tools/Modules/HelpersCommon/HelpersCommon.psm1

…tePassword

…mand (PowerShell#7875)

[Feature] Replace test certificates with cert gen module

3762ab6

rjmholt requested review from JamesWTruher, SteveL-MSFT, adityapatwardhan and markekraus September 26, 2018 22:07

rjmholt requested review from TravisEz13, anmenaga and daxian-dbw as code owners September 26, 2018 22:07

Use fixed certificate module version

2b4e3e2

SteveL-MSFT requested changes Sep 26, 2018

View reviewed changes

rjmholt and others added 11 commits September 26, 2018 16:33

Use correct version parameter for Save-Module

4f5811b

Install SelfSignedCertificate rather than save it

faea431

Generate self-signed certificate for protected certificate test

32d483e

Update certificate-describing documentation

adf0199

Force installation of the self-signed certificate

bd9a9f9

Fix certificate test breakage

2478ca2

Add -AllowClobber to Install-Module

db62e11

Merge branch 'master' of https://github.com/rjmholt/PowerShell into r…

00a1065

…emove-web-cmdlet-test-certs

Merge branch 'remove-web-cmdlet-test-certs' of https://github.com/rjm…

7b82ff3

…holt/PowerShell into remove-web-cmdlet-test-certs

Remove hard-coded test passwords

4c6b542

[Feature] Hide SelfSignedCertificate module name from spell checker

89e27c7

rjmholt force-pushed the remove-web-cmdlet-test-certs branch from 07044ee to 89e27c7 Compare September 27, 2018 04:01

[Feature] De-verbosify x509 enum parameters used in cert creation

39abcd9

[Feature] Use module-qualified name for cert gen command

59d4344

[Feature] Get module name right

70c01d0

rjmholt force-pushed the remove-web-cmdlet-test-certs branch from 72256f8 to 70c01d0 Compare September 27, 2018 17:24

SteveL-MSFT approved these changes Sep 27, 2018

View reviewed changes

Robert Holt added 5 commits September 27, 2018 16:30

[Feature] Move SelfSignedCert module back to test tools module path

d92d44c

[Feature] Correct .gitignore of test tools path

bf39e17

[Feature] Fix module path usage, as asked by @JamesWTruher

dc41fef

[Feature] Put cert gen module in temp dir, revert .gitignore changes

8e8e6d8

[Feature] Fix module path mangling

e539e72

iSazonov reviewed Sep 28, 2018

View reviewed changes

[Feature] Address @iSazonov's feedback

40ac236

iSazonov approved these changes Sep 28, 2018

View reviewed changes

rjmholt assigned rjmholt and adityapatwardhan and unassigned rjmholt Sep 28, 2018

daxian-dbw reviewed Oct 1, 2018

View reviewed changes

build.psm1 Show resolved Hide resolved

daxian-dbw reviewed Oct 1, 2018

View reviewed changes

[Feature] Change global variable to function-exposed module variable

b5cc1ea

adityapatwardhan requested changes Oct 1, 2018

View reviewed changes

Robert Holt added 2 commits October 1, 2018 11:27

[Feature] Prevent use of Get-CertificatePassword before New-Certifica…

2a5722b

…tePassword

[Feature] Factor New-RandomHexString into test helper module

f772d6d

adityapatwardhan approved these changes Oct 1, 2018

View reviewed changes

adityapatwardhan merged commit 649d342 into PowerShell:master Oct 1, 2018

markekraus mentioned this pull request Oct 1, 2018

Move certificateCommon.psm1 to a Common Area and Replace Self-Signed Cert Valid for All tests #4598

Closed

adityapatwardhan pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Apr 9, 2019

Replace test certificates with self-signed certificate generating com…

478fe24

…mand (PowerShell#7875)


		$random = [Random]::new()

		return ((1..$Length).ForEach{ '{0:x}' -f $random.Next(0xf) }) -join ''

Replace test certificates with self-signed certificate generating command #7875

Replace test certificates with self-signed certificate generating command #7875

Uh oh!

Conversation

rjmholt commented Sep 26, 2018

PR Summary

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Sep 27, 2018

Uh oh!

SteveL-MSFT commented Sep 27, 2018

Uh oh!

SteveL-MSFT commented Sep 27, 2018

Uh oh!

rjmholt commented Sep 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rjmholt commented Sep 27, 2018 •

edited

Loading

iSazonov left a comment •

edited

Loading