Skip to content

Commit cca23e5

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 a236dac commit cca23e5

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
@@ -851,9 +851,30 @@ public override uint Receive(out SNIPacket packet, int timeoutInMilliseconds)
851851
}
852852
finally
853853
{
854-
// Reset the socket timeout to Timeout.Infinite after the receive operation is done
855-
// to avoid blocking the thread in case of a timeout error.
856-
_socket.ReceiveTimeout = Timeout.Infinite;
854+
const int resetTimeout = Timeout.Infinite;
855+
856+
try
857+
{
858+
// Reset the socket timeout to Timeout.Infinite after
859+
// the receive operation is done to avoid blocking the
860+
// thread in case of a timeout error.
861+
_socket.ReceiveTimeout = resetTimeout;
862+
863+
}
864+
catch (SocketException ex)
865+
{
866+
// We sometimes see setting the ReceiveTimeout fail
867+
// on macOS. There's isn't much we can do about it
868+
// though, so just log and move on.
869+
SqlClientEventSource.Log.TrySNITraceEvent(
870+
nameof(SNITCPHandle),
871+
EventType.ERR,
872+
"Connection Id {0}, Failed to reset socket " +
873+
"receive timeout to {1}: {2}",
874+
_connectionId,
875+
resetTimeout,
876+
ex.Message);
877+
}
857878
}
858879
}
859880
}

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

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

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

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

0 commit comments

Comments
 (0)