Improve interoperability of NTLM encryption/decryption and authentication #71373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsBuilt on top of #71280, best reviewed commit by commit. Firstly, it fixes an interoperability scenario for non-encrypted NTLM communication passed through Secondly, it fixes a bunch of bugs in the SmtpClient GSSAPI authentication and adds a loopback server test. This was also tested against a real Microsoft Exchange server to ensure that it works properly and that the encryption/decryption is applied correctly in context of NTLM authentication.
|
NegotiateStream on non-encrypted connections with NTLM sends the messages in a special `<signature token><plain text message>` format. That's not something that gss_wrap/gss_unwrap would produce. It can be produced through gss_get_mic/gss_verify_mic calls though so let's do that.
The method names were misleading since they wrapped the EncryptMessage and DecryptMessage native APIs and not the MakeSignature/VerifySignature APIs that also exist.
…/Decrypt The SSPI / GSSAPI providers keep track of the sequence numbers themselves.
This maps directly to the semantics of gss_wrap/gss_unwrap methods that are used in many specifications. It replaces the misleading name which in SSPI API is an equivalent of gss_get_mic/gss_verify_mic. It also fixes the declaration to actually decode the buffers both on Windows and Unix. In NTLM the content of the message is sealed and needs to be decoded. Note that previously on Unix the VerifySignature API didn't decode the content. On Windows it did decode the content inside a buffer that was passed as ReadOnlySpan<byte> but it didn't communicate back the offset of the decoded data. The SMTP GSSAPI authentication code was thus reading incorrect data. In case the underlying authentication was Kerberos the data were not encrypted and they were located at the beginning of the buffer so it was not an issue. In case the underlying authentication was NTLM it was looking at the NTLM signature token which luckily happens to always start with the bytes 01 00 00 00. That exactly matched the expected value by accident.
The last token in the GSSAPI SASL authentication mechanism is a bit mask that specifies the supported security protections offered by the server and the maximum token size. The client is supposed to choose one of the protections and reply back. Relax the check to actually support servers that offer anything but "no protection". As long as the server also offers no protection we can choose it.
Updated the managed NTLM implementation and the fake servers to implement the specification quirk: MS-SPNG section 3.2.5.1 NTLM RC4 Key State for MechListMIC and First Signed Message specifies that the RC4 sealing keys are reset back to the initial state for the first message. Since the managed implementation doesn't expose encryption yet it didn't affect any observable behavior. Likewise the fake servers didn't need this code path yet.
wfurt
approved these changes
Jun 29, 2022
There was a problem hiding this comment.
LGTM.
cc: @SteveSyfuhs for any thoughts on the GSSAPI/SSPI changes.
|
contributes to #19436 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Best reviewed commit by commit.
Firstly, it fixes an interoperability scenario for non-encrypted NTLM communication passed through
NegotiateStreambetween Unix and Windows. It updates theNegotiateStreamPal.Encrypt/DecryptUnix implementation to have the same quirk as the Windows implementation has.Secondly, it fixes a bunch of bugs in the SmtpClient GSSAPI authentication and adds a loopback server test. This was also tested against a real Microsoft Exchange server to ensure that it works properly and that the encryption/decryption is applied correctly in context of NTLM authentication.