dotnet / dotnet/SqlClient

Always Encrypted introduces sporadic errors

Open
#4,199 3 comments 2 reactions 1 assignee Claimed by @paulmedynski View on GitHub
Dominant language
C#
Stars
989
Forks
340
Avg merge
4d 19h
Merged PRs (30d)
72

Description

### Describe the bug
We have recently introduced our first table using always encrypted on SQL Azure Hyperscale + Azure KeyVault as keystore. After deploying it to production we've started experiencing sporadic failures that we've traced back to connections enabled for always encrypted. After narrowing the scope of connections featuring `Column Encryption Setting=Enabled` to the currently rarely exercised code path where its needed, the problem disappeared.

The error would show itself somewhere between minutes to hours apart, resulting in about 1-4 connections failing within the same second in parallel on seperate SqlConnection's opened by independent http requests, with the following exception
```
Microsoft.Data.SqlClient.SqlException (0x80131904): A severe error occurred on the current command. The results, if any, should be discarded.
at void Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, bool callerHasConnectionLock, bool asyncClose)
at TdsOperationStatus Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, out bool dataReady)
at object Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, bool isInternal, string endMethod)
at int Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
at int Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
at void System.Threading.Tasks.TaskFactory.FromAsyncCoreLogic(IAsyncResult iar, Func endFunction, Action endAction, Task promise, bool requiresSynchronization)
at async Task Dapper.SqlMapper.ExecuteImplAsync(IDbConnection cnn, CommandDefinition command, object param) in /_/Dapper/SqlMapper.Async.cs:line 663
ClientConnectionId:3c4124b6-2604-4e7b-ab2e-11aa3da9030c
ClientConnectionId before routing:4f7c97b0-0fbb-434d-adfd-54f87ef5841f
Routing Destination:..germanywestcentral1-a.worker.database.windows.net,11050
```
immediately followed by exactly one exception that is different
```
Microsoft.Data.SqlClient.SqlException (0x80131904): A transport-level error has occurred when receiving results from the server. (provider: Session Provider, error: 19 - Physical connection is not usable)
at int Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
at void System.Threading.Tasks.TaskFactory.FromAsyncCoreLogic(IAsyncResult iar, Func endFunction, Action endAction, Task promise, bool requiresSynchronization)
at async Task Dapper.SqlMapper.ExecuteImplAsync(IDbConnection cnn, CommandDefinition command, object param) in /_/Dapper/SqlMapper.Async.cs:line 663
ClientConnectionId:3c4124b6-2604-4e7b-ab2e-11aa3da9030c
Error Number:-1,State:0,Class:20
ClientConnectionId before routing:4f7c97b0-0fbb-434d-adfd-54f87ef5841f
Routing Destination:..germanywestcentral1-a.worker.database.windows.net,11050
```

One notable observation is that ***all failing connections in a burst of failures all share the exact same ClientConnectionId*** which is suspicious to me. We do have MARS enabled, however MARS should not allow independently constructed SqlConnection's to reuse the same physical connection.

Also note that none of these are actually accessing tables with encrypted columns, it's merely that adding `Column Encryption Setting=Enabled` to the connection string causes these errors as a side effect.

### To reproduce
There's no trivial way to reproduce, presumably it only happens under load as there was only one occurence on our testsystems over two weeks vs. dozens of times a day on production.
The code experiencing the exception roughly looks like this:
```c#
// Data Source=tcp:.database.windows.net,1433;Initial Catalog=;Persist Security Info=False;User ID=;Password=;Multiple Active Result Sets=True;Connect Timeout=30;Encrypt=True;Trust Server Certificate=False;Application Name=;Column Encryption Setting=Enabled

var context = new MyContext();
...
context.EnableEncryptionAccess(true);

var con = context.Database.GetDbConnection();
await using var close = await con.EnsureOpenAsync();

// Create a temp table as a destination for our SqlBulkCopy
await con.ExecuteAsync("SELECT TOP 0 * INTO #tmp FROM UnencryptedTable"); <-- exception **might** occur here at random

// ready a Bulk Copy
var copy = new SqlBulkCopy(con as SqlConnection);
copy.DestinationTableName = "#tmp";

try
{
await copy.WriteToServerAsync(reader, cancellationToken);
```

This is part of our batch data import API that is the single most stressed component on production. It's not actually exercised by the always encrypted table since the ETL pattern used here seemed to be incompatible by design with the always encrypted requirements, so we've built an alternative path but kept the entire batch import API Always Encrypted enabled, leading to the issues. We're unsure if the new code is affected by that issue as well but currently its not yet exercised on prod.

supporting code:

```csharp
public class MyContext : DbContext
{
public MyContext(bool readOnly = false)
{
Database.SetConnectionString(GetConnectionString(readOnly));
}

public static string? ConnectionString { get; set; }

private string GetConnectionString(bool readOnly, bool readEncrypted = false)
{
string conString = ConnectionString!;
var builder = new Microsoft.Data.SqlClient.SqlConnectionStringBuilder(conString)
{
ColumnEncryptionSetting = readEncrypted
? Microsoft.Data.SqlClient.SqlConnectionColumnEncryptionSetting.Enabled
: Microsoft.Data.SqlClient.SqlConnectionColumnEncryptionSetting.Disabled,
ApplicationIntent = readOnly
? Microsoft.Data.SqlClient.ApplicationIntent.ReadOnly
: Microsoft.Data.SqlClient.ApplicationIntent.ReadWrite
};
conString = builder.ToString();

return conString;
}

public void EnableEncryptionAccess(bool enabled)
{
if (Database.GetDbConnection().State != System.Data.ConnectionState.Closed)
{
throw new InvalidOperationException("Cannot switch Always Encrypted setting because the connection is not closed");
}

Database.SetConnectionString(GetConnectionString(ReadOnly, readEncrypted: enabled));
}
}

public static class SqlExtensions
{
public static async Task EnsureOpenAsync(this DbConnection con)
{
if (con.State == System.Data.ConnectionState.Closed)
{
await con.OpenAsync();
return new ClosingWrapper(con);
}
return new ClosingWrapper(null);
}
}

public struct ClosingWrapper(DbConnection? con) : IDisposable, IAsyncDisposable
{
private readonly DbConnection? Connection = con;

public void Dispose() => Connection?.Close();

public async ValueTask DisposeAsync()
{
if (Connection is not null)
await Connection.CloseAsync();
}
}
```

### Expected behavior
Always Encrypted should not affect the connection reliability

### Further technical details
Microsoft.Data.SqlClient version: 7.0
.NET target: .NET 10
SQL Server version: SQL Azure Hyperscale
Operating system: Azure Web Apps Windows Code (non container)

### Additional Info
We do not have `Switch.Microsoft.Data.SqlClient.UseCompatibilityAsyncBehaviour`, `Switch.Microsoft.Data.SqlClient.UseCompatibilityProcessSni` enabled on prod; they were however enabled on the testsystem during that one time where the error occured on the testsystem, so it seems unrelated.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.