diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 18111e7..3db2e8d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,6 +23,17 @@ jobs: shell: cmd run: build.bat + - name: Parse every PowerShell script + shell: pwsh + run: | + $bad = 0 + foreach ($f in Get-ChildItem -Filter *.ps1) { + $tokens = $null; $errors = $null + [System.Management.Automation.Language.Parser]::ParseFile($f.FullName, [ref]$tokens, [ref]$errors) | Out-Null + foreach ($e in $errors) { Write-Host "$($f.Name):$($e.Extent.StartLineNumber): $($e.Message)"; $bad++ } + } + if ($bad) { throw "$bad PowerShell parse error(s)" } + - name: Check that every binary has Control Flow Guard shell: pwsh run: | @@ -37,8 +48,18 @@ jobs: run: > cl /nologo /c /analyze /W4 /EHsc /std:c++17 /DUNICODE /D_UNICODE dll.cpp CTacProvider.cpp CTacCredential.cpp CTacFilter.cpp helpers.cpp - totp.cpp store.cpp verify.cpp statelock.cpp eventlog.cpp enroll.cpp - ap.cpp subauth.cpp + totp.cpp store.cpp store_index.cpp verify.cpp statelock.cpp eventlog.cpp + enroll.cpp ap.cpp subauth.cpp + + # The code that runs inside lsass gets no free pass: a finding there fails + # the build. SDK headers are excluded from the analysis, so only our own + # code can trip it. + - name: Static analysis of the lsass code (/analyze /WX) + shell: cmd + run: > + cl /nologo /c /analyze /analyze:external- /external:anglebrackets /external:W0 + /W4 /WX /EHsc /std:c++17 /DUNICODE /D_UNICODE + ap.cpp subauth.cpp store_index.cpp tests: runs-on: ubuntu-latest @@ -46,7 +67,7 @@ jobs: - uses: actions/checkout@v4 - name: Install OpenSSL headers run: sudo apt-get update && sudo apt-get install -y libssl-dev - - name: Native C++ tests (real totp.cpp / verify.cpp) + - name: Native C++ tests (totp/verify, and the lsass code under ASan/UBSan) run: make -C tests/native - name: Python RFC 6238 check run: python3 tests/totp_check.py diff --git a/.gitignore b/.gitignore index 17315d5..49c899a 100644 --- a/.gitignore +++ b/.gitignore @@ -20,5 +20,6 @@ __pycache__/ # Created by new-guids.ps1 unregister-old.reg -# Native test binary +# Native test binaries tests/native/test_verify +tests/native/test_lsa diff --git a/CMakeLists.txt b/CMakeLists.txt index ea092ff..22383df 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,7 +12,7 @@ set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$:Debug>") # These binaries run in LogonUI and lsass: Control Flow Guard and SDL checks, warning level 4. if(MSVC) add_compile_options(/guard:cf /sdl /W4) - add_link_options(/guard:cf) + add_link_options(/guard:cf /CETCOMPAT) endif() # --- the credential provider + filter DLL --- @@ -24,6 +24,7 @@ add_library(TacProvider SHARED helpers.cpp totp.cpp store.cpp + store_index.cpp verify.cpp statelock.cpp eventlog.cpp) @@ -40,6 +41,7 @@ add_executable(enroll enroll.cpp totp.cpp store.cpp + store_index.cpp statelock.cpp) target_link_libraries(enroll PRIVATE @@ -48,13 +50,13 @@ target_link_libraries(enroll PRIVATE # --- the LSA authentication package (SKELETON) --- # Runs inside lsass.exe. A bug here can leave the machine unbootable. # Load only in a throwaway VM with a snapshot. See ap.h and the article. +# Links only the index reads (store_index.cpp): no DPAPI or TOTP code in lsass. add_library(TacAuthPackage SHARED ap.cpp - store.cpp - totp.cpp) + store_index.cpp) target_link_libraries(TacAuthPackage PRIVATE - advapi32 crypt32 bcrypt) + advapi32) set_target_properties(TacAuthPackage PROPERTIES LINK_FLAGS "/DEF:\"${CMAKE_CURRENT_SOURCE_DIR}/TacAuthPackage.def\"") @@ -65,11 +67,10 @@ set_target_properties(TacAuthPackage PROPERTIES # Same warnings as above: lsass.exe, VM with snapshot only. add_library(TacSubAuth SHARED subauth.cpp - store.cpp - totp.cpp) + store_index.cpp) target_link_libraries(TacSubAuth PRIVATE - advapi32 crypt32 bcrypt) + advapi32) set_target_properties(TacSubAuth PROPERTIES LINK_FLAGS "/DEF:\"${CMAKE_CURRENT_SOURCE_DIR}/TacSubAuth.def\"") diff --git a/CTacFilter.cpp b/CTacFilter.cpp index 29b9f37..f84cdfa 100644 --- a/CTacFilter.cpp +++ b/CTacFilter.cpp @@ -2,6 +2,7 @@ #include "guid.h" #include "dll.h" #include "store.h" +#include #include IFACEMETHODIMP_(ULONG) CTacFilter::AddRef() @@ -56,8 +57,10 @@ IFACEMETHODIMP CTacFilter::Filter(CREDENTIAL_PROVIDER_USAGE_SCENARIO cpus, DWORD // Nothing enrolled: hiding the other tiles would only lock everyone out. // A store that cannot be read is NOT treated as empty - filter as usual. - bool any = false; - if (tac::QueryAnyEnrollment(any) && !any) + // QueryAnyEnrollment counts stored secrets, not registry values, so a + // leftover value of another kind cannot make us hide every way in while + // nothing can actually authenticate. + if (!AnyEnrollment()) return S_OK; for (DWORD i = 0; i < cProviders; ++i) @@ -65,9 +68,24 @@ IFACEMETHODIMP CTacFilter::Filter(CREDENTIAL_PROVIDER_USAGE_SCENARIO cpus, DWORD return S_OK; } +// Is at least one account enrolled? A store that cannot be read counts as +// "yes", so an unreadable store does not quietly switch the second factor off. +bool CTacFilter::AnyEnrollment() +{ + bool any = false; + if (!tac::QueryAnyEnrollment(any)) + return true; + return any; +} + // RDP with NLA delivers the credential before a tile exists. We forward it and // set the target provider to ourselves; copying the incoming CLSID would send // it to a provider we just filtered out, and remote logon would break. +// +// Only while we are actually filtering, though: with nothing enrolled, Filter() +// leaves every other provider visible, and taking the credential here would +// hand it to a tile that cannot authenticate anybody. Then it belongs to +// whoever it was addressed to. IFACEMETHODIMP CTacFilter::UpdateRemoteCredential( const CREDENTIAL_PROVIDER_CREDENTIAL_SERIALIZATION* pcpcsIn, CREDENTIAL_PROVIDER_CREDENTIAL_SERIALIZATION* pcpcsOut) @@ -77,6 +95,21 @@ IFACEMETHODIMP CTacFilter::UpdateRemoteCredential( ZeroMemory(pcpcsOut, sizeof(*pcpcsOut)); if (!pcpcsIn || pcpcsIn->cbSerialization == 0 || !pcpcsIn->rgbSerialization) return E_NOTIMPL; + if (!AnyEnrollment()) + return E_NOTIMPL; + + // RDP can forward other logon types too, a smartcard logon for example. + // Only a password logon has the layout our tile unpacks, so anything else + // stays with the provider it was addressed to instead of being taken and + // then refused. MessageType is the first member of the packed + // KERB_INTERACTIVE_UNLOCK_LOGON. + if (pcpcsIn->cbSerialization < sizeof(KERB_INTERACTIVE_UNLOCK_LOGON)) + return E_NOTIMPL; + const KERB_LOGON_SUBMIT_TYPE type = + reinterpret_cast( + pcpcsIn->rgbSerialization)->Logon.MessageType; + if (type != KerbInteractiveLogon && type != KerbWorkstationUnlockLogon) + return E_NOTIMPL; pcpcsOut->rgbSerialization = static_cast(CoTaskMemAlloc(pcpcsIn->cbSerialization)); diff --git a/CTacFilter.h b/CTacFilter.h index f70994e..8c0f29b 100644 --- a/CTacFilter.h +++ b/CTacFilter.h @@ -25,5 +25,8 @@ class CTacFilter : public ICredentialProviderFilter private: virtual ~CTacFilter() { DllRelease(); } + + // At least one stored secret, or a store that cannot be read. + static bool AnyEnrollment(); LONG _cRef; }; diff --git a/README.md b/README.md index 69994e0..9d3c61f 100644 --- a/README.md +++ b/README.md @@ -25,14 +25,19 @@ use this anywhere, so you don't use the CLSIDs of this repository. In an elevated PowerShell on the test VM: ```powershell -New-Item -ItemType Directory -Force "C:\Program Files\TheAdminCafe" | Out-Null -Copy-Item .\TacProvider.dll, .\enroll.exe "C:\Program Files\TheAdminCafe\" -reg import .\register.reg +.\install-doppio.ps1 -# Enroll every local account you need, including a break-glass admin +# Enroll every local account that signs in at the console or over RDP, +# including a break-glass admin & "C:\Program Files\TheAdminCafe\enroll.exe" alice +& "C:\Program Files\TheAdminCafe\enroll.exe" breakglass ``` +`install-doppio.ps1` copies the files to `C:\Program Files\TheAdminCafe` and +imports `register.reg`, after checking that nobody else's provider already +holds those CLSIDs. (By hand: create the folder, copy `TacProvider.dll` and +`enroll.exe`, `reg import .\register.reg`.) + `enroll.exe` shows the secret and asks for a code from your authenticator app before it stores anything, so a secret mistyped into the app can't lock you out. Log off, click "Sign-in options", choose the brown **2FA** icon and log on @@ -41,9 +46,15 @@ with password and code. Only when this works for every account, hide all other sign-in options: ```powershell -reg import .\register-filter.reg +.\install-doppio.ps1 -EnableFilter ``` +It refuses while an enabled local account is not enrolled, because that account +could not sign in at the console or over RDP afterwards. An account that never +signs in interactively (a service account) can be named with +`-AllowUnenrolled svc-backup`. (By hand: `reg import .\register-filter.reg`, +without that check.) + From then on the logon and unlock screens show **only** the 2FA tiles - no password tile, no PIN, no Windows Hello. That is the point: with any of them left, the code would be optional. Microsoft advises against it, because a @@ -52,7 +63,10 @@ broken provider then leaves no way in. What is left for recovery: - The 2FA provider always shows an **"Other user"** tile at logon, so an account Windows does not list (a hidden break-glass admin) can still sign in. - While **no account is enrolled at all**, the filter hides nothing. Importing - it too early, or after `enroll.exe /purge`, cannot lock you out. + it too early, or after `enroll.exe /purge`, cannot lock you out. Only stored + secrets count, not stray registry values - but the secret of an account you + deleted without `/remove` does count. `enroll.exe /list` shows it as + "(deleted account)"; `enroll.exe /remove ` removes it. - If `TacProvider.dll` cannot be loaded, the filter in the same DLL cannot be created either, and Windows shows all sign-in options again. Confirm this once in the VM by renaming the DLL. @@ -68,11 +82,15 @@ password. The password check is a `LogonUser` call, so each sign-in leaves one extra logon event (4624, type 2, LogonUI.exe) in the Security log. Unlock a user after too many wrong codes: `enroll.exe /unlock alice`. -Remove an enrollment again: `enroll.exe /remove alice`. That drops the secret, -the state and the index entries the LSA packages read. Use it before you delete -the Windows account; `/remove` still cleans up afterwards, but only if you -remember the name. After renaming an enrolled account, or after updating from a -version without the RID index, run `enroll.exe /reindex`. +See who is enrolled, and whether the LSA indexes know about them: +`enroll.exe /list`. Remove an enrollment again: `enroll.exe /remove alice`. +That drops the secret, the state and the index entries the LSA packages read. +Use it before you delete the Windows account; afterwards, take the SID from +`/list` and run `enroll.exe /remove S-1-5-21-...`. After renaming an enrolled +account, or after updating from a version without the RID index, run +`enroll.exe /reindex`. Every `enroll.exe` run (except `/list` and `/purge`) also +re-applies the SYSTEM-and-Administrators ACL to existing store keys; the logon +path no longer does that on every attempt. Uninstall: `enroll.exe /purge` (deletes all secrets, state and indexes), then `reg import .\unregister.reg`, reboot, delete the folder (it also holds `state.lock`, which serializes the state updates of parallel logons). Without @@ -81,6 +99,9 @@ Uninstall: `enroll.exe /purge` (deletes all secrets, state and indexes), then Events (Application log, source "TheAdminCafe 2FA"): 100 password and code accepted, 101 wrong code, 102 code already used, 103/104 lock, 105 not enrolled, 106 state error, 107 password refused, 108 final result from LSA. +From the LSA pieces: 200 authentication package loaded (written while LSA +starts, before the Eventlog service runs, so usually only visible in the +debugger), 201/202 package denied/declined a logon, 210 filter refused a logon. ## What the tile does not cover @@ -116,21 +137,40 @@ alone. On a domain member, a GPO that defines these rights overwrites them. **2. The LSA authentication package** (`ap.cpp`, `TacAuthPackage.dll`). Runs in `lsass.exe` and shows the package interface. It is **not a security control**: a logon only reaches it if the caller selects it by package id, and every such -logon fails anyway, because the package never builds a token. It reads only the -user name from the logon buffer (bounds-checked), never the password, and asks -the store with a single registry read whether that name is enrolled - no DPAPI -decrypt and no name/SID lookup, so nothing re-enters LSA from the logon path. +logon fails anyway, because the package never builds a token - denied with +`STATUS_ACCOUNT_RESTRICTION` for an enrolled account on a non-interactive path, +declined with `STATUS_INVALID_LOGON_TYPE` otherwise. It reads only the user +name from the logon buffer (bounds-checked, as a client address or a packed +offset), never the password, returns it as `AccountName` for the audit record, +and asks the store with a single registry read whether that name is enrolled - +no DPAPI decrypt and no name/SID lookup, so nothing re-enters LSA from the +logon path. **3. The MSV1_0 sub-authentication filter** (`subauth.cpp`, `TacSubAuth.dll`, registered as `Auth0`). MSV1_0 calls it **after** it has validated a logon, and -the filter can only veto: it refuses a network logon for an enrolled account and -has no objection to anything else. MSV1_0 keeps the password check, the account -restrictions and the token. The account is identified by the RID from the SAM, -which survives a rename, with a single registry read (no DPAPI, no lookup). It -reads nothing but the RID and the account name; it never touches the password -hashes or the challenge-response. That credential-safety line is the whole -point: the same LSA seam is where credential-theft malware sits, and this code -deliberately does not read or keep credential material. +the filter can only veto: it refuses a network logon for an enrolled account +with `STATUS_INVALID_WORKSTATION` and has no objection to anything else, nor to +a pass-through logon for an account that lives elsewhere. MSV1_0 keeps the +password check, the account restrictions and the token. The account is +identified by the RID from the SAM, which survives a rename, with a single +registry read (no DPAPI, no lookup). It reads nothing but the RID and the +account name; it never touches the password hashes or the challenge-response. +That credential-safety line is the whole point: the same LSA seam is where +credential-theft malware sits, and this code deliberately does not read or keep +credential material. + +Both LSA pieces follow the same rules (`lsasafe.h`): no heap and no CRT of our +own on the logon path, SEH around the decision, and an enrollment index that +cannot be read counts as "enrolled" (fail closed). They link only +`store_index.cpp` from the store, not the DPAPI and TOTP code. + +The filter leaves `LogoffTime` and `KickoffTime` alone; what MSV1_0 hands in +there is undocumented, so every call writes the incoming values to the +debugger. Logons where the server gets the password in clear (type 8, +NetworkCleartext, e.g. WinRM or IIS with Basic auth) most likely reach the +filter as an interactive logon level and get no objection. Untested: check in +the VM which LogonLevel arrives, and whether the deny right for network logons +catches type 8. Earlier versions exported `Msv1_0SubAuthenticationRoutine` under `Auth1..4`. That is the wrong hook: it only runs for logons that ask for it, and when it @@ -140,6 +180,10 @@ password". `install-subauth.ps1` removes the old registration. Like the deny right, the filter also blocks RDP with NLA for enrolled accounts. Microsoft documents `Auth0` for domain controllers; whether MSV1_0 calls it for local accounts on your Windows version is the first thing to check in the VM. +`install-subauth.ps1` refuses to run on a domain controller, where the filter +would check domain RIDs against a local index. MSV1_0 loads the DLL the first +time it needs it, so a broken filter can surface at the first network logon +after the reboot, not only at boot. ```powershell .\install-authpackage.ps1 # the authentication package @@ -152,10 +196,12 @@ local accounts on your Windows version is the first thing to check in the VM. > Safe Mode. Load them only in a throwaway VM with a snapshot, a BitLocker > recovery key and a WinRE way to edit the registry offline. Debug over a kernel > debugger. They are **learning skeletons, untested on live LSA.** With LSA -> protection (RunAsPPL) on - the default on many current Windows 11 installs - -> lsass refuses to load them at all, because they are not signed. See the -> article "2FA inside LSA". The install scripts refuse to run while LSA -> protection is on (registry check). +> protection (RunAsPPL) on - the default on new, enterprise-joined Windows 11 +> 22H2+ installs with HVCI - lsass refuses to load them at all, because they +> are not signed. See the article "2FA inside LSA". The install scripts refuse +> to run while LSA protection is on: they check `RunAsPPL`/`RunAsPPLBoot` and, +> because the default-on case is not documented to show up there, WinInit +> event 12 ("started as a protected process") since the last boot. ## Limits @@ -175,12 +221,25 @@ local accounts on your Windows version is the first thing to check in the VM. ## Tests -`tests/totp_check.py` checks the algorithm against RFC 6238 in Python. -`tests/native/` compiles the real `totp.cpp` and `verify.cpp` on Linux, against -OpenSSL and an in-memory store, and checks the lockout, replay and fail-closed -behaviour: `make -C tests/native`. `.github/workflows/build.yml` runs both, and -builds everything with MSVC (`/W4 /sdl /guard:cf`, plus `/analyze`) on each push. -None of this loads anything into LogonUI or lsass; that still needs the VM. +`tests/totp_check.py` checks the algorithm against RFC 6238 in Python and +exits non-zero on any failed check. `tests/native/` builds two tests on Linux +with `make -C tests/native`: + +- `test_verify` compiles the real `totp.cpp` and `verify.cpp` against OpenSSL + and an in-memory store, and checks the lockout, replay and fail-closed + behaviour. +- `test_lsa` compiles the real `subauth.cpp`, `ap.cpp` and `store_index.cpp` - + the code that runs inside lsass - against a fake registry, event log and LSA + heap, under AddressSanitizer and UBSan. It checks every veto and fail-closed + path, the `[out]` parameters, hostile logon buffers, non-ASCII names, and that + no LSA heap block or registry handle leaks. SEH is mapped to C++ exceptions + there, so the real `__except` still needs the VM. + +`.github/workflows/build.yml` runs both, parses every PowerShell script, and +builds everything with MSVC (`/W4 /sdl /guard:cf /CETCOMPAT`) on each push. +`/analyze` runs over all sources as a report, and with `/WX` over the lsass +code, where a finding fails the build. None of this loads anything into +LogonUI or lsass; that still needs the VM. ## License diff --git a/ap.cpp b/ap.cpp index 926eff8..48fa838 100644 --- a/ap.cpp +++ b/ap.cpp @@ -1,32 +1,42 @@ #include "ap.h" +#include "lsasafe.h" #include "store.h" -#include -#include #pragma comment(lib, "advapi32.lib") // NTSTATUS values we return. They live in ntstatus.h, which clashes with -// windows.h, so we define the handful we need. +// windows.h, so we define the handful we need. Every one of these is listed in +// the documentation of LSA_AP_LOGON_USER_EX2 as a value this callback may +// return - STATUS_NOT_IMPLEMENTED, which this package used to decline with, +// is not. #ifndef STATUS_SUCCESS -#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) +#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) #endif #ifndef STATUS_ACCOUNT_RESTRICTION #define STATUS_ACCOUNT_RESTRICTION ((NTSTATUS)0xC000006EL) // "account not allowed to log on here/now" #endif +#ifndef STATUS_INVALID_LOGON_TYPE +#define STATUS_INVALID_LOGON_TYPE ((NTSTATUS)0xC000010BL) +#endif +#ifndef STATUS_INVALID_WORKSTATION +#define STATUS_INVALID_WORKSTATION ((NTSTATUS)0xC0000070L) +#endif #ifndef STATUS_NOT_IMPLEMENTED -#define STATUS_NOT_IMPLEMENTED ((NTSTATUS)0xC0000002L) +#define STATUS_NOT_IMPLEMENTED ((NTSTATUS)0xC0000002L) #endif #ifndef STATUS_INVALID_PARAMETER -#define STATUS_INVALID_PARAMETER ((NTSTATUS)0xC000000DL) +#define STATUS_INVALID_PARAMETER ((NTSTATUS)0xC000000DL) #endif -#ifndef STATUS_INSUFFICIENT_RESOURCES -#define STATUS_INSUFFICIENT_RESOURCES ((NTSTATUS)0xC000009AL) +#ifndef STATUS_NO_MEMORY +#define STATUS_NO_MEMORY ((NTSTATUS)0xC0000017L) #endif -// Every export below runs inside lsass.exe. The helpers use std::wstring, -// which can throw std::bad_alloc; each export therefore has a catch-all that -// turns any C++ exception into a failure status. An exception that reached -// LSA would take lsass, and with it the whole machine, down. +// Every export below runs inside lsass.exe, so the rules from lsasafe.h apply: +// no heap of our own and no CRT on the logon path, and SEH rather than C++ +// try/catch, because under /EHsc a catch (...) does not catch the access +// violation a bad pointer from the caller would cause. The only allocations +// here come from the LSA heap, through the dispatch table, because LSA asks +// for the account name that way and frees it itself. // Kerberos message types we recognise in a submit buffer. #ifndef KerbInteractiveLogon @@ -41,20 +51,26 @@ namespace PLSA_DISPATCH_TABLE g_lsa = nullptr; ULONG g_packageId = 0; - // Logging from inside lsass. OutputDebugStringW is the channel you actually - // read under a kernel debugger. ReportEvent is best-effort on top, guarded - // so a logging failure can never affect a logon. We never log a password or - // a code, only metadata. - void ApLog(WORD type, DWORD id, const std::wstring& text) + // UNLEN + 1. + const SIZE_T kApNameCch = 257; + + // Logging from inside lsass. OutputDebugStringW is the channel you + // actually read under a kernel debugger. ReportEvent is best-effort on top, + // guarded so a logging failure can never affect a logon. We never log a + // password or a code, only metadata. + void ApLog(WORD type, DWORD id, const WCHAR* text) { - OutputDebugStringW(L"[Doppio-AP] "); - OutputDebugStringW(text.c_str()); - OutputDebugStringW(L"\n"); + WCHAR debug[320]; + tac::Buf d(debug, ARRAYSIZE(debug)); + d.Str(L"[Doppio-AP] "); + d.Str(text); + d.Str(L"\n"); + OutputDebugStringW(debug); HANDLE h = RegisterEventSourceW(nullptr, L"TheAdminCafe 2FA"); if (h) { - LPCWSTR s[1] = { text.c_str() }; + LPCWSTR s[1] = { text }; ReportEventW(h, type, 0, id, nullptr, 1, 0, s, nullptr); DeregisterEventSource(h); } @@ -92,59 +108,154 @@ namespace } } - // A UNICODE_STRING inside the submit buffer has a Buffer pointer that is - // valid in the CLIENT process, not ours. LSA gives us ClientBufferBase (the - // address the buffer had in the client) so we can relocate it into our copy. - // Everything is bounds-checked against the submit buffer; anything off - // returns an empty string, and the caller fails safe. - std::wstring ReadClientString(const UNICODE_STRING& us, - const void* submitBase, const void* clientBase, - ULONG submitSize) + // A UNICODE_STRING inside the submit buffer has a Buffer field that is not + // a pointer we may follow. Two forms reach us: + // + // * an address in the CLIENT process. LSA hands us ClientBufferBase, the + // address the buffer had there, so the difference is the offset into + // our copy. + // * a plain offset from the start of the submit buffer. That is how + // Winlogon and LogonUI pack a KERB_INTERACTIVE_UNLOCK_LOGON - see + // KerbInteractiveUnlockLogonPack in Microsoft's credential provider + // sample, whose comment says the members "are not actually pointers + // but byte offsets into the overall buffer". The old code only knew + // the first form and read a packed buffer as an empty name. + // + // Everything is bounds-checked against the submit buffer. Anything off + // copies nothing and the caller fails safe. Writes into a caller buffer: + // no heap, no CRT. + bool ReadClientString(const UNICODE_STRING& us, + const void* submitBase, const void* clientBase, ULONG submitSize, + WCHAR* out, SIZE_T outCap, SIZE_T& outCch) { - if (!us.Buffer || us.Length == 0 || (us.Length % sizeof(WCHAR)) != 0) - return std::wstring(); + outCch = 0; + if (outCap) + out[0] = L'\0'; + + if (!submitBase || !outCap || !us.Buffer || us.Length == 0 || + (us.Length % sizeof(WCHAR)) != 0) + return false; const ULONG_PTR p = reinterpret_cast(us.Buffer); const ULONG_PTR base = reinterpret_cast(clientBase); - if (p < base) - return std::wstring(); - const ULONG_PTR off = p - base; - if (off > submitSize || us.Length > submitSize - off) - return std::wstring(); + ULONG_PTR off = 0; + if (clientBase && p >= base && (p - base) <= submitSize) + off = p - base; // client address, relocated + else if (p < submitSize) + off = p; // already an offset + else + return false; + + if ((off % sizeof(WCHAR)) != 0 || us.Length > submitSize - off) + return false; const WCHAR* s = reinterpret_cast( static_cast(submitBase) + off); - return std::wstring(s, us.Length / sizeof(WCHAR)); + + SIZE_T cch = us.Length / sizeof(WCHAR); + if (cch > outCap - 1) + cch = outCap - 1; + + SIZE_T n = 0; + while (n < cch && s[n] != L'\0') + { + out[n] = s[n]; + ++n; + } + out[n] = L'\0'; + outCch = n; + return n > 0; } + // What the decision produced. A plain struct with fixed buffers, so the + // __try frame in the export holds nothing that needs unwinding. + struct Decision + { + bool deny; + bool haveName; + tac::Enrollment enrolled; + WCHAR name[kApNameCch]; + SIZE_T nameCch; + }; + // The account check. We read ONLY the user name from the submit buffer // (never the password field) and ask the store, with a single registry - // read, whether that name is enrolled. IsEnrolledByName does no DPAPI and - // no LSA name/SID lookup, so it is safe on the logon path. Any - // inconsistency returns false (treat as not enrolled -> defer), so a - // malformed buffer cannot crash us. + // read, whether that name is enrolled. EnrollmentByName does no DPAPI and + // no LSA name/SID lookup, so it is safe on the logon path, and it does not + // allocate. // // The name is what the CLIENT typed, and the name index is not updated by // a rename (see store.h), so this check is weaker than the RID check in // the sub-authentication filter. It does not matter here - this package // never lets a logon through either way (see ap.h) - but do not copy it // into anything that does. - bool IsEnrolledAccount(PVOID submit, ULONG size, PVOID clientBase) + void Decide(SECURITY_LOGON_TYPE logonType, PVOID submit, ULONG size, PVOID clientBase, + Decision& d) { + d.deny = false; + d.haveName = false; + d.enrolled = tac::Enrollment::No; + d.name[0] = L'\0'; + d.nameCch = 0; + + // The name first, for every logon type: LSA writes AccountName into + // the audit record whatever we decide. if (!submit || size < sizeof(KERB_INTERACTIVE_LOGON)) - return false; + return; auto* k = static_cast(submit); if (k->MessageType != KerbInteractiveLogon && k->MessageType != KerbWorkstationUnlockLogon) - return false; + return; - std::wstring user = ReadClientString(k->UserName, submit, clientBase, size); - if (user.empty()) - return false; + d.haveName = ReadClientString(k->UserName, submit, clientBase, size, + d.name, ARRAYSIZE(d.name), d.nameCch); - return tac::IsEnrolledByName(user); + if (!IsNonInteractive(logonType)) + return; // the tile already asked for a code + if (!d.haveName) + return; + + d.enrolled = tac::EnrollmentByName(d.name, d.nameCch); + + // Yes and Unknown both deny. "Could not tell" must never come out as + // "not enrolled" - the same rule the sub-authentication filter follows. + d.deny = d.enrolled != tac::Enrollment::No; + } + + // LSA puts this in the audit record and frees it itself, so the account + // name has to come from the LSA heap. The documentation of + // LSA_AP_LOGON_USER_EX2 says it "must always be returned regardless of the + // success or failure of the call"; this package used to leave it null. + PUNICODE_STRING AllocAccountName(const WCHAR* name, SIZE_T cch) + { + if (!g_lsa || !g_lsa->AllocateLsaHeap || !g_lsa->FreeLsaHeap) + return nullptr; + if (cch > kApNameCch - 1) + cch = kApNameCch - 1; + + auto* us = static_cast( + g_lsa->AllocateLsaHeap(sizeof(UNICODE_STRING))); + if (!us) + return nullptr; + + const USHORT cb = static_cast(cch * sizeof(WCHAR)); + WCHAR* buf = static_cast(g_lsa->AllocateLsaHeap(cb + sizeof(WCHAR))); + if (!buf) + { + g_lsa->FreeLsaHeap(us); + return nullptr; + } + + for (SIZE_T i = 0; i < cch; ++i) + buf[i] = name[i]; + buf[cch] = L'\0'; + + us->Length = cb; + us->MaximumLength = static_cast(cb + sizeof(WCHAR)); + us->Buffer = buf; + return us; } } @@ -181,49 +292,27 @@ NTSTATUS NTAPI LsaApInitializePackage( name->Buffer = buf; *AuthenticationPackageName = name; - try - { - ApLog(EVENTLOG_INFORMATION_TYPE, 200, - L"LSA authentication package loaded (id " + std::to_wstring(AuthenticationPackageId) + L")."); - } - catch (...) - { - // The log line is optional; the package is loaded either way. - } + // LSA loads authentication packages while it starts, before the Eventlog + // service is running, so event 200 normally goes nowhere at boot. The + // [Doppio-AP] debugger line is the one you can rely on here. + WCHAR text[160]; + tac::Buf b(text, ARRAYSIZE(text)); + b.Str(L"LSA authentication package loaded (id "); + b.U64(AuthenticationPackageId); + b.Str(L")."); + ApLog(EVENTLOG_INFORMATION_TYPE, 200, text); + return STATUS_SUCCESS; } // We only ever DENY or decline here; we never mint a token. The deny path -// returns STATUS_ACCOUNT_RESTRICTION. Otherwise we return STATUS_NOT_IMPLEMENTED -// to say "this package does not handle this logon". Note: a caller that selects -// this package by id and gets STATUS_NOT_IMPLEMENTED has its logon fail here; it -// is not transparently retried against MSV1_0. Standard network/batch logons do -// not address this package at all - that is what the sub-authentication filter -// (subauth.cpp) is for. See the article, "deny is easy, success is hard". -static NTSTATUS LogonUserEx2Impl(SECURITY_LOGON_TYPE LogonType, PVOID ProtocolSubmitBuffer, - PVOID ClientBufferBase, ULONG SubmitBufferSize, PNTSTATUS SubStatus) -{ - const bool nonInteractive = IsNonInteractive(LogonType); - const bool enrolled = nonInteractive && - IsEnrolledAccount(ProtocolSubmitBuffer, SubmitBufferSize, ClientBufferBase); - - if (nonInteractive && enrolled) - { - ApLog(EVENTLOG_WARNING_TYPE, 201, - std::wstring(L"Denied a ") + LogonTypeName(LogonType) + - L" logon for an enrolled account. This path has no second factor."); - if (SubStatus) *SubStatus = STATUS_ACCOUNT_RESTRICTION; - return STATUS_ACCOUNT_RESTRICTION; - } - - // Not "passed on": nothing is forwarded anywhere. The caller asked for this - // package, and this package never builds a token, so the logon fails. - ApLog(EVENTLOG_INFORMATION_TYPE, 202, - std::wstring(L"Declined a ") + LogonTypeName(LogonType) + - L" logon addressed to this package. It fails; this package never builds a token."); - return STATUS_NOT_IMPLEMENTED; -} - +// returns STATUS_ACCOUNT_RESTRICTION with the reason in SubStatus. Otherwise we +// return STATUS_INVALID_LOGON_TYPE to say "this package does not handle this +// logon". Note: a caller that selects this package by id has its logon fail +// here either way; it is not transparently retried against MSV1_0. Standard +// network/batch logons do not address this package at all - that is what the +// sub-authentication filter (subauth.cpp) is for. See the article, "deny is +// easy, success is hard". NTSTATUS NTAPI LsaApLogonUserEx2( PLSA_CLIENT_REQUEST /*ClientRequest*/, SECURITY_LOGON_TYPE LogonType, @@ -250,16 +339,65 @@ NTSTATUS NTAPI LsaApLogonUserEx2( if (MachineName) *MachineName = nullptr; if (CachedCredentials) *CachedCredentials = nullptr; - try + Decision d = {}; + bool faulted = false; + + // A bad pointer in the submit buffer would otherwise take lsass down with + // it, and with it the machine. Nothing in Decide allocates, so there is no + // state to unwind. + __try + { + Decide(LogonType, ProtocolSubmitBuffer, SubmitBufferSize, ClientBufferBase, d); + } + __except (tac::LsaDecisionExceptionFilter(GetExceptionCode())) + { + faulted = true; + d.deny = IsNonInteractive(LogonType); // fail closed + d.haveName = false; + d.nameCch = 0; + d.name[0] = L'\0'; + } + + WCHAR text[320]; + tac::Buf b(text, ARRAYSIZE(text)); + + NTSTATUS status; + if (d.deny) { - return LogonUserEx2Impl(LogonType, ProtocolSubmitBuffer, ClientBufferBase, - SubmitBufferSize, SubStatus); + b.Str(L"Denied a "); + b.Str(LogonTypeName(LogonType)); + b.Str(L" logon for '"); + b.Str(d.haveName ? d.name : L"?"); + b.Str(faulted ? L"' after an exception while deciding." + : (d.enrolled == tac::Enrollment::Unknown + ? L"': the enrollment index could not be read." + : L"': an enrolled account. This path has no second factor.")); + ApLog(EVENTLOG_WARNING_TYPE, 201, text); + + if (SubStatus) *SubStatus = STATUS_INVALID_WORKSTATION; + status = STATUS_ACCOUNT_RESTRICTION; } - catch (...) + else { - if (SubStatus) *SubStatus = STATUS_INSUFFICIENT_RESOURCES; - return STATUS_INSUFFICIENT_RESOURCES; + // Not "passed on": nothing is forwarded anywhere. The caller asked for + // this package, and this package never builds a token, so the logon + // fails. + b.Str(L"Declined a "); + b.Str(LogonTypeName(LogonType)); + b.Str(L" logon addressed to this package. It fails; this package never " + L"builds a token."); + ApLog(EVENTLOG_INFORMATION_TYPE, 202, text); + + status = STATUS_INVALID_LOGON_TYPE; } + + // Always, success or failure: LSA writes it to the audit record and frees + // it. A name we could not read is reported as "-" rather than left null. + if (AccountName) + *AccountName = d.haveName ? AllocAccountName(d.name, d.nameCch) + : AllocAccountName(L"-", 1); + + return status; } static NTSTATUS CallStub(PVOID* ProtocolReturnBuffer, PULONG ReturnBufferLength, diff --git a/ap.h b/ap.h index bcb38fd..288d561 100644 --- a/ap.h +++ b/ap.h @@ -14,11 +14,17 @@ // // Be clear about what that is worth: NOTHING as a security control. A logon // only reaches this package if the caller asks for it by package id, and every -// such logon fails anyway - denied, or declined with STATUS_NOT_IMPLEMENTED, -// because this package never builds a token. No real logon path is closed by -// loading it. It is here to show the LSA package interface and the logging, -// nothing more. The path that actually matters is the sub-authentication -// filter (subauth.cpp) or, on a real machine, the user rights. +// such logon fails anyway - denied with STATUS_ACCOUNT_RESTRICTION, or declined +// with STATUS_INVALID_LOGON_TYPE, because this package never builds a token. No +// real logon path is closed by loading it. It is here to show the LSA package +// interface and the logging, nothing more. The path that actually matters is +// the sub-authentication filter (subauth.cpp) or, on a real machine, the user +// rights. +// +// It does follow the lsass rules in lsasafe.h all the same (no heap or CRT of +// our own on the logon path, SEH around the decision), because the DLL is +// loaded into lsass at every boot once it is registered, and "it fails the +// logon anyway" is no comfort if it faults while doing so. // // WARNING: this code runs in lsass.exe. A bug here does not fail one tile - it // can crash LSA and leave the machine unbootable, past Safe Mode. Only ever diff --git a/build-lsa.bat b/build-lsa.bat index 4626530..9e4c0c0 100644 --- a/build-lsa.bat +++ b/build-lsa.bat @@ -18,19 +18,19 @@ cd /d "%~dp0" echo. echo === Building TacAuthPackage.dll (LSA authentication package, SKELETON) === cl /nologo /LD /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ - ap.cpp store.cpp totp.cpp ^ + ap.cpp store_index.cpp ^ /Fe:TacAuthPackage.dll ^ - /link /guard:cf /DEF:TacAuthPackage.def ^ - advapi32.lib crypt32.lib bcrypt.lib + /link /guard:cf /CETCOMPAT /DEF:TacAuthPackage.def ^ + advapi32.lib if errorlevel 1 goto :fail echo. echo === Building TacSubAuth.dll (MSV1_0 sub-authentication filter, SKELETON) === cl /nologo /LD /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ - subauth.cpp store.cpp totp.cpp ^ + subauth.cpp store_index.cpp ^ /Fe:TacSubAuth.dll ^ - /link /guard:cf /DEF:TacSubAuth.def ^ - advapi32.lib crypt32.lib bcrypt.lib + /link /guard:cf /CETCOMPAT /DEF:TacSubAuth.def ^ + advapi32.lib if errorlevel 1 goto :fail echo. diff --git a/build.bat b/build.bat index 61160ba..31f813c 100644 --- a/build.bat +++ b/build.bat @@ -9,7 +9,8 @@ REM REM Usage: build.bat REM /MT links the C++ runtime statically, so the target machine needs REM no Visual C++ Redistributable. /O2 optimizes, /guard:cf turns on Control -REM Flow Guard, /sdl adds extra runtime checks and turns a few dangerous +REM Flow Guard, /CETCOMPAT marks the binaries shadow-stack compatible, /sdl +REM adds extra runtime checks and turns a few dangerous REM warnings into errors - worth having in anything LogonUI or lsass loads. REM ===================================================================== @@ -20,38 +21,38 @@ echo. echo === Building TacProvider.dll (credential provider + filter) === cl /nologo /LD /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ dll.cpp CTacProvider.cpp CTacCredential.cpp CTacFilter.cpp ^ - helpers.cpp totp.cpp store.cpp verify.cpp statelock.cpp eventlog.cpp ^ + helpers.cpp totp.cpp store.cpp store_index.cpp verify.cpp statelock.cpp eventlog.cpp ^ /Fe:TacProvider.dll ^ - /link /guard:cf /DEF:TacProvider.def ^ + /link /guard:cf /CETCOMPAT /DEF:TacProvider.def ^ bcrypt.lib crypt32.lib advapi32.lib secur32.lib shlwapi.lib ole32.lib uuid.lib gdi32.lib user32.lib wtsapi32.lib if errorlevel 1 goto :fail echo. echo === Building enroll.exe (enrollment tool) === cl /nologo /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ - enroll.cpp totp.cpp store.cpp statelock.cpp ^ + enroll.cpp totp.cpp store.cpp store_index.cpp statelock.cpp ^ /Fe:enroll.exe ^ - /link /guard:cf bcrypt.lib crypt32.lib advapi32.lib + /link /guard:cf /CETCOMPAT bcrypt.lib crypt32.lib advapi32.lib if errorlevel 1 goto :fail echo. echo === Building TacAuthPackage.dll (LSA authentication package, SKELETON) === echo Runs in lsass.exe. Only load it in a throwaway VM with a snapshot. cl /nologo /LD /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ - ap.cpp store.cpp totp.cpp ^ + ap.cpp store_index.cpp ^ /Fe:TacAuthPackage.dll ^ - /link /guard:cf /DEF:TacAuthPackage.def ^ - advapi32.lib crypt32.lib bcrypt.lib + /link /guard:cf /CETCOMPAT /DEF:TacAuthPackage.def ^ + advapi32.lib if errorlevel 1 goto :fail echo. echo === Building TacSubAuth.dll (MSV1_0 sub-authentication filter, SKELETON) === echo Runs in lsass.exe. Only load it in a throwaway VM with a snapshot. cl /nologo /LD /MT /O2 /guard:cf /EHsc /std:c++17 /W4 /sdl /DUNICODE /D_UNICODE ^ - subauth.cpp store.cpp totp.cpp ^ + subauth.cpp store_index.cpp ^ /Fe:TacSubAuth.dll ^ - /link /guard:cf /DEF:TacSubAuth.def ^ - advapi32.lib crypt32.lib bcrypt.lib + /link /guard:cf /CETCOMPAT /DEF:TacSubAuth.def ^ + advapi32.lib if errorlevel 1 goto :fail echo. diff --git a/enroll.cpp b/enroll.cpp index ff49f46..ca897c2 100644 --- a/enroll.cpp +++ b/enroll.cpp @@ -2,6 +2,7 @@ #include "store.h" #include "statelock.h" #include +#include #include #include #include @@ -75,23 +76,63 @@ static int Unlock(const std::wstring& user, const std::wstring& sid) // entry, and that is the one that would otherwise keep denying network logons. static int Remove(const std::wstring& user, const std::wstring& sid) { + const std::wstring& shown = user.empty() ? sid : user; + if (sid.empty()) - wprintf(L"'%s' is not a local user account (anymore). Removing the name entry only.\n", + wprintf(L"'%s' is not a local user account (anymore). Removing the name entry only.\n" + L"For the secret of a deleted account, use the SID from 'enroll /list'.\n", user.c_str()); if (!tac::RemoveEnrollment(user, sid)) { wprintf(L"Could not remove the enrollment of '%s'. Run this from an elevated prompt.\n", - user.c_str()); + shown.c_str()); return 1; } - wprintf(L"Removed the enrollment of '%s'.\n", user.c_str()); + wprintf(L"Removed the enrollment of '%s'.\n", shown.c_str()); wprintf(L"The account can log on with its password alone again, unless you also\n"); wprintf(L"set the deny rights (deny-noninteractive.ps1).\n"); return 0; } +// Lists every stored secret, the account it belongs to and whether the two +// indexes the LSA packages read know about it. A secret whose SID no longer +// resolves belongs to a deleted account: it can never authenticate, but it +// still counts as "enrolled" for the filter, so remove it by SID. +static int List() +{ + std::vector entries; + if (!tac::ListEnrollments(entries)) + { + wprintf(L"Could not read the enrollments. Run this from an elevated prompt.\n"); + return 1; + } + if (entries.empty()) + { + wprintf(L"No account is enrolled. The filter hides nothing.\n"); + return 0; + } + + bool orphans = false; + for (const tac::EnrollmentEntry& e : entries) + { + const bool gone = e.name.empty(); + orphans = orphans || gone; + wprintf(L" %-20s %s RID index: %s name index: %s\n", + gone ? L"(deleted account)" : e.name.c_str(), e.sid.c_str(), + e.ridIndexed ? L"yes" : L"NO", + gone ? L"-" : (e.nameIndexed ? L"yes" : L"NO")); + } + + if (orphans) + { + wprintf(L"\nA deleted account still counts as enrolled, so the filter keeps hiding the\n"); + wprintf(L"other sign-in options. Remove it with: enroll /remove \n"); + } + return 0; +} + // Rebuilds the RID and name indexes the LSA packages read, from the stored // secrets. Needed once after updating from a version without the RID index, // and after renaming an enrolled account. @@ -175,39 +216,86 @@ static int Purge() return 0; } +// Parses a SID typed on the command line and returns it in the canonical form +// the store uses as value name ("S-1-5-21-..."). Stricter than the "S-1-" +// prefix check, so a local user who happens to be called "S-1-x" is not +// mistaken for one, and "s-1-5-..." in lower case still finds its entries. +static bool CanonicalSid(const wchar_t* text, std::wstring& sid) +{ + sid.clear(); + PSID psid = nullptr; + if (!ConvertStringSidToSidW(text, &psid)) + return false; + + PWSTR canonical = nullptr; + const BOOL ok = ConvertSidToStringSidW(psid, &canonical); + LocalFree(psid); + if (!ok) + return false; + + sid = canonical; + LocalFree(canonical); + return true; +} + int wmain(int argc, wchar_t** argv) { - if (argc == 2 && _wcsicmp(argv[1], L"/reindex") == 0) - return Reindex(); + if (argc == 2 && _wcsicmp(argv[1], L"/list") == 0) + return List(); if (argc == 2 && _wcsicmp(argv[1], L"/purge") == 0) return Purge(); - bool unlock = argc == 3 && _wcsicmp(argv[1], L"/unlock") == 0; - bool remove = argc == 3 && _wcsicmp(argv[1], L"/remove") == 0; - if ((argc != 2 && !unlock && !remove) || (argc == 2 && argv[1][0] == L'/')) + const bool reindex = argc == 2 && _wcsicmp(argv[1], L"/reindex") == 0; + const bool unlock = argc == 3 && _wcsicmp(argv[1], L"/unlock") == 0; + const bool remove = argc == 3 && _wcsicmp(argv[1], L"/remove") == 0; + if (!reindex && + ((argc != 2 && !unlock && !remove) || (argc == 2 && argv[1][0] == L'/'))) { wprintf(L"Usage: enroll \n"); wprintf(L" enroll /unlock \n"); - wprintf(L" enroll /remove \n"); + wprintf(L" enroll /remove \n"); + wprintf(L" enroll /list (enrolled accounts and index state)\n"); wprintf(L" enroll /reindex\n"); wprintf(L" enroll /purge (delete all 2FA data)\n"); return 1; } + // Repair the ACL of store keys an older version created with a weaker one. + // This used to happen on every state write, i.e. on every logon attempt; + // it needs WRITE_DAC and a hive write and belongs here instead. + if (!tac::EnsureStoreAcls()) + wprintf(L"Warning: could not re-apply the ACL to the existing 2FA registry keys.\n"); + + if (reindex) + return Reindex(); + + const wchar_t* arg = argv[(unlock || remove) ? 2 : 1]; + // Only names that exist as a local user account on this machine. The // secret is stored under the SID of that account. - std::wstring user = tac::NormalizeUser(argv[(unlock || remove) ? 2 : 1]); + std::wstring user = tac::NormalizeUser(arg); std::wstring sid; if (!tac::ResolveLocalUserSid(user, sid)) { // For /remove an unresolvable name is not fatal: the name-keyed index - // entry can and should still go. Everything else needs the SID. + // entry can and should still go. A SID (from `enroll /list`) removes + // the secret of an account that was already deleted. Everything else + // needs a live account. if (!remove) { wprintf(L"'%s' is not a local user account on this machine.\n", user.c_str()); return 1; } - sid.clear(); + if (CanonicalSid(arg, sid)) + user.clear(); + else + sid.clear(); + + if (user.empty() && sid.empty()) + { + wprintf(L"'%s' is neither a user name nor a SID.\n", arg); + return 1; + } } if (unlock) diff --git a/install-authpackage.ps1 b/install-authpackage.ps1 index 74c16d7..d7a02a9 100644 --- a/install-authpackage.ps1 +++ b/install-authpackage.ps1 @@ -29,6 +29,19 @@ if (($lsaProps.RunAsPPL -in 1, 2) -or ($lsaProps.RunAsPPLBoot -in 1, 2)) { throw "LSA protection (RunAsPPL) is on. lsass will not load the unsigned $dll.dll. Turn it off in the test VM first (and back on afterwards)." } +# The registry is not the whole story. Windows 11 22H2 and later turns LSA +# protection on by default on new, enterprise-joined, HVCI-capable devices, and +# Microsoft does not document that this shows up in RunAsPPL. What it does +# document is the proof at runtime: WinInit event 12, "LSASS.exe was started +# as a protected process with level: 4", in the System log. Event ID 12 is +# used by other sources too, hence the provider check. +$boot = (Get-CimInstance Win32_OperatingSystem).LastBootUpTime +$ppl = Get-WinEvent -FilterHashtable @{ LogName = 'System'; Id = 12; StartTime = $boot } -ErrorAction SilentlyContinue | + Where-Object { $_.ProviderName -like '*Wininit*' } | Select-Object -First 1 +if ($ppl) { + throw "lsass runs as a protected process since the last boot (WinInit event 12). It will not load the unsigned $dll.dll. Turn LSA protection off in the test VM first (and back on afterwards)." +} + $key = 'HKLM:\SYSTEM\CurrentControlSet\Control\Lsa' $name = 'Authentication Packages' diff --git a/install-doppio.ps1 b/install-doppio.ps1 index 92219b1..b1b0e88 100644 --- a/install-doppio.ps1 +++ b/install-doppio.ps1 @@ -1,17 +1,51 @@ #requires -RunAsAdministrator +<# +.SYNOPSIS + Installs the Doppio credential provider: copies the binaries to Program + Files and registers the provider. The filter that hides every other + sign-in option is a separate, deliberate second step. + +.DESCRIPTION + Step 1 (default): copy and register the provider. Nothing else changes on + the logon screen - the 2FA tile shows up under "Sign-in options", next to + the usual ones. + + Then enroll every account that signs in at the console or over RDP, + including your break-glass admin, and test each one with the 2FA tile. + + Step 2 (-EnableFilter): hide every other sign-in option. Refuses while an + enabled local account is not enrolled, because that account could not sign + in at all afterwards: its 2FA tile has no secret to check, and the tiles it + used before are gone. + +.PARAMETER EnableFilter + Register the filter (register-filter.reg) after checking that every enabled + local account is enrolled. + +.PARAMETER AllowUnenrolled + Local account names that may stay unenrolled with the filter on, because + they never sign in at the console or over RDP (service accounts, for + example). Each one is named explicitly; there is no switch to skip the check. + +.EXAMPLE + .\install-doppio.ps1 + & 'C:\Program Files\TheAdminCafe\enroll.exe' alice + & 'C:\Program Files\TheAdminCafe\enroll.exe' breakglass + # sign out, test both accounts with the 2FA tile, then: + .\install-doppio.ps1 -EnableFilter +#> +param( + [switch]$EnableFilter, + [string[]]$AllowUnenrolled = @() +) + $ErrorActionPreference = 'Stop' -# --------------------------------------------------------------------------- -# Installs the Doppio credential provider: copies the binaries to Program -# Files and registers the provider and its filter. -# -# Part 1 does this with `reg import register.reg`. This script exists because -# a .reg import cannot look before it leaps - it overwrites whatever is at -# those keys and tells you nothing. The registration itself is still the .reg -# files; this only checks first and copies in the right order. -# -# Belongs in the Doppio repository root, next to build.bat. -# --------------------------------------------------------------------------- +# Belongs in the Doppio repository root, next to build.bat. Part 1 does the +# same with `reg import register.reg`; this script exists because a .reg import +# cannot look before it leaps - it overwrites whatever is at those keys and +# tells you nothing. The registration itself is still the .reg files; this only +# checks first and copies in the right order. $dest = 'C:\Program Files\TheAdminCafe' @@ -29,23 +63,92 @@ if ($missing) { $optional = @('TacSubAuth.dll', 'TacAuthPackage.dll', 'register-filter.reg', 'unregister.reg', 'deny-noninteractive.ps1') -# --- is somebody else already at our CLSID? -------------------------------- -# register.reg carries the provider's CLSID. Read it out rather than hard-coding -# one, so this keeps working after new-guids.ps1 has been run. +$regFile = Join-Path $PSScriptRoot 'register.reg' +$filterReg = Join-Path $PSScriptRoot 'register-filter.reg' +$guidRegex = '\{[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}\}' + +# --- step 2: the filter ----------------------------------------------------- + +if ($EnableFilter) { + if (-not (Test-Path -LiteralPath $filterReg)) { + throw "register-filter.reg is missing next to this script." + } + if (-not (Test-Path -LiteralPath (Join-Path $dest 'TacProvider.dll'))) { + throw "Doppio is not installed in $dest yet. Run this script without -EnableFilter first." + } + + # Every stored secret is a REG_BINARY value named by the account's SID. + $store = 'HKLM:\SOFTWARE\TheAdminCafe\2FA' + $enrolled = @() + if (Test-Path $store) { + $item = Get-Item -Path $store + $enrolled = @($item.Property | Where-Object { + $_ -like 'S-1-*' -and $item.GetValueKind($_) -eq [Microsoft.Win32.RegistryValueKind]::Binary + }) + } + if ($enrolled.Count -eq 0) { + throw "No account is enrolled. Enroll every account that signs in at the console or over RDP first: & '$dest\enroll.exe' " + } + + $allowed = @($AllowUnenrolled | ForEach-Object { $_.ToLowerInvariant() }) + $locked = @(Get-LocalUser | Where-Object { + $_.Enabled -and + ($enrolled -notcontains $_.SID.Value) -and + ($allowed -notcontains $_.Name.ToLowerInvariant()) + }) + if ($locked.Count -gt 0) { + $names = ($locked | ForEach-Object { $_.Name }) -join ', ' + throw ("These enabled local accounts are not enrolled and could not sign in at the console or over RDP " + + "once the filter is on: $names. Enroll them, disable them, or - only for accounts that never sign " + + "in interactively - name them with -AllowUnenrolled.") + } -$regText = Get-Content -LiteralPath (Join-Path $PSScriptRoot 'register.reg') -Raw -$clsids = [regex]::Matches($regText, '\{[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}\}') | - ForEach-Object { $_.Value } | Select-Object -Unique + # Same look-before-you-leap as for the provider: never take a filter key + # that someone else's filter already holds. + $filterRoot = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Authentication\Credential Provider Filters' + $filterText = Get-Content -LiteralPath $filterReg -Raw + foreach ($clsid in ([regex]::Matches($filterText, $guidRegex) | ForEach-Object { $_.Value } | Select-Object -Unique)) { + $key = Join-Path $filterRoot $clsid + if (Test-Path $key) { + $owner = (Get-ItemProperty $key -ErrorAction SilentlyContinue).'(default)' + if ($owner -and $owner -notlike '*AdminCafe*' -and $owner -notlike '*Doppio*') { + throw "The filter CLSID $clsid is already registered to '$owner'. Run new-guids.ps1 to get your own CLSIDs." + } + } + } + $out = & reg.exe import $filterReg 2>&1 + if ($LASTEXITCODE -ne 0) { throw "reg import register-filter.reg failed: $out" } + + Write-Host "Registered the filter. The logon and unlock screens now show only the 2FA tiles." + Write-Host '' + Write-Host 'Before you sign out: know your way back. If no tile works, boot WinRE, load the' + Write-Host 'SOFTWARE hive offline and delete the filter key (see README). Practise it once.' + return +} + +# --- step 1: is somebody else already at our CLSIDs? ------------------------ +# register.reg carries the provider's and the filter's CLSIDs (the filter is +# COM-registered here too, only not activated). Read them out rather than +# hard-coding them, so this keeps working after new-guids.ps1 has been run, and +# check every place the import would write to. + +$regText = Get-Content -LiteralPath $regFile -Raw +$clsids = [regex]::Matches($regText, $guidRegex) | ForEach-Object { $_.Value } | Select-Object -Unique if (-not $clsids) { throw "No CLSID found in register.reg. Is it the right file?" } -$cpRoot = 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Authentication\Credential Providers' +$places = @( + 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Authentication\Credential Providers', + 'HKLM:\SOFTWARE\Classes\CLSID' +) foreach ($clsid in $clsids) { - $key = Join-Path $cpRoot $clsid - if (Test-Path $key) { - $owner = (Get-ItemProperty $key -ErrorAction SilentlyContinue).'(default)' - if ($owner -and $owner -notlike '*AdminCafe*' -and $owner -notlike '*Doppio*') { - throw "The credential provider CLSID $clsid is already registered to '$owner'. Run new-guids.ps1 to get your own CLSIDs rather than taking someone else's." + foreach ($root in $places) { + $key = Join-Path $root $clsid + if (Test-Path $key) { + $owner = (Get-ItemProperty $key -ErrorAction SilentlyContinue).'(default)' + if ($owner -and $owner -notlike '*AdminCafe*' -and $owner -notlike '*Doppio*') { + throw "CLSID $clsid is already registered to '$owner' under $root. Run new-guids.ps1 to get your own CLSIDs rather than taking someone else's." + } } } } @@ -71,21 +174,20 @@ foreach ($f in $required + $optional) { Write-Host "" Write-Host "Registering the credential provider..." -$out = & reg.exe import (Join-Path $PSScriptRoot 'register.reg') 2>&1 +$out = & reg.exe import $regFile 2>&1 if ($LASTEXITCODE -ne 0) { throw "reg import register.reg failed: $out" } -$filterReg = Join-Path $PSScriptRoot 'register-filter.reg' -if (Test-Path -LiteralPath $filterReg) { - $out = & reg.exe import $filterReg 2>&1 - if ($LASTEXITCODE -ne 0) { throw "reg import register-filter.reg failed: $out" } - Write-Host "Registered the provider filter (hides the other sign-in options)." -} - Write-Host "" -Write-Host "Installed to $dest." -Write-Host "Reboot, then enroll an account before you deny anything else:" -Write-Host " & '$dest\enroll.exe' " +Write-Host "Installed to $dest. The other sign-in options stay visible for now." +Write-Host "" +Write-Host "Next:" +Write-Host " 1. Enroll every account that signs in at the console or over RDP -" +Write-Host " your break-glass admin too:" +Write-Host " & '$dest\enroll.exe' " +Write-Host " 2. Sign out and sign in once with each of them through the 2FA tile" +Write-Host " (Sign-in options, the brown 2FA icon)." +Write-Host " 3. Only then hide the other sign-in options:" +Write-Host " .\install-doppio.ps1 -EnableFilter" Write-Host "" -Write-Host "Keep a second administrator account that is NOT enrolled until you have" -Write-Host "seen the new tile work. A broken tile with no way back in is the one" -Write-Host "failure this cannot talk you out of." +Write-Host "An account that is not enrolled cannot sign in at the console once the filter" +Write-Host "is on. Step 3 checks that for you; 'enroll.exe /list' shows who is enrolled." diff --git a/install-subauth.ps1 b/install-subauth.ps1 index a2f7633..2ccf1d8 100644 --- a/install-subauth.ps1 +++ b/install-subauth.ps1 @@ -1,13 +1,18 @@ # Registers the Doppio MSV1_0 sub-authentication FILTER (SKELETON). # # The filter is called by MSV1_0 AFTER it has validated a logon, and can only -# veto it. It runs INSIDE lsass.exe. A broken one can leave the machine -# unbootable, past Safe Mode. Take a VM snapshot first. Have your BitLocker -# recovery key and a way to edit the registry offline (WinRE) ready. +# veto it. It runs INSIDE lsass.exe. A broken one can crash lsass - at boot or +# at the first network logon, because MSV1_0 loads a sub-authentication DLL +# the first time it needs it - and leave the machine unbootable, past Safe +# Mode. Take a VM snapshot first. Have your BitLocker recovery key and a way to +# edit the registry offline (WinRE) ready. # # The filter is registered as "Auth0". The numbered slots Auth1..AuthN are a # different hook (Msv1_0SubAuthenticationRoutine) that has to do the whole -# password check itself - this project does not use them. See subauth.h. +# password check itself - this project does not use them. See subauth_filter.h. +# +# Workstations and member servers only. On a domain controller Auth0 sees +# domain accounts, whose RIDs this project's local index knows nothing about. # # Run from an elevated PowerShell. @@ -16,15 +21,33 @@ $dll = 'TacSubAuth' # base name, no extension $src = Join-Path $PSScriptRoot "$dll.dll" if (-not (Test-Path $src)) { throw "Build $dll.dll first (build.bat)." } +# DomainRole 4 = backup DC, 5 = primary DC. +if ((Get-CimInstance Win32_ComputerSystem).DomainRole -ge 4) { + throw "This is a domain controller. The filter's index only knows local accounts by RID; on a DC it would check domain RIDs against it. Refusing." +} + # With LSA protection (RunAsPPL) on, lsass only loads plug-ins with a Microsoft -# LSA signature. This unsigned DLL would be skipped at boot - silently, from the -# point of view of whoever expects it to protect something. Refuse instead. +# LSA signature. This unsigned DLL would be skipped - silently, from the point +# of view of whoever expects it to protect something. Refuse instead. $lsaKey = 'HKLM:\SYSTEM\CurrentControlSet\Control\Lsa' $lsaProps = Get-ItemProperty -Path $lsaKey if (($lsaProps.RunAsPPL -in 1, 2) -or ($lsaProps.RunAsPPLBoot -in 1, 2)) { throw "LSA protection (RunAsPPL) is on. lsass will not load the unsigned $dll.dll. Turn it off in the test VM first (and back on afterwards)." } +# The registry is not the whole story. Windows 11 22H2 and later turns LSA +# protection on by default on new, enterprise-joined, HVCI-capable devices, and +# Microsoft does not document that this shows up in RunAsPPL. What it does +# document is the proof at runtime: WinInit event 12, "LSASS.exe was started +# as a protected process with level: 4", in the System log. Event ID 12 is +# used by other sources too, hence the provider check. +$boot = (Get-CimInstance Win32_OperatingSystem).LastBootUpTime +$ppl = Get-WinEvent -FilterHashtable @{ LogName = 'System'; Id = 12; StartTime = $boot } -ErrorAction SilentlyContinue | + Where-Object { $_.ProviderName -like '*Wininit*' } | Select-Object -First 1 +if ($ppl) { + throw "lsass runs as a protected process since the last boot (WinInit event 12). It will not load the unsigned $dll.dll. Turn LSA protection off in the test VM first (and back on afterwards)." +} + $key = 'HKLM:\SYSTEM\CurrentControlSet\Control\Lsa\MSV1_0' if (-not (Test-Path $key)) { New-Item -Path $key -Force | Out-Null } $props = Get-ItemProperty -Path $key @@ -34,8 +57,9 @@ if ($props.Auth0 -and $props.Auth0 -ne $dll) { throw "Auth0 is already used by '$($props.Auth0)'. Refusing to replace it." } -# MSV1_0 loads sub-auth packages from System32. If lsass has the DLL loaded -# (an older version is registered), this fails: uninstall, reboot, then retry. +# MSV1_0 loads sub-auth packages from the default DLL path (System32). If lsass +# has the DLL loaded (an older version is registered), this fails: uninstall, +# reboot, then retry. try { Copy-Item $src "$env:windir\System32\$dll.dll" -Force } catch { @@ -44,6 +68,7 @@ try { # Older versions of this script registered the DLL under Auth1..Auth4 as a # sub-authentication ROUTINE. That export no longer exists - remove the entry. +# (Those numbers are also in the range Microsoft reserves for itself, 1-127.) 1..4 | ForEach-Object { if ($props."Auth$_" -eq $dll) { Remove-ItemProperty -Path $key -Name "Auth$_" @@ -62,6 +87,9 @@ Write-Host '' Write-Host 'Index existing enrollments by RID, which the filter reads:' Write-Host ' & "C:\Program Files\TheAdminCafe\enroll.exe" /reindex' Write-Host '' -Write-Host 'Reboot to load it. Refusals are logged (source "TheAdminCafe 2FA", event 210);' -Write-Host 'every call writes a [Doppio-SubAuth] OutputDebugString line for the kernel debugger.' +Write-Host 'Reboot. MSV1_0 loads the filter the first time it needs it, so the first' +Write-Host 'network logon after the reboot is the moment of truth, not only the boot itself.' +Write-Host 'Refusals are logged (source "TheAdminCafe 2FA", event 210); every call writes a' +Write-Host '[Doppio-SubAuth] OutputDebugString line for the kernel debugger, including the' +Write-Host 'LogoffTime and KickoffTime MSV1_0 handed in.' Write-Host 'To remove: run uninstall-subauth.ps1 and reboot.' diff --git a/lsasafe.h b/lsasafe.h new file mode 100644 index 0000000..1d802ce --- /dev/null +++ b/lsasafe.h @@ -0,0 +1,75 @@ +#pragma once +// +// Helpers for code that runs INSIDE lsass.exe (the authentication package in +// ap.cpp and the sub-authentication filter in subauth.cpp). The three rules, +// learned the hard way in Part 3 (Ristretto): +// +// * No heap and no CRT of our own on the decision path. std::wstring can +// throw std::bad_alloc; swprintf_s calls the invalid-parameter handler on +// overflow, and the default handler ends the process - here lsass, which +// is a bugcheck. Build log lines with Buf (it cuts instead) and format +// numbers without the CRT. (Windows APIs we call - the registry, +// OutputDebugString, the event log - allocate inside Windows; that is +// theirs to get right, and none of them throws into our code.) +// * SEH, not C++ try/catch. With /EHsc a catch (...) does not catch an +// access violation, and a bad pointer from the caller is the failure that +// actually matters here. The decision runs behind one __except whose +// filter is LsaDecisionExceptionFilter. +// +// Both files shared private copies of this before; keeping one copy means a +// fix lands in both at once. + +#include + +namespace tac +{ + // Bounded, CRT-free wide-string builder. Never writes past `cap`, always + // null-terminates, and simply stops appending when full (no overflow, no + // invalid-parameter handler, nothing that could end lsass). + class Buf + { + public: + Buf(WCHAR* p, SIZE_T cap) : p_(p), cap_(cap), n_(0) + { + if (cap_) p_[0] = L'\0'; + } + + void Str(const WCHAR* s) + { + if (!s) return; + while (*s && n_ + 1 < cap_) p_[n_++] = *s++; + if (cap_) p_[n_] = L'\0'; + } + + void U64(ULONGLONG v) + { + WCHAR t[21]; + int i = 0; + do { t[i++] = (WCHAR)(L'0' + (ULONG)(v % 10)); v /= 10; } while (v && i < 20); + while (i > 0 && n_ + 1 < cap_) p_[n_++] = t[--i]; + if (cap_) p_[n_] = L'\0'; + } + + private: + WCHAR* p_; + SIZE_T cap_; + SIZE_T n_; + }; + + // Which exceptions the __except around a decision swallows. A stack + // overflow is not ours to survive: the guard page is gone by the time we + // run. Everything else here means a bad pointer from the caller, and on + // that one a dead lsass is the worse outcome, so it is caught and the + // caller fails closed. + inline LONG LsaDecisionExceptionFilter(DWORD code) + { + switch (code) + { + case EXCEPTION_STACK_OVERFLOW: + case EXCEPTION_NONCONTINUABLE_EXCEPTION: + return EXCEPTION_CONTINUE_SEARCH; + default: + return EXCEPTION_EXECUTE_HANDLER; + } + } +} diff --git a/store.cpp b/store.cpp index 27789f3..681e876 100644 --- a/store.cpp +++ b/store.cpp @@ -1,20 +1,24 @@ #include "store.h" +#include "storekeys.h" #include "totp.h" #include #include -#include -#include #pragma comment(lib, "crypt32.lib") #pragma comment(lib, "advapi32.lib") +// The index reads that run inside lsass (EnrollmentByRid, EnrollmentByName, +// NormalizeUserInto) live in store_index.cpp, so the LSA DLLs do not have to +// link DPAPI and TOTP code they never use. Everything here runs in LogonUI or +// enroll.exe only. + namespace tac { - static const wchar_t kKeyPath[] = L"SOFTWARE\\TheAdminCafe\\2FA"; - static const wchar_t kStatePath[] = L"SOFTWARE\\TheAdminCafe\\2FA\\State"; - // Enrollment indexes for the LSA hot path (presence only, no secret). - static const wchar_t kNamePath[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Names"; - static const wchar_t kRidPath[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Rids"; + using keys::kSecrets; + using keys::kState; + using keys::kNames; + using keys::kRids; + using keys::kMaxNameCch; // SYSTEM and Administrators only. "P" blocks the inherited ACL of // HKLM\SOFTWARE, which would let every user read the key. @@ -22,13 +26,10 @@ namespace tac std::wstring NormalizeUser(const std::wstring& user) { - std::wstring name = user; - size_t pos = name.find_last_of(L'\\'); - if (pos != std::wstring::npos) - name = name.substr(pos + 1); - for (wchar_t& c : name) - c = static_cast(towlower(c)); - return name; + WCHAR buf[kMaxNameCch]; + if (!NormalizeUserInto(user.c_str(), user.size(), buf, ARRAYSIZE(buf))) + return std::wstring(); + return std::wstring(buf); } static bool Protect(const std::string& plain, std::vector& blob) @@ -99,7 +100,7 @@ namespace tac } // Only SIDs are used as value names. This keeps odd input out of the registry. - static bool IsSidString(const std::wstring& sid) + bool IsSidString(const std::wstring& sid) { return sid.size() > 4 && sid.compare(0, 4, L"S-1-") == 0; } @@ -148,8 +149,10 @@ namespace tac } // Creates or opens a key below HKLM with our ACL. The security attributes - // only apply when the key is created. An existing key might still carry a - // weaker ACL, so it is set again. + // only apply when the key is CREATED; an existing key keeps whatever ACL it + // has. Repairing one needs WRITE_DAC and a hive write, neither of which + // belongs on the logon path (SaveState runs there on every code attempt), + // so that is EnsureStoreAcls' job and enroll.exe calls it. static LSTATUS OpenProtectedKey(const wchar_t* path, HKEY* phKey) { PSECURITY_DESCRIPTOR psd = nullptr; @@ -158,22 +161,40 @@ namespace tac return static_cast(GetLastError()); SECURITY_ATTRIBUTES sa = { sizeof(sa), psd, FALSE }; - DWORD disposition = 0; LSTATUS status = RegCreateKeyExW(HKEY_LOCAL_MACHINE, path, 0, nullptr, 0, - KEY_SET_VALUE | KEY_QUERY_VALUE | WRITE_DAC, - &sa, phKey, &disposition); - if (status == ERROR_SUCCESS && disposition == REG_OPENED_EXISTING_KEY) + KEY_SET_VALUE | KEY_QUERY_VALUE, + &sa, phKey, nullptr); + LocalFree(psd); + return status; + } + + bool EnsureStoreAcls() + { + PSECURITY_DESCRIPTOR psd = nullptr; + if (!ConvertStringSecurityDescriptorToSecurityDescriptorW(kKeySddl, SDDL_REVISION_1, + &psd, nullptr)) + return false; + + bool ok = true; + const wchar_t* paths[] = { kSecrets, kState, kNames, kRids }; + for (const wchar_t* path : paths) { - status = RegSetKeySecurity(*phKey, DACL_SECURITY_INFORMATION, psd); + HKEY hKey = nullptr; + LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, path, 0, WRITE_DAC, &hKey); + if (status == ERROR_FILE_NOT_FOUND) + continue; // not there yet, nothing to repair if (status != ERROR_SUCCESS) { - RegCloseKey(*phKey); - *phKey = nullptr; + ok = false; + continue; } + if (RegSetKeySecurity(hKey, DACL_SECURITY_INFORMATION, psd) != ERROR_SUCCESS) + ok = false; + RegCloseKey(hKey); } LocalFree(psd); - return status; + return ok; } // Deletes one value and treats "was not there" as done. @@ -212,11 +233,48 @@ namespace tac DWORD rid = 0; if (RidFromSidString(sid, rid)) - ridOk = SetFlag(kRidPath, std::to_wstring(rid)); + ridOk = SetFlag(kRids, std::to_wstring(rid)); std::wstring name; if (NameFromSid(sid, name)) - nameOk = SetFlag(kNamePath, name); + nameOk = SetFlag(kNames, name); + } + + // Every REG_BINARY value under a SID name in the secret store. Shared by + // QueryAnyEnrollment, ListEnrollments and ReindexEnrollments so all three + // agree on what counts as an enrollment. + static bool CollectEnrolledSids(std::vector& sids) + { + sids.clear(); + + HKEY hKey = nullptr; + LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kSecrets, 0, KEY_QUERY_VALUE, &hKey); + if (status == ERROR_FILE_NOT_FOUND) + return true; // nothing enrolled yet + if (status != ERROR_SUCCESS) + return false; + + for (DWORD i = 0;; ++i) + { + WCHAR name[256]; + DWORD cch = ARRAYSIZE(name); + DWORD type = 0; + status = RegEnumValueW(hKey, i, name, &cch, nullptr, &type, nullptr, nullptr); + if (status == ERROR_NO_MORE_ITEMS) + break; + if (status == ERROR_MORE_DATA) + continue; // not one of ours, SIDs are short + if (status != ERROR_SUCCESS) + { + RegCloseKey(hKey); + return false; + } + if (type == REG_BINARY && IsSidString(std::wstring(name, cch))) + sids.emplace_back(name, cch); + } + + RegCloseKey(hKey); + return true; } bool StoreSecret(const std::wstring& sid, const std::string& base32Secret) @@ -240,7 +298,7 @@ namespace tac return false; HKEY hKey = nullptr; - LSTATUS status = OpenProtectedKey(kKeyPath, &hKey); + LSTATUS status = OpenProtectedKey(kSecrets, &hKey); if (status == ERROR_SUCCESS) { status = RegSetValueExW(hKey, sid.c_str(), 0, REG_BINARY, @@ -257,7 +315,7 @@ namespace tac return false; HKEY hKey = nullptr; - if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, kKeyPath, 0, KEY_QUERY_VALUE, &hKey) != ERROR_SUCCESS) + if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, kSecrets, 0, KEY_QUERY_VALUE, &hKey) != ERROR_SUCCESS) return false; DWORD type = 0; @@ -292,76 +350,6 @@ namespace tac return true; } - // The RID as the decimal value name of the Rids index, without the CRT. - // This runs inside lsass, where swprintf_s on an overflow would end the - // process. A DWORD has at most 10 digits, so out[12] always fits. - static void RidToValueName(DWORD rid, WCHAR (&out)[12]) - { - WCHAR digits[10]; - int n = 0; - do - { - digits[n++] = static_cast(L'0' + rid % 10); - rid /= 10; - } while (rid != 0 && n < 10); - - int i = 0; - while (n > 0) - out[i++] = digits[--n]; - out[i] = L'\0'; // i <= 10, so this is in bounds - } - - Enrollment EnrollmentByRid(DWORD rid) - { - if (rid == 0) - return Enrollment::No; - - // A single registry read, like IsEnrolledByName below. Only a value - // that is really not there counts as "not enrolled". Any other failure - // is Unknown: a filter that read it as "not enrolled" would wave an - // enrolled account through on one unexpected error. - HKEY hKey = nullptr; - LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kRidPath, 0, KEY_QUERY_VALUE, &hKey); - if (status == ERROR_FILE_NOT_FOUND) - return Enrollment::No; // no index on this machine at all - if (status != ERROR_SUCCESS) - return Enrollment::Unknown; - - WCHAR value[12]; - RidToValueName(rid, value); - status = RegQueryValueExW(hKey, value, nullptr, nullptr, nullptr, nullptr); - RegCloseKey(hKey); - - if (status == ERROR_SUCCESS) - return Enrollment::Yes; - if (status == ERROR_FILE_NOT_FOUND) - return Enrollment::No; - return Enrollment::Unknown; - } - - bool IsEnrolledByRid(DWORD rid) - { - return EnrollmentByRid(rid) == Enrollment::Yes; - } - - bool IsEnrolledByName(const std::wstring& user) - { - std::wstring name = NormalizeUser(user); - if (name.empty()) - return false; - - // A single registry read. No DPAPI, no LookupAccount* - nothing that - // re-enters LSA - so this is safe to call from inside lsass on the - // logon path. - HKEY hKey = nullptr; - if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, kNamePath, 0, KEY_QUERY_VALUE, &hKey) != ERROR_SUCCESS) - return false; - - LSTATUS status = RegQueryValueExW(hKey, name.c_str(), nullptr, nullptr, nullptr, nullptr); - RegCloseKey(hKey); - return status == ERROR_SUCCESS; - } - bool RemoveEnrollment(const std::wstring& user, const std::wstring& sid) { bool ok = true; @@ -371,7 +359,7 @@ namespace tac // account is harmless, the SAM never hands that RID out again. std::wstring typed = NormalizeUser(user); if (!typed.empty()) - ok = DeleteValue(kNamePath, typed) && ok; + ok = DeleteValue(kNames, typed) && ok; if (IsSidString(sid)) { @@ -381,14 +369,14 @@ namespace tac // remove the canonical one as well. Deleting a value twice is free. std::wstring canonical; if (NameFromSid(sid, canonical) && !canonical.empty() && canonical != typed) - ok = DeleteValue(kNamePath, canonical) && ok; + ok = DeleteValue(kNames, canonical) && ok; DWORD rid = 0; if (RidFromSidString(sid, rid)) - ok = DeleteValue(kRidPath, std::to_wstring(rid)) && ok; + ok = DeleteValue(kRids, std::to_wstring(rid)) && ok; - ok = DeleteValue(kKeyPath, sid) && ok; - ok = DeleteValue(kStatePath, sid) && ok; + ok = DeleteValue(kSecrets, sid) && ok; + ok = DeleteValue(kState, sid) && ok; } return ok; } @@ -396,20 +384,47 @@ namespace tac bool QueryAnyEnrollment(bool& any) { any = false; - HKEY hKey = nullptr; - LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kKeyPath, 0, KEY_QUERY_VALUE, &hKey); - if (status == ERROR_FILE_NOT_FOUND) - return true; - if (status != ERROR_SUCCESS) + + // Counts stored secrets, not values: a stray value of another type or + // name is not an enrollment. The credential provider's filter hides + // every other sign-in option as soon as this says yes, and it must not + // do that while nothing on the machine can actually authenticate. + std::vector sids; + if (!CollectEnrolledSids(sids)) return false; - DWORD values = 0; - status = RegQueryInfoKeyW(hKey, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, - &values, nullptr, nullptr, nullptr, nullptr); - RegCloseKey(hKey); - if (status != ERROR_SUCCESS) + any = !sids.empty(); + return true; + } + + bool ListEnrollments(std::vector& out) + { + out.clear(); + + std::vector sids; + if (!CollectEnrolledSids(sids)) return false; - any = values > 0; + + for (const std::wstring& sid : sids) + { + EnrollmentEntry e; + e.sid = sid; + e.rid = 0; + e.ridIndexed = false; + e.nameIndexed = false; + + if (RidFromSidString(sid, e.rid)) + e.ridIndexed = EnrollmentByRid(e.rid) == Enrollment::Yes; + + std::wstring name; + if (NameFromSid(sid, name)) + { + e.name = name; + e.nameIndexed = EnrollmentByName(name.c_str(), name.size()) == Enrollment::Yes; + } + + out.push_back(e); + } return true; } @@ -417,7 +432,7 @@ namespace tac { // HKLM\SOFTWARE\TheAdminCafe\2FA and everything below it, then the // vendor key if nothing else lives there. - LSTATUS status = RegDeleteTreeW(HKEY_LOCAL_MACHINE, kKeyPath); + LSTATUS status = RegDeleteTreeW(HKEY_LOCAL_MACHINE, kSecrets); if (status != ERROR_SUCCESS && status != ERROR_FILE_NOT_FOUND) return false; status = RegDeleteKeyW(HKEY_LOCAL_MACHINE, L"SOFTWARE\\TheAdminCafe"); @@ -428,35 +443,12 @@ namespace tac int ReindexEnrollments() { - HKEY hKey = nullptr; - LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kKeyPath, 0, KEY_QUERY_VALUE, &hKey); - if (status == ERROR_FILE_NOT_FOUND) - return 0; // nothing enrolled yet - if (status != ERROR_SUCCESS) - return -1; - - // Collect first, write afterwards: the index keys are subkeys of this - // one, and enumerating while writing elsewhere is simpler to reason about. + // Collect first, write afterwards: the index keys are subkeys of the + // secret store, and enumerating while writing elsewhere is simpler to + // reason about. std::vector sids; - for (DWORD i = 0;; ++i) - { - WCHAR name[256]; - DWORD cch = ARRAYSIZE(name); - DWORD type = 0; - status = RegEnumValueW(hKey, i, name, &cch, nullptr, &type, nullptr, nullptr); - if (status == ERROR_NO_MORE_ITEMS) - break; - if (status == ERROR_MORE_DATA) - continue; // not one of ours, SIDs are short - if (status != ERROR_SUCCESS) - { - RegCloseKey(hKey); - return -1; - } - if (type == REG_BINARY && IsSidString(name)) - sids.emplace_back(name, cch); - } - RegCloseKey(hKey); + if (!CollectEnrolledSids(sids)) + return -1; // The RID index is what the sub-authentication filter reads, so a // failure there is an error. A missing name is not: the account may @@ -480,7 +472,7 @@ namespace tac return false; HKEY hKey = nullptr; - LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kStatePath, 0, KEY_QUERY_VALUE, &hKey); + LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, kState, 0, KEY_QUERY_VALUE, &hKey); if (status == ERROR_FILE_NOT_FOUND) return true; // nobody has logged on yet if (status != ERROR_SUCCESS) @@ -508,7 +500,7 @@ namespace tac return false; HKEY hKey = nullptr; - LSTATUS status = OpenProtectedKey(kStatePath, &hKey); + LSTATUS status = OpenProtectedKey(kState, &hKey); if (status == ERROR_SUCCESS) { status = RegSetValueExW(hKey, sid.c_str(), 0, REG_BINARY, @@ -526,6 +518,6 @@ namespace tac { if (!IsSidString(sid)) return false; - return DeleteValue(kStatePath, sid); + return DeleteValue(kState, sid); } } diff --git a/store.h b/store.h index 2756103..1cad8f5 100644 --- a/store.h +++ b/store.h @@ -6,9 +6,16 @@ namespace tac { - // "PC\Alice", ".\alice" and "alice" all become "alice". + // "PC\Alice", ".\alice" and "alice" all become "alice". Case folding is + // locale-invariant, so non-ASCII names fold the same way everywhere. std::wstring NormalizeUser(const std::wstring& user); + // The same, for code inside lsass: `user` is counted (cch characters, it + // may also end early at a NUL), the result goes into a caller buffer, and + // nothing allocates. Returns false for an empty name or one that does not + // fit. Lives in store_index.cpp. + bool NormalizeUserInto(const WCHAR* user, SIZE_T cch, WCHAR* out, SIZE_T outCap); + // Looks up COMPUTERNAME\name and returns the SID string ("S-1-5-21-...") // if it is a local user account. Everything else returns false. bool ResolveLocalUserSid(const std::wstring& user, std::wstring& sid); @@ -17,38 +24,49 @@ namespace tac // Returns false for anything that is not a well-formed SID string. bool RidFromSidString(const std::wstring& sid, DWORD& rid); + // Is this a syntactically valid SID string ("S-1-...")? Lets enroll accept + // a SID where a name no longer resolves. + bool IsSidString(const std::wstring& sid); + // The secret is stored per SID, not per name. A renamed or re-created // account is a different account. StoreSecret also mirrors two enrollment // indexes for code that runs inside lsass: one keyed by RID (see - // IsEnrolledByRid) and one keyed by name (see IsEnrolledByName). + // EnrollmentByRid) and one keyed by name (see EnrollmentByName). bool StoreSecret(const std::wstring& sid, const std::string& base32Secret); bool LoadSecretKey(const std::wstring& sid, std::vector& key); // Hot-path enrollment checks for code that runs INSIDE lsass. Each is a // single registry read: no DPAPI decrypt and no account-name or SID lookup, // so nothing calls back into LSA and the logon path cannot deadlock. + // Neither allocates: a caller in lsass must not risk std::bad_alloc. // - // IsEnrolledByRid is the one to use whenever the caller has the account's + // Three outcomes, because a caller that VETOES must not read a failed read + // as "not enrolled" - that is the one wrong answer that lets an enrolled + // account through. A missing index key is No (nothing indexed on this + // machine at all); any other read error is Unknown. + enum class Enrollment { No, Yes, Unknown }; + + // EnrollmentByRid is the one to use whenever the caller has the account's // RID from the SAM (the MSV1_0 sub-authentication filter gets it in // USER_ALL_INFORMATION). A RID survives a rename and the SAM never hands // out a RID twice, so this index can neither miss a renamed account nor // match a different, re-created one. - // - // EnrollmentByRid is the same read with three outcomes, for callers that - // VETO: a read that fails is Unknown, not "not enrolled". A missing Rids - // key is No (nothing indexed on this machine). IsEnrolledByRid is - // EnrollmentByRid(rid) == Yes, so it reads every failure as "not enrolled". - enum class Enrollment { No, Yes, Unknown }; Enrollment EnrollmentByRid(DWORD rid); + + // IsEnrolledByRid is EnrollmentByRid(rid) == Yes, so it reads every failure + // as "not enrolled". Only for callers that do not veto. bool IsEnrolledByRid(DWORD rid); - // IsEnrolledByName is only for callers that have nothing but the name a - // client typed (the authentication package). It is weaker, and fails OPEN - // in one case: after an enrolled account is renamed, the index still holds - // the old name, so the new name reads as "not enrolled" until - // `enroll /reindex` runs. A re-created account with an old name reads as - // enrolled (more restrictive). Prefer IsEnrolledByRid. - bool IsEnrolledByName(const std::wstring& user); + // EnrollmentByName is only for callers that have nothing but the name a + // client typed (the authentication package). `name` is counted, not + // null-terminated, and is normalized into a fixed buffer here. + // + // It is weaker than the RID check and fails OPEN in one case: after an + // enrolled account is renamed, the index still holds the old name, so the + // new name reads as "not enrolled" until `enroll /reindex` runs. A + // re-created account with an old name reads as enrolled (more + // restrictive). Prefer EnrollmentByRid wherever the RID is available. + Enrollment EnrollmentByName(const WCHAR* name, SIZE_T cch); // Removes an enrollment completely: the DPAPI secret, the state and the RID // index entry under the SID, and the name-keyed index entry. Both halves are @@ -60,16 +78,41 @@ namespace tac // counts as removed. bool RemoveEnrollment(const std::wstring& user, const std::wstring& sid); - // Is at least one account enrolled? Counts the stored secrets (one value per - // SID). Returns false if the store exists but cannot be read; a store that - // does not exist yet is "readable, nothing enrolled". + // Is at least one account enrolled? Counts only stored secrets: REG_BINARY + // values whose name is a SID string. A stray value of another type or name + // does not count, because the credential provider's filter hides every + // other sign-in option as soon as this says yes - and it must not do that + // when nothing can actually authenticate. + // Returns false if the store exists but cannot be read; a store that does + // not exist yet is "readable, nothing enrolled". bool QueryAnyEnrollment(bool& any); + // One enrolled account, for `enroll /list`. + struct EnrollmentEntry + { + std::wstring sid; + std::wstring name; // empty when the SID no longer resolves + DWORD rid; + bool ridIndexed; + bool nameIndexed; + }; + + // Every stored secret with its index state. Returns false if the store + // cannot be read; an empty store is "readable, nothing enrolled". + bool ListEnrollments(std::vector& out); + // Removes everything this project keeps in the registry: secrets, state // and both indexes. For a full uninstall. A tree that is not there counts // as removed. bool PurgeAllEnrollments(); + // Re-applies the "SYSTEM and Administrators only" ACL to the store keys + // that already exist. New keys get it when they are created; this repairs + // keys left over from an older version with a weaker ACL. Called by + // enroll.exe, never from the logon path - it needs WRITE_DAC and writes to + // the hive, and neither belongs in a logon. + bool EnsureStoreAcls(); + // Rebuilds the RID and name indexes from the stored secrets. For accounts // that were enrolled before the RID index existed, and after a rename. // Stale name entries of renamed accounts are left alone (they only make the diff --git a/store_index.cpp b/store_index.cpp new file mode 100644 index 0000000..fb1cd58 --- /dev/null +++ b/store_index.cpp @@ -0,0 +1,126 @@ +// The enrollment-index reads that run INSIDE lsass: the sub-authentication +// filter and the authentication package link this file and nothing else from +// the store. No DPAPI, no TOTP, no name or SID lookup, no heap and no CRT of +// our own - see lsasafe.h for why each of those matters in lsass. +// +// Everything that writes the indexes, or needs DPAPI, lives in store.cpp and +// only ever runs in LogonUI or enroll.exe. + +#include "store.h" +#include "storekeys.h" + +#pragma comment(lib, "advapi32.lib") + +namespace tac +{ + bool NormalizeUserInto(const WCHAR* user, SIZE_T cch, WCHAR* out, SIZE_T outCap) + { + if (!user || !out || outCap == 0) + return false; + out[0] = L'\0'; + + // Stop at an embedded NUL: nothing after it belongs to the name. + SIZE_T len = 0; + while (len < cch && user[len] != L'\0') + ++len; + + // Keep only what follows the last backslash. + SIZE_T start = 0; + for (SIZE_T i = 0; i < len; ++i) + if (user[i] == L'\\') + start = i + 1; + + const SIZE_T bare = len - start; + if (bare == 0 || bare >= outCap) + return false; + + // LOCALE_INVARIANT, so "JÖRG" and "jörg" fold the same way on every + // machine. Plain towlower only ever folded ASCII, which made the name + // index miss non-ASCII names. + const int written = LCMapStringW(LOCALE_INVARIANT, LCMAP_LOWERCASE, + user + start, static_cast(bare), + out, static_cast(outCap - 1)); + if (written > 0 && static_cast(written) < outCap) + { + out[written] = L'\0'; + return true; + } + + // NLS unavailable or the mapping did not fit: fold ASCII only, which + // is what every name used to get. + for (SIZE_T i = 0; i < bare; ++i) + { + const WCHAR c = user[start + i]; + out[i] = (c >= L'A' && c <= L'Z') ? static_cast(c - L'A' + L'a') : c; + } + out[bare] = L'\0'; + return true; + } + + // The RID as the decimal value name of the Rids index, without the CRT. + // This runs inside lsass, where swprintf_s on an overflow would end the + // process. A DWORD has at most 10 digits, so out[12] always fits. + static void RidToValueName(DWORD rid, WCHAR (&out)[12]) + { + WCHAR digits[10]; + int n = 0; + do + { + digits[n++] = static_cast(L'0' + rid % 10); + rid /= 10; + } while (rid != 0 && n < 10); + + int i = 0; + while (n > 0) + out[i++] = digits[--n]; + out[i] = L'\0'; // i <= 10, so this is in bounds + } + + // One registry read of an index key, with the three outcomes a vetoing + // caller needs. + static Enrollment QueryIndex(const wchar_t* path, const WCHAR* value) + { + HKEY hKey = nullptr; + LSTATUS status = RegOpenKeyExW(HKEY_LOCAL_MACHINE, path, 0, KEY_QUERY_VALUE, &hKey); + if (status == ERROR_FILE_NOT_FOUND) + return Enrollment::No; // no index on this machine at all + if (status != ERROR_SUCCESS) + return Enrollment::Unknown; + + status = RegQueryValueExW(hKey, value, nullptr, nullptr, nullptr, nullptr); + RegCloseKey(hKey); + + if (status == ERROR_SUCCESS) + return Enrollment::Yes; + if (status == ERROR_FILE_NOT_FOUND) + return Enrollment::No; + // Only a value that is really not there counts as "not enrolled". Any + // other failure is Unknown: a filter that read it as "not enrolled" + // would wave an enrolled account through on one unexpected error. + return Enrollment::Unknown; + } + + Enrollment EnrollmentByRid(DWORD rid) + { + if (rid == 0) + return Enrollment::No; + + WCHAR value[12]; + RidToValueName(rid, value); + return QueryIndex(keys::kRids, value); + } + + bool IsEnrolledByRid(DWORD rid) + { + return EnrollmentByRid(rid) == Enrollment::Yes; + } + + Enrollment EnrollmentByName(const WCHAR* name, SIZE_T cch) + { + WCHAR normalized[keys::kMaxNameCch]; + if (!NormalizeUserInto(name, cch, normalized, ARRAYSIZE(normalized))) + return Enrollment::Unknown; // unusable name: never claim "No" + + return QueryIndex(keys::kNames, normalized); + } +} diff --git a/storekeys.h b/storekeys.h new file mode 100644 index 0000000..b715471 --- /dev/null +++ b/storekeys.h @@ -0,0 +1,20 @@ +#pragma once +// Registry layout of the 2FA store. Internal to store.cpp and store_index.cpp. +#include + +namespace tac +{ + namespace keys + { + // Secrets: one REG_BINARY value per SID, DPAPI-protected. + static const wchar_t kSecrets[] = L"SOFTWARE\\TheAdminCafe\\2FA"; + // Replay and lockout state: one UserState per SID. + static const wchar_t kState[] = L"SOFTWARE\\TheAdminCafe\\2FA\\State"; + // Enrollment indexes for the LSA hot path (presence only, no secret). + static const wchar_t kNames[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Names"; + static const wchar_t kRids[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Rids"; + + // UNLEN + 1. Nothing the SAM stores comes close. + static const SIZE_T kMaxNameCch = 257; + } +} diff --git a/subauth.cpp b/subauth.cpp index 1d90b26..6887d6a 100644 --- a/subauth.cpp +++ b/subauth.cpp @@ -1,4 +1,5 @@ #include "subauth_filter.h" +#include "lsasafe.h" #include "store.h" #pragma comment(lib, "advapi32.lib") @@ -6,57 +7,27 @@ #ifndef STATUS_SUCCESS #define STATUS_SUCCESS ((NTSTATUS)0x00000000L) #endif -#ifndef STATUS_ACCOUNT_RESTRICTION -#define STATUS_ACCOUNT_RESTRICTION ((NTSTATUS)0xC000006EL) +// The documentation of Msv1_0SubAuthenticationFilter lists the values this +// function "must return". STATUS_ACCOUNT_RESTRICTION, which this filter used +// to veto with, is not among them; STATUS_INVALID_WORKSTATION is, and it says +// what we mean: this account may not log on over this path. +#ifndef STATUS_INVALID_WORKSTATION +#define STATUS_INVALID_WORKSTATION ((NTSTATUS)0xC0000070L) #endif -// Everything in this file runs inside lsass.exe, on MSV1_0's logon path. The -// rules, three of them learned the hard way in Part 3 (Ristretto): +#ifndef MSV1_0_PASSTHRU +#define MSV1_0_PASSTHRU 0x01 +#endif + +// Everything in this file runs inside lsass.exe, on MSV1_0's logon path, under +// the rules in lsasafe.h: no heap and no CRT of our own on the decision path, +// and SEH rather than C++ try/catch. On top of those: // -// * No heap: fixed stack buffers, no std::wstring, so no std::bad_alloc. -// * No CRT on the decision path. swprintf_s calls the invalid-parameter -// handler on overflow, and the default handler ends the process - here -// lsass, which is a bugcheck. Log lines are built with Buf, which cuts -// instead. -// * SEH, not C++ try/catch. With /EHsc a catch (...) does not catch an -// access violation, and a bad pointer from the caller is the failure that -// actually matters here. The decision runs behind one __except. // * A failed enrollment read refuses the logon. In a filter whose only job // is the veto, "could not tell" must never come out as "not enrolled". namespace { - // ---- bounded, CRT-free string building --------------------------- - class Buf - { - public: - Buf(WCHAR* p, SIZE_T cap) : p_(p), cap_(cap), n_(0) - { - if (cap_) p_[0] = L'\0'; - } - - void Str(const WCHAR* s) - { - if (!s) return; - while (*s && n_ + 1 < cap_) p_[n_++] = *s++; - if (cap_) p_[n_] = L'\0'; - } - - void U64(ULONGLONG v) - { - WCHAR t[21]; - int i = 0; - do { t[i++] = (WCHAR)(L'0' + (ULONG)(v % 10)); v /= 10; } while (v && i < 20); - while (i > 0 && n_ + 1 < cap_) p_[n_++] = t[--i]; - if (cap_) p_[n_] = L'\0'; - } - - private: - WCHAR* p_; - SIZE_T cap_; - SIZE_T n_; - }; - // The SAM account name, for the log only. The decision runs on the RID. void SamName(const USER_ALL_INFORMATION* ua, WCHAR (&out)[129]) { @@ -80,7 +51,7 @@ namespace out[cch] = L'\0'; } - enum class Verdict { NoObjection, Refused, Unreadable }; + enum class Verdict { NoObjection, Refused, Unreadable, NotOurs }; // Debug output for every decision; the event log only for refusals. The // filter sees every MSV1_0 logon it is called for, so logging approvals to @@ -88,7 +59,7 @@ namespace void Report(Verdict v, ULONG rid, const WCHAR* name, bool isNetwork) { WCHAR text[256]; - Buf b(text, ARRAYSIZE(text)); + tac::Buf b(text, ARRAYSIZE(text)); switch (v) { case Verdict::Refused: @@ -105,6 +76,13 @@ namespace b.U64(rid); b.Str(L"): the enrollment index could not be read."); break; + case Verdict::NotOurs: + b.Str(L"No objection: pass-through logon for '"); + b.Str(name); + b.Str(L"' (RID "); + b.U64(rid); + b.Str(L"), the account does not live on this machine."); + break; default: b.Str(L"No objection to a "); b.Str(isNetwork ? L"network" : L"non-network"); @@ -116,14 +94,14 @@ namespace break; } - WCHAR debug[288]; - Buf d(debug, ARRAYSIZE(debug)); + WCHAR debug[352]; + tac::Buf d(debug, ARRAYSIZE(debug)); d.Str(L"[Doppio-SubAuth] "); d.Str(text); d.Str(L"\n"); OutputDebugStringW(debug); - if (v == Verdict::NoObjection) + if (v == Verdict::NoObjection || v == Verdict::NotOurs) return; HANDLE h = RegisterEventSourceW(nullptr, L"TheAdminCafe 2FA"); if (h) @@ -134,25 +112,26 @@ namespace } } - // Which exceptions the __except below swallows. A stack overflow is not - // ours to survive: the guard page is gone by the time the filter runs. - // Everything else here means a bad pointer from the caller, and on that - // one a dead lsass is the worse outcome, so it is caught. - LONG DecisionFilter(DWORD code) + // The logon hours MSV1_0 passed in, so the VM test can settle what the + // filter is actually handed in LogoffTime and KickoffTime. The + // documentation calls both [out] and does not say what they hold on the + // way in; this filter leaves them alone either way (see the export below), + // and the debugger line is how you check that assumption. + void ReportTimes(const LARGE_INTEGER* logoff, const LARGE_INTEGER* kickoff) { - switch (code) - { - case EXCEPTION_STACK_OVERFLOW: - case EXCEPTION_NONCONTINUABLE_EXCEPTION: - return EXCEPTION_CONTINUE_SEARCH; - default: - return EXCEPTION_EXECUTE_HANDLER; - } + WCHAR text[160]; + tac::Buf b(text, ARRAYSIZE(text)); + b.Str(L"[Doppio-SubAuth] incoming LogoffTime="); + if (logoff) b.U64(static_cast(logoff->QuadPart)); else b.Str(L"(null)"); + b.Str(L" KickoffTime="); + if (kickoff) b.U64(static_cast(kickoff->QuadPart)); else b.Str(L"(null)"); + b.Str(L"\n"); + OutputDebugStringW(text); } // The decision. Split out of the export so the __try frame there holds // nothing that needs unwinding (MSVC error C2712 otherwise). - NTSTATUS Decide(const USER_ALL_INFORMATION* ua, bool isNetwork) + NTSTATUS Decide(const USER_ALL_INFORMATION* ua, bool isNetwork, bool passthru) { // No SAM record, no decision: MSV1_0 has already validated the logon, // so "no objection" does not let anything through that MSV1_0 refused. @@ -169,6 +148,15 @@ namespace return STATUS_SUCCESS; } + // MSV1_0_PASSTHRU means "the user is not connecting to this machine". + // Our index only knows local accounts, and a RID from somewhere else + // could collide with a local one, so we have nothing to say. + if (passthru) + { + Report(Verdict::NotOurs, rid, name, true); + return STATUS_SUCCESS; + } + // A single registry read - no DPAPI, no LSA name/SID lookup - so // nothing here re-enters LSA from inside MSV1_0. switch (tac::EnrollmentByRid(rid)) @@ -182,12 +170,12 @@ namespace // enrolled" - that is the one wrong answer that lets an enrolled // account through. Report(Verdict::Unreadable, rid, name, true); - return STATUS_ACCOUNT_RESTRICTION; + return STATUS_INVALID_WORKSTATION; case tac::Enrollment::Yes: default: Report(Verdict::Refused, rid, name, true); - return STATUS_ACCOUNT_RESTRICTION; + return STATUS_INVALID_WORKSTATION; } } } @@ -195,10 +183,20 @@ namespace // MSV1_0 calls this AFTER it has validated the logon. We only add a veto; // password check, account restrictions and token stay with MSV1_0. // -// The output parameters are left as MSV1_0 passed them in, except WhichFields -// (nothing to write back to the SAM) and Authoritative on a refusal. In -// particular LogoffTime and KickoffTime are NOT overwritten: they carry the -// account's logon hours, and a filter has no business extending them. +// The documentation calls WhichFields, UserFlags, Authoritative, LogoffTime and +// KickoffTime [out], so we write the three we can answer for: +// +// * WhichFields = 0: nothing to write back to the SAM. +// * UserFlags = 0: we claim neither LOGON_GUEST nor LOGON_NOENCRYPTION. +// * Authoritative = TRUE: the documentation says this "should return valid +// information regardless of the return value", and our answer is final on +// every path - there is no other domain controller that would decide +// differently about a local account. +// +// LogoffTime and KickoffTime are deliberately NOT written: they carry the +// account's logon hours, and a filter has no business extending them. What +// MSV1_0 hands us there is undocumented, so every call logs the incoming +// values for the VM test (ReportTimes above). // // IMPORTANT: we read ONLY the RID and the account name. We never touch the // password hashes in UserAll or the challenge-response in LogonInformation. @@ -207,19 +205,26 @@ namespace NTSTATUS NTAPI Msv1_0SubAuthenticationFilter( NETLOGON_LOGON_INFO_CLASS LogonLevel, PVOID /*LogonInformation*/, - ULONG /*Flags*/, + ULONG Flags, PUSER_ALL_INFORMATION UserAll, PULONG WhichFields, - PULONG /*UserFlags*/, + PULONG UserFlags, PBOOLEAN Authoritative, - PLARGE_INTEGER /*LogoffTime*/, - PLARGE_INTEGER /*KickoffTime*/) + PLARGE_INTEGER LogoffTime, + PLARGE_INTEGER KickoffTime) { if (WhichFields) *WhichFields = 0; + if (UserFlags) + *UserFlags = 0; + if (Authoritative) + *Authoritative = TRUE; const bool isNetwork = (LogonLevel == NetlogonNetworkInformation || LogonLevel == NetlogonNetworkTransitiveInformation); + const bool passthru = (Flags & MSV1_0_PASSTHRU) != 0; + + ReportTimes(LogoffTime, KickoffTime); NTSTATUS result; @@ -229,17 +234,15 @@ NTSTATUS NTAPI Msv1_0SubAuthenticationFilter( // state to unwind. __try { - result = Decide(UserAll, isNetwork); + result = Decide(UserAll, isNetwork, passthru); } - __except (DecisionFilter(GetExceptionCode())) + __except (tac::LsaDecisionExceptionFilter(GetExceptionCode())) { OutputDebugStringW(L"[Doppio-SubAuth] Exception while deciding a logon - " L"refused if it was a network logon (fail closed).\n"); - result = isNetwork ? STATUS_ACCOUNT_RESTRICTION : STATUS_SUCCESS; + result = (isNetwork && !passthru) ? STATUS_INVALID_WORKSTATION : STATUS_SUCCESS; } - if (result != STATUS_SUCCESS && Authoritative) - *Authoritative = TRUE; // the refusal is final return result; } diff --git a/subauth_filter.h b/subauth_filter.h index 2a3fd47..7e29c88 100644 --- a/subauth_filter.h +++ b/subauth_filter.h @@ -24,7 +24,13 @@ // // Policy of this skeleton: refuse a NETWORK logon for an enrolled account (no UI // to type a code there), and raise no objection to anything else. A network -// logon whose enrollment cannot be read is refused too (fail closed). +// logon whose enrollment cannot be read is refused too (fail closed). A +// pass-through logon (MSV1_0_PASSTHRU: the account does not live on this +// machine) gets no objection either, because the index only knows local RIDs. +// +// The veto is STATUS_INVALID_WORKSTATION, which is one of the values the +// documentation lists for this function. Earlier versions returned +// STATUS_ACCOUNT_RESTRICTION, which is not in that list. // // The account is identified by the RID that MSV1_0 read from the SAM // (USER_ALL_INFORMATION::UserId), not by the name the client sent. The RID diff --git a/tests/native/Makefile b/tests/native/Makefile index e13f589..fa0f13a 100644 --- a/tests/native/Makefile +++ b/tests/native/Makefile @@ -1,15 +1,28 @@ -# Native tests of totp.cpp and verify.cpp on Linux (g++ and OpenSSL). +# Native tests on Linux (g++), no Windows needed: +# test_verify - the real totp.cpp and verify.cpp, against OpenSSL and an +# in-memory store. +# test_lsa - the real subauth.cpp, ap.cpp and store_index.cpp (the code +# that runs inside lsass), against a fake registry, event log +# and LSA heap, under AddressSanitizer and UBSan. # make -C tests/native ROOT := ../.. CXXFLAGS := -std=c++17 -Wall -Wextra -Wno-unknown-pragmas -Ishim -I$(ROOT) -I. -SOURCES := test_verify.cpp fakes.cpp $(ROOT)/totp.cpp $(ROOT)/verify.cpp +SANITIZE := -g -fsanitize=address,undefined -fno-sanitize-recover=all + +VERIFY_SOURCES := test_verify.cpp fakes.cpp $(ROOT)/totp.cpp $(ROOT)/verify.cpp +LSA_SOURCES := test_lsa.cpp lsa_fakes.cpp $(ROOT)/subauth.cpp $(ROOT)/ap.cpp $(ROOT)/store_index.cpp +SHIMS := $(wildcard shim/*.h) .PHONY: test clean -test: test_verify +test: test_verify test_lsa ./test_verify + ./test_lsa + +test_verify: $(VERIFY_SOURCES) fakes.h $(SHIMS) + $(CXX) $(CXXFLAGS) $(VERIFY_SOURCES) -lcrypto -o $@ -test_verify: $(SOURCES) fakes.h shim/windows.h shim/bcrypt.h - $(CXX) $(CXXFLAGS) $(SOURCES) -lcrypto -o $@ +test_lsa: $(LSA_SOURCES) lsa_fakes.h $(SHIMS) $(ROOT)/lsasafe.h $(ROOT)/store.h $(ROOT)/storekeys.h + $(CXX) $(CXXFLAGS) $(SANITIZE) $(LSA_SOURCES) -o $@ clean: - rm -f test_verify + rm -f test_verify test_lsa diff --git a/tests/native/lsa_fakes.cpp b/tests/native/lsa_fakes.cpp new file mode 100644 index 0000000..f98e7d3 --- /dev/null +++ b/tests/native/lsa_fakes.cpp @@ -0,0 +1,136 @@ +// In-memory replacements for the handful of Windows APIs the lsass code calls. +#include "lsa_fakes.h" +#include +#include + +namespace fake +{ + std::map> keys; + + LSTATUS openStatus = ERROR_SUCCESS; + LSTATUS queryStatus = ERROR_SUCCESS; + bool throwOnOpen = false; + bool failLcMap = false; + int allocBudget = -1; + + std::vector debug; + std::vector events; + int opened = 0, closed = 0; + int lsaAllocs = 0, lsaFrees = 0; + + // Open handles: index + 1 -> key path. + static std::vector handles; + + static PVOID NTAPI Alloc(ULONG cb) + { + if (allocBudget == 0) + return nullptr; + if (allocBudget > 0) + --allocBudget; + ++lsaAllocs; + return std::calloc(1, cb ? cb : 1); + } + + static VOID NTAPI Free(PVOID p) + { + if (p) + ++lsaFrees; + std::free(p); + } + + LSA_DISPATCH_TABLE lsa = { &Alloc, &Free }; + + void Reset() + { + keys.clear(); + openStatus = ERROR_SUCCESS; + queryStatus = ERROR_SUCCESS; + throwOnOpen = false; + failLcMap = false; + allocBudget = -1; + debug.clear(); + events.clear(); + handles.clear(); + opened = closed = 0; + lsaAllocs = lsaFrees = 0; + } + + bool DebugContains(const wchar_t* text) + { + for (const std::wstring& line : debug) + if (line.find(text) != std::wstring::npos) + return true; + return false; + } +} + +int LCMapStringW(LCID, DWORD, LPCWSTR src, int cchSrc, LPWSTR dst, int cchDst) +{ + if (fake::failLcMap || cchSrc <= 0 || cchSrc > cchDst) + return 0; + for (int i = 0; i < cchSrc; ++i) + dst[i] = static_cast(std::towlower(static_cast(src[i]))); + return cchSrc; +} + +LSTATUS RegOpenKeyExW(HKEY root, LPCWSTR path, DWORD, REGSAM, PHKEY out) +{ + *out = nullptr; + if (fake::throwOnOpen) + throw 1; + if (root != HKEY_LOCAL_MACHINE || !path) + return ERROR_FILE_NOT_FOUND; + if (fake::keys.find(path) == fake::keys.end()) + return ERROR_FILE_NOT_FOUND; + if (fake::openStatus != ERROR_SUCCESS) + return fake::openStatus; + + fake::handles.push_back(path); + ++fake::opened; + *out = reinterpret_cast(static_cast(fake::handles.size())); + return ERROR_SUCCESS; +} + +LSTATUS RegQueryValueExW(HKEY h, LPCWSTR value, LPDWORD, LPDWORD, LPBYTE, LPDWORD) +{ + if (fake::queryStatus != ERROR_SUCCESS) + return fake::queryStatus; + + const size_t index = static_cast(reinterpret_cast(h)); + if (index == 0 || index > fake::handles.size()) + return ERROR_FILE_NOT_FOUND; + const auto& values = fake::keys[fake::handles[index - 1]]; + return values.count(value) ? ERROR_SUCCESS : ERROR_FILE_NOT_FOUND; +} + +LSTATUS RegCloseKey(HKEY) +{ + ++fake::closed; + return ERROR_SUCCESS; +} + +void OutputDebugStringW(LPCWSTR text) +{ + fake::debug.emplace_back(text ? text : L""); +} + +HANDLE RegisterEventSourceW(LPCWSTR, LPCWSTR) +{ + return reinterpret_cast(static_cast(1)); +} + +BOOL ReportEventW(HANDLE, WORD, WORD, DWORD id, PSID, WORD, DWORD, LPCWSTR*, LPVOID) +{ + fake::events.push_back(id); + return TRUE; +} + +BOOL DeregisterEventSource(HANDLE) +{ + return TRUE; +} + +BOOL DisableThreadLibraryCalls(HMODULE) +{ + return TRUE; +} diff --git a/tests/native/lsa_fakes.h b/tests/native/lsa_fakes.h new file mode 100644 index 0000000..3063faa --- /dev/null +++ b/tests/native/lsa_fakes.h @@ -0,0 +1,38 @@ +#pragma once +// A fake registry, event log and LSA heap, so the real ap.cpp, subauth.cpp and +// store_index.cpp can run on Linux. See lsa_fakes.cpp. +#include +#include +#include +#include +#include +#include + +namespace fake +{ + // Registry: key path (as the code passes it) -> value names present. + extern std::map> keys; + + // Failure injection. + extern LSTATUS openStatus; // returned by RegOpenKeyExW for keys that exist + extern LSTATUS queryStatus; // returned by RegQueryValueExW + extern bool throwOnOpen; // stand-in for an access violation; thrown + // before a handle exists, like a fault in + // the caller's buffers would be + extern bool failLcMap; // LCMapStringW returns 0 + extern int allocBudget; // LSA heap blocks left before AllocateLsaHeap + // returns null; -1 = unlimited + + // What the code under test left behind. + extern std::vector debug; // OutputDebugStringW lines + extern std::vector events; // ReportEventW ids + extern int opened, closed; // registry handles + extern int lsaAllocs, lsaFrees; // LSA heap blocks + + extern LSA_DISPATCH_TABLE lsa; + + // Clears everything above except the dispatch table. + void Reset(); + + bool DebugContains(const wchar_t* text); +} diff --git a/tests/native/shim/ntsecapi.h b/tests/native/shim/ntsecapi.h new file mode 100644 index 0000000..99e68fd --- /dev/null +++ b/tests/native/shim/ntsecapi.h @@ -0,0 +1,51 @@ +// Stand-in for : the LSA types ap.cpp and subauth.cpp use. +#pragma once +#include "windows.h" + +typedef struct _UNICODE_STRING +{ + USHORT Length; // bytes, without a terminating NUL + USHORT MaximumLength; // bytes + PWSTR Buffer; +} UNICODE_STRING, *PUNICODE_STRING; + +typedef struct _LSA_STRING +{ + USHORT Length; + USHORT MaximumLength; + PCHAR Buffer; +} LSA_STRING, *PLSA_STRING; + +typedef enum _SECURITY_LOGON_TYPE +{ + UndefinedLogonType = 0, + Interactive = 2, + Network, + Batch, + Service, + Proxy, + Unlock, + NetworkCleartext, + NewCredentials, + RemoteInteractive, + CachedInteractive, + CachedRemoteInteractive, + CachedUnlock +} SECURITY_LOGON_TYPE; + +typedef enum _KERB_LOGON_SUBMIT_TYPE +{ + KerbInteractiveLogon = 2, + KerbSmartCardLogon = 6, + KerbWorkstationUnlockLogon = 7 +} KERB_LOGON_SUBMIT_TYPE; + +typedef struct _KERB_INTERACTIVE_LOGON +{ + KERB_LOGON_SUBMIT_TYPE MessageType; + UNICODE_STRING LogonDomainName; + UNICODE_STRING UserName; + UNICODE_STRING Password; +} KERB_INTERACTIVE_LOGON; + +typedef struct _LUID { DWORD LowPart; LONG HighPart; } LUID, *PLUID; diff --git a/tests/native/shim/ntsecpkg.h b/tests/native/shim/ntsecpkg.h new file mode 100644 index 0000000..6ca8041 --- /dev/null +++ b/tests/native/shim/ntsecpkg.h @@ -0,0 +1,21 @@ +// Stand-in for : the dispatch table and the opaque types in the +// authentication package's export signatures. +#pragma once +#include "ntsecapi.h" + +typedef PVOID* PLSA_CLIENT_REQUEST; + +// The real table has many more members; ap.cpp only uses these two. +typedef struct _LSA_DISPATCH_TABLE +{ + PVOID (NTAPI *AllocateLsaHeap)(ULONG Length); + VOID (NTAPI *FreeLsaHeap)(PVOID Base); +} LSA_DISPATCH_TABLE, *PLSA_DISPATCH_TABLE; + +typedef enum _LSA_TOKEN_INFORMATION_TYPE { LsaTokenInformationNull } LSA_TOKEN_INFORMATION_TYPE, + *PLSA_TOKEN_INFORMATION_TYPE; + +struct _SECPKG_PRIMARY_CRED; +typedef struct _SECPKG_PRIMARY_CRED* PSECPKG_PRIMARY_CRED; +struct _SECPKG_SUPPLEMENTAL_CRED_ARRAY; +typedef struct _SECPKG_SUPPLEMENTAL_CRED_ARRAY* PSECPKG_SUPPLEMENTAL_CRED_ARRAY; diff --git a/tests/native/shim/sspi.h b/tests/native/shim/sspi.h new file mode 100644 index 0000000..e499878 --- /dev/null +++ b/tests/native/shim/sspi.h @@ -0,0 +1,4 @@ +// Stand-in for . ap.h and subauth_filter.h include it; nothing from +// it is used by the code under test. +#pragma once +#include "windows.h" diff --git a/tests/native/shim/subauth.h b/tests/native/shim/subauth.h new file mode 100644 index 0000000..8a94277 --- /dev/null +++ b/tests/native/shim/subauth.h @@ -0,0 +1,26 @@ +// Stand-in for the SDK's : what Msv1_0SubAuthenticationFilter's +// signature and subauth.cpp use. +#pragma once +#include "ntsecapi.h" + +typedef enum _NETLOGON_LOGON_INFO_CLASS +{ + NetlogonInteractiveInformation = 1, + NetlogonNetworkInformation, + NetlogonServiceInformation, + NetlogonGenericInformation, + NetlogonInteractiveTransitiveInformation, + NetlogonNetworkTransitiveInformation, + NetlogonServiceTransitiveInformation +} NETLOGON_LOGON_INFO_CLASS; + +// The real structure is much larger; the filter reads only these two members, +// and the test builds them directly. +typedef struct _USER_ALL_INFORMATION +{ + UNICODE_STRING UserName; + ULONG UserId; +} USER_ALL_INFORMATION, *PUSER_ALL_INFORMATION; + +#define MSV1_0_PASSTHRU 0x01 +#define MSV1_0_GUEST_LOGON 0x02 diff --git a/tests/native/shim/windows.h b/tests/native/shim/windows.h index d1946e9..0ac8179 100644 --- a/tests/native/shim/windows.h +++ b/tests/native/shim/windows.h @@ -1,6 +1,9 @@ -// Minimal stand-in for : just enough for totp.cpp, verify.cpp and -// statelock.h to compile on Linux. Only used by the native tests. +// Minimal stand-in for : just enough for totp.cpp, verify.cpp, +// store_index.cpp, ap.cpp, subauth.cpp and their headers to compile on Linux. +// Only used by the native tests. The functions declared at the bottom are +// defined by the fakes of the test that needs them (lsa_fakes.cpp). #pragma once +#include #include #include #include @@ -10,17 +13,99 @@ typedef unsigned char BYTE; typedef unsigned char UCHAR; typedef UCHAR* PUCHAR; typedef unsigned long ULONG; +typedef ULONG* PULONG; typedef uint32_t DWORD; +typedef DWORD* LPDWORD; +typedef BYTE* LPBYTE; +typedef int32_t LONG; typedef long NTSTATUS; +typedef NTSTATUS* PNTSTATUS; typedef void* HANDLE; -typedef const wchar_t* LPCWSTR; +typedef wchar_t WCHAR; +typedef WCHAR* PWSTR; +typedef WCHAR* LPWSTR; +typedef const WCHAR* LPCWSTR; +typedef char* PCHAR; +typedef void* PVOID; +typedef void* LPVOID; +typedef void VOID; +typedef size_t SIZE_T; +typedef uintptr_t ULONG_PTR; +typedef uint64_t ULONGLONG; +typedef int64_t LONGLONG; +typedef uint16_t USHORT; +typedef uint16_t WORD; +typedef unsigned char BOOLEAN; +typedef BOOLEAN* PBOOLEAN; +typedef int BOOL; +typedef LONG LSTATUS; +typedef DWORD LCID; +typedef DWORD REGSAM; +typedef void* PSID; typedef void* BCRYPT_ALG_HANDLE; typedef void* BCRYPT_HASH_HANDLE; +struct HKEY__; +typedef HKEY__* HKEY; +typedef HKEY* PHKEY; +struct HINSTANCE__; +typedef HINSTANCE__* HMODULE; + +typedef union _LARGE_INTEGER { LONGLONG QuadPart; } LARGE_INTEGER, *PLARGE_INTEGER; + +#define NTAPI +#define APIENTRY +#define TRUE 1 +#define FALSE 0 + #define INVALID_HANDLE_VALUE (reinterpret_cast(-1)) +#define HKEY_LOCAL_MACHINE (reinterpret_cast(static_cast(0x80000002))) + +#define ARRAYSIZE(a) (sizeof(a) / sizeof((a)[0])) + +#define ERROR_SUCCESS 0L +#define ERROR_FILE_NOT_FOUND 2L +#define ERROR_ACCESS_DENIED 5L +#define ERROR_OUTOFMEMORY 14L + +#define KEY_QUERY_VALUE 0x0001 + +#define EVENTLOG_ERROR_TYPE 0x0001 +#define EVENTLOG_WARNING_TYPE 0x0002 +#define EVENTLOG_INFORMATION_TYPE 0x0004 + +#define LOCALE_INVARIANT 0x007F +#define LCMAP_LOWERCASE 0x00000100 + +#define DLL_PROCESS_ATTACH 1 + +#define EXCEPTION_STACK_OVERFLOW 0xC00000FDu +#define EXCEPTION_NONCONTINUABLE_EXCEPTION 0xC0000025u +#define EXCEPTION_EXECUTE_HANDLER 1 +#define EXCEPTION_CONTINUE_SEARCH 0 + +// SEH does not exist in g++ on Linux. The tests map __try/__except to C++ +// try/catch (...), and the fake registry throws to stand in for an access +// violation. libstdc++ defines __try the same way. +#ifndef __try +#define __try try +#endif +#define __except(filter) catch (...) inline void SecureZeroMemory(void* p, size_t n) { volatile unsigned char* v = static_cast(p); while (n--) *v++ = 0; } + +// Defined by tests/native/lsa_fakes.cpp. +int LCMapStringW(LCID, DWORD, LPCWSTR src, int cchSrc, LPWSTR dst, int cchDst); +LSTATUS RegOpenKeyExW(HKEY root, LPCWSTR path, DWORD, REGSAM, PHKEY out); +LSTATUS RegQueryValueExW(HKEY, LPCWSTR value, LPDWORD, LPDWORD, LPBYTE, LPDWORD); +LSTATUS RegCloseKey(HKEY); +void OutputDebugStringW(LPCWSTR text); +HANDLE RegisterEventSourceW(LPCWSTR server, LPCWSTR source); +BOOL ReportEventW(HANDLE, WORD type, WORD category, DWORD id, PSID, WORD count, + DWORD, LPCWSTR* strings, LPVOID); +BOOL DeregisterEventSource(HANDLE); +BOOL DisableThreadLibraryCalls(HMODULE); diff --git a/tests/native/test_lsa.cpp b/tests/native/test_lsa.cpp new file mode 100644 index 0000000..da223c0 --- /dev/null +++ b/tests/native/test_lsa.cpp @@ -0,0 +1,375 @@ +// Native tests for the code that runs inside lsass: the real subauth.cpp, +// ap.cpp and store_index.cpp, against a fake registry, event log and LSA heap +// (lsa_fakes.cpp). Built with AddressSanitizer and UBSan. See README, "Tests". +// +// What this cannot show: how MSV1_0 and LSA really call these exports, and +// real SEH. __try/__except are mapped to try/catch here, and the fake +// registry throws to stand in for an access violation. It throws before a key +// is opened: between RegOpenKeyExW and RegCloseKey the real code touches no +// pointer of the caller's, so that is not where a fault would come from. +#include "ap.h" +#include "subauth_filter.h" +#include "store.h" +#include "lsa_fakes.h" +#include +#include +#include +#include + +static int g_failed = 0; +#define CHECK(c) do { if (!(c)) { std::printf("FAIL line %d: %s\n", __LINE__, #c); ++g_failed; } \ + else std::printf("ok %s\n", #c); } while (0) + +static const NTSTATUS kSuccess = 0x00000000L; +static const NTSTATUS kAccountRestrict = 0xC000006EL; +static const NTSTATUS kInvalidWorkst = 0xC0000070L; +static const NTSTATUS kInvalidLogonType = 0xC000010BL; +static const NTSTATUS kNoMemory = 0xC0000017L; + +static const wchar_t kRids[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Rids"; +static const wchar_t kNames[] = L"SOFTWARE\\TheAdminCafe\\2FA\\Names"; + +// ---- the sub-authentication filter ----------------------------------------- + +struct FilterResult +{ + NTSTATUS status; + ULONG which; + ULONG userFlags; + BOOLEAN authoritative; + LONGLONG logoff; + LONGLONG kickoff; +}; + +static FilterResult Filter(NETLOGON_LOGON_INFO_CLASS level, ULONG flags, ULONG rid, + std::wstring name, bool noUser = false) +{ + USER_ALL_INFORMATION ua = {}; + ua.UserId = rid; + ua.UserName.Buffer = name.empty() ? nullptr : &name[0]; + ua.UserName.Length = static_cast(name.size() * sizeof(WCHAR)); + ua.UserName.MaximumLength = ua.UserName.Length; + + ULONG which = 0xFFFF, userFlags = 0xFFFF; + BOOLEAN authoritative = 2; + LARGE_INTEGER logoff, kickoff; + logoff.QuadPart = 111; + kickoff.QuadPart = 222; + + NTSTATUS s = Msv1_0SubAuthenticationFilter(level, nullptr, flags, noUser ? nullptr : &ua, + &which, &userFlags, &authoritative, + &logoff, &kickoff); + return { s, which, userFlags, authoritative, logoff.QuadPart, kickoff.QuadPart }; +} + +// The outputs the documentation calls [out] are valid on every path, and the +// logon hours MSV1_0 handed in are left alone. +static bool OutputsSane(const FilterResult& r) +{ + return r.which == 0 && r.userFlags == 0 && r.authoritative == TRUE && + r.logoff == 111 && r.kickoff == 222; +} + +static void TestFilter() +{ + std::printf("-- sub-auth filter: decisions\n"); + fake::Reset(); + fake::keys[kRids] = { L"1001", L"4294967295" }; + + FilterResult r = Filter(NetlogonInteractiveInformation, 0, 1001, L"alice"); + CHECK(r.status == kSuccess && OutputsSane(r)); // not network: no objection + CHECK(fake::events.empty()); + + r = Filter(NetlogonNetworkInformation, 0, 1002, L"bob"); + CHECK(r.status == kSuccess && OutputsSane(r)); // network, not enrolled + CHECK(fake::events.empty()); + + r = Filter(NetlogonNetworkInformation, 0, 1001, L"alice"); + CHECK(r.status == kInvalidWorkst && OutputsSane(r)); // network, enrolled + CHECK(fake::events.size() == 1 && fake::events[0] == 210); + + r = Filter(NetlogonNetworkTransitiveInformation, 0, 1001, L"alice"); + CHECK(r.status == kInvalidWorkst && OutputsSane(r)); + + r = Filter(NetlogonNetworkInformation, 0, 4294967295u, L"max"); + CHECK(r.status == kInvalidWorkst); // 10-digit RID value name + + r = Filter(NetlogonNetworkInformation, 0, 0, L"rid0"); + CHECK(r.status == kSuccess); // RID 0 is never enrolled + + std::printf("-- sub-auth filter: pass-through and missing record\n"); + r = Filter(NetlogonNetworkInformation, MSV1_0_PASSTHRU, 1001, L"alice"); + CHECK(r.status == kSuccess && OutputsSane(r)); // not our account + CHECK(fake::DebugContains(L"pass-through")); + + r = Filter(NetlogonNetworkInformation, 0, 1001, L"", true); + CHECK(r.status == kSuccess && OutputsSane(r)); // no SAM record + + std::printf("-- sub-auth filter: fail closed\n"); + fake::keys.erase(kRids); + r = Filter(NetlogonNetworkInformation, 0, 1001, L"alice"); + CHECK(r.status == kSuccess); // no index at all = No + + fake::keys[kRids] = { L"1001" }; + fake::openStatus = ERROR_ACCESS_DENIED; + r = Filter(NetlogonNetworkInformation, 0, 1002, L"bob"); + CHECK(r.status == kInvalidWorkst && OutputsSane(r)); // unreadable = refuse + CHECK(fake::DebugContains(L"could not be read")); + fake::openStatus = ERROR_SUCCESS; + + fake::queryStatus = ERROR_OUTOFMEMORY; + r = Filter(NetlogonNetworkInformation, 0, 1002, L"bob"); + CHECK(r.status == kInvalidWorkst); + fake::queryStatus = ERROR_SUCCESS; + + fake::throwOnOpen = true; + r = Filter(NetlogonNetworkInformation, 0, 1002, L"bob"); + CHECK(r.status == kInvalidWorkst && OutputsSane(r)); // fault on network: refuse + CHECK(fake::DebugContains(L"Exception while deciding")); + r = Filter(NetlogonInteractiveInformation, 0, 1002, L"bob"); + CHECK(r.status == kSuccess); // no read, no fault + fake::throwOnOpen = false; + + std::printf("-- sub-auth filter: logging\n"); + fake::debug.clear(); + Filter(NetlogonNetworkInformation, 0, 1002, std::wstring(L"ev\0il", 5)); + CHECK(fake::DebugContains(L"'ev'") && !fake::DebugContains(L"il'")); // stops at NUL + Filter(NetlogonNetworkInformation, 0, 1002, std::wstring(L"a\x01" L"b")); + CHECK(fake::DebugContains(L"'a.b'")); // control char masked + CHECK(fake::DebugContains(L"incoming LogoffTime=111 KickoffTime=222")); + Filter(NetlogonNetworkInformation, 0, 1002, std::wstring(400, L'x')); + CHECK(!fake::debug.empty() && fake::debug.back().size() < 352); // cut, not overflowed + + // Null out-parameters must not crash. + USER_ALL_INFORMATION ua = {}; + ua.UserId = 1001; + CHECK(Msv1_0SubAuthenticationFilter(NetlogonNetworkInformation, nullptr, 0, &ua, nullptr, + nullptr, nullptr, nullptr, nullptr) == kInvalidWorkst); + + CHECK(fake::opened == fake::closed); +} + +// ---- the authentication package -------------------------------------------- + +static const ULONG_PTR kClientBase = 0x7ff000000000ull; + +// A KERB_INTERACTIVE_LOGON with the user name behind it, the name either as a +// client-process address (relative to kClientBase) or as a packed offset. +static std::vector Submit(const std::wstring& user, bool packed, + KERB_LOGON_SUBMIT_TYPE type = KerbInteractiveLogon) +{ + const size_t off = sizeof(KERB_INTERACTIVE_LOGON); + std::vector b(off + user.size() * sizeof(WCHAR)); + auto* k = reinterpret_cast(b.data()); + k->MessageType = type; + if (!user.empty()) + std::memcpy(b.data() + off, user.data(), user.size() * sizeof(WCHAR)); + k->UserName.Length = static_cast(user.size() * sizeof(WCHAR)); + k->UserName.MaximumLength = k->UserName.Length; + k->UserName.Buffer = reinterpret_cast(packed ? off : kClientBase + off); + return b; +} + +struct ApResult +{ + NTSTATUS status; + NTSTATUS sub; + bool haveAccount; + std::wstring account; +}; + +static ApResult Logon(SECURITY_LOGON_TYPE type, std::vector submit, bool wantAccount = true) +{ + PVOID profile = reinterpret_cast(1); + ULONG profileSize = 99; + LUID luid = {}; + NTSTATUS sub = 12345; + LSA_TOKEN_INFORMATION_TYPE tokenType = LsaTokenInformationNull; + PVOID token = nullptr; + PUNICODE_STRING account = nullptr; + PUNICODE_STRING authority = reinterpret_cast(1); + PUNICODE_STRING machine = reinterpret_cast(1); + PSECPKG_SUPPLEMENTAL_CRED_ARRAY cached = reinterpret_cast(1); + + NTSTATUS s = LsaApLogonUserEx2(nullptr, type, + submit.empty() ? nullptr : submit.data(), + reinterpret_cast(kClientBase), + static_cast(submit.size()), + &profile, &profileSize, &luid, &sub, &tokenType, &token, + wantAccount ? &account : nullptr, + &authority, &machine, nullptr, &cached); + + CHECK(profile == nullptr && profileSize == 0 && authority == nullptr && + machine == nullptr && cached == nullptr); + + ApResult r = { s, sub, account != nullptr, L"" }; + if (account) + { + const size_t cch = account->Length / sizeof(WCHAR); + CHECK(account->MaximumLength == account->Length + sizeof(WCHAR) && + account->Buffer[cch] == L'\0'); + r.account.assign(account->Buffer, cch); + fake::lsa.FreeLsaHeap(account->Buffer); // as LSA would + fake::lsa.FreeLsaHeap(account); + } + return r; +} + +static void TestAuthPackage() +{ + std::printf("-- authentication package: initialize\n"); + fake::Reset(); + PLSA_STRING name = reinterpret_cast(1); + + fake::allocBudget = 1; // second block fails + CHECK(LsaApInitializePackage(7, &fake::lsa, nullptr, nullptr, &name) == kNoMemory); + CHECK(name == nullptr && fake::lsaAllocs == fake::lsaFrees); // first block given back + + fake::allocBudget = -1; + CHECK(LsaApInitializePackage(7, &fake::lsa, nullptr, nullptr, &name) == kSuccess); + CHECK(name && name->Length == 14 && std::strcmp(name->Buffer, "TacAuthPackage") == 0); + CHECK(fake::DebugContains(L"[Doppio-AP] LSA authentication package loaded (id 7).")); + fake::lsa.FreeLsaHeap(name->Buffer); + fake::lsa.FreeLsaHeap(name); + + std::printf("-- authentication package: decisions\n"); + fake::keys[kNames] = { L"alice", L"jörg" }; + + ApResult r = Logon(Network, Submit(L"alice", false)); + CHECK(r.status == kAccountRestrict && r.sub == kInvalidWorkst); // enrolled, network + CHECK(r.account == L"alice"); + CHECK(!fake::events.empty() && fake::events.back() == 201); + + r = Logon(Network, Submit(L"alice", true)); // packed offsets + CHECK(r.status == kAccountRestrict && r.account == L"alice"); + + r = Logon(Batch, Submit(L"PC\\ALICE", false)); // normalized + CHECK(r.status == kAccountRestrict && r.account == L"PC\\ALICE"); + + r = Logon(Network, Submit(L"bob", false)); // not enrolled + CHECK(r.status == kInvalidLogonType && r.account == L"bob"); + CHECK(fake::events.back() == 202); + + r = Logon(Interactive, Submit(L"alice", false)); // tile path: decline + CHECK(r.status == kInvalidLogonType && r.account == L"alice"); // name still audited + + r = Logon(Network, Submit(L"x", false, KerbSmartCardLogon)); // not our layout + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + r = Logon(Network, {}); // no buffer + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + std::printf("-- authentication package: non-ASCII names (locale-invariant folding)\n"); + if (std::setlocale(LC_CTYPE, "C.UTF-8")) + { + r = Logon(Network, Submit(L"JÖRG", false)); + CHECK(r.status == kAccountRestrict); + fake::failLcMap = true; // ASCII-only fallback + r = Logon(Network, Submit(L"JÖRG", false)); + CHECK(r.status == kInvalidLogonType); + r = Logon(Network, Submit(L"ALICE", false)); + CHECK(r.status == kAccountRestrict); + fake::failLcMap = false; + std::setlocale(LC_CTYPE, "C"); + } + else + { + std::printf("skip C.UTF-8 locale not available\n"); + } + + std::printf("-- authentication package: hostile buffers\n"); + std::vector b = Submit(L"alice", false); + reinterpret_cast(b.data())->UserName.Length += sizeof(WCHAR); + r = Logon(Network, b); // runs past the buffer + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + b = Submit(L"alice", false); + reinterpret_cast(b.data())->UserName.Length = 3; + r = Logon(Network, b); // odd byte count + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + b = Submit(L"alice", false); + reinterpret_cast(b.data())->UserName.Buffer = + reinterpret_cast(kClientBase - 64); + r = Logon(Network, b); // points before it + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + b = Submit(L"alice", false); + b.resize(sizeof(KERB_INTERACTIVE_LOGON) - 1); + r = Logon(Network, b); // truncated header + CHECK(r.status == kInvalidLogonType && r.account == L"-"); + + r = Logon(Network, Submit(std::wstring(L"al\0ice", 6), false)); // stops at NUL + CHECK(r.account == L"al"); + + r = Logon(Network, Submit(std::wstring(300, L'n'), false)); // capped at UNLEN + CHECK(r.account.size() == 256); + + std::printf("-- authentication package: fail closed\n"); + fake::openStatus = ERROR_ACCESS_DENIED; + r = Logon(Network, Submit(L"bob", false)); // unreadable index + CHECK(r.status == kAccountRestrict && r.account == L"bob"); + CHECK(fake::DebugContains(L"could not be read")); + fake::openStatus = ERROR_SUCCESS; + + fake::throwOnOpen = true; + r = Logon(Network, Submit(L"bob", false)); // fault: deny + CHECK(r.status == kAccountRestrict && r.account == L"-"); + CHECK(fake::DebugContains(L"after an exception")); + r = Logon(Interactive, Submit(L"bob", false)); // no read, no fault + CHECK(r.status == kInvalidLogonType); + fake::throwOnOpen = false; + + std::printf("-- authentication package: AccountName edge cases\n"); + r = Logon(Network, Submit(L"alice", false), false); // caller passed null + CHECK(r.status == kAccountRestrict && !r.haveAccount); + + fake::allocBudget = 1; // struct ok, buffer fails + r = Logon(Network, Submit(L"alice", false)); + CHECK(r.status == kAccountRestrict && !r.haveAccount); + fake::allocBudget = 0; + r = Logon(Network, Submit(L"alice", false)); + CHECK(r.status == kAccountRestrict && !r.haveAccount); + fake::allocBudget = -1; + + CHECK(fake::lsaAllocs == fake::lsaFrees); // nothing leaked + CHECK(fake::opened == fake::closed); +} + +// ---- the index reads ------------------------------------------------------- + +static void TestIndex() +{ + std::printf("-- index: NormalizeUserInto\n"); + fake::Reset(); + WCHAR out[8]; + CHECK(tac::NormalizeUserInto(L"PC\\Alice", 8, out, 8) && std::wcscmp(out, L"alice") == 0); + CHECK(tac::NormalizeUserInto(L".\\bob", 5, out, 8) && std::wcscmp(out, L"bob") == 0); + CHECK(!tac::NormalizeUserInto(L"", 0, out, 8)); + CHECK(!tac::NormalizeUserInto(L"PC\\", 3, out, 8)); + CHECK(!tac::NormalizeUserInto(L"abcdefgh", 8, out, 8)); // does not fit + CHECK(tac::NormalizeUserInto(L"abcdefg", 7, out, 8) && std::wcscmp(out, L"abcdefg") == 0); + CHECK(!tac::NormalizeUserInto(nullptr, 3, out, 8)); + + std::printf("-- index: three outcomes\n"); + CHECK(tac::EnrollmentByRid(1001) == tac::Enrollment::No); // no key at all + fake::keys[kRids] = { L"1001" }; + CHECK(tac::EnrollmentByRid(1001) == tac::Enrollment::Yes); + CHECK(tac::EnrollmentByRid(1002) == tac::Enrollment::No); + fake::queryStatus = ERROR_ACCESS_DENIED; + CHECK(tac::EnrollmentByRid(1001) == tac::Enrollment::Unknown); + CHECK(!tac::IsEnrolledByRid(1001)); // non-vetoing view + fake::queryStatus = ERROR_SUCCESS; + CHECK(tac::EnrollmentByName(L"", 0) == tac::Enrollment::Unknown); // unusable name + CHECK(fake::opened == fake::closed); +} + +int main() +{ + TestFilter(); + TestAuthPackage(); + TestIndex(); + + std::printf("\n%s (%d failed)\n", g_failed ? "SOME CHECKS FAILED" : "ALL CHECKS PASS", g_failed); + return g_failed ? 1 : 0; +} diff --git a/tests/totp_check.py b/tests/totp_check.py index 2a1a49a..420b621 100644 --- a/tests/totp_check.py +++ b/tests/totp_check.py @@ -1,4 +1,4 @@ -import hmac, hashlib, struct +import hmac, hashlib, struct, sys BASE32="ABCDEFGHIJKLMNOPQRSTUVWXYZ234567" @@ -61,6 +61,7 @@ def totp_at(key, unixtime, step=30, digits=6): # Known: base32 of ASCII "12345678901234567890" is GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ print("matches known GEZD... :", enc=="GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ") +ok_all &= dec==seed and enc=="GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ" # --- faithful port of my C++ ValidateTotp (with replay protection) --- def validate(key, code, now, last_step, step=30, digits=6, window=1): @@ -89,3 +90,6 @@ def validate(key, code, now, last_step, step=30, digits=6, window=1): ok_all &= all(checks.values()) print("\nALL CHECKS PASS:", ok_all) + +# CI runs this script on its own; without a failing exit code it could never go red. +sys.exit(0 if ok_all else 1)