From ff3270fce99bb418886cc52439bdf839ea448511 Mon Sep 17 00:00:00 2001 From: Michael Grafnetter Date: Sun, 21 Apr 2019 09:59:11 +0200 Subject: [PATCH] DN with binary parsing performance --- Documentation/CHANGELOG.md | 4 +++ .../PowerShell/Test-PasswordQuality.md | 4 +-- .../StringExtensionsTester.cs | 26 ++++++++++++++++++- .../Extensions/StringExtensions.cs | 16 +++++++----- Src/DSInternals.Common/Validator.cs | 9 +++++++ .../en-US/DSInternals.PowerShell.dll-Help.xml | 6 ++--- 6 files changed, 53 insertions(+), 12 deletions(-) diff --git a/Documentation/CHANGELOG.md b/Documentation/CHANGELOG.md index 013d3ed..690d8b0 100644 --- a/Documentation/CHANGELOG.md +++ b/Documentation/CHANGELOG.md @@ -7,6 +7,10 @@ All notable changes to this project will be documented in this file. The format - The [Test-PasswordQuality](PowerShell/Test-PasswordQuality.md) cmdlet now has a parameter called `-WeakPasswordHashesSortedFile`. This parameter should be used with ordered hash files downloaded from [HaveIBeenPwned](https://haveibeenpwned.com/Passwords) as it has huge performance benefits over the older `-WeakPasswordHashesFile` parameter due to the usage of binary search algorithm. - The [Test-PasswordQuality](PowerShell/Test-PasswordQuality.md) cmdlet now has a proper documentation, including usage examples. +### Fixed + - The speed of processing the `-WeakPasswordHashesFile` and `-WeakPasswordsFile` parameters of the [Test-PasswordQuality](PowerShell/Test-PasswordQuality.md) cmdlet has significantly been increased. + - Parsing of roamed credentials is now slightly faster. + ## [3.3] - 2019-03-02 ### Changed - Implemented a slightly more secure handling of [GMSA passwords](../Src/DSInternals.Common/Data/Principals/ManagedPassword.cs). diff --git a/Documentation/PowerShell/Test-PasswordQuality.md b/Documentation/PowerShell/Test-PasswordQuality.md index 57431d5..f18c5ec 100644 --- a/Documentation/PowerShell/Test-PasswordQuality.md +++ b/Documentation/PowerShell/Test-PasswordQuality.md @@ -89,7 +89,7 @@ Performs an offline credential hygiene audit of AD database against HIBP. ### Example 2 ```powershell PS C:\> $results = Get-ADReplAccount -All -NamingContext 'DC=contoso,DC=com' -Server LON-DC1 | - Test-PasswordQuality -WeakPasswords 'Pa$$w0rd','April2019' -WeakPasswordHashesFile pwned-passwords-ntlm-ordered-by-hash-v4.txt + Test-PasswordQuality -WeakPasswords 'Pa$$w0rd','April2019' -WeakPasswordHashesSortedFile pwned-passwords-ntlm-ordered-by-hash-v4.txt ``` Performs an online credential hygiene audit of AD against HIBP + a custom wordlist. @@ -190,7 +190,7 @@ Accept wildcard characters: False ``` ### -WeakPasswords -List of passwords that are considered weak, e.g. Password123 or April2019. +List of passwords that are considered weak, e.g. Password123 or April2019. If more than a handful passwords are to be tested, the WeakPasswordsFile parameter should be used instead. ```yaml Type: String[] diff --git a/Src/DSInternals.Common.Test/StringExtensionsTester.cs b/Src/DSInternals.Common.Test/StringExtensionsTester.cs index 74991b1..c8cc0ce 100644 --- a/Src/DSInternals.Common.Test/StringExtensionsTester.cs +++ b/Src/DSInternals.Common.Test/StringExtensionsTester.cs @@ -1,5 +1,6 @@ namespace DSInternals.Common.Test { + using System; using Microsoft.VisualStudio.TestTools.UnitTesting; [TestClass] @@ -37,7 +38,7 @@ } [TestMethod] - public void StringExtensions_DNWithBinary() + public void StringExtensions_DNWithBinary_Vector1() { // By converting the value back and forth, we should get the original string. string input = "B:828:0002000020000120717AE052FCCF546AAD0D51E878AAD69CE04FDC39F5A8D8E3CEBA6BCB4DA0E720000214B7474E61C1001D3E546CFED8E387CFC1AC86A2CA7B3CDCF1267614585E2A341B0103525341310008000003000000000100000000000000000000010001C1A78914457758B0B13C70C710C7F8548F3F9ED56AD4640B6E6A112655C98ECAC1CBD68A298F5686C08439428A97FE6FDF58D78EA481905182BAD684C2D9C5CDE1CDE34AA19742E8BBF58B953EAC4C562FCF598CC176B02DBE9FFFEF5937A65815C236F92892F7E511A1FEDD5483CB33F1EA715D68106180DED2432A293367114A6E325E62F93F73D7ECE4B6A2BCDB829D95C8645C3073B94BA7CB7515CD29042F0967201C6E24A77821E92A6C756DF79841ACBAAE11D90CA03B9FCD24EF9E304B5D35248A7BD70557399960277058AE3E99C7C7E2284858B7BF8B08CDD286964186A50A7FCBCC6A24F00FEE5B9698BBD3B1AEAD0CE81FEA461C0ABD716843A50100040101000500100006E377F547D0D20A4A8ACAE0501098BDE40200070100080008417BD66E6603D401080009417BD66E6603D401:CN = Admin,CN = Users,DC = contoso,DC = com"; @@ -45,5 +46,28 @@ string result = binaryData.ToDNWithBinary("CN = Admin,CN = Users,DC = contoso,DC = com"); Assert.AreEqual(input, result); } + + [TestMethod] + [ExpectedException(typeof(ArgumentNullException))] + public void StringExtensions_DNWithBinary_NullInput() + { + byte[] binaryData = ((string)null).FromDNWithBinary(); + } + + [TestMethod] + [ExpectedException(typeof(ArgumentNullException))] + public void StringExtensions_DNWithBinary_EmptyInput() + { + // By converting the value back and forth, we should get the original string. + byte[] binaryData = String.Empty.FromDNWithBinary(); + } + + [TestMethod] + [ExpectedException(typeof(ArgumentException))] + public void StringExtensions_DNWithBinary_MalformedInput() + { + string input = "B:828::CN = Admin,CN=Users,DC=contoso,DC=com"; + byte[] binaryData = input.FromDNWithBinary(); + } } } diff --git a/Src/DSInternals.Common/Extensions/StringExtensions.cs b/Src/DSInternals.Common/Extensions/StringExtensions.cs index 85d9866..77611da 100644 --- a/Src/DSInternals.Common/Extensions/StringExtensions.cs +++ b/Src/DSInternals.Common/Extensions/StringExtensions.cs @@ -9,7 +9,7 @@ public static class StringExtensions { private const char DBWithBinarySeparator = ':'; - private const string DBWithBinaryPrefix = "B"; + private const string DBWithBinaryPrefix = "B:"; private const string DNWithBinaryFormat = "B:{0}:{1}:{2}"; public static string TrimEnd(this string input, string suffix) @@ -53,16 +53,20 @@ /// public static byte[] FromDNWithBinary(this string input) { - Validator.AssertNotNull(input, nameof(input)); - string[] parts = input.Split(DBWithBinarySeparator); + Validator.AssertNotNullOrEmpty(input, nameof(input)); - // We do not need to perform a more thorough validation. - if (parts.Length != 4 || parts[0] != DBWithBinaryPrefix) + bool hasCorrectPrefix = input.StartsWith(DBWithBinaryPrefix); + int valueLeadingColonIndex = input.IndexOf(DBWithBinarySeparator, DBWithBinaryPrefix.Length); + int valueTrailingColonIndex = input.LastIndexOf(DBWithBinarySeparator); + bool has4Parts = valueLeadingColonIndex >= 3 && (valueLeadingColonIndex + 1) < valueTrailingColonIndex; + + if(!hasCorrectPrefix || !has4Parts) { + // We do not need to perform a more thorough validation. throw new ArgumentException(Resources.NotDNWithBinaryMessage, nameof(input)); } - return parts[2].HexToBinary(); + return input.HexToBinary(valueLeadingColonIndex + 1, valueTrailingColonIndex - valueLeadingColonIndex - 1); } public static string ToDNWithBinary(this byte[] binaryData, string distinguishedName) diff --git a/Src/DSInternals.Common/Validator.cs b/Src/DSInternals.Common/Validator.cs index 926831d..011dbb8 100644 --- a/Src/DSInternals.Common/Validator.cs +++ b/Src/DSInternals.Common/Validator.cs @@ -79,6 +79,15 @@ namespace DSInternals.Common throw new ArgumentNullException(paramName); } } + + public static void AssertNotNullOrEmpty(string value, string paramName) + { + if (String.IsNullOrEmpty(value)) + { + throw new ArgumentNullException(paramName); + } + } + public static void AssertNotNullOrWhiteSpace(string value, string paramName) { if(string.IsNullOrWhiteSpace(value)) diff --git a/Src/DSInternals.PowerShell/en-US/DSInternals.PowerShell.dll-Help.xml b/Src/DSInternals.PowerShell/en-US/DSInternals.PowerShell.dll-Help.xml index aa68fb8..1c4cee2 100644 --- a/Src/DSInternals.PowerShell/en-US/DSInternals.PowerShell.dll-Help.xml +++ b/Src/DSInternals.PowerShell/en-US/DSInternals.PowerShell.dll-Help.xml @@ -7908,7 +7908,7 @@ $initTask.RunAsTask() WeakPasswords - List of passwords that are considered weak, e.g. Password123 or April2019. + List of passwords that are considered weak, e.g. Password123 or April2019. If more than a handful passwords are to be tested, the WeakPasswordsFile parameter should be used instead. String[] @@ -7995,7 +7995,7 @@ $initTask.RunAsTask() WeakPasswords - List of passwords that are considered weak, e.g. Password123 or April2019. + List of passwords that are considered weak, e.g. Password123 or April2019. If more than a handful passwords are to be tested, the WeakPasswordsFile parameter should be used instead. String[] @@ -8104,7 +8104,7 @@ These accounts are not required to have a password: -------------------------- Example 2 -------------------------- PS C:\> $results = Get-ADReplAccount -All -NamingContext 'DC=contoso,DC=com' -Server LON-DC1 | - Test-PasswordQuality -WeakPasswords 'Pa$$w0rd','April2019' -WeakPasswordHashesFile pwned-passwords-ntlm-ordered-by-hash-v4.txt + Test-PasswordQuality -WeakPasswords 'Pa$$w0rd','April2019' -WeakPasswordHashesSortedFile pwned-passwords-ntlm-ordered-by-hash-v4.txt Performs an online credential hygiene audit of AD against HIBP + a custom wordlist.