Repository navigation
Enable securestring cmdlets for non-Windows - #9199
Conversation
|
Reviewed the CodeFactor issues, all are for code I didn't change (but wrapped in |
Rain Sallow (/u/ta11ow) (vexx32)
left a comment
Rain Sallow (/u/ta11ow) (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?
|
From mentioned docs:
It seems we need to correct our already merged docs. Also have we problems in remote scenarios? |
|
Ilya (@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. |
Paul Higinbotham (PaulHigin)
left a comment
Paul Higinbotham (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-Bot (@PoshChan) Remind me in 1 minute |
|
Steve Lee (@SteveL-MSFT), I do not understand: Remind me in 1 minute |
|
PoshChan-Bot (@PoshChan) Please remind me in 1 minute |
|
Steve Lee (@SteveL-MSFT), will remind you in 1 minute |
|
Steve Lee (@SteveL-MSFT), this is the reminder you requested 1 minute ago |
Co-Authored-By: SteveL-MSFT <[email protected]>
Ilya (iSazonov)
added
the
CL-General
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 <[email protected]>
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 PowerShell#1654 Co-authored-by: Travis Plunk <[email protected]>
7 participants
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