Update PSRP protocol to deprecate session key exchange between newer client and server #25774
Conversation
…client and server
| internal static readonly Version ProtocolVersionWin7RC = new Version(2, 0); | ||
| internal static readonly Version ProtocolVersionWin7RTM = new Version(2, 1); | ||
| internal static readonly Version ProtocolVersionWin8RTM = new Version(2, 2); | ||
| internal static readonly Version ProtocolVersionWin10RTM = new Version(2, 3); | ||
| internal static readonly Version ProtocolVersion_2_0 = new(2, 0); // Window 7 RC | ||
| internal static readonly Version ProtocolVersion_2_1 = new(2, 1); // Window 7 RTM | ||
| internal static readonly Version ProtocolVersion_2_2 = new(2, 2); // Window 8 RTM | ||
| internal static readonly Version ProtocolVersion_2_3 = new(2, 3); // Window 10 RTM | ||
| internal static readonly Version ProtocolVersion_2_4 = new(2, 4); // PowerShell 7.6 |
There was a problem hiding this comment.
Renamed the variable names because:
- It's hard to map the original names to the true protocol versions when reading code
- The change to v2.4 doesn't map to a Windows RTM.
There was a problem hiding this comment.
This makes perfect sense
| Version clientProtocolVersion = session.Context.ClientCapability.ProtocolVersion; | ||
| if (clientProtocolVersion >= RemotingConstants.ProtocolVersion_2_4) | ||
| { | ||
| // For client v2.4+, we no longer encrypt secure strings, but rely on the underlying secure transport to do the right thing. |
There was a problem hiding this comment.
So does this mean that there is no check to see if a "secure" transport is used? What happens if someone is using a custom transport that isn't secure, uses something like WSMan without encryption (or weak NTLM encryption), etc?
There was a problem hiding this comment.
Thanks for taking a look!
The recommendation from the internal crypto board is to require secure transport for network connections. Perhaps it's possible to check if the WSMan connection is secure, but I doubt if that's doable in general.
The plan is to update docs to add this requirement for network connections, but I'm not sure if this change would be considered a "regression" for users who are currently using PSRP over in-secure network transports. PowerShell remoting over WinRM uses HTTP by default. Even though it uses Kerberos or NTLM session key to encrypt at the message level, it's not considered as secure in untrusted networks. So, this protocol change may also have implications to it.
The Security WG will review and discuss about this on Monday. I will post the update afterwards.
There was a problem hiding this comment.
Yea the fact that custom transports can do whatever they want makes it hard to guarantee a "secure transport". It's a pity that this won't be updated to a newer standard so that even on insecure transports SecureString's will stay encrypted but I understand not wanting to be in the same situation in x years time.
While I doubt many people are using custom transports it is a possibility and this change would mean SecureStrings are essentially plaintext.
| @@ -454,7 +454,7 @@ private static NamedPipeServerStream CreateNamedPipe( | |||
| SafePipeHandle pipeHandle = NamedPipeNative.CreateNamedPipe( | |||
| fullPipeName, | |||
| NamedPipeNative.PIPE_ACCESS_DUPLEX | NamedPipeNative.FILE_FLAG_FIRST_PIPE_INSTANCE | NamedPipeNative.FILE_FLAG_OVERLAPPED, | |||
| NamedPipeNative.PIPE_TYPE_MESSAGE | NamedPipeNative.PIPE_READMODE_MESSAGE, | |||
| NamedPipeNative.PIPE_TYPE_MESSAGE | NamedPipeNative.PIPE_READMODE_MESSAGE | NamedPipeNative.PIPE_REJECT_REMOTE_CLIENTS, | |||
There was a problem hiding this comment.
This seems like it would be better off as a separate commit or at a minimum pointed out that this has changed here.
There was a problem hiding this comment.
This is actually related to this protocol change. PowerShell has built-in support for 3 non-network transports:
- Standard I/O
- Named pipe
- HyperV direct
For HyperV direct, we've confirmed with the HyperV team that the man-in-the-middle attacks are not possible with HyperV sockets.
For standard I/O, both client and server are local, so we should be good.
For named pipe, it's possible for a remote client to connect to named pipe, which 1) may not be intended from the beginning of this feature 2) could be problematic after SecureString is no longer encrypted with a session key. So, it's better off to disable remote client for it.
- Updated the
pipe_optionfor creating the named pipe (used forEnter-PSHostProcess) in PowerShell to reject remote client.
I called it out in the PR description above. I will add more about it in the description.
|
This PR was approved by the security-wg |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
📣 Hey @@daxian-dbw, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Today, a
Session_Keyis used to encrypt aSecureStringbefore sending it on wire and decrypt it after receiving one. The PowerShell Remoting Protocol (PSRP) does theSession_Keyexchange between client and server when aSecureStringobject needs to be transferred. It involves the following steps:SecureStringobject before sending it, and decrypt after receiving one.However, the padding algorithm
RSAEncryptionPadding.Pkcs1used in the PSRPSession_Keyexchange is NOT secure, and therefore, the PSRP needs to be used on top of a secure transport and theSession_Keydoesn't add any extra security.So, we decided to deprecate the
Session_Keyexchange in PSRP, and instead, require secure transportation layer for secure data transfer between PSRP clients and servers.This PR increments the protocol version to v2.4 with the following changes:
SecureStringare skipped when both client and server are v2.4+pipe_optionfor creating the named pipe (used forEnter-PSHostProcess) in PowerShell to reject remote client.SecureStringis no longer encrypted with a session key. So, it's better off to disable remote client (done by thepipe_optionchange above).PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header