Skip to content

Allow for .pem and .key client cert authentication#2

Merged
rodchristiansen merged 4 commits intowindowsadmins:mainfrom
aysiu:aysiu-patch-1
Apr 11, 2026
Merged

Allow for .pem and .key client cert authentication#2
rodchristiansen merged 4 commits intowindowsadmins:mainfrom
aysiu:aysiu-patch-1

Conversation

@aysiu
Copy link
Copy Markdown
Contributor

@aysiu aysiu commented Apr 9, 2026

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.

aysiu and others added 4 commits April 9, 2026 13:56
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>
@rodchristiansen
Copy link
Copy Markdown
Contributor

rodchristiansen commented Apr 11, 2026

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 (672fe33) that does a few things:

Fixes to the PEM path you added

  1. Gate file-based cert loading behind auth.use_mtls. In the original patch, setting CRYPT_CLIENT_CERT_PATH alone would silently enable mTLS even when use_mtls was false, and if you set both cert-store options and PEM paths, two different client certs got added to the same handler. Now everything routes through a single GetClientCertificate(AuthConfig) method that tries strategies in priority order and stops at the first success.
  2. Route the new fields through ConfigService.GetX() helpers. The original read CRYPT_CLIENT_CERT_PATH / CRYPT_CLIENT_KEY_PATH inline, which skipped the registry / CSP / OMA-URI Intune deployment path that every other auth field goes through. I added GetClientCertPath() and GetClientKeyPath() helpers matching the pattern of GetCertificateSubject().
  3. Replace the obsolete new X509Certificate2(byte[]) ctor — it triggers SYSLIB0057 on .NET 9+ — with X509CertificateLoader.LoadPkcs12. The PFX round-trip on Windows is still needed so HttpClient can use the key during the TLS handshake; the byte buffer is now cleared after use, and the resulting cert is tracked on the instance so Dispose() cleans it up.
  4. Distinct error messages for each failure mode (file missing, parse failure) so operators can self-diagnose configuration problems.

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 .pfx on disk whose passphrase is pulled from Windows Credential Manager at load time. The passphrase never touches YAML, env vars, or the registry. Provisioning is a one-liner:

cmdkey /generic:CryptPfxPassword /user:cryptescrow /pass:<secret>

New config fields are pfx_path / pfx_password_credential (YAML), CRYPT_PFX_PATH / CRYPT_PFX_PASSWORD_CRED (env vars), and matching registry values for CSP deployment.

Implementation is a LibraryImport wrapper over advapi32!CredReadW in Services/CredentialManager.cs — no new NuGet dependency. <AllowUnsafeBlocks>true</AllowUnsafeBlocks> is required by the LibraryImport source generator and was added to the csproj.

Priority order is: GetClientCertificate now tries, 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 nuke cert resolution.

README updated to rank the three mTLS strategies by security with provisioning examples for each, and the new env vars / registry values are documented.

@rodchristiansen rodchristiansen merged commit 649ddf0 into windowsadmins:main Apr 11, 2026
rodchristiansen added a commit that referenced this pull request Apr 12, 2026
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants