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

Windows DNS resolution fails when impersonation is used #13612

Closed
timmaraju opened this issue May 13, 2024 · 12 comments
Closed

Windows DNS resolution fails when impersonation is used #13612

timmaraju opened this issue May 13, 2024 · 12 comments
Labels
name lookup DNS and related tech Windows Windows-specific

Comments

@timmaraju
Copy link

timmaraju commented May 13, 2024

I did this

Our windows service calls ImpersonateLoggedOnUser before any curl calls so that the proxy calls can be authenticated using the logged on user's token and not from the system.

DNS resolution fails when this is called because of the change in commit a6bbc87 to use GetAddrInfoExW. GetAddrInfoExW() keeps returning WSATRY_AGAIN and never succeeds even after many retries.

I expected the following

DNS resolution works even with impersonation.

curl/libcurl version

libcurl 8.7.1

operating system

Windows

@bagder bagder added Windows Windows-specific name lookup DNS and related tech labels May 13, 2024
@bagder
Copy link
Member

bagder commented May 13, 2024

GetAddrInfoExW() keeps returning WSATRY_AGAIN and never succeeds even after many retries.

How many retries/over how long time did you retry? I found some other references to this problem but they seem to fix it by retrying again a little later.

@timmaraju
Copy link
Author

Retries are every 5 seconds and unlimited, but each call to curl returns Host not Found error (11002).

@edmcln
Copy link

edmcln commented May 15, 2024

A simple approach is to verify whether the thread is impersonating. If it is, we don't invoke the GetAddrInfoExW function.

BOOL WINAPI IsImpersonating()
{
	HANDLE hToken = NULL;
	BOOL bRes = FALSE;
	if ( OpenThreadToken( GetCurrentThread(), TOKEN_QUERY, TRUE, &hToken ) )
	{
		CloseHandle( hToken );
		bRes = TRUE;
	}

	// failure means the thread is not impersonating or OpenThreadToken has failed.

	return bRes;
}

Add && ( !IsImpersonating() ) to the if statement:

curl/lib/asyn-thread.c

Lines 639 to 642 in 17fbed2

#ifdef _WIN32
if(Curl_isWindows8OrGreater && Curl_FreeAddrInfoExW &&
Curl_GetAddrInfoExCancel && Curl_GetAddrInfoExW) {
#define MAX_NAME_LEN 256 /* max domain name is 253 chars */

I didn't have time to test this, but in theory it should fix the issue.

@jay
Copy link
Member

jay commented May 17, 2024

Would this always be true of impersonation? Is it possible the reporter is impersonating without the appropriate permissions in the token which is why winsock consistently returns WSATRY_AGAIN?

@bagder
Copy link
Member

bagder commented May 17, 2024

Would this always be true of impersonation?

And the other way around: isn't there a risk that it returns WSATRY_AGAIN even when not impersonating?

@bagder
Copy link
Member

bagder commented May 17, 2024

/cc @pps83

@jay
Copy link
Member

jay commented May 17, 2024

And the other way around: isn't there a risk that it returns WSATRY_AGAIN even when not impersonating?

Possibly but I don't think that's something we need to address. According to the doc WSATRY_AGAIN "usually" means the host was not found because the authoritative DNS server did not reply.

edit: WSATRY_AGAIN is the winsock equivalent of EAGAIN. Like how getaddrinfo can return EAGAIN to indicate temporary failure. So it's definitely possible but I think we continue to treat it as an error just like we do for getaddrinfo.

@pps83
Copy link
Contributor

pps83 commented May 18, 2024

Not sure if it's some sort of winapi or the user's code issue, perhaps, we can add an option to disable use of GetAddrInfoExW.
If it's some sort of winapi issue, a minimal example with GetAddrInfoExW wrapped to a synchronous call vs regular getaddrinfo should be reported to MS (this would also rule out if it's winapi vs user's code issue).
@timmaraju perhaps, as a workaround, you can build with c-ares for now.

@edmcln
Copy link

edmcln commented May 18, 2024

Hi all, today I spent some time on this issue, and wrote a simple Windows service that reproduces it. I used two different methods to reproduce it, using LogonUser & ImpersonateLoggedOnUser, and WTSQueryUserToken& ImpersonateLoggedOnUser, but my tests are based on the second method, that impersonates the logged-on user.

The service tries to resolve an address (localhost in this case) every 5 seconds, and it succeeds without impersonation (it runs as LOCAL SYSTEM), but it fails with impersonation if UAC is enabled, and succeeds with impersonation if UAC is disabled.

Please note that my test user belongs to the Administrators group, and UAC is enabled, which is the default behavior.

Here is the code:

// stdafx.h : include file for standard system include files,
// or project specific include files that are used frequently, but
// are changed infrequently
//

#pragma once

//#include "targetver.h"

#include <stdio.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <tchar.h>
#include <WtsApi32.h>
#include <UserEnv.h>
#pragma comment( lib, "ws2_32.lib" )
#pragma comment( lib, "wtsapi32.lib" )

#define MAX_ADDRESS_STRING_LENGTH   64
#define SERVICE_NAME TEXT( "IMP DNS Service" )

typedef struct _QueryContext
{
	OVERLAPPED      QueryOverlapped;
	PADDRINFOEX     QueryResults;
	HANDLE          CompleteEvent;
} QUERY_CONTEXT, *PQUERY_CONTEXT;
// ImpSrv.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

SERVICE_STATUS g_ServiceStatus;
SERVICE_STATUS_HANDLE g_StatusHandle;
HANDLE g_ServiceStopEvent;

/*BOOL WINAPI IsImpersonating()
{
	HANDLE hToken = NULL;
	BOOL bRes = FALSE;
	if ( OpenThreadToken( GetCurrentThread(), TOKEN_QUERY, TRUE, &hToken ) )
	{
		CloseHandle( hToken );
		bRes = TRUE;
	}

	// failure means the thread is not impersonating or OpenThreadToken has failed.

	return bRes;
}*/

BOOL WINAPI PrintUserInfo()
{
	TCHAR szInfo [MAX_PATH] = { 0 }, szMsg [MAX_PATH] = { 0 };
	DWORD dwSize = _countof( szInfo );
	BOOL Ret = FALSE;

	if ( !GetUserName( szInfo, &dwSize ) )
		OutputDebugString( TEXT( "GetUserName failed.\n" ) );
	else
	{
		_stprintf_s( szMsg, _countof( szMsg ), TEXT( "User name: %s\n" ), szInfo );
		OutputDebugString( szMsg );
		Ret = TRUE;
	}

	return Ret;
}

BOOL WINAPI Impersonate( PHANDLE hToken )
{
	BOOL Status = FALSE;
	//
	__try {
		DWORD dwSessionID = WTSGetActiveConsoleSessionId();
		if ( dwSessionID == 0xFFFFFFFF )
		{
			OutputDebugString( TEXT( "WTSGetActiveConsoleSessionId failed.\n" ) );
			__leave;
		}

		if ( !WTSQueryUserToken( dwSessionID, hToken ) )
		{
			OutputDebugString( TEXT( "WTSQueryUserToken failed.\n" ) );
			__leave;
		}

		if ( !ImpersonateLoggedOnUser( *hToken ) )
		{
			OutputDebugString( TEXT( "ImpersonateLoggedOnUser failed.\n" ) );
			__leave;
		}

		Status = TRUE;
	}

	__finally {
		//
	}

	return Status;
}

VOID
WINAPI
QueryCompleteCallback(
_In_ DWORD Error,
_In_ DWORD Bytes,
_In_ LPOVERLAPPED Overlapped
)
{
	PQUERY_CONTEXT  QueryContext = NULL;
	PADDRINFOEX     QueryResults = NULL;
	WCHAR           AddrString [MAX_ADDRESS_STRING_LENGTH];
	DWORD           AddressStringLength;
	TCHAR           szMsg[MAX_PATH] = { 0 };

	UNREFERENCED_PARAMETER( Bytes );

	__try {
		QueryContext = CONTAINING_RECORD( Overlapped,
			QUERY_CONTEXT,
			QueryOverlapped );

		if ( Error != ERROR_SUCCESS )
		{
			_stprintf_s( szMsg, _countof( szMsg ), TEXT( "ResolveName failed with %d\n" ), Error );
			OutputDebugString( szMsg );
			__leave;
		}

		OutputDebugString( TEXT( "ResolveName succeeded. Query Results:\n" ) );

		QueryResults = QueryContext->QueryResults;

		while ( QueryResults )
		{
			AddressStringLength = MAX_ADDRESS_STRING_LENGTH;

			WSAAddressToString( QueryResults->ai_addr,
				( DWORD ) QueryResults->ai_addrlen,
				NULL,
				AddrString,
				&AddressStringLength );

			_stprintf_s( szMsg, _countof( szMsg ), TEXT( "Ip Address: %s\n" ), AddrString );
			OutputDebugString( szMsg );
			QueryResults = QueryResults->ai_next;
		}
	}

	__finally {
		if ( QueryContext->QueryResults )
			FreeAddrInfoEx( QueryContext->QueryResults );

		//
		//  Notify caller that the query completed
		//

		SetEvent( QueryContext->CompleteEvent );
	}

	return;
}

INT WINAPI DNSCore()
{
	INT                 Error = ERROR_SUCCESS;
	WSADATA             wsaData;
	BOOL                IsWSAStartupCalled = FALSE;
	ADDRINFOEX          Hints;
	QUERY_CONTEXT       QueryContext;
	HANDLE              CancelHandle = NULL;
	DWORD               QueryTimeout = 4 * 1000; // 4 seconds
	TCHAR               szMsg [MAX_PATH] = { 0 };

	__try {
		ZeroMemory( &QueryContext, sizeof( QueryContext ) );

		PrintUserInfo();

		//
		//  All Winsock functions require WSAStartup() to be called first
		//

		Error = WSAStartup( MAKEWORD( 2, 2 ), &wsaData );
		if ( Error != 0 )
		{
			//
			_stprintf_s( szMsg, _countof( szMsg ), TEXT( "WSAStartup failed with %d\n" ), Error );
			OutputDebugString( szMsg );
			__leave;
		}

		IsWSAStartupCalled = TRUE;

		ZeroMemory( &Hints, sizeof( Hints ) );
		Hints.ai_family = AF_UNSPEC;

		//
		//  Note that this is a simple sample that waits/cancels a single
		//  asynchronous query. The reader may extend this to support
		//  multiple asynchronous queries.
		//

		QueryContext.CompleteEvent = CreateEvent( NULL, TRUE, FALSE, NULL );

		if ( QueryContext.CompleteEvent == NULL )
		{
			Error = GetLastError();
			_stprintf_s( szMsg, _countof( szMsg ), TEXT( "Failed to create completion event: Error %d\n" ), Error );
			OutputDebugString( szMsg );
			__leave;
		}

		//
		//  Initiate asynchronous GetAddrInfoExW.
		//
		//  Note GetAddrInfoEx can also be invoked asynchronously using an event
		//  in the overlapped object (Just set hEvent in the Overlapped object
		//  and set NULL as completion callback.)
		//
		//  This sample uses the completion callback method.
		//

		Error = GetAddrInfoExW( TEXT( "localhost" ),
			NULL,
			NS_DNS,
			NULL,
			&Hints,
			&QueryContext.QueryResults,
			NULL,
			&QueryContext.QueryOverlapped,
			QueryCompleteCallback,
			&CancelHandle );

		//
		//  If GetAddrInfoExW() returns  WSA_IO_PENDING, GetAddrInfoExW will invoke
		//  the completion routine. If GetAddrInfoExW returned anything else we must
		//  invoke the completion directly.
		//

		if ( Error != WSA_IO_PENDING )
		{
			QueryCompleteCallback( Error, 0, &QueryContext.QueryOverlapped );
			__leave;
		}

		//
		//  Wait for query completion for 5 seconds and cancel the query if it has
		//  not yet completed.
		//

		if ( WaitForSingleObject( QueryContext.CompleteEvent,
			QueryTimeout ) == WAIT_TIMEOUT )
		{

			//
			//  Cancel the query: Note that the GetAddrInfoExCancelcancel call does
			//  not block, so we must wait for the completion routine to be invoked.
			//  If we fail to wait, WSACleanup() could be called while an
			//  asynchronous query is still in progress, possibly causing a crash.
			//

			_stprintf_s( szMsg, _countof( szMsg ), TEXT( "The query took longer than %d seconds to complete; cancelling the query...\n" ), QueryTimeout / 1000 );
			OutputDebugString( szMsg );

			GetAddrInfoExCancel( &CancelHandle );

			WaitForSingleObject( QueryContext.CompleteEvent,
				INFINITE );
		}
	}

	__finally {
		if ( IsWSAStartupCalled )
		{
			WSACleanup();
		}

		if ( QueryContext.CompleteEvent )
		{
			CloseHandle( QueryContext.CompleteEvent );
		}
	}

	return Error;
}

VOID WINAPI ServiceControlHandler( DWORD CtrlCode )
{
	switch ( CtrlCode )
	{
		case SERVICE_CONTROL_INTERROGATE:
			break;

		case SERVICE_CONTROL_SHUTDOWN:
		case SERVICE_CONTROL_STOP:
			g_ServiceStatus.dwCurrentState = SERVICE_STOP_PENDING;
			SetServiceStatus( g_StatusHandle, &g_ServiceStatus );

			SetEvent( g_ServiceStopEvent );
			return;

		case SERVICE_CONTROL_PAUSE:
			break;

		case SERVICE_CONTROL_CONTINUE:
			break;

		default:
			if ( CtrlCode >= 128 && CtrlCode <= 255 )
				// user defined control code
				break;
			else
				// unrecognised control code
				break;
	}

	SetServiceStatus( g_StatusHandle, &g_ServiceStatus );
}

VOID WINAPI ServiceMain( DWORD argc, TCHAR* argv[] )
{
	// Init
	g_ServiceStatus.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
	g_ServiceStatus.dwCurrentState = SERVICE_START_PENDING;
	g_ServiceStatus.dwControlsAccepted = 0;
	g_ServiceStatus.dwWin32ExitCode = NO_ERROR;
	g_ServiceStatus.dwServiceSpecificExitCode = NO_ERROR;
	g_ServiceStatus.dwCheckPoint = 0;
	g_ServiceStatus.dwWaitHint = 0;

	g_StatusHandle = RegisterServiceCtrlHandler( SERVICE_NAME, ServiceControlHandler );

	if ( g_StatusHandle )
	{
		// service is starting
		g_ServiceStatus.dwCurrentState = SERVICE_START_PENDING;
		SetServiceStatus( g_StatusHandle, &g_ServiceStatus );

		// do initialisation here
		g_ServiceStopEvent = CreateEvent( NULL, FALSE, FALSE, NULL );

		// running
		g_ServiceStatus.dwControlsAccepted |= ( SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN );
		g_ServiceStatus.dwCurrentState = SERVICE_RUNNING;
		SetServiceStatus( g_StatusHandle, &g_ServiceStatus );

		/*HANDLE hToken = NULL;
		Impersonate( &hToken );*/

		do
		{
			//
			DNSCore();
		} while ( WaitForSingleObject( g_ServiceStopEvent, 5000 ) == WAIT_TIMEOUT );

		//
		//RevertToSelf();
		
		//if ( hToken ) CloseHandle( hToken );

		g_ServiceStatus.dwCurrentState = SERVICE_STOP_PENDING;
		SetServiceStatus( g_StatusHandle, &g_ServiceStatus );

		//
		CloseHandle( g_ServiceStopEvent );
		g_ServiceStopEvent = NULL;
		//

		//
		g_ServiceStatus.dwControlsAccepted &= ~( SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN );
		g_ServiceStatus.dwCurrentState = SERVICE_STOPPED;
		SetServiceStatus( g_StatusHandle, &g_ServiceStatus );
	}
}

VOID WINAPI RunService()
{
	SERVICE_TABLE_ENTRY serviceTable[] =
	{
		{ SERVICE_NAME, ServiceMain },
		{ NULL, NULL }
	};

	StartServiceCtrlDispatcher( serviceTable );
}

VOID WINAPI InstallService()
{
	SC_HANDLE hSrvCtrlMgr = OpenSCManager( NULL, NULL, SC_MANAGER_CREATE_SERVICE );

	if ( hSrvCtrlMgr )
	{
		TCHAR szPath[ MAX_PATH + 1 ];
		if ( GetModuleFileName( NULL, szPath, sizeof( szPath ) / sizeof( szPath [0] ) ) > 0 )
		{
			SC_HANDLE hService = CreateService( hSrvCtrlMgr,
				SERVICE_NAME, SERVICE_NAME, SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS, 
				SERVICE_DEMAND_START, SERVICE_ERROR_IGNORE, szPath, NULL, NULL, NULL, NULL, NULL );
			if ( hService )
				CloseServiceHandle( hService );
		}

		CloseServiceHandle( hSrvCtrlMgr );
	}
}

VOID WINAPI UninstallService()
{
	SC_HANDLE hSrvCtrlMgr = OpenSCManager( NULL, NULL, SC_MANAGER_CONNECT );

	if ( hSrvCtrlMgr )
	{
		SC_HANDLE hService = OpenService( hSrvCtrlMgr,
			SERVICE_NAME, SERVICE_QUERY_STATUS | DELETE );
		if ( hService )
		{
			SERVICE_STATUS Status;
			if ( QueryServiceStatus( hService, &Status ) )
			{
				if ( Status.dwCurrentState == SERVICE_STOPPED )
					DeleteService( hService );
			}

			CloseServiceHandle( hService );
		}

		CloseServiceHandle( hSrvCtrlMgr );
	}
}

int _tmain( int argc, TCHAR* argv[] )
{
	if ( argc > 1 && _tcsicmp( argv[1], TEXT("install") ) == 0 )
	{
		InstallService();
	}
	else if ( argc > 1 && _tcsicmp( argv [1], TEXT( "uninstall" ) ) == 0 )
	{
		UninstallService();
	}
	else
	{
		RunService();
	}

	return 0;
}

Steps to reproduce (after compiling the code):

  • Run ImpSrv.exe install to install the service, and ImpSrv.exe uninstall to uninstall it.
  • Download DebugView from Microsoft to monitor the debug output (it requires Admin privileges, and don't forget to Enable Capture Global Win32).
  • Start the service and view the service output in DebugView, you should get something like this:

Svc

00000001	0.00000000	[4688] User name: SYSTEM	
00000002	0.02582780	[4688] ResolveName succeeded. Query Results:	
00000003	0.02587420	[4688] Ip Address: ::1	
00000004	0.02637750	[4688] Ip Address: 127.0.0.1	
00000005	5.02249575	[4688] User name: SYSTEM	
00000006	5.04618120	[4688] ResolveName succeeded. Query Results:	
00000007	5.04635906	[4688] Ip Address: ::1	
00000008	5.04654169	[4688] Ip Address: 127.0.0.1
  • Stop the service, and enable impersonation by uncommenting four line of the code, and recompiling it.
HANDLE hToken = NULL;
Impersonate( &hToken );
RevertToSelf();
if ( hToken ) CloseHandle( hToken );
  • Restart the service and view the debug output, which should look like this (UAC is enabled, also note that the username is redacted here):

LogFail

  • Stop the service, launch the RegistryEditor and Disable the UAC by setting EnableLUA to 0 (located at HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Policies\System - also note that I set the ConsentPromptBehaviorAdmin to 0, which is not necessary)

UAC_REG

After rebooting the system and restarting the service, the debug output should look like this:

LogSuccess

The research still continues, and I'm trying to find the root cause of the issue to fix it properly.

@timmaraju
Copy link
Author

timmaraju commented May 21, 2024

Would this always be true of impersonation? Is it possible the reporter is impersonating without the appropriate permissions in the token which is why winsock consistently returns WSATRY_AGAIN?

I checked all the necessary permissions on the token and they are as recommended and even then winsock consistently responds WSATRY_AGAIN. Apparently the impersonation and the async operations don't work together. There are multiple issues in .NET repo for a similar problem.

dotnet/runtime#29351 (The thread is long and the information is dispersed all over)

pps83 added a commit to pps83/curl that referenced this issue May 21, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
@pps83
Copy link
Contributor

pps83 commented May 21, 2024

dotnet/runtime#29351 (The thread is long and the information is dispersed all over)

It does seem like winapi bug. Let me know if the fix works for you

pps83 added a commit to pps83/curl that referenced this issue May 21, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
pps83 added a commit to pps83/curl that referenced this issue May 21, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
pps83 added a commit to pps83/curl that referenced this issue May 22, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
pps83 added a commit to pps83/curl that referenced this issue May 23, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
pps83 added a commit to pps83/curl that referenced this issue May 23, 2024
Multiple reports suggest that GetAddrInfoExW fails when impersonation is used. This PR checks if thread is impersonating and avoids using GetAddrInfoExW api.

fixes curl#13612
@timmaraju
Copy link
Author

timmaraju commented May 23, 2024

It does seem like winapi bug. Let me know if the fix works for you

Yes, the fix works, now the Windows service can resolve the DNS addresses even with impersonation! 👍

@bagder bagder closed this as completed in 0caadc1 May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
name lookup DNS and related tech Windows Windows-specific
Development

Successfully merging a pull request may close this issue.

5 participants