Fix Windows netonly connection pool identity and replenishment

doc/connection-pooling-impersonation.md

@@ -0,0 +1,79 @@
+# Connection pooling and Windows impersonation
+
+On Windows, integrated-security connections are pooled by Windows identity and
+logon session, in addition to the connection-string pool group. This applies to
+both the wait-handle pool and the channel pool, with native or managed networking.
+
+Network-only logons (`LOGON32_LOGON_NEW_CREDENTIALS`, also used by `runas /netonly`)
+can retain the process user's local SID while supplying different credentials for
+outbound authentication. The token's `AuthenticationId` identifies its logon
+session and prevents these connections from sharing a pool with the process
+account or another network-only logon session.
+
+Duplicated tokens in the same logon session can reuse the same pool. Separate
+logon sessions, even for the same account, use separate pools. Applications that
+create a fresh network-only logon for every operation can therefore create more
+pools and should consider reusing a suitably scoped token.
+
+Background replenishment creates connections only when its effective identity
+matches the pool's identity, including the logon-session ID. Returning an expired
+connection outside impersonation can cause replenishment to skip that pool.
+`Min Pool Size` is therefore not guaranteed across impersonation boundaries;
+subsequent opens under the owning identity can create connections on demand.
+
+Open connections inside the intended impersonation scope, including awaiting
+asynchronous opens within that scope (see the
+[complete sample](samples/ConnectionPool_Impersonation.cs)):
+
+```csharp
+// token is a SafeAccessTokenHandle obtained from LogonUser.
+await WindowsIdentity.RunImpersonated(token, async () =>
+{
+    using var connection = new SqlConnection(connectionString);
+    await connection.OpenAsync();
+    using var command = new SqlCommand("SELECT ORIGINAL_LOGIN()", connection);
+    Console.WriteLine(await command.ExecuteScalarAsync());
+});
+```
+
+## Regression tests
+
+`DbConnectionPoolIdentityTest` uses synthetic network-only tokens and stub
+connections; it requires Windows but no SQL Server or valid outbound credentials.
+
+`PoolIdentityImpersonationTest` exercises real SQL Server connections using both
+pools and synchronous/asynchronous APIs.
+
+The `InvalidNetOnlyCredentials_*` tests need only an integrated-security
+`TCPConnectionString` (local or remote; on a local instance a named-pipe or
+shared-memory data source also works). They use a network-only token with
+placeholder credentials, which cannot open a new physical connection, so any
+successful open under that token proves it received a process-account connection.
+If the environment accepts the placeholder credentials (an unpooled probe open
+succeeds, as can happen with some loopback configurations), these tests skip.
+They also verify that the failed open does not put the process account's pool into
+its error blocking period.
+
+The remaining tests require an alternate account, supplied through the
+`SQLCLIENT_NETONLY_USER`, `SQLCLIENT_NETONLY_DOMAIN`, and
+`SQLCLIENT_NETONLY_PASSWORD` environment variables. Do not store credentials in
+source or test configuration. The login-reversion test requires both the process
+account and the alternate account to authenticate successfully as different SQL
+logins; the replenishment test requires only the alternate account to succeed. Use
+a TCP connection and verify the configured account is actually authenticated:
+some loopback configurations may fall back to the process identity. Local
+named-pipe and shared-memory transports do not exercise outbound credentials.
+Managed networking is selected through the
+existing `Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows` setting.
+
+Windows local-server Set 3 CI jobs for the default SQL instance provision a unique
+non-administrator local account and a SQL Windows login with access to `master`.
+The generated password is passed only as a secret test-process environment
+variable. An always-run cleanup step removes the SQL login and local account.
+These jobs set `SQLCLIENT_NETONLY_REQUIRED=true` and
+`SQLCLIENT_NETONLY_CONNECTION_STRING` to an explicit TCP endpoint using `master`;
+missing prerequisites fail the successful-login tests rather than skipping them.
+Both successful-login tests first verify that an unpooled connection authenticates
+as the configured alternate account. Required CI coverage also verifies that the
+process account authenticates as a different login, preventing loopback fallback
+from producing a false pass.

doc/samples/ConnectionPool_Impersonation.cs

@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+// <Snippet_ConnectionPool_Impersonation>
+using System.Security.Principal;
+using System.Threading.Tasks;
+using Microsoft.Data.SqlClient;
+using Microsoft.Win32.SafeHandles;
+
+public static class ConnectionPoolImpersonationSample
+{
+    /// <summary>
+    /// Opens a pooled integrated-security connection under the supplied Windows token and
+    /// returns the authenticated SQL login. Requires Windows and an integrated-security
+    /// connection string. The caller retains ownership of the token.
+    /// </summary>
+    /// <param name="token">The Windows token supplying the intended outbound credentials.</param>
+    /// <param name="connectionString">A connection string with integrated security enabled.</param>
+    /// <returns>The login that authenticated the SQL connection.</returns>
+    public static Task<string> GetLoginAsync(SafeAccessTokenHandle token, string connectionString)
+    {
+        return WindowsIdentity.RunImpersonated(token, async () =>
+        {
+            using var connection = new SqlConnection(connectionString);
+            await connection.OpenAsync();
+            using var command = new SqlCommand("SELECT ORIGINAL_LOGIN()", connection);
+            return (string)await command.ExecuteScalarAsync();
+        });
+    }
+}
+// </Snippet_ConnectionPool_Impersonation>

eng/pipelines/common/templates/jobs/ci-run-tests-job.yml

@@ -341,6 +341,11 @@ jobs:
   - pwsh: New-Item -ItemType Directory -Path TestResults -Force | Out-Null
     displayName: 'Create TestResults Directory'
 
+  - ${{ if and(eq(parameters.operatingSystem, 'Windows'), eq(parameters.configSqlFor, 'local'), eq(parameters.testSet, '3'), eq(coalesce(parameters.configProperties.instanceName, 'MSSQLSERVER'), 'MSSQLSERVER')) }}:
+    - template: /eng/pipelines/common/templates/steps/configure-netonly-account-step.yml@self
+      parameters:
+        mode: setup
+
   # Gate: record that all setup steps (SDK install, build, SQL config, x86 SDK
   # install, etc.) completed successfully.  Test steps condition on this variable
   # so they are skipped when setup fails, yet remain independent of each
@@ -359,6 +364,7 @@ jobs:
         operatingSystem: ${{ parameters.operatingSystem }}
         packageVersion: ${{ parameters.packageVersion }}
         sqlServerPackageVersion: ${{ parameters.sqlServerPackageVersion }}
+        enableNetOnlyTests: ${{ and(eq(parameters.operatingSystem, 'Windows'), eq(parameters.configSqlFor, 'local'), eq(parameters.testSet, '3'), eq(coalesce(parameters.configProperties.instanceName, 'MSSQLSERVER'), 'MSSQLSERVER')) }}
 
   - ${{ if and(eq(parameters.enableX86Test, true), eq(parameters.operatingSystem, 'Windows')) }}:
     - template: /eng/pipelines/common/templates/steps/run-all-tests-step.yml@self
@@ -373,6 +379,12 @@ jobs:
         operatingSystem: ${{ parameters.operatingSystem }}
         packageVersion: ${{ parameters.packageVersion }}
         sqlServerPackageVersion: ${{ parameters.sqlServerPackageVersion }}
+        enableNetOnlyTests: ${{ and(eq(parameters.operatingSystem, 'Windows'), eq(parameters.configSqlFor, 'local'), eq(parameters.testSet, '3'), eq(coalesce(parameters.configProperties.instanceName, 'MSSQLSERVER'), 'MSSQLSERVER')) }}
+
+  - ${{ if and(eq(parameters.operatingSystem, 'Windows'), eq(parameters.configSqlFor, 'local'), eq(parameters.testSet, '3'), eq(coalesce(parameters.configProperties.instanceName, 'MSSQLSERVER'), 'MSSQLSERVER')) }}:
+    - template: /eng/pipelines/common/templates/steps/configure-netonly-account-step.yml@self
+      parameters:
+        mode: cleanup
 
   - template: /eng/pipelines/common/templates/steps/publish-test-results-step.yml@self
     parameters:

eng/pipelines/common/templates/steps/configure-netonly-account-step.yml

@@ -0,0 +1,47 @@
+#################################################################################
+# Licensed to the .NET Foundation under one or more agreements.                 #
+# The .NET Foundation licenses this file to you under the MIT license.          #
+# See the LICENSE file in the project root for more information.                #
+#################################################################################
+
+# Invoke setup and cleanup in the same Windows job, with cleanup after the tests.
+# The agent must be a local administrator with permission to manage SQL logins.
+# Setup publishes netOnlyUser, netOnlyDomain, netOnlySid, netOnlyConnectionString,
+# and secret netOnlyPassword.
+# No server roles or database users are granted; tests must use master.
+parameters:
+  - name: mode
+    type: string
+    default: setup
+    values:
+      - setup
+      - cleanup
+
+  - name: instanceName
+    type: string
+    default: MSSQLSERVER
+
+  # Must match the SQL listener, not a client alias.
+  - name: port
+    type: string
+    default: '1433'
+
+steps:
+  - task: PowerShell@2
+    displayName: '${{ parameters.mode }} temporary net-only account [Win]'
+    ${{ if eq(parameters.mode, 'cleanup') }}:
+      condition: always()
+    inputs:
+      targetType: inline
+      pwsh: false
+      script: |
+        & "$(Build.SourcesDirectory)\eng\scripts\Configure-NetOnlyAccount.ps1" `
+          -Mode $env:NETONLY_MODE -InstanceName $env:NETONLY_INSTANCE -Port $env:NETONLY_PORT `
+          -AccountName $env:NETONLY_ACCOUNT -AccountSid $env:NETONLY_SID
+    env:
+      NETONLY_MODE: ${{ parameters.mode }}
+      NETONLY_INSTANCE: ${{ parameters.instanceName }}
+      NETONLY_PORT: ${{ parameters.port }}
+      # Undefined ADO macros are treated as absent by cleanup.
+      NETONLY_ACCOUNT: $(netOnlyUser)
+      NETONLY_SID: $(netOnlySid)

eng/pipelines/common/templates/steps/run-all-tests-step.yml

@@ -68,6 +68,10 @@ parameters:
     type: number
     default: 2
 
+  - name: enableNetOnlyTests
+    type: boolean
+    default: false
+
 steps:
 - ${{ if parameters.debug }}:
   - powershell: 'dotnet sdk check'
@@ -192,6 +196,13 @@ steps:
   - task: DotNetCoreCLI@2
     displayName: 'Run Manual Tests ${{parameters.msbuildArchitecture }}'
     condition: and(eq(variables['setupSucceeded'], 'true'), succeededOrFailed())
+    ${{ if parameters.enableNetOnlyTests }}:
+      env:
+        SQLCLIENT_NETONLY_REQUIRED: 'true'
+        SQLCLIENT_NETONLY_USER: $(netOnlyUser)
+        SQLCLIENT_NETONLY_DOMAIN: $(netOnlyDomain)
+        SQLCLIENT_NETONLY_PASSWORD: $(netOnlyPassword)
+        SQLCLIENT_NETONLY_CONNECTION_STRING: $(netOnlyConnectionString)
     inputs:
       command: build
       projects: build.proj

eng/pipelines/pr/jobs/test-sqlclientmanual-job.yml

@@ -208,9 +208,21 @@ jobs:
           Write-Host "##vso[task.setvariable variable=TEST_MDS_CONFIG]$configPath"
         displayName: 'Generate manual test config'
 
+      - ${{ if and(eq(parameters.platformOperatingSystem, 'Windows'), parameters.isLocalServer, eq(parameters.configDisplayName, 'localhost'), contains(parameters.testSet, '3')) }}:
+        - template: /eng/pipelines/common/templates/steps/configure-netonly-account-step.yml@self
+          parameters:
+            mode: setup
+
       # Execute TestSqlClientManual target from build.proj
       - task: DotNetCoreCLI@2
         displayName: 'Test: SqlClientManual ${{ parameters.testSet }}'
+        ${{ if and(eq(parameters.platformOperatingSystem, 'Windows'), parameters.isLocalServer, eq(parameters.configDisplayName, 'localhost'), contains(parameters.testSet, '3')) }}:
+          env:
+            SQLCLIENT_NETONLY_REQUIRED: 'true'
+            SQLCLIENT_NETONLY_USER: $(netOnlyUser)
+            SQLCLIENT_NETONLY_DOMAIN: $(netOnlyDomain)
+            SQLCLIENT_NETONLY_PASSWORD: $(netOnlyPassword)
+            SQLCLIENT_NETONLY_CONNECTION_STRING: $(netOnlyConnectionString)
         inputs:
           command: build
           projects: build.proj
@@ -224,6 +236,11 @@ jobs:
             -p:TestSet=${{ parameters.testSet }}
             -p:TestResultsFolderPath="${{ variables.testResultsPath }}"
 
+      - ${{ if and(eq(parameters.platformOperatingSystem, 'Windows'), parameters.isLocalServer, eq(parameters.configDisplayName, 'localhost'), contains(parameters.testSet, '3')) }}:
+        - template: /eng/pipelines/common/templates/steps/configure-netonly-account-step.yml@self
+          parameters:
+            mode: cleanup
+
       # Publish test results
       - template: /eng/pipelines/pr/steps/publish-test-results-step.yml@self
         parameters: