From 8da6737d64bec79c430b48746415b2d2a0051172 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett (MSFT)" Date: Tue, 21 May 2019 13:29:16 -0700 Subject: [PATCH] Switch to v5 UUIDs as profile GUIDs for the default profiles (#913) This commit switches the GUIDs for default profiles from being randomly generated to being version 5 UUIDs. More info in #870. ## PR Checklist * [x] Closes #870 * [x] CLA signed * [x] Tests added/passed * [x] Requires documentation to be updated (#883) * [x] I've discussed this with core contributors already. ## Detailed Description of the Pull Request / Additional comments This commit has a number of changes that seem ancillary, but they're general goodness. Let me explain: * I've added a whole new Types test library with only two tests in * Since UUIDv5 generation requires SHA1, we needed to take a dependency on bcrypt * I honestly don't think we should have to link bcrypt in conhost, but LTO should take care of that * I considered adding a new Terminal-specific Utils/Types library, but that seemed like a waste * The best way to link bcrypt turned out to be in line with a discussion @miniksa and I had, where we decided we both love APISets and think that the console should link against them exclusively... so I've added `onecore_apiset.lib` to the front of the link line, where it will deflect the linker away from most of the other libs automagically. ``` StartGroup: UuidTests::TestV5UuidU8String Verify: AreEqual(uuidExpected, uuidActual) EndGroup: UuidTests::TestV5UuidU8String [Passed] StartGroup: UuidTests::TestV5UuidU16String Verify: AreEqual(uuidExpected, uuidActual) EndGroup: UuidTests::TestV5UuidU16String [Passed] ``` --- OpenConsole.sln | 21 ++++++++ src/cascadia/TerminalApp/CascadiaSettings.cpp | 32 +++++++++-- src/cascadia/TerminalApp/CascadiaSettings.h | 1 + src/cascadia/TerminalApp/Profile.cpp | 8 ++- src/cascadia/TerminalApp/Profile.h | 2 + src/common.build.exe.or.dll.props | 2 +- src/testlist/Microsoft.Console.Tests.testlist | 1 + src/types/dirs | 2 +- src/types/inc/utils.hpp | 30 +++++++++++ src/types/precomp.h | 12 +++-- src/types/ut_types/DefaultResource.rc | 12 +++++ src/types/ut_types/Types.Unit.Tests.vcxproj | 34 ++++++++++++ src/types/ut_types/UuidTests.cpp | 46 ++++++++++++++++ src/types/ut_types/product.pbxproj | 4 ++ src/types/ut_types/sources | 34 ++++++++++++ src/types/utils.cpp | 53 ++++++++++++++++++- tools/OpenConsole.psm1 | 2 +- tools/runut.cmd | 1 + tools/tests.xml | 1 + 19 files changed, 285 insertions(+), 13 deletions(-) create mode 100644 src/types/ut_types/DefaultResource.rc create mode 100644 src/types/ut_types/Types.Unit.Tests.vcxproj create mode 100644 src/types/ut_types/UuidTests.cpp create mode 100644 src/types/ut_types/product.pbxproj create mode 100644 src/types/ut_types/sources diff --git a/OpenConsole.sln b/OpenConsole.sln index 8ca6a8c45f8..c4241b6e992 100644 --- a/OpenConsole.sln +++ b/OpenConsole.sln @@ -228,6 +228,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Buffer", "Buffer", "{1E4A06 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{89CDCC5C-9F53-4054-97A4-639D99F169CD}" EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "Types.Unit.Tests", "src\types\ut_types\Types.Unit.Tests.vcxproj", "{34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution AuditMode|ARM64 = AuditMode|ARM64 @@ -1099,6 +1101,24 @@ Global {EF3E32A7-5FF6-42B4-B6E2-96CD7D033F00}.Release|x64.Build.0 = Release|x64 {EF3E32A7-5FF6-42B4-B6E2-96CD7D033F00}.Release|x86.ActiveCfg = Release|Win32 {EF3E32A7-5FF6-42B4-B6E2-96CD7D033F00}.Release|x86.Build.0 = Release|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|ARM64.ActiveCfg = AuditMode|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|ARM64.Build.0 = AuditMode|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|x64.ActiveCfg = AuditMode|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|x64.Build.0 = AuditMode|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|x86.ActiveCfg = AuditMode|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.AuditMode|x86.Build.0 = AuditMode|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|ARM64.ActiveCfg = Debug|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|ARM64.Build.0 = Debug|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|x64.ActiveCfg = Debug|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|x64.Build.0 = Debug|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|x86.ActiveCfg = Debug|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Debug|x86.Build.0 = Debug|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|ARM64.ActiveCfg = Release|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|ARM64.Build.0 = Release|ARM64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|x64.ActiveCfg = Release|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|x64.Build.0 = Release|x64 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|x86.ActiveCfg = Release|Win32 + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73}.Release|x86.Build.0 = Release|Win32 EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1157,6 +1177,7 @@ Global {F1995847-4AE5-479A-BBAF-382E51A63532} = {89CDCC5C-9F53-4054-97A4-639D99F169CD} {05500DEF-2294-41E3-AF9A-24E580B82836} = {89CDCC5C-9F53-4054-97A4-639D99F169CD} {1E4A062E-293B-4817-B20D-BF16B979E350} = {89CDCC5C-9F53-4054-97A4-639D99F169CD} + {34DE34D3-1CD6-4EE3-8BD9-A26B5B27EC73} = {89CDCC5C-9F53-4054-97A4-639D99F169CD} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {3140B1B7-C8EE-43D1-A772-D82A7061A271} diff --git a/src/cascadia/TerminalApp/CascadiaSettings.cpp b/src/cascadia/TerminalApp/CascadiaSettings.cpp index 7801fc8ac93..eab35094c1c 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.cpp +++ b/src/cascadia/TerminalApp/CascadiaSettings.cpp @@ -13,6 +13,11 @@ using namespace ::TerminalApp; using namespace winrt::Microsoft::Terminal::TerminalControl; using namespace winrt::TerminalApp; +// {2bde4a90-d05f-401c-9492-e40884ead1d8} +// uuidv5 properties: name format is UTF-16LE bytes +static constexpr GUID TERMINAL_PROFILE_NAMESPACE_GUID = +{ 0x2bde4a90, 0xd05f, 0x401c, { 0x94, 0x92, 0xe4, 0x8, 0x84, 0xea, 0xd1, 0xd8 } }; + CascadiaSettings::CascadiaSettings() : _globals{}, _profiles{} @@ -182,16 +187,15 @@ void CascadiaSettings::_CreateDefaultSchemes() // - void CascadiaSettings::_CreateDefaultProfiles() { - Profile cmdProfile{}; + auto cmdProfile{ _CreateDefaultProfile(L"cmd") }; cmdProfile.SetFontFace(L"Consolas"); cmdProfile.SetCommandline(L"cmd.exe"); cmdProfile.SetStartingDirectory(DEFAULT_STARTING_DIRECTORY); cmdProfile.SetColorScheme({ L"Campbell" }); cmdProfile.SetAcrylicOpacity(0.75); cmdProfile.SetUseAcrylic(true); - cmdProfile.SetName(L"cmd"); - Profile powershellProfile{}; + auto powershellProfile{ _CreateDefaultProfile(L"PowerShell") }; // If the user has installed PowerShell Core, we add PowerShell Core as a default. // PowerShell Core default folder is "%PROGRAMFILES%\PowerShell\[Version]\". std::wstring psCmdline = L"powershell.exe"; @@ -210,7 +214,6 @@ void CascadiaSettings::_CreateDefaultProfiles() powershellProfile.SetColorScheme({ L"Campbell" }); powershellProfile.SetDefaultBackground(RGB(1, 36, 86)); powershellProfile.SetUseAcrylic(false); - powershellProfile.SetName(L"PowerShell"); _profiles.emplace_back(powershellProfile); _profiles.emplace_back(cmdProfile); @@ -462,3 +465,24 @@ std::wstring CascadiaSettings::ExpandEnvironmentVariableString(std::wstring_view result.resize(requiredSize-1); return result; } + +// Method Description: +// - Helper function for creating a skeleton default profile with a pre-populated +// guid and name. +// Arguments: +// - name: the name of the new profile. +// Return Value: +// - A Profile, ready to be filled in +Profile CascadiaSettings::_CreateDefaultProfile(const std::wstring_view& name) +{ + Profile newProfile{ + Microsoft::Console::Utils::CreateV5Uuid( + TERMINAL_PROFILE_NAMESPACE_GUID, + gsl::as_bytes(gsl::make_span(name)) + ) + }; + + newProfile.SetName(static_cast(name)); + + return newProfile; +} diff --git a/src/cascadia/TerminalApp/CascadiaSettings.h b/src/cascadia/TerminalApp/CascadiaSettings.h index 102d208830d..3491f59623e 100644 --- a/src/cascadia/TerminalApp/CascadiaSettings.h +++ b/src/cascadia/TerminalApp/CascadiaSettings.h @@ -69,4 +69,5 @@ class TerminalApp::CascadiaSettings final static std::optional _LoadAsUnpackagedApp(); static bool _IsPowerShellCoreInstalled(std::wstring_view programFileEnv, std::filesystem::path& cmdline); static std::wstring ExpandEnvironmentVariableString(std::wstring_view source); + static Profile _CreateDefaultProfile(const std::wstring_view& name); }; diff --git a/src/cascadia/TerminalApp/Profile.cpp b/src/cascadia/TerminalApp/Profile.cpp index 95dae7c60c8..f5e18992278 100644 --- a/src/cascadia/TerminalApp/Profile.cpp +++ b/src/cascadia/TerminalApp/Profile.cpp @@ -49,7 +49,12 @@ static const std::wstring CURSORSHAPE_FILLEDBOX{ L"filledBox" }; static const std::wstring CURSORSHAPE_EMPTYBOX{ L"emptyBox" }; Profile::Profile() : - _guid{}, + Profile(Utils::CreateGuid()) +{ +} + +Profile::Profile(const winrt::guid& guid): + _guid(guid), _name{ L"Default" }, _schemeName{}, @@ -73,7 +78,6 @@ Profile::Profile() : _padding{ DEFAULT_PADDING }, _icon{ } { - UuidCreate(&_guid); } Profile::~Profile() diff --git a/src/cascadia/TerminalApp/Profile.h b/src/cascadia/TerminalApp/Profile.h index 1605ba0d7ad..a9fee235443 100644 --- a/src/cascadia/TerminalApp/Profile.h +++ b/src/cascadia/TerminalApp/Profile.h @@ -25,7 +25,9 @@ class TerminalApp::Profile final { public: + Profile(const winrt::guid& guid); Profile(); + ~Profile(); winrt::Microsoft::Terminal::Settings::TerminalSettings CreateTerminalSettings(const std::vector<::TerminalApp::ColorScheme>& schemes) const; diff --git a/src/common.build.exe.or.dll.props b/src/common.build.exe.or.dll.props index 04b586666f2..6846cf7a8a8 100644 --- a/src/common.build.exe.or.dll.props +++ b/src/common.build.exe.or.dll.props @@ -6,7 +6,7 @@ $(OutDir)$(TargetName)FullPDB.pdb - dwrite.lib;dxgi.lib;d2d1.lib;d3d11.lib;shcore.lib;uxtheme.lib;dwmapi.lib;winmm.lib;pathcch.lib;propsys.lib;uiautomationcore.lib;Shlwapi.lib;ntdll.lib;%(AdditionalDependencies) + onecore_apiset.lib;dwrite.lib;dxgi.lib;d2d1.lib;d3d11.lib;shcore.lib;uxtheme.lib;dwmapi.lib;winmm.lib;pathcch.lib;propsys.lib;uiautomationcore.lib;Shlwapi.lib;ntdll.lib;%(AdditionalDependencies) + + + + diff --git a/src/types/ut_types/UuidTests.cpp b/src/types/ut_types/UuidTests.cpp new file mode 100644 index 00000000000..4788542af85 --- /dev/null +++ b/src/types/ut_types/UuidTests.cpp @@ -0,0 +1,46 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" +#include "WexTestClass.h" +#include "..\..\inc\consoletaeftemplates.hpp" + +#include "..\inc\utils.hpp" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +using namespace Microsoft::Console::Utils; + +class UuidTests +{ + // {AD56DE9E-5167-41B6-80EB-FB19F7927D1A} + static constexpr GUID TEST_NAMESPACE_GUID{ 0xad56de9e, 0x5167, 0x41b6, + { 0x80, 0xeb, 0xfb, 0x19, 0xf7, 0x92, 0x7d, 0x1a } }; + + TEST_CLASS(UuidTests); + + TEST_METHOD(TestV5UuidU8String) + { + const GUID uuidExpected{ 0x8b9d4336, 0x0c82, 0x54c4, + { 0xb3, 0x15, 0xf1, 0xd2, 0xd2, 0x7e, 0xc6, 0xda} }; + + std::string name{ "testing" }; + auto uuidActual = CreateV5Uuid(TEST_NAMESPACE_GUID, gsl::as_bytes(gsl::make_span(name))); + + VERIFY_ARE_EQUAL(uuidExpected, uuidActual); + } + + TEST_METHOD(TestV5UuidU16String) + { + const GUID uuidExpected{ 0xe04fb1f7, 0x739d, 0x5d63, + { 0xbb, 0x18, 0xe0, 0xea, 0x00, 0xb1, 0x9e, 0xe8 } }; + + // This'll come out in little endian; the reference GUID was generated as such. + std::wstring name{ L"testing" }; + auto uuidActual = CreateV5Uuid(TEST_NAMESPACE_GUID, gsl::as_bytes(gsl::make_span(name))); + + VERIFY_ARE_EQUAL(uuidExpected, uuidActual); + } +}; diff --git a/src/types/ut_types/product.pbxproj b/src/types/ut_types/product.pbxproj new file mode 100644 index 00000000000..9e5ef983069 --- /dev/null +++ b/src/types/ut_types/product.pbxproj @@ -0,0 +1,4 @@ + + + + diff --git a/src/types/ut_types/sources b/src/types/ut_types/sources new file mode 100644 index 00000000000..a09fa19cf40 --- /dev/null +++ b/src/types/ut_types/sources @@ -0,0 +1,34 @@ +!include ..\..\project.unittest.inc + +# ------------------------------------- +# Program Information +# ------------------------------------- + +TARGETNAME = Microsoft.Console.Types.UnitTests +TARGETTYPE = DYNLINK +DLLDEF = + +# ------------------------------------- +# Sources, Headers, and Libraries +# ------------------------------------- + +SOURCES = \ + $(SOURCES) \ + UuidTests.cpp \ + DefaultResource.rc \ + +INCLUDES = \ + .. \ + $(INCLUDES) \ + +TARGETLIBS = \ + $(WINCORE_OBJ_PATH)\console\open\src\types\lib\$(O)\ConTypes.lib \ + $(TARGETLIBS) \ + +# ------------------------------------- +# Localization +# ------------------------------------- + +# Autogenerated. Sets file name for Device Guard whitelisting effort, used in RC.exe. +C_DEFINES = $(C_DEFINES) -D___TARGETNAME="""$(TARGETNAME).$(TARGETTYPE)""" +MUI_VERIFY_NO_LOC_RESOURCE = 1 diff --git a/src/types/utils.cpp b/src/types/utils.cpp index cd099e88fdc..32ef395518b 100644 --- a/src/types/utils.cpp +++ b/src/types/utils.cpp @@ -3,7 +3,7 @@ #include "precomp.h" #include "inc/utils.hpp" -#include + using namespace Microsoft::Console; // Function Description: @@ -43,6 +43,17 @@ GUID Utils::GuidFromString(const std::wstring wstr) return result; } +// Method Description: +// - Creates a GUID, but not via an out parameter. +// Return Value: +// - A GUID if there's enough randomness; otherwise, an exception. +GUID Utils::CreateGuid() +{ + GUID result{}; + THROW_IF_FAILED(::CoCreateGuid(&result)); + return result; +} + // Function Description: // - Creates a String representation of a color, in the format "#RRGGBB" // Arguments: @@ -406,3 +417,43 @@ void Utils::SetColorTableAlpha(gsl::span& table, const BYTE newAlpha) WI_UpdateFlagsInMask(color, 0xff000000, shiftedAlpha); } } + +// Function Description: +// - Generate a Version 5 UUID (specified in RFC4122 4.3) +// v5 UUIDs are stable given the same namespace and "name". +// Arguments: +// - namespaceGuid: The GUID of the v5 UUID namespace, which provides both +// a seed and a tacit agreement that all UUIDs generated +// with it will follow the same data format. +// - name: Bytes comprising the name (in a namespace-specific format) +// Return Value: +// - a new stable v5 UUID +GUID Utils::CreateV5Uuid(const GUID& namespaceGuid, const gsl::span& name) +{ + // v5 uuid generation happens over values in network byte order, so let's enforce that + auto correctEndianNamespaceGuid{ EndianSwap(namespaceGuid) }; + + wil::unique_bcrypt_hash hash; + THROW_IF_NTSTATUS_FAILED(BCryptCreateHash(BCRYPT_SHA1_ALG_HANDLE, &hash, nullptr, 0, nullptr, 0, 0)); + + // According to N4713 8.2.1.11 [basic.lval], accessing the bytes underlying an object + // through unsigned char or char pointer *is defined*. + THROW_IF_NTSTATUS_FAILED(BCryptHashData(hash.get(), reinterpret_cast(&correctEndianNamespaceGuid), sizeof(GUID), 0)); + // BCryptHashData is ill-specified in that it leaves off "const" qualification for pbInput + THROW_IF_NTSTATUS_FAILED(BCryptHashData(hash.get(), reinterpret_cast(const_cast(name.data())), gsl::narrow(name.size()), 0)); + + std::array buffer; + THROW_IF_NTSTATUS_FAILED(BCryptFinishHash(hash.get(), buffer.data(), gsl::narrow(buffer.size()), 0)); + + buffer[6] = (buffer[6] & 0x0F) | 0x50; // set the uuid version to 5 + buffer[8] = (buffer[8] & 0x3F) | 0x80; // set the variant to 2 (RFC4122) + + // We're using memcpy here pursuant to N4713 6.7.2/3 [basic.types], + // "...the underlying bytes making up the object can be copied into an array + // of char or unsigned char...array is copied back into the object..." + // std::copy may compile down to ::memcpy for these types, but using it might + // contravene the standard and nobody's got time for that. + GUID newGuid{ 0 }; + ::memcpy_s(&newGuid, sizeof(GUID), buffer.data(), sizeof(GUID)); + return EndianSwap(newGuid); +} diff --git a/tools/OpenConsole.psm1 b/tools/OpenConsole.psm1 index 953f7200785..b9b28afe605 100644 --- a/tools/OpenConsole.psm1 +++ b/tools/OpenConsole.psm1 @@ -91,7 +91,7 @@ function Invoke-OpenConsoleTests() [switch]$FTOnly, [parameter(Mandatory=$false)] - [ValidateSet('host', 'interactivityWin32', 'terminal', 'adapter', 'feature', 'uia', 'textbuffer')] + [ValidateSet('host', 'interactivityWin32', 'terminal', 'adapter', 'feature', 'uia', 'textbuffer', 'types')] [string]$Test, [parameter(Mandatory=$false)] diff --git a/tools/runut.cmd b/tools/runut.cmd index d144b0eaf89..51dffb5e0d8 100644 --- a/tools/runut.cmd +++ b/tools/runut.cmd @@ -9,4 +9,5 @@ call %TAEF% ^ %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\Conhost.Interactivity.Win32.Unit.Tests.dll ^ %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\ConParser.Unit.Tests.dll ^ %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\ConAdapter.Unit.Tests.dll ^ + %OPENCON%\bin\%PLATFORM%\%_LAST_BUILD_CONF%\Types.Unit.Tests.dll ^ %* diff --git a/tools/tests.xml b/tools/tests.xml index 5350f8e62b2..3a64144b843 100644 --- a/tools/tests.xml +++ b/tools/tests.xml @@ -5,6 +5,7 @@ +