Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible threading issues in version 3.0 when using Azure Managed Identity auth #1209

Closed
ccarpediem opened this issue Aug 12, 2021 · 10 comments · Fixed by #1213
Closed

Possible threading issues in version 3.0 when using Azure Managed Identity auth #1209

ccarpediem opened this issue Aug 12, 2021 · 10 comments · Fixed by #1213
Labels
🐛 Bug! Issues that are bugs in the drivers we maintain.

Comments

@ccarpediem
Copy link

After upgrading our app from Microsoft.Data.SqlClient 2.1.3 to version 3.0, our application simply seems to "freeze" upon opening any sql connection using Azure Managed Identity auth. This did work in 2.1.3 and the connection does work if we change it to use SQL auth. When it "freezes" there are no errors/exceptions thrown; it just hangs indefinitely. The only diagnostic information directly available seems to be from the SQL Server (Azure SQL Managed Instance). That error is Error: 33155, Severity: 20, State: 1 (A disconnect event was raised when server is waiting for Federated Authentication token. This could be due to client close or server timeout expired).

I've created a very simple sample app to further diagnose the issue and my findings thus far are:
• If you set the project to be a console app all works as expected with Managed Identity
• If you set the project to be a WinForm app running the exact same code, it will experience the "freeze"
• If you update the WinForm code to use "con.OpenAsync().Wait();" instead of "con.Open();" again all works without the freeze

Is this a known/expected issue? While changing the connection to use OpenAsync() in a small sample app isn't a big deal, it is a much larger problem for huge legacy application with many modules so hoping this isn't expected behavior going forward.

To reproduce

WindowsFormsApp4.zip

Further technical details

Microsoft.Data.SqlClient version: 3.0
Framework 4.7.2
SQL Server version: Azure SQL Managed Instance
SQL Authentication: Azure Managed Identity
Operating system: Windows 2019

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Aug 12, 2021

Hi @ccarpediem

This is a strange behavior and not expected.
OpenAsync() and Open() should ideally not have any change in behavior, but we'll test out your repro to confirm that.

Could you also confirm:

  • Your client is an Azure VM (as that's the only way you can get token from Managed Identity)
  • Is your Azure DB Server configured for Proxy/Redirect connection policy? (You can find that in Azure SQL Server > Firewall settings)
  • Does changing that to redirect resolve the issue?

@ccarpediem
Copy link
Author

  • I can confirm that yes my testing is on an Azure VM properly configured for Managed Identity and the Managed Identity is properly configured on the SQL Managed Instance to have rights to login (validated by the fact it does work properly if using OpenAsync().
    -The SQL Managed Instance is currently configured to use the Redirect option.
  • Changing to Proxy option did not impact the results

@ccarpediem
Copy link
Author

I've also added a SQLClient EventListener to my sample code (attached as Form1.txt). The results from running Open() vs OpenAsync() are also attached. I'm not sure it really help, but figure it cannot hurt.

OpenAsync_Log.txt
Open_Log.txt
Form1.txt

@cheenamalhotra
Copy link
Member

I think this problem is coming from MSAL library we're using here and seems to impact all AAD modes for Windows Forms.
You could run a quick test by updating your repro to:

  • Provide <tenantId>
  • Acquire token for yourself (interactively)
using Microsoft.Identity.Client;
...
       private void button1_Click(object sender, EventArgs e)
        {
            var appId = "2fd908ad-0664-4344-b9be-cd3e8b574c38"; // use anything here.
            var authority = "https://login.windows.net/<tenantId>/";
            var redirectUri = "https://login.microsoftonline.com/common/oauth2/nativeclient";
            CancellationTokenSource ctsInteractive = new CancellationTokenSource();
            ctsInteractive.CancelAfter(180000);
            IPublicClientApplication app = PublicClientApplicationBuilder.Create(appId)
                .WithAuthority(authority)
                .WithRedirectUri(redirectUri)
                .Build();
            run().Wait();
            async Task run() => await app.AcquireTokenInteractive(new string[] { "https://database.windows.net//.default" })
                .WithLoginHint("")
                .WithCorrelationId(Guid.NewGuid())
                .ExecuteAsync(ctsInteractive.Token);
            textBox1.Text = "Token acquired!";
        }

Alternatively, also if you update to using Azure Identity's API, it continues to freeze.

using Azure.Identity;
using Azure.Core;
...
       private void button1_Click(object sender, EventArgs e)
        {
            CancellationTokenSource ctsInteractive = new CancellationTokenSource();
            InteractiveBrowserCredential credential = new InteractiveBrowserCredential();
            getToken(credential).Wait();
            async Task getToken(TokenCredential cred) => await cred.GetTokenAsync(
                new TokenRequestContext(new string[] { "https://database.windows.net//.default" }), ctsInteractive.Token);
        }

Or use ManagedIdentityCredential here instead of InteractiveBrowserCredential, still cannot get token.
I updated all related packages for Azure.Identity and MSAL libraries to latest versions but issue continues to persist.

The same code however works when called via OpenAsync() which is a strange mystery to me I'll look more into. But I would highly recommend raising this issue with MSAL team with these repro cases as it can be reproduced directly with MSAL.

@ghost
Copy link

ghost commented Aug 14, 2021

Judging by the trace that stops on "Generating federated authentication token", I believe this line in SqlInternalConnectionTds.GetFedAuthToken is very likely to cause a deadlock because of the blocking call to .Result:

_fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken();

By the way, this seems to be the same issue: #1138

@ccarpediem
Copy link
Author

Thanks @AnKor1985! @cheenamalhotra yes I think it may have been a change committed as part to #1010 which changed how those these are called within SqlInternalConnectionTds as shown below.

image

@cheenamalhotra
Copy link
Member

Hi @ccarpediem

Yes I believe that exact change caused issues on Windows Forms. I was investigating it too and just opened PR #1213 with necessary changes, which we'll include in next release and plan to hotfix 3.0.

@cheenamalhotra cheenamalhotra added the 🐛 Bug! Issues that are bugs in the drivers we maintain. label Aug 16, 2021
@janecosham
Copy link

Hi @cheenamalhotra

I'm currently working on a release which is switching from System.Data.SqlClient to Microsoft.Data.SqlClient using 3.0. Everything is working fine except I just found this issue with our WinForms tools which will block the release.
You said this is planned to be in a hotfix - can you share expected timing on this? Is it quite likely to be in the next 2 weeks for example? If it is then we can wait, but if it is not likely to be in the next 2 weeks then that's ok too and we will downgrade to 2.1.3.

Thanks!

@cheenamalhotra
Copy link
Member

I will be likely in next 2-3 weeks.
Backport PRs are under preparation.

@janecosham
Copy link

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug! Issues that are bugs in the drivers we maintain.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants