From 3deeca767ffe86c1a86b6692bfd1ebfa46ed36ff Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 9 Jan 2024 21:01:01 +0100 Subject: [PATCH 1/7] Make NegotiateAuthentication.ComputeIntegrityCheck/VerifyIntegrityCheck APIs public --- .../System/Net/Security/NegotiateAuthentication.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs index 30db44ec27d041..b70815b5094427 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs @@ -356,21 +356,23 @@ public NegotiateAuthenticationStatusCode UnwrapInPlace(Span input, out int /// Computes the message integrity check of a given message. /// /// Input message for MIC calculation. - /// Buffer writer where the MIC is written. + /// Buffer writer where the MIC is written. /// + /// Implements the GSSAPI GetMIC operation. + /// /// The method modifies the internal state and may update sequence numbers depending on the /// selected algorithm. Two successive invocations thus don't produce the same result and /// it's important to carefully pair GetMIC and VerifyMIC calls on the both sides of the /// authenticated session. /// - internal void GetMIC(ReadOnlySpan message, IBufferWriter signature) + public void ComputeIntegrityCheck(ReadOnlySpan message, IBufferWriter signatureWriter) { if (!IsAuthenticated || _isDisposed) { throw new InvalidOperationException(SR.net_auth_noauth); } - _pal.GetMIC(message, signature); + _pal.GetMIC(message, signatureWriter); } /// @@ -380,12 +382,14 @@ internal void GetMIC(ReadOnlySpan message, IBufferWriter signature) /// MIC to be verified. /// For successfully verified MIC, the method returns true. /// + /// Implements the GSSAPI VerifyMIC operation. + /// /// The method modifies the internal state and may update sequence numbers depending on the /// selected algorithm. Two successive invocations thus don't produce the same result and /// it's important to carefully pair GetMIC and VerifyMIC calls on the both sides of the /// authenticated session. /// - internal bool VerifyMIC(ReadOnlySpan message, ReadOnlySpan signature) + public bool VerifyIntegrityCheck(ReadOnlySpan message, ReadOnlySpan signature) { if (!IsAuthenticated || _isDisposed) { From 6d897809a297281460df23da6890882ee4f7cfbb Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 9 Jan 2024 21:19:11 +0100 Subject: [PATCH 2/7] Update ref source --- src/libraries/System.Net.Security/ref/System.Net.Security.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Net.Security/ref/System.Net.Security.cs b/src/libraries/System.Net.Security/ref/System.Net.Security.cs index b94c0efa91dd73..a075727ebba38d 100644 --- a/src/libraries/System.Net.Security/ref/System.Net.Security.cs +++ b/src/libraries/System.Net.Security/ref/System.Net.Security.cs @@ -57,6 +57,8 @@ public void Dispose() { } public System.Net.Security.NegotiateAuthenticationStatusCode Unwrap(System.ReadOnlySpan input, System.Buffers.IBufferWriter outputWriter, out bool wasEncrypted) { throw null; } public System.Net.Security.NegotiateAuthenticationStatusCode UnwrapInPlace(System.Span input, out int unwrappedOffset, out int unwrappedLength, out bool wasEncrypted) { throw null; } public System.Net.Security.NegotiateAuthenticationStatusCode Wrap(System.ReadOnlySpan input, System.Buffers.IBufferWriter outputWriter, bool requestEncryption, out bool isEncrypted) { throw null; } + public void ComputeIntegrityCheck(System.ReadOnlySpan message, System.Buffers.IBufferWriter signatureWriter) { } + public bool VerifyIntegrityCheck(System.ReadOnlySpan message, System.ReadOnlySpan signature) { throw null; } } public partial class NegotiateAuthenticationClientOptions { From 7ae11f040a7c1deab97975fe94d3fcf6a6282081 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 9 Jan 2024 21:19:38 +0100 Subject: [PATCH 3/7] Update usage of the internal APIs --- .../src/System/Net/Security/NegotiateStream.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs index f26a837e0c110f..033b82800dba94 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs @@ -398,7 +398,7 @@ private async ValueTask ReadAsync(Memory buffer, Cancella const int NtlmSignatureLength = 16; if (readBytes < NtlmSignatureLength || - !_context.VerifyMIC(_readBuffer.AsSpan(NtlmSignatureLength, readBytes - NtlmSignatureLength), _readBuffer.AsSpan(0, NtlmSignatureLength))) + !_context.VerifyIntegrityCheck(_readBuffer.AsSpan(NtlmSignatureLength, readBytes - NtlmSignatureLength), _readBuffer.AsSpan(0, NtlmSignatureLength))) { statusCode = NegotiateAuthenticationStatusCode.InvalidToken; } @@ -526,7 +526,7 @@ private async Task WriteAsync(ReadOnlyMemory buffer, Cancellat if (isNtlm && !isEncrypted) { // Non-encrypted NTLM uses an encoding quirk - _context.GetMIC(bufferToWrap.Span, _writeBuffer); + _context.ComputeIntegrityCheck(bufferToWrap.Span, _writeBuffer); _writeBuffer.Write(bufferToWrap.Span); statusCode = NegotiateAuthenticationStatusCode.Completed; } From b9b9d360f0fb3e807be083fb70140cca9f674168 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 9 Jan 2024 21:21:02 +0100 Subject: [PATCH 4/7] Add unit test --- .../UnitTests/NegotiateAuthenticationTests.cs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/libraries/System.Net.Security/tests/UnitTests/NegotiateAuthenticationTests.cs b/src/libraries/System.Net.Security/tests/UnitTests/NegotiateAuthenticationTests.cs index 56d86c06b51e27..5be16753dc4926 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/NegotiateAuthenticationTests.cs +++ b/src/libraries/System.Net.Security/tests/UnitTests/NegotiateAuthenticationTests.cs @@ -248,6 +248,43 @@ public void NtlmSignatureTest() Assert.Equal(s_Hello, output.WrittenSpan.ToArray()); } + [ConditionalFact(nameof(IsNtlmAvailable))] + public void NtlmIntegrityCheckTest() + { + using FakeNtlmServer fakeNtlmServer = new FakeNtlmServer(s_testCredentialRight); + NegotiateAuthentication ntAuth = new NegotiateAuthentication( + new NegotiateAuthenticationClientOptions + { + Package = "NTLM", + Credential = s_testCredentialRight, + TargetName = "HTTP/foo", + RequiredProtectionLevel = ProtectionLevel.EncryptAndSign + }); + + DoNtlmExchange(fakeNtlmServer, ntAuth); + + Assert.True(fakeNtlmServer.IsAuthenticated); + + ArrayBufferWriter output = new ArrayBufferWriter(); + for (int i = 0; i < 3; i++) + { + // Test ComputeIntegrityCheck on client side and decoding it on server side + ntAuth.ComputeIntegrityCheck(s_Hello, output); + Assert.Equal(16, output.WrittenCount); + // Verify the signature computation + fakeNtlmServer.VerifyMIC(s_Hello, output.WrittenSpan); + // Prepare buffer for reuse + output.Clear(); + } + + Span signature = stackalloc byte[16]; + for (int i = 0; i < 3; i++) + { + fakeNtlmServer.GetMIC(s_Hello, signature); + Assert.True(ntAuth.VerifyIntegrityCheck(s_Hello, signature)); + } + } + private void DoNtlmExchange(FakeNtlmServer fakeNtlmServer, NegotiateAuthentication ntAuth) { NegotiateAuthenticationStatusCode statusCode; From 85c4a766ed8e47fea101403e8299bed030c9e847 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Tue, 9 Jan 2024 21:43:32 +0100 Subject: [PATCH 5/7] Add exception documentation --- .../src/System/Net/Security/NegotiateAuthentication.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs index b70815b5094427..f80af220626300 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs @@ -365,6 +365,7 @@ public NegotiateAuthenticationStatusCode UnwrapInPlace(Span input, out int /// it's important to carefully pair GetMIC and VerifyMIC calls on the both sides of the /// authenticated session. /// + /// Authentication failed or has not occurred. public void ComputeIntegrityCheck(ReadOnlySpan message, IBufferWriter signatureWriter) { if (!IsAuthenticated || _isDisposed) @@ -389,6 +390,7 @@ public void ComputeIntegrityCheck(ReadOnlySpan message, IBufferWriter + /// Authentication failed or has not occurred. public bool VerifyIntegrityCheck(ReadOnlySpan message, ReadOnlySpan signature) { if (!IsAuthenticated || _isDisposed) From e2974c189472406415ee064e2eca829b6808d042 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 10 Jan 2024 05:18:09 +0100 Subject: [PATCH 6/7] Fix Windows implementation of GetMIC --- .../src/System/Net/NegotiateAuthenticationPal.Windows.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Windows.cs index 10fc41dbf14723..cbabb8898eecac 100644 --- a/src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Windows.cs @@ -566,7 +566,7 @@ public override unsafe void GetMIC(ReadOnlySpan message, IBufferWriter signatureBuffer = signature.GetSpan(sizes.cbSecurityTrailer); + Span signatureBuffer = signature.GetSpan(sizes.cbMaxSignature); fixed (byte* messagePtr = message) fixed (byte* signaturePtr = signatureBuffer) @@ -577,7 +577,7 @@ public override unsafe void GetMIC(ReadOnlySpan message, IBufferWriterBufferType = SecurityBufferType.SECBUFFER_TOKEN; tokenBuffer->pvBuffer = (IntPtr)signaturePtr; - tokenBuffer->cbBuffer = sizes.cbSecurityTrailer; + tokenBuffer->cbBuffer = sizes.cbMaxSignature; dataBuffer->BufferType = SecurityBufferType.SECBUFFER_DATA; dataBuffer->pvBuffer = (IntPtr)messagePtr; dataBuffer->cbBuffer = message.Length; @@ -597,7 +597,7 @@ public override unsafe void GetMIC(ReadOnlySpan message, IBufferWritercbBuffer); } } finally From 7c7097910c761c0a437a547916ae980acc7ecf92 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Wed, 10 Jan 2024 13:07:32 +0100 Subject: [PATCH 7/7] Update src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com> --- .../src/System/Net/Security/NegotiateAuthentication.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs index f80af220626300..0042f73605919f 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateAuthentication.cs @@ -353,7 +353,7 @@ public NegotiateAuthenticationStatusCode UnwrapInPlace(Span input, out int } /// - /// Computes the message integrity check of a given message. + /// Computes the integrity check of a given message. /// /// Input message for MIC calculation. /// Buffer writer where the MIC is written.