Implement support for adding sid certificate extension into offline r…

…equests
Sleepw4lker · Sleepw4lker · Aug 11, 2022 · Apr 21, 2022 · May 14, 2022 · May 17, 2022
commit 919c8d983803804e954b5dfb1e5e92f417347713
diff --git a/TameMyCerts/CCertServerPolicyExtensions.cs b/TameMyCerts/CCertServerPolicyExtensions.cs
@@ -187,5 +187,36 @@ public static byte[] GetBinaryRequestPropertyOrDefault(this CCertServerPolicy se
         }
 
         #endregion
+
+        #region SetCertificateExtension
+
+        public static void SetCertificateExtension(this CCertServerPolicy serverPolicy, string oid, string value, bool critical = false)
+        {
+            var rawData = Convert.FromBase64String(value);
+
+            IntPtr pBstr = Marshal.AllocHGlobal(rawData.Length + 4);
+            Marshal.WriteInt32(pBstr, 0, rawData.Length);
+            Marshal.Copy(rawData, 0, pBstr + 4, rawData.Length);
+            var variant = new OleAut32.VARIANT
+            {
+                vt = 8, // VT_BSTR
+                pvRecord = pBstr + 4
+            };
+            IntPtr pvarValue = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(OleAut32.VARIANT)));
+            Marshal.StructureToPtr(variant, pvarValue, false);
+            Int32 dwCritical = critical ? 1 : 0;
+
+            try
+            {
+                serverPolicy.SetCertificateExtension(oid, CertSrv.PROPTYPE_BINARY, dwCritical, pvarValue);
+            }
+            finally
+            {
+                Marshal.FreeHGlobal(pBstr);
+                Marshal.FreeHGlobal(pvarValue);
+            }
+        }
+
+        #endregion
     }
 }
diff --git a/TameMyCerts/CERTCLILIB.il b/TameMyCerts/CERTCLILIB.il
@@ -800,7 +800,8 @@
           instance object 
           marshal( struct) 
           GetCertificateExtension([in] string  marshal( bstr) strExtensionName,
-                                  [in] int32 Type) runtime managed internalcall
+                                  [in] int32 Type,
+                                  [out] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 05 00 02 60 00 00 )                         // .....`..
   } // end of method ICertServerPolicy::GetCertificateExtension
@@ -815,7 +816,7 @@
           instance void  SetCertificateExtension([in] string  marshal( bstr) strExtensionName,
                                                  [in] int32 Type,
                                                  [in] int32 ExtFlags,
-                                                 [in] object&  marshal( struct) pvarValue) runtime managed internalcall
+                                                 [in] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 07 00 02 60 00 00 )                         // .....`..
   } // end of method ICertServerPolicy::SetCertificateExtension
@@ -939,7 +940,8 @@
           instance object 
           marshal( struct) 
           GetCertificateExtension([in] string  marshal( bstr) strExtensionName,
-                                  [in] int32 Type) runtime managed internalcall
+                                  [in] int32 Type,
+                                  [out] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 05 00 02 60 00 00 )                         // .....`..
     .override CERTCLILib.ICertServerPolicy::GetCertificateExtension
@@ -956,7 +958,7 @@
           instance void  SetCertificateExtension([in] string  marshal( bstr) strExtensionName,
                                                  [in] int32 Type,
                                                  [in] int32 ExtFlags,
-                                                 [in] object&  marshal( struct) pvarValue) runtime managed internalcall
+                                                 [in] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 07 00 02 60 00 00 )                         // .....`..
     .override CERTCLILib.ICertServerPolicy::SetCertificateExtension
@@ -1054,7 +1056,8 @@
           instance object 
           marshal( struct) 
           GetCertificateExtension([in] string  marshal( bstr) strExtensionName,
-                                  [in] int32 Type) runtime managed internalcall
+                                  [in] int32 Type,
+                                  [out] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 04 00 02 60 00 00 )                         // .....`..
   } // end of method ICertServerExit::GetCertificateExtension
@@ -1175,7 +1178,8 @@
           instance object 
           marshal( struct) 
           GetCertificateExtension([in] string  marshal( bstr) strExtensionName,
-                                  [in] int32 Type) runtime managed internalcall
+                                  [in] int32 Type,
+                                  [out] native int pvarValue) runtime managed internalcall
   {
     .custom instance void [mscorlib]System.Runtime.InteropServices.DispIdAttribute::.ctor(int32) = ( 01 00 04 00 02 60 00 00 )                         // .....`..
     .override CERTCLILib.ICertServerExit::GetCertificateExtension

diff --git a/TameMyCerts/CERTCLILib.dll b/TameMyCerts/CERTCLILib.dll
diff --git a/TameMyCerts/CertificateRequestValidationResult.cs b/TameMyCerts/CertificateRequestValidationResult.cs
@@ -32,6 +32,7 @@ public CertificateRequestValidationResult(bool auditOnly, string notAfter)
         public bool AuditOnly { get; }
         public List<string> Description { get; set; } = new List<string>();
         public List<KeyValuePair<string, string>> Identities { get; set; } = new List<KeyValuePair<string, string>>();
+        public List<KeyValuePair<string, string>> Extensions { get; set; } = new List<KeyValuePair<string, string>>();
 
         public void SetFailureStatus()
         {

diff --git a/TameMyCerts/CertificateRequestValidator.cs b/TameMyCerts/CertificateRequestValidator.cs
@@ -224,7 +224,6 @@ public CertificateRequestValidationResult VerifyRequest(string certificateReques
 
                 #region Process request extensions
 
-                // TODO: KB5014754. Validation logic is yet to be established. I assume we will need this for mobile devices until May 9, 2023.
                 if (certificateRequestPolicy.SecurityIdentifierExtension.Equals("Deny",
                         StringComparison.InvariantCultureIgnoreCase) &&
                     certificateRequestPkcs10.HasExtension(WinCrypt.szOID_DS_CA_SECURITY_EXT))

diff --git a/TameMyCerts/DirectoryServicesValidator.cs b/TameMyCerts/DirectoryServicesValidator.cs
@@ -1,6 +1,22 @@
-using System;
+// Copyright 2021 Uwe Gradenegger <uwe@gradenegger.eu>
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System;
+using System.Collections.Generic;
 using System.DirectoryServices;
 using System.Linq;
+using System.Security.Principal;
 
 namespace TameMyCerts
 {
@@ -101,6 +117,8 @@ public CertificateRequestValidationResult VerifyRequest(CertificateRequestPolicy
                 return result;
             }
 
+            #region process memberships of allowed groups
+
             if (dsMapping.AllowedSecurityGroups.Count > 0)
             {
                 var matchFound = false;
@@ -122,23 +140,36 @@ public CertificateRequestValidationResult VerifyRequest(CertificateRequestPolicy
                 }
             }
 
+            #endregion
+
             #region process memberships of disallowed groups
 
-            if (dsMapping.DisallowedSecurityGroups.Count <= 0)
+            if (dsMapping.DisallowedSecurityGroups.Count > 0)
             {
-                return result;
+                for (var index = 0; index < searchResults[0].Properties["memberOf"].Count; index++)
+                {
+                    var group = searchResults[0].Properties["memberOf"][index];
+                    if (dsMapping.DisallowedSecurityGroups.Any(x =>
+                            x.Equals(group.ToString(), StringComparison.InvariantCultureIgnoreCase)))
+                    {
+                        result.SetFailureStatus(WinError.CERTSRV_E_TEMPLATE_DENIED, string.Format(
+                            LocalizedStrings.DirVal_Account_Groups_Disallowed,
+                            dsMapping.ObjectCategory, identity, group));
+                    }
+                }
             }
 
-            for (var index = 0; index < searchResults[0].Properties["memberOf"].Count; index++)
+            #endregion
+
+            #region Process SID certificate extension construction
+
+            if (certificateRequestPolicy.SecurityIdentifierExtension.Equals("Add",
+                    StringComparison.InvariantCultureIgnoreCase))
             {
-                var group = searchResults[0].Properties["memberOf"][index];
-                if (dsMapping.DisallowedSecurityGroups.Any(x =>
-                        x.Equals(group.ToString(), StringComparison.InvariantCultureIgnoreCase)))
-                {
-                    result.SetFailureStatus(WinError.CERTSRV_E_TEMPLATE_DENIED, string.Format(
-                        LocalizedStrings.DirVal_Account_Groups_Disallowed,
-                        dsMapping.ObjectCategory, identity, group));
-                }
+                var objectSid =
+                    new SecurityIdentifier((byte[]) searchResults[0].Properties["objectSid"][0], 0).ToString();
+                result.Extensions.Add(new KeyValuePair<string, string>(WinCrypt.szOID_DS_CA_SECURITY_EXT,
+                    new SidCertificateExtension(objectSid).value));
             }
 
             #endregion

diff --git a/TameMyCerts/OleAut32.cs b/TameMyCerts/OleAut32.cs
@@ -0,0 +1,36 @@
+// Copyright 2021 Uwe Gradenegger <uwe@gradenegger.eu>
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Kudos to Vadims podans for his research and support!
+
+using System;
+using System.Runtime.InteropServices;
+
+namespace TameMyCerts {
+    class OleAut32 {
+        public const Int16 VT_BSTR = 0x8;
+
+        [DllImport("OleAut32.dll", SetLastError = true)]
+        public static extern Int32 VariantClear(IntPtr pvarg);
+
+        [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
+        public struct VARIANT {
+            public Int16 vt;
+            public Int16 wReserved1;
+            public Int16 wReserved2;
+            public Int16 wReserved3;
+            public IntPtr pvRecord;
+        }
+    }
+}
diff --git a/TameMyCerts/Policy.cs b/TameMyCerts/Policy.cs
@@ -137,8 +137,6 @@ public int VerifyRequest(string strConfig, int context, int bNewRequest, int fla
 
             #region Process insecure flag/attribute combinations
 
-            // TODO: Move this into CertificateRequestValidator, remove dependency on EditFlags (it is never a good idea to use the san extension)
-
             // Deny requests containing the "san" request attribute
             if ((_editFlags & CertSrv.EDITF_ATTRIBUTESUBJECTALTNAME2) == CertSrv.EDITF_ATTRIBUTESUBJECTALTNAME2 &&
                 requestAttributeList.Any(x => x.Key.Equals("san", StringComparison.InvariantCultureIgnoreCase)))
@@ -241,6 +239,11 @@ public int VerifyRequest(string strConfig, int context, int bNewRequest, int fla
             {
                 validationResult =
                     _directoryServicesValidator.VerifyRequest(requestPolicy, validationResult);
+
+                foreach (var extension in validationResult.Extensions)
+                {
+                    certServerPolicy.SetCertificateExtension(extension.Key, extension.Value);
+                }
             }
 
             // No reason to deny the request, let's issue the certificate

diff --git a/TameMyCerts/SidCertificateExtension.cs b/TameMyCerts/SidCertificateExtension.cs
@@ -0,0 +1,85 @@
+// Copyright 2021 Uwe Gradenegger <uwe@gradenegger.eu>
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+
+// http://www.apache.org/licenses/LICENSE-2.0
+
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System;
+using System.IO;
+using System.Linq;
+
+namespace TameMyCerts
+{
+    public class SidCertificateExtension
+    {
+        public string value;
+
+        public SidCertificateExtension(string sid)
+        {
+            var result = ConvertStringToDerNode("04", ConvertStringToHexString(sid));
+            result = ConvertStringToDerNode("A0", result);
+            result = $"060A2B060104018237190201{result}";
+            result = ConvertStringToDerNode("A0", result);
+            result = ConvertStringToDerNode("30", result);
+            result = Convert.ToBase64String(HexStringToByteArray(result));
+
+            value = result;
+        }
+
+        private static byte[] HexStringToByteArray(string input)
+        {
+            var outputLength = input.Length / 2;
+            var output = new byte[outputLength];
+            using (var sr = new StringReader(input))
+            {
+                for (var i = 0; i < outputLength; i++)
+                {
+                    output[i] = Convert.ToByte(new string(new[] {(char) sr.Read(), (char) sr.Read()}), 16);
+                }
+            }
+
+            return output;
+        }
+
+        private static string ConvertStringToDerNode(string identifier, string content)
+        {
+            return $"{identifier}{GetAsn1LengthOctets(content.Length)}{content}";
+        }
+
+        private static string ConvertStringToHexString(string content)
+        {
+            return content.ToCharArray().Aggregate("", (current, c) => current + $"{(int) c:X2}");
+        }
+
+        private static string GetAsn1LengthOctets(int length)
+        {
+            var x = (int) Math.Ceiling(length / 2m);
+
+            var numBits = Convert.ToString(x, 2).Length;
+            var numOctets = Math.Ceiling(numBits / 8m);
+
+            if (numBits <= 7)
+            {
+                // Short form (for lengths between 0 and 127). One octet. 
+                // Bit 8 has value "0" and bits 7-1 give the length.
+                return $"{x:X2}";
+            }
+
+            // Long form. Two to 127 octets.
+
+            // Second and following octets give the length, base 256, most significant digit first.
+            var result = string.Format("{0:X" + numOctets * 2 + "}", x);
+
+            // Bit 8 of first octet has value "1" and bits 7-1 give the number of additional length octets.
+            return string.Format("{0:X2}", (int) (128 + numOctets)) + result;
+        }
+    }
+}
diff --git a/TameMyCerts/TameMyCerts.csproj b/TameMyCerts/TameMyCerts.csproj
@@ -68,12 +68,14 @@
       <DependentUpon>LocalizedStrings.resx</DependentUpon>
     </Compile>
     <Compile Include="Logger.cs" />
+    <Compile Include="OleAut32.cs" />
     <Compile Include="Policy.cs" />
     <Compile Include="PolicyManage.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="CertificateRequestValidator.cs" />
     <Compile Include="CertificateTemplateInfo.cs" />
     <Compile Include="Headers.cs" />
+    <Compile Include="SidCertificateExtension.cs" />
   </ItemGroup>
   <ItemGroup>
     <None Include="CERTCLILib.il" />