[BUG] CertificatePath accepts unusable non-PKCS#12 files and silently ignores a missing file #269

Summary

DockerUpdateGuard:DockerInstances:<n>:CertificatePath is meant to supply the client certificate for TLS-secured Docker Engine access. Today a file that is not .pfx/.p12 is loaded as a bare certificate without private key, which cannot authenticate a TLS client, and a path that does not exist is silently ignored. In both cases the operator believes mutual TLS is configured, while the connection is attempted without a usable client certificate and fails later with an opaque TLS/HTTP error (or, against a misconfigured engine, succeeds without client authentication). Only PKCS#12 must be accepted, and a wrong, missing or unreadable file must fail validation.

Area

• docs/areas/docker-instances.md (TLS options, Limits and errors), startup validation in docs/areas/hosting.md
• Security areas (.squad/project.md): TLS and transport relaxations, Docker Engine access (DockerInstanceClient)

Current behavior

src/DockerUpdateGuard/Docker/DockerInstanceClient.cs:297-304 – a missing file is skipped without any log entry:

var certificatePath = instanceOptions.CertificatePath;

if (string.IsNullOrWhiteSpace(certificatePath) == false
    && File.Exists(certificatePath))
{
    handler.SslOptions.ClientCertificates ??= new X509CertificateCollection();
    handler.SslOptions.ClientCertificates.Add(LoadCertificate(certificatePath));
}

src/DockerUpdateGuard/Docker/DockerInstanceClient.cs:485-497 – any extension other than .pfx/.p12 goes through X509CertificateLoader.LoadCertificateFromFile, which loads only the public certificate (PEM/DER), never a private key:

return string.Equals(extension, ".pfx", ...) || string.Equals(extension, ".p12", ...)
           ? X509CertificateLoader.LoadPkcs12FromFile(certificatePath, password: null)
           : X509CertificateLoader.LoadCertificateFromFile(certificatePath);

src/DockerUpdateGuard/Configuration/DockerUpdateGuardOptionsValidator.cs:296-316 (ValidateDockerInstance) does not look at CertificatePath at all. A PKCS#12 file that is corrupt or password-protected is only detected when the first HTTP client for the instance is built, i.e. as an exception inside an Engine call.

Failure scenarios:

1. CertificatePath: /certs/client-cert.pem (the usual Docker cert.pem + key.pem layout): startup succeeds, every Engine call fails the TLS handshake (tlsverify engines reject the client) and the scan runs end failed with a handshake error that does not mention the certificate.
2. CertificatePath: /certs/client.pfx but the file was not mounted into the container (README, Notes for Docker-based deployments, asks to mount it): startup succeeds, no warning, the connection is made without a client certificate.
3. A password-protected .pfx: startup succeeds; every Engine call fails with a cryptographic exception message.

README (README.md:168) only says "Optional client certificate path for TLS-secured engine access" and does not state the required format.

Expected behavior

Decision (product manager): only PKCS#12 is accepted; any other file and a missing or unreadable file fail validation.

• When CertificatePath is set (non-whitespace) for an enabled instance, startup validation fails unless:
  • the extension is .pfx or .p12 (case-insensitive), and
  • the file exists and can be read by the application user, and
  • it loads as a password-less PKCS#12 bundle containing at least one certificate with a private key.
• Each failure is reported with its key (DockerUpdateGuard:DockerInstances:<name>:CertificatePath) and the reason (unsupported format — only .pfx/.p12; file not found; file cannot be read or is not a password-less PKCS#12 bundle; no private key). The message names the path but never file contents.
• DockerInstanceClient no longer has a silent-skip branch and no non-PKCS#12 load branch; a certificate that becomes unreadable after startup makes the Engine operation end failed with a message naming the instance and that the client certificate could not be loaded (never an unhandled exception, see the failure model).
• README documents CertificatePath as: password-less PKCS#12 (.pfx/.p12) with certificate and private key, must be readable by the container user (UID 64000), with a short openssl pkcs12 -export -in cert.pem -inkey key.pem -out client.pfx -passout pass: conversion hint for the Docker cert.pem/key.pem layout. It also states that the option is ignored for http, non-TLS tcp, unix and npipe instances.

Acceptance criteria

• Validation fails for an enabled instance with CertificatePath ending in .pem, .crt, .cer, .key or no extension.
• Validation fails for a .pfx path that does not exist.
• Validation fails for a .pfx that is corrupt, password-protected, or contains no private key.
• Validation passes for a valid password-less .pfx/.p12 with private key, and for an empty/whitespace CertificatePath.
• A disabled instance (Enabled: false) is not validated for CertificatePath (consistent with Engine calls answering not configured for it) — or, if the Lead decides otherwise, that is documented in the area document.
• DockerInstanceClient loads only PKCS#12; the LoadCertificateFromFile branch and the File.Exists skip are removed; a load failure at runtime ends the operation failed with a clear message.
• Unit tests (MSTest, naming per docs/UNIT_TESTS.md), e.g. in DockerUpdateGuardOptionsValidatorTests: DockerUpdateGuardOptionsValidatorValidateWithPemCertificatePathFails, …WithMissingCertificateFileFails, …WithPasswordProtectedPfxFails, …WithPfxWithoutPrivateKeyFails, …WithValidPfxSucceeds; in DockerInstanceClientTests a test for the runtime load failure. Test certificates are generated in the test (as DockerInstanceClientCreateHttpClientWithClientCertificateReturnsConfiguredClient does at src/Tests/DockerUpdateGuard.Tests/DockerInstanceClientTests.cs:572), never checked in.
• README configuration reference (README.md:168) and the deployment note (README.md:294) describe the format, the readability requirement and the conversion hint; docs/ARCHITECTURE.md, Configuration model, is updated if it mentions the option.
• The area document docs/areas/docker-instances.md describes the new behavior and its Known gaps entries for this issue (currently Open questions items 2 and 3) are removed.

Implementation notes

• The validator is currently pure (no I/O). Reading a file in validation is new; keep it testable: either inject a small file-access abstraction into DockerUpdateGuardOptionsValidator (constructed via DI, src/DockerUpdateGuard/ServiceCollectionExtensions.cs:53) or write temp files in tests. Recommendation: share one ClientCertificateLoader helper (load PKCS#12, check HasPrivateKey, return a result with a reason) used by both the validator and DockerInstanceClient, so the rules cannot drift.
• Dispose the certificate loaded during validation.
• Validation also runs when a reloaded configuration is read (docs/areas/hosting.md, Configuration reload). A certificate file deleted while the app runs would then make the reloaded options invalid; that interaction is already a known gap of the hosting area and must not be worsened silently — mention it in the area document.
• No password option is added (out of scope); a password-protected bundle is rejected with a message saying that only password-less bundles are supported.
• Pool key: BuildHttpClientCacheKey (DockerInstanceClient.cs:249-258) includes the path; a renewed certificate under the same path is picked up only when the pool entry is rebuilt. Keep as is, but do not regress it.
• Must not change: SkipCertificateValidation stays opt-in with its once-per-instance warning; no secrets (certificate contents) in logs, messages or telemetry; integration clients return outcomes instead of throwing (.squad/project.md, Guarantees). This is a breaking configuration change for operators using PEM files: call it out in the PR description and README.

Related

• PR [Docs] Describe the current behavior by area #263 (area documents)

Generated by Claude Code