From 5ae7dc40af9864a24c0d9fbcba673d11b831db64 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 8 May 2026 14:35:52 +0200 Subject: [PATCH] Revert "fix(backend/security): pin client cert chain trust to embedded Intune root" This reverts commit 7e21591b55c5e60cd371e7ab117778ad1e0ce95b. --- .../CertificateValidatorTests.cs | 127 --------------- .../Security/CertificateValidator.cs | 149 +++++++++--------- 2 files changed, 75 insertions(+), 201 deletions(-) delete mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/CertificateValidatorTests.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/CertificateValidatorTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/CertificateValidatorTests.cs deleted file mode 100644 index 38a77b5d..00000000 --- a/src/Backend/AutopilotMonitor.Functions.Tests/CertificateValidatorTests.cs +++ /dev/null @@ -1,127 +0,0 @@ -using System.Security.Cryptography; -using System.Security.Cryptography.X509Certificates; -using AutopilotMonitor.Functions.Security; - -namespace AutopilotMonitor.Functions.Tests; - -/// -/// Tests for CertificateValidator chain-trust enforcement. -/// -/// The validator pins trust to embedded Intune root certs via X509ChainTrustMode.CustomRootTrust. -/// A self-signed leaf cert whose Subject contains "Microsoft Intune" must NOT validate - -/// guards against the prior bypass where AllowUnknownCertificateAuthority + DN-substring -/// matching let any forged self-signed cert pass. -/// -public class CertificateValidatorTests -{ - [Fact] - public void ValidateCertificate_WithEmptyBase64_ReturnsInvalid() - { - var result = CertificateValidator.ValidateCertificate(""); - - Assert.False(result.IsValid); - Assert.Contains("No certificate", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); - } - - [Fact] - public void ValidateCertificate_WithNull_ReturnsInvalid() - { - var result = CertificateValidator.ValidateCertificate(null); - - Assert.False(result.IsValid); - } - - [Fact] - public void ValidateCertificate_WithMalformedBase64_ReturnsInvalid() - { - var result = CertificateValidator.ValidateCertificate("not-base64!!!"); - - Assert.False(result.IsValid); - } - - [Fact] - public void ValidateCertificate_WithSelfSignedCertImpersonatingIntune_ReturnsInvalid() - { - // Regression test: a self-signed leaf with Subject CN="Microsoft Intune MDM Device CA" - // and Client-Auth EKU previously passed validation under - // AllowUnknownCertificateAuthority + DN substring match. Under CustomRootTrust the - // chain has no signature link to a pinned Intune root and must be rejected. - var b64 = CreateSelfSignedCertBase64( - subjectCn: "Microsoft Intune MDM Device CA", - includeClientAuthEku: true); - - var result = CertificateValidator.ValidateCertificate(b64); - - Assert.False(result.IsValid); - Assert.Contains("chain validation failed", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); - } - - [Fact] - public void ValidateCertificate_WithSelfSignedCertImpersonatingMdmDeviceCa_ReturnsInvalid() - { - // Second variant of the Subject-substring bypass. - var b64 = CreateSelfSignedCertBase64( - subjectCn: "MDM Device CA", - includeClientAuthEku: true); - - var result = CertificateValidator.ValidateCertificate(b64); - - Assert.False(result.IsValid); - } - - [Fact] - public void ValidateCertificate_WithSelfSignedNonIntuneCert_ReturnsInvalid() - { - var b64 = CreateSelfSignedCertBase64( - subjectCn: "test-device.contoso.example", - includeClientAuthEku: true); - - var result = CertificateValidator.ValidateCertificate(b64); - - Assert.False(result.IsValid); - } - - [Fact] - public void ValidateCertificate_WithExpiredSelfSignedCert_ReturnsInvalid() - { - var b64 = CreateSelfSignedCertBase64( - subjectCn: "Microsoft Intune MDM Device CA", - includeClientAuthEku: true, - notBefore: DateTimeOffset.UtcNow.AddYears(-2), - notAfter: DateTimeOffset.UtcNow.AddYears(-1)); - - var result = CertificateValidator.ValidateCertificate(b64); - - Assert.False(result.IsValid); - Assert.Contains("expired", result.ErrorMessage, StringComparison.OrdinalIgnoreCase); - } - - private static string CreateSelfSignedCertBase64( - string subjectCn, - bool includeClientAuthEku, - DateTimeOffset? notBefore = null, - DateTimeOffset? notAfter = null) - { - using var rsa = RSA.Create(2048); - var req = new CertificateRequest( - $"CN={subjectCn}", - rsa, - HashAlgorithmName.SHA256, - RSASignaturePadding.Pkcs1); - - if (includeClientAuthEku) - { - // 1.3.6.1.5.5.7.3.2 = Client Authentication - req.CertificateExtensions.Add( - new X509EnhancedKeyUsageExtension( - new OidCollection { new Oid("1.3.6.1.5.5.7.3.2") }, - critical: false)); - } - - var nb = notBefore ?? DateTimeOffset.UtcNow.AddDays(-1); - var na = notAfter ?? DateTimeOffset.UtcNow.AddYears(1); - using var cert = req.CreateSelfSigned(nb, na); - - return Convert.ToBase64String(cert.Export(X509ContentType.Cert)); - } -} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs index 9a448ba9..ebab0cac 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs @@ -15,9 +15,9 @@ namespace AutopilotMonitor.Functions.Security /// /// /// Security measures: - /// - X.509 chain trust pinned to embedded Intune root certificate(s) via - /// X509ChainTrustMode.CustomRootTrust (OS trust store is ignored) + /// - Full X.509 chain validation using X509Chain.Build() /// - No revocation checking (Intune certificates have no CRL/OCSP endpoints) + /// - Validates entire chain contains Microsoft Intune CA /// - Prevents self-signed certificate attacks /// - Validates Enhanced Key Usage for Client Authentication /// - In-memory cache for validated certificates (5 minute TTL) @@ -29,13 +29,8 @@ namespace AutopilotMonitor.Functions.Security private static readonly ConcurrentDictionary _validationCache = new(); private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(5); - // Lazily loaded Intune CA certs from embedded PEM resources, split by role: - // roots -> pinned trust anchors (chain.ChainPolicy.CustomTrustStore) - // intermediates -> bridge candidates (chain.ChainPolicy.ExtraStore) - private static readonly Lazy _intuneRootCerts = - new(() => LoadIntunePemBundle("intune-root-ca.pem")); - private static readonly Lazy _intuneIntermediateCerts = - new(() => LoadIntunePemBundle("intune-intermediate-ca.pem")); + // Lazily loaded Intune CA certs from embedded resources + private static readonly Lazy _intuneCaCerts = new(LoadIntuneCaCertificates); private class CachedValidationResult { @@ -106,50 +101,31 @@ namespace AutopilotMonitor.Functions.Security }; } - // 2. Validate certificate chain against pinned Intune roots (no revocation - no CRL/OCSP). + // 2. Validate certificate chain (without revocation check - Intune certs have no CRL/OCSP) // - // CustomTrustStore + CustomRootTrust pins trust to the embedded Intune root cert(s) - // only and ignores the OS trust store entirely - works on Linux/Flex Consumption - // (no machine store) and replaces the previous AllowUnknownCertificateAuthority - // workaround, which was bypassable by self-signed leaf certs whose Subject contained - // "Microsoft Intune". - var roots = _intuneRootCerts.Value; - var intermediates = _intuneIntermediateCerts.Value; - - if (roots.Length == 0) - { - logger?.LogError("No embedded Intune root certificates loaded - failing closed"); - return new CertificateValidationResult - { - IsValid = false, - ErrorMessage = "Server misconfiguration: no Intune trust anchors available", - Thumbprint = thumbprint - }; - } - + // On Linux (Azure Functions flex consumption plan) the OS trust store does not contain + // Microsoft Intune CAs. We load them from embedded PEM resources into ExtraStore and set + // AllowUnknownCertificateAuthority so chain.Build() can resolve the issuer without the OS + // store. The issuer identity is still enforced in step 3. using var chain = new X509Chain(); chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck; chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain; - chain.ChainPolicy.VerificationFlags = X509VerificationFlags.NoFlag; + chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; chain.ChainPolicy.VerificationTime = DateTime.UtcNow; chain.ChainPolicy.UrlRetrievalTimeout = TimeSpan.FromSeconds(10); - chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; - foreach (var root in roots) - chain.ChainPolicy.CustomTrustStore.Add(root); - foreach (var intermediate in intermediates) - chain.ChainPolicy.ExtraStore.Add(intermediate); - - logger?.LogDebug( - "Chain build with {RootCount} pinned Intune root(s) + {IntermediateCount} intermediate(s)", - roots.Length, intermediates.Length); + // Load embedded Intune CA certs into ExtraStore + var caCerts = _intuneCaCerts.Value; + logger?.LogDebug("Loading {Count} embedded Intune CA certificate(s) into chain ExtraStore", caCerts.Length); + foreach (var caCert in caCerts) + chain.ChainPolicy.ExtraStore.Add(caCert); var chainValid = chain.Build(certificate); if (!chainValid) { var chainErrors = string.Join(", ", chain.ChainStatus.Select(s => s.StatusInformation)); - logger?.LogWarning("Certificate chain validation failed: {ChainErrors}", chainErrors); + logger?.LogWarning($"Certificate chain validation failed: {chainErrors}"); return new CertificateValidationResult { @@ -159,12 +135,29 @@ namespace AutopilotMonitor.Functions.Security }; } - // chainValid == true under CustomRootTrust proves the leaf is transitively signed - // by one of the pinned Intune roots. The previous Subject/Issuer DN substring check - // is intentionally removed - it added no security on top of root pinning and would - // false-negative if Microsoft renames the CA. + // 3. Verify issuer chain contains Microsoft Intune CA + var hasValidIssuer = chain.ChainElements + .Cast() + .Any(element => + element.Certificate.Issuer.Contains("Microsoft Intune", StringComparison.OrdinalIgnoreCase) || + element.Certificate.Issuer.Contains("MDM Device CA", StringComparison.OrdinalIgnoreCase) || + element.Certificate.Subject.Contains("Microsoft Intune", StringComparison.OrdinalIgnoreCase) || + element.Certificate.Subject.Contains("MDM Device CA", StringComparison.OrdinalIgnoreCase)); - // 3. Check Enhanced Key Usage for Client Authentication (1.3.6.1.5.5.7.3.2) + if (!hasValidIssuer) + { + var chainInfo = string.Join(" -> ", chain.ChainElements.Cast().Select(e => e.Certificate.Subject)); + logger?.LogWarning($"Certificate chain does not contain Microsoft Intune CA: {chainInfo}"); + + return new CertificateValidationResult + { + IsValid = false, + ErrorMessage = $"Certificate not issued by Microsoft Intune CA", + Thumbprint = thumbprint + }; + } + + // 4. Check Enhanced Key Usage for Client Authentication (1.3.6.1.5.5.7.3.2) var hasClientAuth = certificate.Extensions .OfType() .Any(ext => ext.EnhancedKeyUsages @@ -214,40 +207,48 @@ namespace AutopilotMonitor.Functions.Security } /// - /// Loads one or more X.509 certs from a PEM-bundle embedded resource under - /// Security/Certificates/. Roles are filename-segregated: - /// intune-root-ca.pem -> pinned trust anchors (CustomTrustStore) - /// intune-intermediate-ca.pem -> chain bridge candidates (ExtraStore) + /// Loads Intune CA certificates from embedded PEM resources. + /// Files: Security/Certificates/intune-intermediate-ca.pem + /// Security/Certificates/intune-root-ca.pem /// - private static X509Certificate2[] LoadIntunePemBundle(string fileName) + private static X509Certificate2[] LoadIntuneCaCertificates() { - var resourceName = $"AutopilotMonitor.Functions.Security.Certificates.{fileName}"; var assembly = Assembly.GetExecutingAssembly(); - - using var stream = assembly.GetManifestResourceStream(resourceName); - if (stream == null) - return []; - - using var reader = new StreamReader(stream); - var pem = reader.ReadToEnd(); - - if (pem.Contains("PLACEHOLDER_REPLACE_WITH_BASE64")) - return []; - - // PEM may contain multiple cert blocks (e.g. cross-signed roots); parse each. - const string begin = "-----BEGIN CERTIFICATE-----"; - const string end = "-----END CERTIFICATE-----"; - var certs = new List(); - int idx = 0; - while ((idx = pem.IndexOf(begin, idx, StringComparison.Ordinal)) >= 0) + var resourceNames = new[] { - var endIdx = pem.IndexOf(end, idx, StringComparison.Ordinal); - if (endIdx < 0) break; - var block = pem.Substring(idx, endIdx + end.Length - idx); - try { certs.Add(X509Certificate2.CreateFromPem(block)); } - catch { /* malformed block - skip; chain.Build() fails closed if no roots remain */ } - idx = endIdx + end.Length; + "AutopilotMonitor.Functions.Security.Certificates.intune-intermediate-ca.pem", + "AutopilotMonitor.Functions.Security.Certificates.intune-root-ca.pem" + }; + + List certs = []; + + foreach (var resourceName in resourceNames) + { + using var stream = assembly.GetManifestResourceStream(resourceName); + if (stream == null) + { + // Resource not found - not fatal, chain validation will fall back to OS store + continue; + } + + using var reader = new StreamReader(stream); + var pem = reader.ReadToEnd(); + + // Skip placeholder files that haven't been filled with a real cert yet + if (pem.Contains("PLACEHOLDER_REPLACE_WITH_BASE64")) + continue; + + try + { + var cert = X509Certificate2.CreateFromPem(pem); + certs.Add(cert); + } + catch + { + // Malformed PEM - skip silently, chain validation continues with OS store + } } + return certs.ToArray(); } }