Minor fixes and enhancements (#78)

* Extend documentation

* Rename Yubikey integration test

* Rename variable in Yubikey validator

* Extend logging of detection of san request attribute

* Fix installer handling of ETW manifest

Author: Sleepw4lker

TameMyCerts.IntegrationTests/Tests/User_Online_YubiKey.ps1 renamed to TameMyCerts.IntegrationTests/Tests/User_Online_YubiKey.Tests.ps1

@@ -9,8 +9,6 @@ Describe 'User_Online_Yubikey.Tests' {
 
     It 'Given YubiKey Policy is enabled and a request contains a valid attestation, a certificate is issued' {
 
-        #$Csr = Get-Content -Path "User_Online_Yubikey.pem" -Raw
-
         $Csr =  "MIIIxDCCB6wCAQAwHDEaMBgGA1UEAwwRVGFtZU15Q2VydHNfNS43LjIwggEiMA0G" +
                 "CSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC8PsNNUt67rgq4+EGGnBU46XiiXv4g" +
                 "ADdAhOB8IQp1eZ+7Dq8JOQ9XDjjBObPj7TnEzxS+PSpXTmwV46FA4gykcVj+tRhT" +

TameMyCerts.Tests/RequestAttributeValidatorTests.cs

@@ -1,4 +1,5 @@
 using Microsoft.VisualStudio.TestPlatform.ObjectModel.Client;
+using Newtonsoft.Json.Linq;
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
@@ -187,12 +188,15 @@ public void Allow_StartDate()
     }
 
     [Fact]
-    public void Deny_invalid_flags()
+    public void Deny_SAN_request_attribute_when_insecure_flag_is_set()
     {
+        const string attrName = "saN"; // note that we also test case sensitivity
+        const string attrValue = "doesnt-matter";
+
         var caConfig = new CertificateAuthorityConfiguration(EditFlag.EDITF_ATTRIBUTESUBJECTALTNAME2);
 
         var dbRow = new CertificateDatabaseRow(_defaultCsr, CertCli.CR_IN_PKCS10,
-            new Dictionary<string, string> { { "saN", "doesnt-matter" } });
+            new Dictionary<string, string> { { attrName, attrValue } });
 
         var result = new CertificateRequestValidationResult(dbRow);
 
@@ -202,15 +206,19 @@ public void Deny_invalid_flags()
 
         Assert.True(result.DeniedForIssuance);
         Assert.True(result.StatusCode == WinError.NTE_FAIL);
+        Assert.Contains($"{attrName}:{attrValue}", string.Join("\n", result.Warnings));
     }
 
     [Fact]
-    public void Allow_valid_flags()
+    public void Allow_SAN_request_attribute_when_insecure_flag_is_not_set()
     {
+        const string attrName = "saN"; // note that we also test case sensitivity
+        const string attrValue = "doesnt-matter";
+
         var caConfig = new CertificateAuthorityConfiguration(0);
 
         var dbRow = new CertificateDatabaseRow(_defaultCsr, CertCli.CR_IN_PKCS10,
-            new Dictionary<string, string> { { "saN", "doesnt-matter" } });
+            new Dictionary<string, string> { { attrName, attrValue } });
 
         var result = new CertificateRequestValidationResult(dbRow);
 
@@ -220,6 +228,7 @@ public void Allow_valid_flags()
 
         Assert.False(result.DeniedForIssuance);
         Assert.True(result.StatusCode == WinError.ERROR_SUCCESS);
+        Assert.Contains($"{attrName}:{attrValue}", string.Join("\n", result.Warnings));
     }
 
     [Fact]

TameMyCerts/LocalizedStrings.Designer.cs

@@ -142,7 +142,7 @@
 {2}</value>
   </data>
   <data name="Events_REQUEST_CONTAINS_WARNINGS" xml:space="preserve">
-    <value>The following warnings have been logged during the processing of request {0} for {1}:
+    <value>The following warning has been logged during the processing of request {0} for {1}:
 {2}</value>
   </data>
   <data name="Events_REQUEST_DENIED_AUDIT" xml:space="preserve">
@@ -213,11 +213,21 @@
   <data name="ReqVal_Crypto_Provider_Unknown" xml:space="preserve">
     <value>Unable to determine the cryptographic provider that was used to create the certificate request, but policy requires this information. Probably the certificate request does not contain such information.</value>
   </data>
-  <data name="AttribVal_Insecure_Flags" xml:space="preserve">
-    <value>The certificate request metadata contains the "san" request attribute, and the certification authority is configured with the EDITF_ATTRIBUTESUBJECTALTNAME2 flag. This is a highly dangerous configuration. The request was therefore denied.</value>
+  <data name="AttribVal_Insecure_Flags_denied" xml:space="preserve">
+    <value>The certificate request metadata contains the "san" request attribute, and the certification authority is configured with the EDITF_ATTRIBUTESUBJECTALTNAME2 flag. This is a highly dangerous configuration. The request was therefore denied.
+
+The request contains the following request attributes:
+
+{0}</value>
   </data>
   <data name="AttribVal_Insecure_Flags_detected" xml:space="preserve">
-    <value>The certificate request metadata contains the "san" request attribute. If the certification authority is configured with the EDITF_ATTRIBUTESUBJECTALTNAME2 flag, this is a highly dangerous configuration.</value>
+    <value>The certificate request metadata contains the "san" request attribute. If the certification authority is configured with the EDITF_ATTRIBUTESUBJECTALTNAME2 flag, this is a highly dangerous configuration.
+
+The request contains the following request attributes:
+
+{0}
+
+Requester name is {1}</value>
   </data>
   <data name="ReqVal_Err_NotAfter_Invalid" xml:space="preserve">
     <value>Unable to parse the configured expiration date specified in certificate request policy.</value>

TameMyCerts/LocalizedStrings.resx

@@ -35,6 +35,7 @@ public CertificateDatabaseRow(CCertServerPolicy serverPolicy)
         PublicKey = serverPolicy.GetBinaryCertificatePropertyOrDefault("RawPublicKey");
         RawRequest = serverPolicy.GetBinaryRequestPropertyOrDefault("RawRequest");
         RequestID = serverPolicy.GetLongRequestPropertyOrDefault("RequestID");
+        RequesterName = serverPolicy.GetStringRequestPropertyOrDefault("RequesterName");
         RequestType = serverPolicy.GetLongRequestPropertyOrDefault("RequestType") ^ CertCli.CR_IN_FULLRESPONSE;
         Upn = serverPolicy.GetStringCertificatePropertyOrDefault("UPN") ?? string.Empty;
         DistinguishedName = serverPolicy.GetStringRequestPropertyOrDefault("Request.DistinguishedName") ??
@@ -136,6 +137,11 @@ public CertificateDatabaseRow(string request, int requestType,
     /// </summary>
     public int RequestID { get; }
 
+    /// <summary>
+    ///     The requester name.
+    /// </summary>
+    public string RequesterName { get; }
+
     /// <summary>
     ///     The request type as defined in certcli.h (PKCS#10, PKCS#7 or CMS).
     /// </summary>

TameMyCerts/Models/CertificateDatabaseRow.cs

@@ -216,10 +216,9 @@ public int VerifyRequest(string strConfig, int context, int isNewRequest, int fl
 
         #region Log warnings, if any
 
-        if (result.Warnings.Count > 0)
+        foreach (var warning in result.Warnings.Distinct().ToList())
         {
-            _logger.Log(Events.REQUEST_CONTAINS_WARNINGS, requestId, template.Name,
-                string.Join("\n", result.Warnings.Distinct().ToList()));
+            _logger.Log(Events.REQUEST_CONTAINS_WARNINGS, requestId, template.Name, warning);
         }
 
         #endregion

TameMyCerts/Policy.cs

@@ -14,6 +14,7 @@
 
 using System;
 using System.Globalization;
+using System.Linq;
 using TameMyCerts.Enums;
 using TameMyCerts.Models;
 
@@ -39,12 +40,16 @@ public CertificateRequestValidationResult VerifyRequest(CertificateRequestValida
 
         if (dbRow.RequestAttributes.ContainsKey("san"))
         {
-            result.AddWarning(LocalizedStrings.AttribVal_Insecure_Flags_detected);
+            var requestAttributes = string.Join("\n",
+                dbRow.RequestAttributes.Select(x => $"{x.Key}:{x.Value}").ToList());
+
+            result.AddWarning(string.Format(LocalizedStrings.AttribVal_Insecure_Flags_detected, requestAttributes, dbRow.RequesterName));
 
             if (caConfig.EditFlags.HasFlag(EditFlag.EDITF_ATTRIBUTESUBJECTALTNAME2) &&
                 !caConfig.TmcFlags.HasFlag(TmcFlag.TMC_WARN_ONLY_ON_INSECURE_FLAGS))
             {
-                result.SetFailureStatus(WinError.NTE_FAIL, LocalizedStrings.AttribVal_Insecure_Flags);
+                result.SetFailureStatus(WinError.NTE_FAIL,
+                    string.Format(LocalizedStrings.AttribVal_Insecure_Flags_denied, requestAttributes, dbRow.RequesterName));
             }
         }
 

TameMyCerts/Validators/RequestAttributeValidator.cs

@@ -253,10 +253,10 @@ private bool ObjectMatchesPolicy(YubikeyPolicy policy, YubikeyObject yubikey)
     /// <returns></returns>
     private static X509Certificate2Collection GetCertificatesFromMachineStore(string storeName)
     {
-        var rootCaStore = new X509Store(storeName, StoreLocation.LocalMachine);
-        rootCaStore.Open(OpenFlags.ReadOnly);
-        var certificates = rootCaStore.Certificates;
-        rootCaStore.Close();
+        var store = new X509Store(storeName, StoreLocation.LocalMachine);
+        store.Open(OpenFlags.ReadOnly);
+        var certificates = store.Certificates;
+        store.Close();
 
         return certificates;
     }