Skip to content

Commit 6dacbf1

Browse files
committed
Add CodeQL suppression for DefaultAzureCredential use in Production (#3542)
- Adjusted CodeQL suppression to meet the strict requirements of where it may appear relative to the flagged code. - Adding catch for macOS socket error to log and ignore.
1 parent f0a4542 commit 6dacbf1

File tree

2 files changed

+46
-4
lines changed

2 files changed

+46
-4
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -867,9 +867,30 @@ public override uint Receive(out SNIPacket packet, int timeoutInMilliseconds)
867867
}
868868
finally
869869
{
870-
// Reset the socket timeout to Timeout.Infinite after the receive operation is done
871-
// to avoid blocking the thread in case of a timeout error.
872-
_socket.ReceiveTimeout = Timeout.Infinite;
870+
const int resetTimeout = Timeout.Infinite;
871+
872+
try
873+
{
874+
// Reset the socket timeout to Timeout.Infinite after
875+
// the receive operation is done to avoid blocking the
876+
// thread in case of a timeout error.
877+
_socket.ReceiveTimeout = resetTimeout;
878+
879+
}
880+
catch (SocketException ex)
881+
{
882+
// We sometimes see setting the ReceiveTimeout fail
883+
// on macOS. There's isn't much we can do about it
884+
// though, so just log and move on.
885+
SqlClientEventSource.Log.TrySNITraceEvent(
886+
nameof(SNITCPHandle),
887+
EventType.ERR,
888+
"Connection Id {0}, Failed to reset socket " +
889+
"receive timeout to {1}: {2}",
890+
_connectionId,
891+
resetTimeout,
892+
ex.Message);
893+
}
873894
}
874895
}
875896
}

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationProvider.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,28 @@ private static TokenCredentialData CreateTokenCredentialInstance(TokenCredential
599599
defaultAzureCredentialOptions.WorkloadIdentityClientId = tokenCredentialKey._clientId;
600600
}
601601

602-
return new TokenCredentialData(new DefaultAzureCredential(defaultAzureCredentialOptions), GetHash(secret));
602+
// SqlClient is a library and provides support to acquire access
603+
// token using 'DefaultAzureCredential' on user demand when they
604+
// specify 'Authentication = Active Directory Default' in
605+
// connection string.
606+
//
607+
// Default Azure Credential is instantiated by the calling
608+
// application when using "Active Directory Default"
609+
// authentication code to connect to Azure SQL instance.
610+
// SqlClient is a library, doesn't instantiate the credential
611+
// without running application instructions.
612+
//
613+
// Note that CodeQL suppression support can only detect
614+
// suppression comments that appear immediately above the
615+
// flagged statement, or appended to the end of the statement.
616+
// Multi-line justifications are not supported.
617+
//
618+
// https://eng.ms/docs/cloud-ai-platform/devdiv/one-engineering-system-1es/1es-docs/codeql/codeql-semmle#guidance-on-suppressions
619+
//
620+
// CodeQL [SM05137] See above for justification.
621+
DefaultAzureCredential cred = new(defaultAzureCredentialOptions);
622+
623+
return new TokenCredentialData(cred, GetHash(secret));
603624
}
604625

605626
TokenCredentialOptions tokenCredentialOptions = new() { AuthorityHost = new Uri(tokenCredentialKey._authority) };

0 commit comments

Comments
 (0)