Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,10 @@ The workaround is strictly opt-in and stays inert unless a PIN is available: wit
credential is left untouched so the normal Windows prompt path is used. It only activates for a
marshaled certificate credential; any other credential is passed through unchanged.

When the certificate records the legacy **Microsoft Base Smart Card Crypto Provider**, the credential is
repointed at the **Microsoft Smart Card Key Storage Provider** instead. A card minidriver is reachable
both through the Base CSP (CAPI) and through the KSP (CNG) under the same container name, but
[KB5066793](https://support.microsoft.com/help/5066793) stopped honouring the CAPI route for RSA smart
card keys. A certificate still associated with the legacy CSP therefore yields a credential LSASS cannot
service — the Kerberos certificate logon is declined locally, SPNEGO falls back to NTLM, and where NTLM
is disabled the failure surfaces as *"Authentication failed because NTLM authentication has been
disabled"* rather than as a smart card error. Naming the KSP reaches the same key over the path that
remains supported.

The provider name is rewritten in place, so the credential keeps the exact byte layout
`CredPackAuthenticationBufferW` produced; the two provider names are the same length, so nothing moves.
Certificates recording any other provider — including third-party CSPs, which do not imply a minidriver
— are left exactly as they were.
The credential is passed on exactly as `CredPackAuthenticationBufferW` produced it. The provider and
container names it records come from the certificate, and the server resolves them through the redirected
reader — so the same smart card middleware has to be installed on the remote host as on the client. A
mismatch surfaces as *"the key container does not exist on the smart card"*.

`KerbCertificateLogon` is independent of `PasswordContainsSCardPin`. The latter is the stock RDP
setting that tells the client the password field holds a smart card PIN, so that a smart card
Expand Down
77 changes: 30 additions & 47 deletions dll/Sspi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

#include <MsRdpEx/Detours.h>

static bool g_PcapInitialized = false;
static INIT_ONCE g_PcapEnvInitOnce = INIT_ONCE_STATIC_INIT;

static MsRdpEx_PcapFile* g_PcapFile = NULL;
static bool g_PcapEnabled = false;
Expand All @@ -34,13 +34,12 @@ void MsRdpEx_SetPcapFilePath(const char* pcapFilePath)
strcpy_s(g_PcapFilePath, MSRDPEX_MAX_PATH, pcapFilePath);
}

void MsRdpEx_PcapEnvInit()
// Reached from AcquireCredentialsHandleW and from the message paths, both of which run concurrently on
// several connection worker threads, so first use has to be serialized rather than guarded by a plain flag.
static BOOL CALLBACK MsRdpEx_PcapEnvInitOnce(PINIT_ONCE, PVOID, PVOID*)
{
char* envvar;

if (g_PcapInitialized)
return;

bool pcapDump = MsRdpEx_GetEnvBool("MSRDPEX_PCAP_DUMP", false);
MsRdpEx_SetPcapEnabled(pcapDump);

Expand All @@ -58,7 +57,12 @@ void MsRdpEx_PcapEnvInit()

free(envvar);

g_PcapInitialized = true;
return TRUE;
}

void MsRdpEx_PcapEnvInit()
{
InitOnceExecuteOnce(&g_PcapEnvInitOnce, MsRdpEx_PcapEnvInitOnce, NULL, NULL);
}

static MsRdpEx_PcapFile* MsRdpEx_GetPcapFile()
Expand Down Expand Up @@ -989,33 +993,6 @@ static void sspi_FormatCspInfoName(const MsRdpEx_KerbSmartCardCspInfo* cspInfo,
}
}

// Locate the KERB_SMARTCARD_CSP_INFO inside a packed credential, rejecting anything that does not match the
// layout this module assumes. KERB_SMARTCARD_CSP_INFO is not in the SDK, so its offsets are asserted at
// compile time and re-checked here against real bytes: the blob repeats its own length, so a disagreement
// with CspDataLength means this is not the structure we think it is.
static MsRdpEx_KerbSmartCardCspInfo* sspi_TryGetCspInfo(void* pLogon, DWORD cbLogon)
{
KERB_CERTIFICATE_LOGON* logon = (KERB_CERTIFICATE_LOGON*)pLogon;
MsRdpEx_KerbSmartCardCspInfo* cspInfo = NULL;

if (!pLogon || !MsRdpEx_CanReadUnsafePtr(pLogon, sizeof(KERB_CERTIFICATE_LOGON)))
return NULL;

if (logon->MessageType != KerbCertificateLogon)
return NULL;

if ((logon->CspDataLength < MSRDPEX_CSP_INFO_HEADER_SIZE) || !logon->CspData)
return NULL;

cspInfo = (MsRdpEx_KerbSmartCardCspInfo*) sspi_ResolvePackedField(pLogon, cbLogon,
logon->CspData, logon->CspDataLength);

if (!cspInfo || (cspInfo->dwCspInfoLen != logon->CspDataLength))
return NULL;

return cspInfo;
}

// Dump a KERB_CERTIFICATE_LOGON without revealing secrets: the PIN is reported as a length only, and the
// certificate itself never appears (the credential carries a container name, not the certificate).
static void sspi_LogKerbCertificateLogon(const char* label, void* pLogon, DWORD cbLogon)
Expand All @@ -1037,6 +1014,16 @@ static void sspi_LogKerbCertificateLogon(const char* label, void* pLogon, DWORD
return;
}

// CanReadUnsafePtr proves the pages are committed, not that the allocation extends this far, so a known
// size too small to hold the fixed header has to be rejected before the first dereference. Zero means the
// size is simply unknown (a buffer that is not a LocalAlloc block) and is handled further down.
if (cbLogon && (cbLogon < sizeof(KERB_CERTIFICATE_LOGON)))
{
MsRdpEx_LogPrint(DEBUG, "KerbCertificateLogon(%s): truncated, size=%u is below the header size",
label, cbLogon);
return;
}

logon = (KERB_CERTIFICATE_LOGON*)pLogon;

if (logon->MessageType != KerbCertificateLogon)
Expand All @@ -1057,6 +1044,16 @@ static void sspi_LogKerbCertificateLogon(const char* label, void* pLogon, DWORD
return;
}

// Without a known allocation size the extent check in sspi_ResolvePackedField cannot run, and walking the
// blob would then trust CspDataLength alone and could read past the credential. The fields above are
// already bounded by CanReadUnsafePtr, so report them and stop. A credential arriving in-band often has
// no LocalAlloc size (reconnect passes such a buffer), so this is a normal outcome, not a failure.
if (!cbLogon)
Comment thread
thenextman marked this conversation as resolved.
{
MsRdpEx_LogPrint(DEBUG, "KerbCertificateLogon(%s): CspData not parsed, allocation size unknown", label);
return;
}

cspInfo = (MsRdpEx_KerbSmartCardCspInfo*) sspi_ResolvePackedField(pLogon, cbLogon,
logon->CspData, logon->CspDataLength);

Expand Down Expand Up @@ -1283,20 +1280,6 @@ static bool sspi_IsCertificateMarshaledUserName(WCHAR* userName)
return result;
}

// Repoint a packed credential at the smart card KSP by overwriting, in place, the provider name that
// CredPackAuthenticationBufferW copied from the certificate.
//
// The Base Smart Card Crypto Provider is the CAPI front end over a card minidriver, so a certificate that
// records it is by definition on a minidriver card -- and every minidriver card is also reachable through the
// Smart Card KSP, under the same container name. KB5066793 stopped honouring the CAPI route for RSA smart card
// keys, so naming the KSP reaches the same key over the path that remains supported. No probe of the card is
// needed to know the route exists, which matters: touching the card from this code path (NCrypt -> KSP ->
// minidriver -> WinSCard) faults inside the smart card stack while the RDP client is mid-connect.
//
// Patched in place rather than rebuilt, so the buffer keeps the byte layout Windows produced. A hand-built
// equivalent carrying identical field values but a different internal layout is rejected by LSASS, which
// evidently derives the CspData extent from the buffer rather than trusting the offsets alone. The two
// provider names are both 41 characters, so the strings after this one do not move.
static bool sspi_CreatePackedCertificateLogon(WCHAR* marshaledCertificateUserName,
WCHAR* pin, HLOCAL* phPackedCredentials, DWORD* pcbPackedCredentials)
{
Expand Down