Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fixing assert conditions for System.Net.Security. #5642

Merged
merged 1 commit into from
Jan 25, 2016
Merged

Fixing assert conditions for System.Net.Security. #5642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Common/src/Interop/Windows/SChannel/SslConnectionInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ internal unsafe SslConnectionInfo(byte[] nativeBuffer)
{
GlobalLog.Assert("SslConnectionInfo::.ctor", "Negative size.");
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tend to add blank lines after clauses (such as if clauses) since that was/is the StyleCop guidelines for formatting C# code. This particular formatting is not detailed in the current CoreFx guidelines. Personally, I do like it as it improves readability. We might want to consider adding this guidelines for the official CoreFx style guidelines.

Debug.Fail("SslConnectionInfo::.ctor", "Negative size.");
throw;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Common/src/Interop/Windows/sspicli/SSPIAuthType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ public unsafe int DecryptMessage(SafeDeleteContext context, Interop.SspiCli.Secu
context.DangerousRelease();
}


if (status == 0 && qop == Interop.SspiCli.SECQOP_WRAP_NO_ENCRYPT)
{
if (GlobalLog.IsEnabled)
{
GlobalLog.Assert("SspiCli.DecryptMessage", "Expected qop = 0, returned value = " + qop.ToString("x", CultureInfo.InvariantCulture));
}

Debug.Fail("SspiCli.DecryptMessage", "Expected qop = 0, returned value = " + qop.ToString("x", CultureInfo.InvariantCulture));
throw new InvalidOperationException(SR.net_auth_message_not_encrypted);
}
Expand Down
5 changes: 5 additions & 0 deletions src/Common/src/Interop/Windows/sspicli/SSPIWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ private unsafe static int EncryptDecryptHelper(OP op, SSPIInterface secModule, S
{
GlobalLog.Assert("SSPIWrapper::EncryptDecryptHelper", "Unknown OP: " + op);
}

Debug.Fail("SSPIWrapper::EncryptDecryptHelper", "Unknown OP: " + op);
throw NotImplemented.ByDesignWithMessage(SR.net_MethodNotImplementedException);
}
Expand Down Expand Up @@ -458,6 +459,7 @@ private unsafe static int EncryptDecryptHelper(OP op, SSPIInterface secModule, S
{
GlobalLog.Assert("SSPIWrapper::EncryptDecryptHelper", "Output buffer out of range.");
}

Debug.Fail("SSPIWrapper::EncryptDecryptHelper", "Output buffer out of range.");
iBuffer.size = 0;
iBuffer.offset = 0;
Expand All @@ -473,14 +475,17 @@ private unsafe static int EncryptDecryptHelper(OP op, SSPIInterface secModule, S
{
GlobalLog.AssertFormat("SSPIWrapper::EncryptDecryptHelper|'offset' out of range. [{0}]", iBuffer.offset);
}

Debug.Fail("SSPIWrapper::EncryptDecryptHelper|'offset' out of range. [" + iBuffer.offset + "]");
}

if (iBuffer.size < 0 || iBuffer.size > (iBuffer.token == null ? 0 : iBuffer.token.Length - iBuffer.offset))
{
if (GlobalLog.IsEnabled)
{
GlobalLog.AssertFormat("SSPIWrapper::EncryptDecryptHelper|'size' out of range. [{0}]", iBuffer.size);
}

Debug.Fail("SSPIWrapper::EncryptDecryptHelper|'size' out of range. [" + iBuffer.size + "]");
}
}
Expand Down
1 change: 1 addition & 0 deletions src/Common/src/Interop/Windows/sspicli/SecSizes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal unsafe SecSizes(byte[] memory)
{
GlobalLog.Assert("SecSizes::.ctor", "Negative size.");
}

Debug.Fail("SecSizes::.ctor", "Negative size.");
throw;
}
Expand Down
5 changes: 5 additions & 0 deletions src/Common/src/Interop/Windows/sspicli/SecuritySafeHandles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ internal unsafe static int InitializeSecurityContext(
{
GlobalLog.Assert("SafeDeleteContext::InitializeSecurityContext()|outSecBuffer != null");
}

Debug.Fail("SafeDeleteContext::InitializeSecurityContext()|outSecBuffer != null");
}
if (inSecBuffer != null && inSecBuffers != null)
Expand All @@ -557,6 +558,7 @@ internal unsafe static int InitializeSecurityContext(
{
GlobalLog.Assert("SafeDeleteContext::InitializeSecurityContext()|inSecBuffer == null || inSecBuffers == null");
}

Debug.Fail("SafeDeleteContext::InitializeSecurityContext()|inSecBuffer == null || inSecBuffers == null");
}

Expand Down Expand Up @@ -859,6 +861,7 @@ internal unsafe static int AcceptSecurityContext(
{
GlobalLog.Assert("SafeDeleteContext::AcceptSecurityContext()|outSecBuffer != null");
}

Debug.Fail("SafeDeleteContext::AcceptSecurityContext()|outSecBuffer != null");
}
if (inSecBuffer != null && inSecBuffers != null)
Expand All @@ -867,6 +870,7 @@ internal unsafe static int AcceptSecurityContext(
{
GlobalLog.Assert("SafeDeleteContext::AcceptSecurityContext()|inSecBuffer == null || inSecBuffers == null");
}

Debug.Fail("SafeDeleteContext::AcceptSecurityContext()|outSecBuffer != null");
}

Expand Down Expand Up @@ -1142,6 +1146,7 @@ internal unsafe static int CompleteAuthToken(
{
GlobalLog.Assert("SafeDeleteContext::CompleteAuthToken()|inSecBuffers == null");
}

Debug.Fail("SafeDeleteContext::CompleteAuthToken()|inSecBuffers == null");
}

Expand Down
1 change: 1 addition & 0 deletions src/Common/src/Interop/Windows/sspicli/StreamSizes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ internal unsafe StreamSizes(byte[] memory)
{
GlobalLog.Assert("StreamSizes::.ctor", "Negative size.");
}

Debug.Fail("StreamSizes::.ctor", "Negative size.");
throw;
}
Expand Down
5 changes: 5 additions & 0 deletions src/Common/src/System/Net/ContextAwareResult.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal WindowsIdentity Identity
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::Identity|Called on completed result.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::Identity |Called on completed result.");
}

Expand All @@ -49,6 +50,7 @@ internal WindowsIdentity Identity
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::Identity|No identity captured - specify captureIdentity.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::Identity |No identity captured - specify captureIdentity.");
}

Expand All @@ -62,8 +64,10 @@ internal WindowsIdentity Identity
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::Identity|Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling Identity (unless it's only called after FinishPostingAsyncOp).", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::Identity |Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling Identity (unless it's only called after FinishPostingAsyncOp).");
}

lock (_lock) { }
}

Expand All @@ -75,6 +79,7 @@ internal WindowsIdentity Identity
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::Identity|Result became completed during call.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::Identity |Result became completed during call.");
}

Expand Down
7 changes: 7 additions & 0 deletions src/Common/src/System/Net/ContextAwareResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ internal ExecutionContext ContextCopy
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::ContextCopy|Called on completed result.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::ContextCopy |Called on completed result.");
}

Expand All @@ -157,6 +158,7 @@ internal ExecutionContext ContextCopy
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::ContextCopy|No context captured - specify a callback or forceCaptureContext.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::ContextCopy |No context captured - specify a callback or forceCaptureContext.");
}

Expand All @@ -170,6 +172,7 @@ internal ExecutionContext ContextCopy
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::ContextCopy|Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling ContextCopy (unless it's only called after FinishPostingAsyncOp).", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) +"::ContextCopy |Must lock (StartPostingAsyncOp()) { ... FinishPostingAsyncOp(); } when calling ContextCopy (unless it's only called after FinishPostingAsyncOp).");
}
lock (_lock) { }
Expand All @@ -183,6 +186,7 @@ internal ExecutionContext ContextCopy
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::ContextCopy|Result became completed during call.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::ContextCopy |Result became completed during call.");
}

Expand Down Expand Up @@ -221,6 +225,7 @@ internal object StartPostingAsyncOp(bool lockCapture)
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::StartPostingAsyncOp|Called on completed result.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::StartPostingAsyncOp |Called on completed result.");
}

Expand Down Expand Up @@ -318,6 +323,7 @@ private bool CaptureOrComplete(ref ExecutionContext cachedContext, bool returnCo
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::CaptureOrComplete|Called without calling StartPostingAsyncOp.", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::CaptureOrComplete |Called without calling StartPostingAsyncOp.");
}

Expand Down Expand Up @@ -384,6 +390,7 @@ private bool CaptureOrComplete(ref ExecutionContext cachedContext, bool returnCo
{
GlobalLog.AssertFormat("ContextAwareResult#{0}::CaptureOrComplete|Didn't capture context, but didn't complete synchronously!", LoggingHash.HashString(this));
}

Debug.Fail("ContextAwareResult#" + LoggingHash.HashString(this) + "::CaptureOrComplete |Didn't capture context, but didn't complete synchronously!");
}
}
Expand Down
1 change: 1 addition & 0 deletions src/Common/src/System/Net/InternalException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ internal InternalException()
{
GlobalLog.Assert("InternalException thrown.");
}

Debug.Fail("InternalException thrown.");
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/Common/src/System/Net/LazyAsyncResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ internal LazyAsyncResult(object myObject, object myState, AsyncCallback myCallBa
{
GlobalLog.AssertFormat("LazyAsyncResult#{0}::.ctor()|Result can't be set to DBNull - it's a special internal value.", LoggingHash.HashString(this));
}

Debug.Fail("LazyAsyncResult#" + LoggingHash.HashString(this) + "::.ctor()|Result can't be set to DBNull - it's a special internal value.");
}

Expand Down Expand Up @@ -335,14 +336,17 @@ internal object Result
{
GlobalLog.AssertFormat("LazyAsyncResult#{0}::set_Result()|Result can't be set to DBNull - it's a special internal value.", LoggingHash.HashString(this));
}

Debug.Fail("LazyAsyncResult#" + LoggingHash.HashString(this) + "::set_Result()|Result can't be set to DBNull - it's a special internal value.");
}

if (InternalPeekCompleted)
{
if (GlobalLog.IsEnabled)
{
GlobalLog.AssertFormat("LazyAsyncResult#{0}::set_Result()|Called on completed result.", LoggingHash.HashString(this));
}

Debug.Fail("LazyAsyncResult#" + LoggingHash.HashString(this) + "::set_Result()|Called on completed result.");
}
_result = value;
Expand Down
2 changes: 2 additions & 0 deletions src/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ internal static string[] GetRequestCertificateAuthorities(SafeDeleteContext secu
{
GlobalLog.Assert("SecureChannel::GetIssuers()", "Interop.SspiCli._CERT_CHAIN_ELEMENT size is not positive: " + pIL2->cbSize.ToString());
}

Debug.Fail("SecureChannel::GetIssuers()", "Interop.SspiCli._CERT_CHAIN_ELEMENT size is not positive: " + pIL2->cbSize.ToString());
}

Expand Down Expand Up @@ -254,6 +255,7 @@ internal static X509Store EnsureStoreOpened(bool isMachineStore)
{
GlobalLog.Assert("SecureChannel::EnsureStoreOpened()", "Failed to open cert store, location:" + storeLocation + " exception:" + exception);
}

Debug.Fail("SecureChannel::EnsureStoreOpened()", "Failed to open cert store, location:" + storeLocation + " exception:" + exception);
return null;
}
Expand Down
22 changes: 16 additions & 6 deletions src/System.Net.Security/src/System/Net/NTAuthentication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,13 @@ internal SecSizes Sizes
{
get
{
if ((IsCompleted && IsValidContext))
if (!(IsCompleted && IsValidContext))
{
if (GlobalLog.IsEnabled)
{
GlobalLog.Assert("NTAuthentication#{0}::MaxDataSize|The context is not completed or invalid.", LoggingHash.HashString(this));
GlobalLog.AssertFormat("NTAuthentication#{0}::MaxDataSize|The context is not completed or invalid.", LoggingHash.HashString(this));
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::MaxDataSize |The context is not completed or invalid.");
}

Expand Down Expand Up @@ -339,21 +340,23 @@ private void Initialize(bool isServer, string package, NetworkCredential credent
// This token can be used for impersonation. We use it to create a WindowsIdentity and hand it out to the server app.
internal SecurityContextTokenHandle GetContextToken(out Interop.SecurityStatus status)
{
if ((IsCompleted && IsValidContext))
if (!(IsCompleted && IsValidContext))
{
if (GlobalLog.IsEnabled)
{
GlobalLog.AssertFormat("NTAuthentication#{0}::GetContextToken|Should be called only when completed with success, currently is not!", LoggingHash.HashString(this));
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::GetContextToken |Should be called only when completed with success, currently is not!");
}

if (IsServer)
if (!IsServer)
{
if (GlobalLog.IsEnabled)
{
GlobalLog.AssertFormat("NTAuthentication#{0}::GetContextToken|The method must not be called by the client side!", LoggingHash.HashString(this));
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::GetContextToken |The method must not be called by the client side!");
}

Expand Down Expand Up @@ -525,12 +528,13 @@ internal byte[] GetOutgoingBlob(byte[] incomingBlob, bool throwOnError, out Inte
if (statusCode == Interop.SecurityStatus.OK)
{
// Success.
if ((statusCode == Interop.SecurityStatus.OK))
if (statusCode != Interop.SecurityStatus.OK)
{
if (GlobalLog.IsEnabled)
{
GlobalLog.AssertFormat("NTAuthentication#{0}::GetOutgoingBlob()|statusCode:[0x{1:x8}] ({2}) m_SecurityContext#{3}::Handle:[{4}] [STATUS != OK]", LoggingHash.HashString(this), (int)statusCode, statusCode, LoggingHash.HashString(_securityContext), LoggingHash.ObjectToString(_securityContext));
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::GetOutgoingBlob()|statusCode:[0x" + ((int)statusCode).ToString("x8") + "] (" + statusCode + ") m_SecurityContext#" + LoggingHash.HashString(_securityContext) + "::Handle:[" + LoggingHash.ObjectToString(_securityContext) + "] [STATUS != OK]");
}

Expand Down Expand Up @@ -571,6 +575,7 @@ internal int Encrypt(byte[] buffer, int offset, int count, ref byte[] output, ui
{
GlobalLog.Assert("NTAuthentication#" + LoggingHash.HashString(this) + "::Encrypt", "Arguments out of range.");
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::Encrypt", "Arguments out of range.");
}

Expand Down Expand Up @@ -652,6 +657,7 @@ internal int Decrypt(byte[] payload, int offset, int count, out int newOffset, u
{
GlobalLog.Assert("NTAuthentication#" + LoggingHash.HashString(this) + "::Decrypt", "Argument 'offset' out of range.");
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::Decrypt", "Argument 'offset' out of range.");

throw new ArgumentOutOfRangeException("offset");
Expand All @@ -663,6 +669,7 @@ internal int Decrypt(byte[] payload, int offset, int count, out int newOffset, u
{
GlobalLog.Assert("NTAuthentication#" + LoggingHash.HashString(this) + "::Decrypt", "Argument 'count' out of range.");
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::Decrypt", "Argument 'count' out of range.");

throw new ArgumentOutOfRangeException("count");
Expand Down Expand Up @@ -710,12 +717,13 @@ internal int Decrypt(byte[] payload, int offset, int count, out int newOffset, u

private string GetClientSpecifiedSpn()
{
if ((IsValidContext && IsCompleted))
if (!(IsValidContext && IsCompleted))
{
if (GlobalLog.IsEnabled)
{
GlobalLog.Assert("NTAuthentication: Trying to get the client SPN before handshaking is done!");
}

Debug.Fail("NTAuthentication: Trying to get the client SPN before handshaking is done!");
}

Expand All @@ -738,6 +746,7 @@ private int DecryptNtlm(byte[] payload, int offset, int count, out int newOffset
{
GlobalLog.Assert("NTAuthentication#" + LoggingHash.HashString(this) + "::DecryptNtlm", "Argument 'count' out of range.");
}

Debug.Fail("NTAuthentication#" + LoggingHash.HashString(this) + "::DecryptNtlm", "Argument 'count' out of range.");

throw new ArgumentOutOfRangeException("count");
Expand Down Expand Up @@ -767,6 +776,7 @@ private int DecryptNtlm(byte[] payload, int offset, int count, out int newOffset
{
GlobalLog.Print("NTAuthentication#" + LoggingHash.HashString(this) + "::Decrypt() throw Error = " + errorCode.ToString("x", NumberFormatInfo.InvariantInfo));
}

throw new Win32Exception(errorCode);
}

Expand Down
1 change: 1 addition & 0 deletions src/System.Net.Security/src/System/Net/SSPIHandleCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ internal static void CacheCredential(SafeFreeCredentials newHandle)
{
GlobalLog.Assert("SSPIHandlCache", "Attempted to throw: " + e.ToString());
}

Debug.Fail("SSPIHandlCache", "Attempted to throw: " + e.ToString());
}
}
Expand Down
Loading