Allow for .pem and .key client cert authentication#2
Allow for .pem and .key client cert authentication#2rodchristiansen merged 4 commits intowindowsadmins:mainfrom
Conversation
Review of the original PEM change surfaced four issues that are fixed here, and adds a third, more secure file-based strategy alongside them. Fixes to the PEM path: 1. Gate all file-based cert loading behind auth.use_mtls. Previously, setting CRYPT_CLIENT_CERT_PATH alone silently enabled mTLS even when use_mtls was false, and setting both cert store options + PEM paths added two certificates to the handler. 2. Route the new config fields through ConfigService.GetX() helpers so that Intune / CSP / OMA-URI registry deployment works for them, the same way it already does for every other auth field. 3. Replace the obsolete `new X509Certificate2(byte[])` ctor (SYSLIB0057 on .NET 9+) with X509CertificateLoader.LoadPkcs12, clear the PFX byte buffer after use, and track the loaded cert on the instance so Dispose() can release it alongside the HttpClient. 4. Check File.Exists and log distinct errors for each failure mode (file missing vs parse failure vs credential missing) so operators can self-diagnose configuration problems. New: PFX + Credential Manager strategy Introduces a third mTLS strategy that is preferred over PEM+key files: a .pfx on disk whose passphrase is pulled from Windows Credential Manager at load time. The passphrase never touches YAML, environment variables, or the registry. Provision with: cmdkey /generic:CryptPfxPassword /user:cryptescrow /pass:<secret> Configuration is either yaml (pfx_path, pfx_password_credential) or the new env vars (CRYPT_PFX_PATH, CRYPT_PFX_PASSWORD_CRED) or registry values (PfxPath, PfxPasswordCredential). Strategies in GetClientCertificate are now tried in priority order, most secure first: PFX+CredMgr -> PEM+key -> Cert Store thumbprint -> Cert Store subject. First match wins; failures fall through so a typo'd path doesn't break cert resolution. Implementation of the Credential Manager read is a ~60 LOC LibraryImport wrapper over advapi32!CredReadW in Services/CredentialManager.cs — no new NuGet dependency. AllowUnsafeBlocks is required by the LibraryImport source generator; added to the csproj. README is rewritten to rank the three mTLS strategies by security and document the new env vars / registry values / YAML fields. Co-authored-by: aysiu <aysiu@users.noreply.github.com>
|
Thanks for the PR, @aysiu! Helps me cover ground I haven't yet. As we chatted about doing extra tweaks to carry the idea forward, I pushed a follow-up commit directly to your branch ( Fixes to the PEM path you added
New: PFX + Credential Manager strategy I also added a third mTLS strategy that's more secure than PEM+key files, after a discussion about security trade-offs: a cmdkey /generic:CryptPfxPassword /user:cryptescrow /pass:<secret>New config fields are Implementation is a Priority order is: README updated to rank the three mTLS strategies by security with provisioning examples for each, and the new env vars / registry values are documented. |
Consolidates the four PRs merged today into a single semver entry for humans, and triggers a new CI release whose body will link back to the individual timestamp-tagged releases (2308, 2314, 2344, 2347). The entry credits @aysiu explicitly for the PEM client cert contribution in PR #2, which was the original spark for the mTLS feature. PFX + Credential Manager and the strategy-priority refactor were layered on as maintainer tweaks. Also documents: - xUnit test suite + dotnet test CI integration (PR #3) - Node.js 24 opt-in for JavaScript Actions (PR #5) - REG_DWORD/QWORD handling fix in GetRegistryValue, closing #4 (PR #6)
Not necessarily the ideal implementation, but we've had a brief Slack discussion about it, and it sounds as if you might be able to make additional tweaks to make it better.