Don't use Win32 native APIs on non-Windows for crypto of secure string over remoting#8746
Conversation
|
The CodeFactor issues are all on existing code complaining about Hungarian notation which none of them are |
PaulHigin
left a comment
There was a problem hiding this comment.
This seems Ok as a temporary change to prevent a hang, although the result for a client initiated key exchange is the termination of the remoting connection. Only slightly better. For a server initiated exchange, a Windows DPAPI dll exception is generated and returned to the user, but the connection remains. We can easily change this (in serialization->HandleSecureString) to throw a better exception, but I see that the original exception gets lost and the user gets an obscure "Root element is missing" error, probably because the remoting packet XML is malformed (serializer is not supposed to throw but simply return nothing on errors). But this may be Ok until we get this working correctly.
|
@PaulHigin can you re-review? |
PR Summary
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-Credentialis 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.
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