Enable securestring cmdlets for non-Windows#9199
Enable securestring cmdlets for non-Windows#9199TravisEz13 merged 5 commits intoPowerShell:masterfrom
Conversation
|
Reviewed the CodeFactor issues, all are for code I didn't change (but wrapped in |
vexx32
left a comment
There was a problem hiding this comment.
Should we also perhaps add a warning / obsolete message alerting Unix users to the fact that it's not... actually secure, and even less so than Windows?
src/System.Management.Automation/security/SecureStringHelper.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/security/SecureStringHelper.cs
Outdated
Show resolved
Hide resolved
|
From mentioned docs:
It seems we need to correct our already merged docs. Also have we problems in remote scenarios? |
|
@iSazonov Looking at the corefx sources, it appears that on Windows, it does call into the crypt32 apis and encrypts the in memory representation in SecureString. I'll submit a PR to update the docs to reflect this. As far as remoting, SecureString encryption is local to the machine so it is always decrypted before sending over the encrypted PSRP channel before being re-secured as a SecureString, so no impact there. |
PaulHigin
left a comment
There was a problem hiding this comment.
The changes look Ok. But I feel that since we used to throw an error before and now succeed without protecting the string (non-Windows only), there should be some kind of user notification. I am not sure if a warning should be displayed by SecureString cmdlets, but at least documentation should be updated. Can you create a doc bug?
|
@PoshChan Remind me in 1 minute |
|
@SteveL-MSFT, I do not understand: Remind me in 1 minute |
|
@PoshChan Please remind me in 1 minute |
|
@SteveL-MSFT, will remind you in 1 minute |
|
@SteveL-MSFT, this is the reminder you requested 1 minute ago |
src/System.Management.Automation/security/SecureStringHelper.cs
Outdated
Show resolved
Hide resolved
Co-Authored-By: SteveL-MSFT <slee@microsoft.com>
On Unix systems, fallback to plaintext manipulation instead of using the DPAPI which is not available. ## PR Context Currently, existing scripts that use SecureString cmdlets fail with an error complaining about crypt32.dll not being available. This change allows these cmdlets to be used, but there is no encryption of the string. .Net already [states](https://docs.microsoft.com/en-us/dotnet/api/system.security.securestring?view=netcore-2.1#remarks) that the contents of a SecureString are not encrypted on .Net Core. Fix #1654 Co-authored-by: Travis Plunk <travis.plunk@microsoft.com>
PR Summary
On Unix systems, fallback to plaintext manipulation instead of using the DPAPI which is not available.
PR Context
Currently, existing scripts that use SecureString cmdlets fail with an error complaining about crypt32.dll not being available. This change allows these cmdlets to be used, but there is no encryption of the string.
.Net already states that the contents of a SecureString are not encrypted on .Net Core.
Fix #1654
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.[feature]to your commit messages if the change is significant or affects feature tests