From 249e39bc687b00950476f2c7918d4e46c3e22f02 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Mon, 24 Aug 2026 16:03:05 +0200 Subject: [PATCH] feat(security): cert tenant binding, stage 1 shadow mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chain-valid Intune MDM certificate only proves "issued to some tenant" — the pinned roots are shared by every Intune tenant. Tenant scoping rested entirely on the device serial in the downstream validators, which is not a secret. Bind the tenant stamped into the auth cert to the requested tenant. The Entra tenant id sits in OID 1.2.840.113556.5.14 of the cert the agent already presents for mTLS, so this needs no Graph call, no consent and no agent release. Not 5.6 (Intune Account id, a different GUID) and not 5.4 (device id, stored as 16 bare bytes) — the decoder handles both encodings and now lives once in Shared, used by agent and backend. Stage 1 observes only: ObserveCertTenantBinding returns void and swallows its own exceptions, so it cannot fail an otherwise valid request. Match is logged only on cert-cache miss, keeping the denominator without per-request volume. Grep marker for stage 2: CERT-TENANT-BINDING-SHADOW. Backend 4604/4604, Agent V2.Core 2519/2519, 0 warnings. Co-Authored-By: Claude Opus 5 (1M context) --- docs/backend/client-cert-tenant-binding.md | 127 +++++++ docs/index.md | 1 + docs/log.md | 4 + .../Security/EntraDeviceCertHelper.cs | 65 +--- .../CertTenantBindingTests.cs | 330 ++++++++++++++++++ .../Security/CertTenantBinding.cs | 81 +++++ .../Security/CertificateValidator.cs | 91 ++++- .../Security/SecurityValidator.cs | 71 ++++ .../Security/MsDeviceCertificateOids.cs | 130 +++++++ 9 files changed, 846 insertions(+), 54 deletions(-) create mode 100644 docs/backend/client-cert-tenant-binding.md create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/CertTenantBindingTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs create mode 100644 src/Shared/AutopilotMonitor.Shared/Security/MsDeviceCertificateOids.cs diff --git a/docs/backend/client-cert-tenant-binding.md b/docs/backend/client-cert-tenant-binding.md new file mode 100644 index 00000000..0f846161 --- /dev/null +++ b/docs/backend/client-cert-tenant-binding.md @@ -0,0 +1,127 @@ +--- +type: Concept +title: Client-Certificate Tenant Binding — Closing the Cross-Tenant Gap +description: Why a chain-valid Intune MDM certificate does not prove the caller belongs to the tenant it claims, which certificate extension carries the Entra tenant id (and which two look like it but do not), and the shadow-then-enforce rollout that measures field coverage before it can lock devices out. +resource: src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs +tags: + - backend + - security + - authentication + - certificates + - intune + - multi-tenant +timestamp: 2026-08-24T00:00:00+02:00 +--- + +# Problem + +Agent requests authenticate with mTLS. `CertificateValidator` pins the chain to the embedded +Intune roots (`X509ChainTrustMode.CustomRootTrust`), checks the validity window and requires the +Client-Authentication EKU. All of that together proves exactly one thing: **the certificate was +issued by the Microsoft Intune MDM Device CA to some tenant**. + +Those roots are shared by every Intune tenant on the planet. Nothing in the certificate stage ties +the caller to the tenant in the request. The tenant scoping came entirely from the downstream +device validators — the serial number must appear in that tenant's Autopilot / imported-device / +Cloud PC inventory. Device serial numbers are not secrets; they are printed on the chassis. + +So the pre-existing attack was: enroll a device in your own Intune tenant, obtain a legitimate +device certificate, learn one serial number of the victim tenant, and submit telemetry as that +tenant. Certificate validation passes, device validation passes. + +# Mechanism + +The Intune MDM Device CA stamps the customer's identifiers into the leaf certificate. Since the +agent presents this certificate in the TLS handshake, its contents are proof of possession and the +backend can compare them against the requested tenant — no Graph call, no additional consent, and +nothing the agent has to send. + +## Which extension carries what + +Verified by decoding a real field certificate (issued 2026-05-08, `Functions.Tests/device-cert-sample.pem`): + +| OID | Encoding | Content | +| --- | --- | --- | +| `1.2.840.113556.5.14` | nested OCTET STRING (`04 10` + 16 bytes) | **Entra tenant id** — the one to compare | +| `1.2.840.113556.5.6` | nested OCTET STRING (`04 10` + 16 bytes) | Intune **Account** id — a *different* GUID, never equal to the tenant id | +| `1.2.840.113556.5.4` | **16 raw bytes, no wrapper** | Intune device id, identical to the certificate's Subject CN | + +Two traps are worth stating explicitly, because both cost real debugging time: + +1. `…5.6` is widely described as "the tenant id (AccountID)". It is not the Entra tenant id. + Comparing it against the requested tenant would reject every request in production. +2. The encodings are not uniform. `…5.4` holds the GUID bare while the other two nest it in a + second OCTET STRING, so a decoder written against one shape silently fails on the other. + `MsDeviceCertificateOids.TryParseGuid` accepts both, and all GUIDs are Microsoft + little-endian (what `new Guid(byte[])` expects). + +A 16-byte input is inherently ambiguous — it is indistinguishable from a truncated wrapper and is +always read as a bare GUID. That is harmless here: the value is only ever compared for equality +against a known tenant id, never trusted as a standalone claim. + +## Where it is NOT + +`EntraDeviceCertHelper` reads tenant and device ids from the **MS-Organization-Access** certificate +(`1.2.840.113556.1.5.284.*`). That certificate is not part of the authentication path at all — the +agent selects the Intune MDM certificate by exact issuer match (`CN=Microsoft Intune MDM Device CA`, +see `CertificateHelper.SelectMdmCertificate`) and the backend pins only the MDM chain. The Entra +certificate is used purely as a local fallback for `TenantIdResolver`; it never leaves the device. + +Sending it in a header instead would prove nothing: only the certificate used in the TLS handshake +demonstrates possession of a private key. A certificate copied into a custom header can be replayed +by anyone who has seen it once. + +# Rollout + +Enforcing immediately would be reckless: any device population whose certificates predate the +extension, or whose CA behaves differently (sovereign clouds), would be locked out at once. The +rollout therefore has staged consequences. + +**Stage 1 — shadow (current).** `SecurityValidator.ObserveCertTenantBinding` evaluates the binding +after certificate validation and logs the outcome. It returns `void`, changes no state, and swallows +its own exceptions, so it cannot fail a request that would otherwise be accepted. The pure comparison +lives in `CertTenantBinding.Evaluate`; the outcome codes (`Match`, `Mismatch`, `ExtensionMissing`, +`Unparseable`, `RequestTenantNotAGuid`) are stable and queried by exact match in KQL. +`WouldRejectUnderEnforcement` records what stage 2 *would* have done, so the telemetry can be read +without re-deriving the rule at query time. + +Volume control: the outcomes that inform the decision (mismatch, missing, undecodable) are always +logged, because they are expected to be rare and each is individually interesting. The bulk `Match` +case is logged only when the certificate was freshly validated rather than served from the 5-minute +thumbprint cache (`CertificateValidationResult.FromCache`). Both numerator and denominator are +therefore counted per certificate-window rather than per request, which keeps the ratio meaningful +while cutting the volume by roughly the request-per-device factor. + +**Stage 2 — enforce.** Once telemetry shows the `ExtensionMissing` rate is negligible, `Mismatch` +becomes a rejection. The decision that still has to be made is the policy for certificates without +the extension: skip with a warning, or reject. Grep marker for every stage-2 site: +`CERT-TENANT-BINDING-SHADOW`. + +The bootstrap-token path (`X-Bootstrap-Token`) bypasses certificate validation by design and is +unaffected by either stage. + +# Examples + +Read the shadow telemetry: + +```kusto +traces +| where message has "AgentCertTenantBinding" +| extend Outcome = tostring(customDimensions.Outcome), + Tenant = tostring(customDimensions.TenantId) +| summarize count() by Outcome, Tenant +``` + +Anything in `Mismatch` is a cross-tenant certificate; anything in `ExtensionMissing` is a device +that stage-2 enforcement would have to make a policy decision about. + +# Citations + +* `src/Shared/AutopilotMonitor.Shared/Security/MsDeviceCertificateOids.cs` — OID catalog and the + shared ASN.1 GUID decoder (one implementation for agent and backend). +* `src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs` — extension extraction + alongside chain validation, cached per thumbprint. +* `src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs` — the pure comparison. +* `src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs` — shadow observation. +* [W365 Cloud PC Device Validation](cloudpc-device-validation.md) — the other consumer of this + certificate's identity fields (Subject CN = Intune device id). diff --git a/docs/index.md b/docs/index.md index 52d5d163..1c140072 100644 --- a/docs/index.md +++ b/docs/index.md @@ -76,6 +76,7 @@ https://docs.autopilotmonitor.com (separate repository). # Security & Identity * [MCP OAuth Flow](mcp-oauth-flow.md) - Who authenticates where when connecting an AI client to the MCP server; two identities, three parties plus Entra ID. +* [Client-Certificate Tenant Binding](backend/client-cert-tenant-binding.md) - Why a chain-valid Intune MDM certificate only proves "issued to some tenant", which extension actually carries the Entra tenant id (`…5.14`) versus the two that look like it (`…5.6` Intune Account id, `…5.4` device id, different encodings), why the Entra join certificate is not part of the auth path, and the shadow-then-enforce rollout with its volume-controlled telemetry. # Conventions for this bundle diff --git a/docs/log.md b/docs/log.md index f01d15b5..a77998c1 100644 --- a/docs/log.md +++ b/docs/log.md @@ -1,5 +1,9 @@ # Log +## 2026-08-24 + +* **New**: `backend/client-cert-tenant-binding.md` — the agent's mTLS certificate chains to Microsoft's shared Intune roots, so certificate validation alone never proved tenant membership; tenant scoping rested entirely on the (non-secret) device serial in the downstream validators. The Intune MDM leaf carries the Entra tenant id in OID `1.2.840.113556.5.14` (nested OCTET STRING), which is NOT `…5.6` (Intune Account id, a different GUID) and NOT `…5.4` (device id, stored as 16 bare bytes) — all decoded by one shared `MsDeviceCertificateOids.TryParseGuid` now used by agent and backend. Stage 1 ships as shadow only: `CertTenantBinding.Evaluate` (pure) plus `SecurityValidator.ObserveCertTenantBinding` (returns void, swallows its own exceptions, cannot change an authorization outcome), with `Match` sampled via `CertificateValidationResult.FromCache` so the denominator survives without per-request volume. Grep marker for the stage-2 enforcement change: `CERT-TENANT-BINDING-SHADOW`. + ## 2026-08-23 * **New**: `backend/telemetry-ingest-shaping.md` — `RequestTelemetryMiddleware` sets `SamplingPercentage=100` on its own item (worker request rows were adaptively sampled, ItemCount 2–5 on weekdays, despite host.json `excludedTypes`), the redundant host HTTP request copy is dropped by a workspace transformation DCR (`infra/appinsights-workspace-transforms.bicep`, timer/queue rows kept), and `StorageDependencyFilterProcessor` now also drops expected storage outcomes (404/412/409, both InProc and HTTP shapes) and successful SignalR REST calls; the host-emitted SignalR binding calls (bulk of the volume, SDK `rdddsc`) are dropped by a second `AppDependencies` flow in the same DCR. Measured ≈ −1.3 GB/week. diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/EntraDeviceCertHelper.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/EntraDeviceCertHelper.cs index f3b05429..6fdc5392 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/EntraDeviceCertHelper.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Security/EntraDeviceCertHelper.cs @@ -2,6 +2,7 @@ using System; using System.Linq; using System.Security.Cryptography.X509Certificates; using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Shared.Security; namespace AutopilotMonitor.Agent.V2.Core.Security { @@ -49,8 +50,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Security public static class EntraDeviceCertHelper { public const string MsOrganizationAccessIssuer = "MS-Organization-Access"; - public const string TenantIdOid = "1.2.840.113556.1.5.284.5"; - public const string DeviceIdOid = "1.2.840.113556.1.5.284.2"; + public const string TenantIdOid = MsDeviceCertificateOids.EntraJoinCertTenantIdOid; + public const string DeviceIdOid = MsDeviceCertificateOids.EntraJoinCertDeviceIdOid; /// /// Reads the Entra TenantId from the MS-Organization-Access device cert. @@ -144,57 +145,17 @@ namespace AutopilotMonitor.Agent.V2.Core.Security /// 1.2.840.113556.5.4 — verified against a live Intune MDM device cert, whose raw value is /// exactly 16 bytes with no 04 10 prefix. /// - /// The two forms cannot collide: a valid OCTET-STRING wrapper around 16 bytes is at least 18 - /// bytes long, so a 16-byte payload is never a truncated wrapper even when it happens to start - /// with 0x04. + /// A valid OCTET-STRING wrapper around 16 bytes is at least 18 bytes long, so the two forms + /// never collide for well-formed input. A 16-byte value is always read as the bare form — + /// a truncated wrapper of exactly that length is indistinguishable from it. /// Returns null on any structural mismatch — never throws. + /// + /// The decoding itself lives in so the agent + /// and the backend's client-cert tenant binding share one implementation; this stays as the + /// agent-side nullable-returning façade. + /// /// - public static Guid? ParseGuidFromExtension(byte[] raw) - { - if (raw == null) return null; - - // Bare 16-byte GUID (Intune 1.2.840.113556.5.4) — no ASN.1 wrapper to strip. - if (raw.Length == 16) return ToGuid(raw, 0); - - if (raw.Length < 2) return null; - - // ASN.1 OCTET STRING tag. - if (raw[0] != 0x04) return null; - - int contentStart; - int contentLen; - - if (raw[1] < 0x80) - { - // short-form length - contentLen = raw[1]; - contentStart = 2; - } - else - { - // long-form length: lower 7 bits = number of following length bytes. - int numLenBytes = raw[1] & 0x7F; - if (numLenBytes < 1 || numLenBytes > 4 || raw.Length < 2 + numLenBytes) return null; - - contentLen = 0; - for (int i = 0; i < numLenBytes; i++) - contentLen = (contentLen << 8) | raw[2 + i]; - - contentStart = 2 + numLenBytes; - } - - if (contentLen != 16 || contentStart + 16 > raw.Length) return null; - - return ToGuid(raw, contentStart); - } - - private static Guid? ToGuid(byte[] raw, int offset) - { - var guidBytes = new byte[16]; - Buffer.BlockCopy(raw, offset, guidBytes, 0, 16); - - try { return new Guid(guidBytes); } - catch { return null; } - } + public static Guid? ParseGuidFromExtension(byte[] raw) => + MsDeviceCertificateOids.TryParseGuid(raw, out var value) ? value : (Guid?)null; } } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/CertTenantBindingTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/CertTenantBindingTests.cs new file mode 100644 index 00000000..93c8c1c7 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/CertTenantBindingTests.cs @@ -0,0 +1,330 @@ +using System.Net; +using System.Security.Cryptography.X509Certificates; +using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using AutopilotMonitor.Shared.Security; +using Microsoft.Azure.Functions.Worker.Http; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// CERT-TENANT-BINDING-SHADOW — stage 1 coverage. +/// +/// The agent authenticates with the Intune MDM device certificate, whose chain is pinned to +/// Microsoft's Intune roots. Those roots are shared by every Intune tenant, so a valid chain alone +/// does not prove the caller belongs to the tenant it claims. The CA stamps the Entra tenant id +/// into OID 1.2.840.113556.5.14, which lets the backend bind the two. +/// +/// These tests pin three things: +/// - the tenant id is actually extracted from a REAL field certificate (not a synthetic fixture), +/// - the pure comparison covers every outcome the telemetry can report, +/// - shadow mode observes without deciding: a cross-tenant certificate must still pass the +/// certificate stage in stage 1, or we would be enforcing before we measured. +/// +public class CertTenantBindingTests +{ + private const string DeviceSamplePem = "device-cert-sample.pem"; + + /// + /// Tenant stamped into device-cert-sample.pem by the Intune MDM Device CA. The sample is + /// a public-key-only cert from an internal test device; the GUID is the test tenant's own id. + /// + private static readonly Guid SampleCertTenantId = Guid.Parse("b54dc1af-5320-4f60-b5d4-821e0cf2a359"); + + private static string SampleCertBase64() + { + var assemblyDir = Path.GetDirectoryName(typeof(CertTenantBindingTests).Assembly.Location)!; + var pem = File.ReadAllText(Path.Combine(assemblyDir, DeviceSamplePem)); + using var cert = X509Certificate2.CreateFromPem(pem); + return Convert.ToBase64String(cert.Export(X509ContentType.Cert)); + } + + // ---------------------------------------------------------------- extraction + + [Fact] + public void ValidateCertificate_RealIntuneCert_ExtractsEntraTenantId() + { + // The load-bearing test for the whole feature: if Microsoft ever stops stamping OID + // 1.2.840.113556.5.14, or changes its encoding, enforcement would lock every device out. + var result = CertificateValidator.ValidateCertificate(SampleCertBase64()); + + Assert.True(result.IsValid, $"Real device cert rejected: {result.ErrorMessage}"); + Assert.Equal(CertTenantIdStatus.Present, result.CertTenantIdStatus); + Assert.Equal(SampleCertTenantId, result.CertTenantId); + } + + [Fact] + public void ValidateCertificate_CachedResult_KeepsTenantIdAndFlagsCacheHit() + { + // FromCache is what samples the high-volume "Match" telemetry down to ~one line per cert + // per cache window, so it has to survive the cache and not corrupt the cached entry. + var b64 = SampleCertBase64(); + + _ = CertificateValidator.ValidateCertificate(b64); + var second = CertificateValidator.ValidateCertificate(b64); + var third = CertificateValidator.ValidateCertificate(b64); + + Assert.True(second.FromCache); + Assert.Equal(SampleCertTenantId, second.CertTenantId); + + // The cache-hit copy must not have mutated the shared cached instance. + Assert.True(third.FromCache); + Assert.Equal(SampleCertTenantId, third.CertTenantId); + Assert.True(third.IsValid); + } + + // ---------------------------------------------------------------- encoding + + [Fact] + public void TryParseGuid_PinsRealFieldEncodings() + { + // Exact bytes lifted from the sample cert. OID 5.14 nests the GUID in an OCTET STRING, + // OID 5.4 stores the 16 bytes bare — both shapes must decode, and both are little-endian. + var wrapped = Convert.FromHexString("0410afc14db52053604fb5d4821e0cf2a359"); + var bare = Convert.FromHexString("6701bc076190b743b2430bb6aeda736e"); + + Assert.True(MsDeviceCertificateOids.TryParseGuid(wrapped, out var tenantId)); + Assert.Equal(SampleCertTenantId, tenantId); + + Assert.True(MsDeviceCertificateOids.TryParseGuid(bare, out var deviceId)); + // The Intune device id is also the certificate's Subject CN. + Assert.Equal(Guid.Parse("07bc0167-9061-43b7-b243-0bb6aeda736e"), deviceId); + } + + [Fact] + public void TryParseGuid_AccountIdOidIsNotTheTenantId() + { + // Guards the mistake this feature was nearly built on: OID 5.6 carries the Intune ACCOUNT + // id, a different GUID. Comparing it against the tenant id would reject every request. + var accountIdExt = Convert.FromHexString("04103d85dede74e26b409d0d46b6fe65b440"); + + Assert.True(MsDeviceCertificateOids.TryParseGuid(accountIdExt, out var accountId)); + Assert.NotEqual(SampleCertTenantId, accountId); + } + + [Theory] + [InlineData("")] // empty + [InlineData("0400")] // OCTET STRING, zero length + [InlineData("020102")] // not an OCTET STRING + [InlineData("0411afc14db52053604fb5d4821e0cf2a35900")] // wrapper claims 17 bytes + [InlineData("0410afc14db52053604fb5d4821e0cf2a3")] // wrapper claims 16, only 15 present + public void TryParseGuid_RejectsMalformed(string hex) + { + Assert.False(MsDeviceCertificateOids.TryParseGuid(Convert.FromHexString(hex), out _)); + } + + [Fact] + public void TryParseGuid_AnySixteenBytesDecodeAsBareGuid() + { + // Documents a deliberate limit rather than a gap: the bare form (OID 5.4) is exactly 16 + // bytes with no tag, so a 16-byte input is indistinguishable from a truncated wrapper and + // is always read as a GUID. Harmless here — the value is only ever compared for equality + // against a known tenant id, never trusted as a claim in its own right. + Assert.True(MsDeviceCertificateOids.TryParseGuid( + Convert.FromHexString("0410afc14db52053604fb5d4821e0cf2"), out _)); + } + + [Fact] + public void TryParseGuid_RejectsNull() + { + Assert.False(MsDeviceCertificateOids.TryParseGuid(null!, out _)); + } + + // ---------------------------------------------------------------- comparison + + [Fact] + public void Evaluate_MatchingTenant_ReturnsMatch() + { + var outcome = CertTenantBinding.Evaluate( + SampleCertTenantId, CertTenantIdStatus.Present, SampleCertTenantId.ToString()); + + Assert.Equal(CertTenantBinding.Outcome.Match, outcome); + Assert.False(CertTenantBinding.WouldRejectUnderEnforcement(outcome)); + } + + [Fact] + public void Evaluate_MatchingTenant_IsCaseInsensitive() + { + // Tenant ids travel as strings in headers/routes and casing must not decide access. + var outcome = CertTenantBinding.Evaluate( + SampleCertTenantId, CertTenantIdStatus.Present, SampleCertTenantId.ToString().ToUpperInvariant()); + + Assert.Equal(CertTenantBinding.Outcome.Match, outcome); + } + + [Fact] + public void Evaluate_ForeignTenantCert_ReturnsMismatch() + { + // The attack this feature exists for: a valid Intune cert from the attacker's own tenant, + // replayed against a victim tenant whose device serial the attacker knows. + var outcome = CertTenantBinding.Evaluate( + SampleCertTenantId, CertTenantIdStatus.Present, "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + + Assert.Equal(CertTenantBinding.Outcome.Mismatch, outcome); + Assert.True(CertTenantBinding.WouldRejectUnderEnforcement(outcome)); + } + + [Theory] + [InlineData(CertTenantIdStatus.ExtensionMissing, CertTenantBinding.Outcome.ExtensionMissing)] + [InlineData(CertTenantIdStatus.Unparseable, CertTenantBinding.Outcome.Unparseable)] + public void Evaluate_UndecodableCert_ReportsWhyWithoutClaimingMismatch( + CertTenantIdStatus status, string expected) + { + // These must stay distinguishable from Mismatch: they measure rollout readiness + // (how many field certs can be enforced against), not an attack. + var outcome = CertTenantBinding.Evaluate(null, status, SampleCertTenantId.ToString()); + + Assert.Equal(expected, outcome); + Assert.False(CertTenantBinding.WouldRejectUnderEnforcement(outcome)); + } + + [Fact] + public void Evaluate_PresentButNullGuid_FallsBackToExtensionMissing() + { + var outcome = CertTenantBinding.Evaluate(null, CertTenantIdStatus.Present, SampleCertTenantId.ToString()); + + Assert.Equal(CertTenantBinding.Outcome.ExtensionMissing, outcome); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not-a-guid")] + public void Evaluate_NonGuidRequestTenant_IsReportedSeparately(string? requestedTenantId) + { + var outcome = CertTenantBinding.Evaluate( + SampleCertTenantId, CertTenantIdStatus.Present, requestedTenantId); + + Assert.Equal(CertTenantBinding.Outcome.RequestTenantNotAGuid, outcome); + Assert.False(CertTenantBinding.WouldRejectUnderEnforcement(outcome)); + } + + // ---------------------------------------------------------------- shadow mode + + [Fact] + public async Task ValidateRequestAsync_ForeignTenantCert_IsObservedButNotRejected() + { + // Stage 1 contract: a cross-tenant certificate is logged and still passes the certificate + // stage. If this test ever fails, enforcement was switched on without the telemetry review. + var logger = new CapturingLogger(); + var validator = BuildValidator(logger); + + var result = await validator.ValidateRequestAsync( + BuildRequestWithCert(SampleCertBase64()), + tenantId: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + + // Deliberately accepted: stage 1 measures, it does not enforce. This assertion is the one + // that must be inverted (to a 403) when stage 2 turns enforcement on. + Assert.True(result.IsValid, $"Shadow mode changed the outcome: {result.ErrorMessage} / {result.Details}"); + Assert.NotEqual(HttpStatusCode.Unauthorized, result.StatusCode); + + var observed = Assert.Single(logger.Entries, e => e.Message.Contains("AgentCertTenantBinding")); + Assert.Equal(LogLevel.Warning, observed.Level); + Assert.Contains(CertTenantBinding.Outcome.Mismatch, observed.Message); + Assert.Contains("enforced=False", observed.Message); + Assert.Contains("wouldReject=True", observed.Message); + } + + [Fact] + public async Task ValidateRequestAsync_MatchingTenantCert_IsAccepted() + { + var logger = new CapturingLogger(); + var validator = BuildValidator(logger); + + var result = await validator.ValidateRequestAsync( + BuildRequestWithCert(SampleCertBase64()), + tenantId: SampleCertTenantId.ToString()); + + Assert.True(result.IsValid, $"Matching-tenant request rejected: {result.ErrorMessage}"); + + // Match is only logged on a fresh validation; the cache is process-wide and shared with the + // other tests here, so assert the outcome only when a line was actually emitted. + var observed = logger.Entries.FirstOrDefault(e => e.Message.Contains("AgentCertTenantBinding")); + if (observed != null) + { + Assert.Equal(LogLevel.Information, observed.Level); + Assert.Contains(CertTenantBinding.Outcome.Match, observed.Message); + } + } + + // ---------------------------------------------------------------- harness + + private static SecurityValidator BuildValidator(ILogger logger) + { + var configRepo = Mock.Of(); + var cache = new MemoryCache(new MemoryCacheOptions()); + + // AllowInsecureAgentRequests clears the "at least one device validator" gate so the request + // reaches — and passes — the certificate stage without needing Graph mocks. + var config = new TenantConfiguration + { + AllowInsecureAgentRequests = true, + ManufacturerWhitelist = "*", + ModelWhitelist = "*", + }; + + var configServiceMock = new Mock( + configRepo, Mock.Of>(), cache) + { CallBase = false }; + configServiceMock + .Setup(x => x.TryGetConfigurationAsync(It.IsAny())) + .ReturnsAsync((config, true)); + + var adminConfigServiceMock = new Mock( + configRepo, Mock.Of>(), cache) + { CallBase = false }; + adminConfigServiceMock + .Setup(x => x.GetConfigurationAsync()) + .ReturnsAsync(new AdminConfiguration()); + + return new SecurityValidator( + configService: configServiceMock.Object, + adminConfigService: adminConfigServiceMock.Object, + rateLimitService: new RateLimitService(cache, Mock.Of>()), + autopilotDeviceValidator: null!, + corporateIdentifierValidator: null!, + logger: logger, + bootstrapSessionService: null, + deviceAssociationValidator: null); + } + + private static HttpRequestData BuildRequestWithCert(string certBase64) + { + var contextMock = new Mock(); + var reqMock = new Mock(contextMock.Object); + + var headers = new HttpHeadersCollection + { + { "X-ARR-ClientCert", certBase64 }, + // The hardware whitelist stage runs after the certificate stage and rejects requests + // with no hardware headers, which would mask what these tests are actually asserting. + { "X-Device-Manufacturer", "Contoso" }, + { "X-Device-Model", "TestBook" }, + { "X-Device-SerialNumber", "TESTSERIAL01" }, + }; + reqMock.SetupGet(r => r.Headers).Returns(headers); + reqMock.SetupGet(r => r.Url).Returns(new Uri("https://example.invalid/api/telemetry")); + + return reqMock.Object; + } + + private sealed record LogEntry(LogLevel Level, string Message); + + private sealed class CapturingLogger : ILogger + { + public List Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Entries.Add(new LogEntry(logLevel, formatter(state, exception))); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs b/src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs new file mode 100644 index 00000000..5b5137ce --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Security/CertTenantBinding.cs @@ -0,0 +1,81 @@ +using System; + +namespace AutopilotMonitor.Functions.Security +{ + /// + /// Compares the Entra TenantId stamped into the agent's mTLS client certificate against the + /// tenant the request claims to belong to. + /// + /// Without this, a validated certificate only proves "issued by the Intune MDM Device CA to + /// some tenant" — the chain is pinned to Microsoft's roots, which every Intune tenant on the + /// planet shares. The tenant scoping comes solely from the downstream device validators, so an + /// attacker with their own Intune tenant and a known serial number of the victim tenant would + /// pass certificate validation. Binding the cert's tenant to the requested tenant closes that. + /// + /// + /// + /// SHADOW MODE (stage 1): the outcome is observed and logged only — it never changes an + /// authorization decision. Enforcement is stage 2, once telemetry shows how many field + /// certificates actually carry the extension. Grep marker for the stage-2 change: + /// CERT-TENANT-BINDING-SHADOW. + /// + public static class CertTenantBinding + { + /// + /// Stable outcome codes emitted in the AgentCertTenantBinding structured log. + /// Keep these strings stable — they are queried by exact match in KQL + /// (customDimensions.Outcome == "Mismatch"). + /// + public static class Outcome + { + /// Certificate tenant equals the requested tenant — the expected case. + public const string Match = "Match"; + + /// Certificate belongs to a different tenant than the request claims. + public const string Mismatch = "Mismatch"; + + /// Certificate carries no tenant extension (older enrollment / unexpected CA). + public const string ExtensionMissing = "ExtensionMissing"; + + /// Tenant extension present but undecodable. + public const string Unparseable = "Unparseable"; + + /// Requested tenant id was not a GUID, so no meaningful comparison is possible. + public const string RequestTenantNotAGuid = "RequestTenantNotAGuid"; + } + + /// + /// Evaluates the binding. Pure — no logging, no I/O, no side effects. + /// + /// Tenant GUID decoded from the client certificate, if any. + /// Whether the certificate carried a decodable tenant extension. + /// The tenant id the request is scoped to. + /// One of the codes. + public static string Evaluate(Guid? certTenantId, CertTenantIdStatus status, string? requestedTenantId) + { + switch (status) + { + case CertTenantIdStatus.ExtensionMissing: + return Outcome.ExtensionMissing; + case CertTenantIdStatus.Unparseable: + return Outcome.Unparseable; + } + + if (certTenantId == null) + return Outcome.ExtensionMissing; + + if (!Guid.TryParse(requestedTenantId, out var requested)) + return Outcome.RequestTenantNotAGuid; + + return certTenantId.Value == requested ? Outcome.Match : Outcome.Mismatch; + } + + /// + /// Whether an outcome would block the request once stage-2 enforcement is switched on. + /// Nothing consumes this for authorization yet — it exists so the shadow telemetry can be + /// read as "would this have been rejected?" without re-deriving the rule at query time. + /// + public static bool WouldRejectUnderEnforcement(string outcome) => + outcome == Outcome.Mismatch; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs index a173415c..4c5e23ed 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/CertificateValidator.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Reflection; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using AutopilotMonitor.Shared.Security; using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Security @@ -138,7 +139,8 @@ namespace AutopilotMonitor.Functions.Security if (cached.ExpiresAt > DateTime.UtcNow) { logger?.LogDebug($"Certificate validation cache HIT for thumbprint {thumbprint}"); - return cached.Result; + // Copy so FromCache is per-call state and never mutates the shared cached instance. + return cached.Result.AsCacheHit(); } else { @@ -253,6 +255,12 @@ namespace AutopilotMonitor.Functions.Security }; } + // 4. Extract the Entra TenantId the CA stamped into the cert (OID 1.2.840.113556.5.14). + // Pure function of the cert, so it is cached alongside the validation result. + // Read only - the tenant binding it enables is evaluated (and currently only observed) + // by SecurityValidator; see CertTenantBinding. + var (certTenantId, certTenantIdStatus) = ExtractEntraTenantId(certificate); + // All checks passed - create and cache success result logger?.LogInformation($"Certificate validated successfully: Thumbprint={thumbprint}"); var successResult = new CertificateValidationResult @@ -260,7 +268,9 @@ namespace AutopilotMonitor.Functions.Security IsValid = true, Thumbprint = thumbprint, Subject = certificate.Subject, - Issuer = certificate.Issuer + Issuer = certificate.Issuer, + CertTenantId = certTenantId, + CertTenantIdStatus = certTenantIdStatus }; // Cache successful validation @@ -288,6 +298,30 @@ namespace AutopilotMonitor.Functions.Security } } + /// + /// Reads the Entra TenantId the Intune MDM Device CA stamps into the client certificate + /// (OID 1.2.840.113556.5.14). Never throws — a cert without the extension, or with an + /// undecodable one, yields the corresponding status instead of an error, because this value + /// is currently observed only and must never turn a valid cert into a rejected one. + /// + /// + /// Deliberately NOT OID 1.2.840.113556.5.6: that one carries the Intune Account + /// ID, which is a different GUID from the Entra tenant ID. + /// + private static (Guid? TenantId, CertTenantIdStatus Status) ExtractEntraTenantId(X509Certificate2 certificate) + { + var ext = certificate.Extensions + .OfType() + .FirstOrDefault(e => e.Oid?.Value == MsDeviceCertificateOids.IntuneCertEntraTenantIdOid); + + if (ext?.RawData == null) + return (null, CertTenantIdStatus.ExtensionMissing); + + return MsDeviceCertificateOids.TryParseGuid(ext.RawData, out var tenantId) + ? (tenantId, CertTenantIdStatus.Present) + : (null, CertTenantIdStatus.Unparseable); + } + /// /// Loads all certs whose embedded resource filename starts with the given prefix. /// Multiple PEM blocks per file are supported (cross-signed bundles). Duplicates @@ -378,5 +412,58 @@ namespace AutopilotMonitor.Functions.Security /// Error message if validation failed /// public string? ErrorMessage { get; set; } + + /// + /// Entra TenantId stamped into the certificate by the Intune MDM Device CA + /// (OID 1.2.840.113556.5.14), or null when + /// is not . + /// + public Guid? CertTenantId { get; set; } + + /// + /// Whether the certificate carried a decodable Entra TenantId extension. + /// + public CertTenantIdStatus CertTenantIdStatus { get; set; } + + /// + /// True when this result was served from the 5-minute thumbprint cache rather than freshly + /// validated. Used to sample per-request observability down to roughly one line per + /// certificate per cache window. + /// + public bool FromCache { get; set; } + + /// + /// Returns a copy of this result marked as a cache hit, so the shared cached instance is + /// never mutated by a caller. + /// + internal CertificateValidationResult AsCacheHit() => new() + { + IsValid = IsValid, + Thumbprint = Thumbprint, + Subject = Subject, + Issuer = Issuer, + ErrorMessage = ErrorMessage, + CertTenantId = CertTenantId, + CertTenantIdStatus = CertTenantIdStatus, + FromCache = true + }; + } + + /// + /// Whether a validated client certificate carried a decodable Entra TenantId extension. + /// + public enum CertTenantIdStatus + { + /// Certificate was not validated far enough to look, or validation failed. + NotEvaluated = 0, + + /// Extension present and decoded into a GUID. + Present = 1, + + /// Certificate carries no such extension (expected on older enrollments). + ExtensionMissing = 2, + + /// Extension present but its value could not be decoded as a GUID. + Unparseable = 3 } } diff --git a/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs b/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs index 8b1e9917..09f77563 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/SecurityValidator.cs @@ -275,6 +275,12 @@ namespace AutopilotMonitor.Functions.Security }; } + // 1b. CERT-TENANT-BINDING-SHADOW — observe only, never decide. + // The cert chain is pinned to Microsoft's Intune roots, which every Intune tenant shares, + // so a valid cert alone does not prove the caller belongs to THIS tenant. Stage 1 measures + // how reliably the certificate's own tenant stamp matches before stage 2 enforces it. + ObserveCertTenantBinding(certValidation, tenantId, req, sessionId); + // 2. Check rate limit (DoS protection) // Effective limit = per-tenant override if set, otherwise the global default. var rateLimitValue = await ResolveDeviceRateLimitAsync(config); @@ -514,6 +520,71 @@ namespace AutopilotMonitor.Functions.Security return false; } + /// + /// CERT-TENANT-BINDING-SHADOW — stage 1 observability for the client-certificate tenant + /// binding. Evaluates whether the tenant stamped into the agent's certificate matches the + /// tenant the request claims, and logs the outcome. It deliberately returns nothing and + /// changes no state: enforcement is stage 2, after this telemetry shows the miss rate. + /// + /// + /// Volume control: outcomes that need enforcement decisions (mismatch, missing or + /// undecodable extension) are always logged — they are expected to be rare and each one is + /// individually interesting. The bulk "Match" case is logged only when the certificate was + /// freshly validated rather than served from the 5-minute thumbprint cache, which samples it + /// down to roughly one line per certificate per cache window while keeping the ratio between + /// numerator and denominator meaningful (both are counted per cert-window, not per request). + /// + private void ObserveCertTenantBinding( + CertificateValidationResult certValidation, + string tenantId, + HttpRequestData req, + string? sessionId) + { + try + { + var outcome = CertTenantBinding.Evaluate( + certValidation.CertTenantId, + certValidation.CertTenantIdStatus, + tenantId); + + if (outcome == CertTenantBinding.Outcome.Match && certValidation.FromCache) + return; + + var agentVersion = req.Headers.Contains("X-Agent-Version") + ? req.Headers.GetValues("X-Agent-Version").FirstOrDefault() ?? "n/a" + : "n/a"; + + // Only the cert's tenant is logged, never the raw certificate. On a mismatch this is + // the attacker-controlled side of the comparison and the single most useful field. + const string template = + "AgentCertTenantBinding outcome={Outcome} enforced={Enforced} wouldReject={WouldReject} " + + "tenant={TenantId} certTenant={CertTenantId} status={Status} thumbprint={Thumbprint} " + + "session={SessionId} ver={AgentVersion}"; + + var certTenant = certValidation.CertTenantId?.ToString() ?? "n/a"; + var wouldReject = CertTenantBinding.WouldRejectUnderEnforcement(outcome); + + if (outcome == CertTenantBinding.Outcome.Match) + { + _logger.LogInformation(template, + outcome, false, wouldReject, tenantId, certTenant, certValidation.CertTenantIdStatus, + certValidation.Thumbprint ?? "n/a", sessionId ?? "n/a", agentVersion); + } + else + { + _logger.LogWarning(template, + outcome, false, wouldReject, tenantId, certTenant, certValidation.CertTenantIdStatus, + certValidation.Thumbprint ?? "n/a", sessionId ?? "n/a", agentVersion); + } + } + catch (Exception ex) + { + // Shadow-mode observability must never be able to fail a request that would + // otherwise have been accepted. + _logger.LogWarning(ex, "AgentCertTenantBinding observation failed for tenant {TenantId}", tenantId); + } + } + /// /// Emits a structured warning with request-side context (path, hardware headers, agent /// version) at every fail-fast rejection point. Pairs with the cert-side warnings emitted diff --git a/src/Shared/AutopilotMonitor.Shared/Security/MsDeviceCertificateOids.cs b/src/Shared/AutopilotMonitor.Shared/Security/MsDeviceCertificateOids.cs new file mode 100644 index 00000000..76d5e524 --- /dev/null +++ b/src/Shared/AutopilotMonitor.Shared/Security/MsDeviceCertificateOids.cs @@ -0,0 +1,130 @@ +using System; + +namespace AutopilotMonitor.Shared.Security +{ + /// + /// Microsoft device-certificate extension OIDs and the shared ASN.1 decoding for the + /// GUID-bearing ones. Canonical for both the agent (local identity probing) and the + /// backend (client-cert tenant binding) so the two cannot drift apart. + /// + /// + /// Two certificate families carry these, and they are NOT interchangeable: + /// + /// + /// MS-Organization-Access (Entra/AAD-Join, 1.2.840.113556.1.5.284.*) — installed + /// on AAD-join. The agent reads it locally to learn its own TenantId; it is never used for + /// backend authentication. + /// + /// + /// Microsoft Intune MDM Device CA (1.2.840.113556.5.*) — this is the cert the + /// agent presents as the mTLS client certificate, so it is the only one whose contents the + /// backend can trust as proof of possession. + /// + /// + /// Values verified against a real field certificate (issued 2026-05-08) — see + /// Functions.Tests/device-cert-sample.pem. Note that the Intune Account ID + /// (5.6) is a DIFFERENT GUID from the Entra tenant ID (5.14); they must never + /// be compared against each other. + /// + public static class MsDeviceCertificateOids + { + /// Issuer CN of the Intune MDM device certificate used for mTLS client auth. + public const string IntuneMdmIssuer = "CN=Microsoft Intune MDM Device CA"; + + /// + /// Intune MDM device certificate → Entra (Azure AD) TenantId. Encoded as a nested + /// ASN.1 OCTET STRING wrapping a 16-byte little-endian GUID. + /// + public const string IntuneCertEntraTenantIdOid = "1.2.840.113556.5.14"; + + /// + /// Intune MDM device certificate → Intune Account ID. Distinct from the Entra + /// tenant ID — do not use it for tenant comparison. + /// + public const string IntuneCertAccountIdOid = "1.2.840.113556.5.6"; + + /// + /// Intune MDM device certificate → Intune device ID (matches the certificate's Subject CN). + /// Unlike the other two this extension holds the 16 GUID bytes raw, without the + /// nested OCTET STRING wrapper — decode it with , which accepts + /// both shapes. + /// + public const string IntuneCertDeviceIdOid = "1.2.840.113556.5.4"; + + /// MS-Organization-Access (Entra join) certificate → TenantId. + public const string EntraJoinCertTenantIdOid = "1.2.840.113556.1.5.284.5"; + + /// MS-Organization-Access (Entra join) certificate → DeviceId. + public const string EntraJoinCertDeviceIdOid = "1.2.840.113556.1.5.284.2"; + + /// + /// Decodes a 16-byte Microsoft binary GUID out of an X.509 extension value + /// (X509Extension.RawData). + /// + /// Accepts both encodings seen in the wild: the GUID wrapped in a nested ASN.1 OCTET + /// STRING (short form 04 10 … and long form 04 81 10 …), and the 16 raw + /// bytes with no wrapper at all (Intune OID 5.4). + /// + /// The byte order is Microsoft's — little-endian for the first three groups, i.e. exactly + /// what expects. + /// + /// true and the decoded GUID, or false on any structural mismatch. Never throws. + public static bool TryParseGuid(byte[] raw, out Guid value) + { + value = Guid.Empty; + if (raw == null) return false; + + // Unwrapped form: exactly the 16 GUID bytes (Intune OID 5.4). + if (raw.Length == 16) + return TryFromBytes(raw, 0, out value); + + if (raw.Length < 2) return false; + + // ASN.1 OCTET STRING tag. + if (raw[0] != 0x04) return false; + + int contentStart; + int contentLen; + + if (raw[1] < 0x80) + { + // short-form length + contentLen = raw[1]; + contentStart = 2; + } + else + { + // long-form length: lower 7 bits = number of following length bytes. + int numLenBytes = raw[1] & 0x7F; + if (numLenBytes < 1 || numLenBytes > 4 || raw.Length < 2 + numLenBytes) return false; + + contentLen = 0; + for (int i = 0; i < numLenBytes; i++) + contentLen = (contentLen << 8) | raw[2 + i]; + + contentStart = 2 + numLenBytes; + } + + if (contentLen != 16 || contentStart + 16 > raw.Length) return false; + + return TryFromBytes(raw, contentStart, out value); + } + + private static bool TryFromBytes(byte[] source, int offset, out Guid value) + { + value = Guid.Empty; + var guidBytes = new byte[16]; + Buffer.BlockCopy(source, offset, guidBytes, 0, 16); + + try + { + value = new Guid(guidBytes); + return true; + } + catch + { + return false; + } + } + } +}