fix(agent): cert GUID parser accepts bare 16-byte extension values

Intune's MDM device cert stores the DeviceId in OID 1.2.840.113556.5.4 as a bare 16-byte GUID with no ASN.1 wrapper, so the strict OCTET-STRING parser bailed on the tag check and returned null. Verified against a live Intune device cert: 5.4 is 16 raw bytes, while 5.6 / 5.11 / 5.14 carry the usual 04 10 wrapper. - ParseGuidFromAsn1OctetString -> ParseGuidFromExtension (the old name no longer describes what it parses); both encodings now handled, with the shared LE-GUID construction pulled into ToGuid(). The forms cannot collide: a wrapped 16-byte GUID is always >= 18 bytes. - Document the Intune MDM cert OID arc on the class: 5.4 DeviceId, 5.6 Intune AccountId (NOT the Entra tenant), 5.14 Entra TenantId, 5.11 undocumented GUID, 5.15/.16/.17/.18/.19/.23 INTEGER enums. - 13 tests pinning both encodings plus the malformed-input rejections, including a bare GUID whose first byte is the 0x04 OCTET STRING tag. No behaviour change for the MS-Organization-Access probe the agent uses; TryGetTenantIdFromCert is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-24 12:42:38 +00:00 · 2026-08-24 14:41:00 +02:00
parent 73373a2c9e
commit 6bd9139869
2 changed files with 192 additions and 7 deletions
@@ -0,0 +1,146 @@
+using System;
+using AutopilotMonitor.Agent.V2.Core.Security;
+using Xunit;
+
+namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
+{
+    /// <summary>
+    /// Pins <see cref="EntraDeviceCertHelper.ParseGuidFromExtension"/>, the pure byte-level part of the
+    /// cert probe. Both encodings Microsoft actually ships are covered: the ASN.1 OCTET STRING wrapper
+    /// used by the MS-Organization-Access TenantId/DeviceId extensions (and Intune's
+    /// <c>1.2.840.113556.5.6</c> / <c>5.14</c>), and the BARE 16-byte payload used by Intune's DeviceId
+    /// extension <c>1.2.840.113556.5.4</c> — the latter was silently returning <c>null</c> before.
+    /// The store-iteration shell around it needs a real machine cert store and is not unit-tested.
+    /// </summary>
+    public sealed class EntraDeviceCertHelperParseTests
+    {
+        // Synthetic — never a real tenant/device id. ToByteArray() gives the MS little-endian layout
+        // that both cert encodings carry, so every expectation below is derived, not hand-transcribed.
+        private static readonly Guid Sample = new Guid("11223344-5566-7788-99aa-bbccddeeff00");
+
+        private static byte[] Wrap(byte[] payload, params byte[] header)
+        {
+            var buf = new byte[header.Length + payload.Length];
+            Buffer.BlockCopy(header, 0, buf, 0, header.Length);
+            Buffer.BlockCopy(payload, 0, buf, header.Length, payload.Length);
+            return buf;
+        }
+
+        [Fact]
+        public void OctetString_ShortForm_Parses()
+        {
+            var raw = Wrap(Sample.ToByteArray(), 0x04, 0x10);
+
+            Assert.Equal(Sample, EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void OctetString_LongForm_Parses()
+        {
+            var raw = Wrap(Sample.ToByteArray(), 0x04, 0x81, 0x10);
+
+            Assert.Equal(Sample, EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        /// <summary>
+        /// The regression this class exists for: Intune's <c>1.2.840.113556.5.4</c> extension value is
+        /// exactly 16 bytes with no <c>04 10</c> prefix, so the strict OCTET-STRING parser bailed on the
+        /// tag check and the DeviceId was unreadable.
+        /// </summary>
+        [Fact]
+        public void BareSixteenBytes_Parses()
+        {
+            var raw = Sample.ToByteArray();
+
+            Assert.Equal(Sample, EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        /// <summary>
+        /// A bare GUID may legitimately start with 0x04 (the OCTET STRING tag). That must not be
+        /// mistaken for a wrapper: a wrapped 16-byte GUID is always >= 18 bytes, so the two forms are
+        /// disjoint by length and the bare path has to win here.
+        /// </summary>
+        [Fact]
+        public void BareSixteenBytes_StartingWithOctetStringTag_StillParses()
+        {
+            var guid = new Guid("aabbcc04-0110-4020-8030-405060708090");
+            var raw = guid.ToByteArray();
+
+            Assert.Equal(0x04, raw[0]); // guard: the byte layout really does start with the ASN.1 tag
+            Assert.Equal(16, raw.Length);
+            Assert.Equal(guid, EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void OctetString_IsLittleEndianMsLayout()
+        {
+            // 04 10 + the canonical MS binary layout of {11223344-...} => first group byte-reversed.
+            var raw = new byte[]
+            {
+                0x04, 0x10,
+                0x44, 0x33, 0x22, 0x11,
+                0x66, 0x55,
+                0x88, 0x77,
+                0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00
+            };
+
+            Assert.Equal(Sample, EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void Null_ReturnsNull()
+        {
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(null));
+        }
+
+        [Fact]
+        public void Empty_ReturnsNull()
+        {
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(new byte[0]));
+        }
+
+        [Theory]
+        [InlineData(15)]
+        [InlineData(17)]
+        public void BarePayload_OfWrongLength_ReturnsNull(int length)
+        {
+            var raw = new byte[length];
+            for (var i = 0; i < length; i++) raw[i] = (byte)(0x40 + i); // no 0x04 tag, no valid wrapper
+
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void NonOctetStringTag_ReturnsNull()
+        {
+            // e.g. Intune's 1.2.840.113556.5.15 => ASN.1 INTEGER 2.
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(new byte[] { 0x02, 0x01, 0x02 }));
+        }
+
+        [Fact]
+        public void OctetString_WithNonGuidContentLength_ReturnsNull()
+        {
+            var raw = Wrap(new byte[8], 0x04, 0x08);
+
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void OctetString_TruncatedContent_ReturnsNull()
+        {
+            // Declares 16 content bytes but only carries 10.
+            var raw = Wrap(new byte[10], 0x04, 0x10);
+
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+
+        [Fact]
+        public void OctetString_LongFormWithOversizedLengthHeader_ReturnsNull()
+        {
+            // 0x85 => 5 length bytes, above the 4 the parser accepts.
+            var raw = Wrap(Sample.ToByteArray(), 0x04, 0x85, 0x00, 0x00, 0x00, 0x00, 0x10);
+
+            Assert.Null(EntraDeviceCertHelper.ParseGuidFromExtension(raw));
+        }
+    }
+}
@@ -23,6 +23,24 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
    /// <item><description><c>1.2.840.113556.1.5.284.2</c> → DeviceId (same encoding)</description></item>
    /// </list>
    /// <para>
+    /// The sibling <c>Microsoft Intune MDM Device CA</c> client cert (the one the agent authenticates
+    /// with, see <see cref="CertificateHelper"/>) carries a DIFFERENT OID arc that identifies the same
+    /// device from Intune's side. Observed on a live Intune-enrolled Windows 11 device:
+    /// </para>
+    /// <list type="table">
+    /// <listheader><term>OID</term><description>Meaning / encoding</description></listheader>
+    /// <item><term><c>1.2.840.113556.5.4</c></term><description>Intune DeviceId — BARE 16-byte GUID, no ASN.1 wrapper. Equals the cert's Subject CN.</description></item>
+    /// <item><term><c>1.2.840.113556.5.6</c></term><description>Intune AccountId (the Intune tenant account, NOT the Entra TenantId) — OCTET STRING GUID.</description></item>
+    /// <item><term><c>1.2.840.113556.5.14</c></term><description>Entra TenantId — OCTET STRING GUID. This is the one to compare against an Entra tenant id.</description></item>
+    /// <item><term><c>1.2.840.113556.5.11</c></term><description>Further GUID, OCTET STRING; semantics not documented by Microsoft and not verified here.</description></item>
+    /// <item><term><c>1.2.840.113556.5.15/.16/.17/.18/.19/.23</c></term><description>ASN.1 INTEGER enums (platform / enrollment flavour); semantics undocumented.</description></item>
+    /// </list>
+    /// <para>
+    /// The bare-GUID encoding of <c>1.2.840.113556.5.4</c> is why
+    /// <see cref="ParseGuidFromExtension"/> accepts an unwrapped 16-byte payload as well —
+    /// a strict OCTET-STRING parser silently returns <c>null</c> for that extension.
+    /// </para>
+    /// <para>
    /// All probes are best-effort: never throw, return <c>null</c> on miss, log a Warning
    /// with the specific failure reason (no matching cert / extension missing / ASN.1
    /// parse failed) so the agent log explains why the fallback didn't help.
@@ -88,7 +106,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                            continue;
                        }

-                        var guid = ParseGuidFromAsn1OctetString(ext.RawData);
+                        var guid = ParseGuidFromExtension(ext.RawData);
                        if (guid != null)
                        {
                            logger?.Info(
@@ -98,7 +116,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                        }

                        logger?.Warning(
-                            $"EntraDeviceCertHelper: failed to parse {label} ASN.1 OCTET STRING in cert thumbprint={cert.Thumbprint} " +
+                            $"EntraDeviceCertHelper: failed to parse {label} GUID extension in cert thumbprint={cert.Thumbprint} " +
                            $"(raw {ext.RawData.Length} bytes); trying next candidate.");
                    }

@@ -116,13 +134,29 @@ namespace AutopilotMonitor.Agent.V2.Core.Security

        /// <summary>
        /// Parses a 16-byte MS-format binary GUID (little-endian for the first three groups,
-        /// per <see cref="Guid(byte[])"/>) wrapped in an ASN.1 OCTET STRING. Handles both
-        /// short-form (<c>04 10 ...</c>) and long-form (<c>04 81 10 ...</c>) length encoding.
+        /// per <see cref="Guid(byte[])"/>) out of a certificate extension's raw value.
+        /// Two encodings are accepted, because Microsoft uses both:
+        /// <list type="number">
+        /// <item><description>Wrapped in an ASN.1 OCTET STRING — short-form (<c>04 10 ...</c>) or
+        /// long-form (<c>04 81 10 ...</c>) length. Used by the MS-Organization-Access TenantId/DeviceId
+        /// extensions and by the Intune <c>…5.6</c> / <c>…5.14</c> extensions.</description></item>
+        /// <item><description>A bare, unwrapped 16-byte payload. Used by the Intune DeviceId extension
+        /// <c>1.2.840.113556.5.4</c> — verified against a live Intune MDM device cert, whose raw value is
+        /// exactly 16 bytes with no <c>04 10</c> prefix.</description></item>
+        /// </list>
+        /// 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 <c>0x04</c>.
        /// Returns <c>null</c> on any structural mismatch — never throws.
        /// </summary>
-        public static Guid? ParseGuidFromAsn1OctetString(byte[] raw)
+        public static Guid? ParseGuidFromExtension(byte[] raw)
        {
-            if (raw == null || raw.Length < 2) return null;
+            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;
@@ -151,8 +185,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Security

            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, contentStart, guidBytes, 0, 16);
+            Buffer.BlockCopy(raw, offset, guidBytes, 0, 16);

            try { return new Guid(guidBytes); }
            catch { return null; }