Skip to content

Commit 5ece96a

Browse files
SteveL-MSFTTravisEz13
authored andcommitted
Don't use Win32 native APIs on non-Windows for crypto of secure string over remoting (#8746)
Remoting relies on Windows native crypto APIs to pass a secure string. Of course these APIs don't work on non-Windows. However, the remoting code does not expect this to fail so ends up in a hang waiting for an event that never happens. Have not figured out how to return an error to the user in the case `Get-Credential` is called within a PSSession. Currently it just quietly closes the remote connection. @PaulHigin can you give a pointer to where this code would exist? Debugging the current code, it has a bunch of error handlers which just result in the pssession being closed and not sure where throwing is appropriate. Fix #8723 ## PR Context Fix is to provide implementations of the Win32 native pinvoke apis that throw an exception. The reason to do it this way is that there's a bunch of code that calls some of the APIs of the wrapper .Net class to setup structures that eventually get passed to the win32 api. Throwing in the managed class causes other things to fail early that should work on non-Windows.
1 parent 26a5867 commit 5ece96a

File tree

4 files changed

+179
-41
lines changed

4 files changed

+179
-41
lines changed

src/System.Management.Automation/engine/remoting/client/clientremotesession.cs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -364,12 +364,14 @@ internal override void StartKeyExchange()
364364

365365
SessionDataStructureHandler.StateMachine.RaiseEvent(eventArgs);
366366
}
367+
else
368+
{
369+
// send using data structure handler
370+
eventArgs = new RemoteSessionStateMachineEventArgs(RemoteSessionEvent.KeySent);
371+
SessionDataStructureHandler.StateMachine.RaiseEvent(eventArgs);
367372

368-
// send using data structure handler
369-
eventArgs = new RemoteSessionStateMachineEventArgs(RemoteSessionEvent.KeySent);
370-
SessionDataStructureHandler.StateMachine.RaiseEvent(eventArgs);
371-
372-
SessionDataStructureHandler.SendPublicKeyAsync(localPublicKey);
373+
SessionDataStructureHandler.SendPublicKeyAsync(localPublicKey);
374+
}
373375
}
374376
}
375377

src/System.Management.Automation/engine/serialization.cs

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1108,47 +1108,42 @@ private bool HandleSecureString(object source, string streamName, string propert
11081108
{
11091109
// the principle used in serialization is that serialization
11101110
// never throws, and if something can't be serialized nothing
1111-
// is written. So we write the elements only if encryption succeeds
1112-
try
1111+
// is written. So we write the elements only if encryption succeeds.
1112+
// However, in the case for non-Windows where secure string encryption
1113+
// is not yet supported, a PSCryptoException will be thrown.
1114+
string encryptedString;
1115+
if (_context.cryptoHelper != null)
11131116
{
1114-
string encryptedString;
1115-
if (_context.cryptoHelper != null)
1116-
{
1117-
encryptedString = _context.cryptoHelper.EncryptSecureString(secureString);
1118-
}
1119-
else
1120-
{
1121-
encryptedString = Microsoft.PowerShell.SecureStringHelper.Protect(secureString);
1122-
}
1117+
encryptedString = _context.cryptoHelper.EncryptSecureString(secureString);
1118+
}
1119+
else
1120+
{
1121+
encryptedString = Microsoft.PowerShell.SecureStringHelper.Protect(secureString);
1122+
}
11231123

1124-
if (property != null)
1125-
{
1126-
WriteStartElement(SerializationStrings.SecureStringTag);
1127-
WriteNameAttribute(property);
1128-
}
1129-
else
1130-
{
1131-
WriteStartElement(SerializationStrings.SecureStringTag);
1132-
}
1124+
if (property != null)
1125+
{
1126+
WriteStartElement(SerializationStrings.SecureStringTag);
1127+
WriteNameAttribute(property);
1128+
}
1129+
else
1130+
{
1131+
WriteStartElement(SerializationStrings.SecureStringTag);
1132+
}
11331133

1134-
if (streamName != null)
1135-
{
1136-
WriteAttribute(SerializationStrings.StreamNameAttribute, streamName);
1137-
}
1134+
if (streamName != null)
1135+
{
1136+
WriteAttribute(SerializationStrings.StreamNameAttribute, streamName);
1137+
}
11381138

1139-
// Note: We do not use WriteRaw for serializing secure string. WriteString
1140-
// does necessary escaping which may be needed for certain
1141-
// characters.
1142-
_writer.WriteString(encryptedString);
1139+
// Note: We do not use WriteRaw for serializing secure string. WriteString
1140+
// does necessary escaping which may be needed for certain
1141+
// characters.
1142+
_writer.WriteString(encryptedString);
11431143

1144-
_writer.WriteEndElement();
1144+
_writer.WriteEndElement();
11451145

1146-
return true;
1147-
}
1148-
catch (PSCryptoException)
1149-
{
1150-
// do nothing
1151-
}
1146+
return true;
11521147
}
11531148
}
11541149

src/System.Management.Automation/resources/SecuritySupportStrings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,4 +138,7 @@
138138
<data name="CouldNotUseCertificate" xml:space="preserve">
139139
<value>ERROR: Could not find or use certificate: {0}</value>
140140
</data>
141+
<data name="CannotEncryptSecureString" xml:space="preserve">
142+
<value>Session key not available to encrypt secure string.</value>
143+
</data>
141144
</root>

src/System.Management.Automation/utils/CryptoUtils.cs

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,143 @@ internal class PSCryptoNativeUtils
8080
{
8181
#region Functions
8282

83+
#if UNIX
84+
/// Return Type: BOOL->int
85+
///hProv: HCRYPTPROV->ULONG_PTR->unsigned int
86+
///Algid: ALG_ID->unsigned int
87+
///dwFlags: DWORD->unsigned int
88+
///phKey: HCRYPTKEY*
89+
public static bool CryptGenKey(
90+
PSSafeCryptProvHandle hProv,
91+
uint Algid,
92+
uint dwFlags,
93+
ref PSSafeCryptKey phKey)
94+
{
95+
throw new PSCryptoException();
96+
}
97+
98+
/// Return Type: BOOL->int
99+
///hKey: HCRYPTKEY->ULONG_PTR->unsigned int
100+
public static bool CryptDestroyKey(IntPtr hKey)
101+
{
102+
throw new PSCryptoException();
103+
}
104+
105+
/// Return Type: BOOL->int
106+
///phProv: HCRYPTPROV*
107+
///szContainer: LPCWSTR->WCHAR*
108+
///szProvider: LPCWSTR->WCHAR*
109+
///dwProvType: DWORD->unsigned int
110+
///dwFlags: DWORD->unsigned int
111+
public static bool CryptAcquireContext(ref PSSafeCryptProvHandle phProv,
112+
[InAttribute()] [MarshalAsAttribute(UnmanagedType.LPWStr)] string szContainer,
113+
[InAttribute()] [MarshalAsAttribute(UnmanagedType.LPWStr)] string szProvider,
114+
uint dwProvType,
115+
uint dwFlags)
116+
{
117+
throw new PSCryptoException();
118+
}
119+
120+
/// Return Type: BOOL->int
121+
///hProv: HCRYPTPROV->ULONG_PTR->unsigned int
122+
///dwFlags: DWORD->unsigned int
123+
public static bool CryptReleaseContext(IntPtr hProv, uint dwFlags)
124+
{
125+
throw new PSCryptoException();
126+
}
127+
128+
129+
/// Return Type: BOOL->int
130+
///hKey: HCRYPTKEY->ULONG_PTR->unsigned int
131+
///hHash: HCRYPTHASH->ULONG_PTR->unsigned int
132+
///Final: BOOL->int
133+
///dwFlags: DWORD->unsigned int
134+
///pbData: BYTE*
135+
///pdwDataLen: DWORD*
136+
///dwBufLen: DWORD->unsigned int
137+
public static bool CryptEncrypt(PSSafeCryptKey hKey,
138+
IntPtr hHash,
139+
[MarshalAsAttribute(UnmanagedType.Bool)] bool Final,
140+
uint dwFlags,
141+
byte[] pbData,
142+
ref int pdwDataLen,
143+
int dwBufLen)
144+
{
145+
throw new PSCryptoException();
146+
}
147+
148+
149+
/// Return Type: BOOL->int
150+
///hKey: HCRYPTKEY->ULONG_PTR->unsigned int
151+
///hHash: HCRYPTHASH->ULONG_PTR->unsigned int
152+
///Final: BOOL->int
153+
///dwFlags: DWORD->unsigned int
154+
///pbData: BYTE*
155+
///pdwDataLen: DWORD*
156+
public static bool CryptDecrypt(PSSafeCryptKey hKey,
157+
IntPtr hHash,
158+
[MarshalAsAttribute(UnmanagedType.Bool)] bool Final,
159+
uint dwFlags,
160+
byte[] pbData,
161+
ref int pdwDataLen)
162+
{
163+
throw new PSCryptoException();
164+
}
165+
166+
/// Return Type: BOOL->int
167+
///hKey: HCRYPTKEY->ULONG_PTR->unsigned int
168+
///hExpKey: HCRYPTKEY->ULONG_PTR->unsigned int
169+
///dwBlobType: DWORD->unsigned int
170+
///dwFlags: DWORD->unsigned int
171+
///pbData: BYTE*
172+
///pdwDataLen: DWORD*
173+
public static bool CryptExportKey(PSSafeCryptKey hKey,
174+
PSSafeCryptKey hExpKey,
175+
uint dwBlobType,
176+
uint dwFlags,
177+
byte[] pbData,
178+
ref uint pdwDataLen)
179+
{
180+
throw new PSCryptoException();
181+
}
182+
183+
/// Return Type: BOOL->int
184+
///hProv: HCRYPTPROV->ULONG_PTR->unsigned int
185+
///pbData: BYTE*
186+
///dwDataLen: DWORD->unsigned int
187+
///hPubKey: HCRYPTKEY->ULONG_PTR->unsigned int
188+
///dwFlags: DWORD->unsigned int
189+
///phKey: HCRYPTKEY*
190+
public static bool CryptImportKey(PSSafeCryptProvHandle hProv,
191+
byte[] pbData,
192+
int dwDataLen,
193+
PSSafeCryptKey hPubKey,
194+
uint dwFlags,
195+
ref PSSafeCryptKey phKey)
196+
{
197+
throw new PSCryptoException();
198+
}
199+
200+
/// Return Type: BOOL->int
201+
///hKey: HCRYPTKEY->ULONG_PTR->unsigned int
202+
///pdwReserved: DWORD*
203+
///dwFlags: DWORD->unsigned int
204+
///phKey: HCRYPTKEY*
205+
public static bool CryptDuplicateKey(PSSafeCryptKey hKey,
206+
ref uint pdwReserved,
207+
uint dwFlags,
208+
ref PSSafeCryptKey phKey)
209+
{
210+
throw new PSCryptoException();
211+
}
212+
213+
/// Return Type: DWORD->unsigned int
214+
public static uint GetLastError()
215+
{
216+
throw new PSCryptoException();
217+
}
218+
#else
219+
83220
/// Return Type: BOOL->int
84221
///hProv: HCRYPTPROV->ULONG_PTR->unsigned int
85222
///Algid: ALG_ID->unsigned int
@@ -200,6 +337,7 @@ public static extern bool CryptDuplicateKey(PSSafeCryptKey hKey,
200337
/// Return Type: DWORD->unsigned int
201338
[DllImportAttribute(PinvokeDllNames.GetLastErrorDllName, EntryPoint = "GetLastError")]
202339
public static extern uint GetLastError();
340+
#endif
203341

204342
#endregion Functions
205343

@@ -1022,7 +1160,7 @@ protected string EncryptSecureStringCore(SecureString secureString)
10221160
}
10231161
else
10241162
{
1025-
Dbg.Assert(false, "Session key not available to encrypt secure string");
1163+
throw new PSCryptoException(SecuritySupportStrings.CannotEncryptSecureString);
10261164
}
10271165

10281166
return encryptedDataAsString;

0 commit comments

Comments
 (0)