From c9a0bcdcd3462d169ba2c9b0211ced96197db32d Mon Sep 17 00:00:00 2001 From: Gary Daniels Date: Tue, 28 Jan 2025 15:23:06 -0500 Subject: [PATCH] enforce signing of midi 2 components unless in developer mode --- src/api/Inc/midi_utils.h | 94 +++++++++++++++++++ .../Service/Exe/MidiConfigurationManager.cpp | 44 +++++---- src/api/Service/Exe/MidiDeviceManager.cpp | 6 ++ src/api/Service/Exe/MidiDevicePipe.cpp | 4 + .../Exe/MidiEndpointProtocolWorker.cpp | 2 + src/api/Service/Exe/MidiTransformPipe.cpp | 4 + src/api/Service/Exe/stdafx.h | 1 + .../Midi2.KSAggregateMidiInProxy.cpp | 4 + .../Midi2.KSAggregateMidiOutProxy.cpp | 4 + src/api/Transport/KSAggregateTransport/pch.h | 1 + 10 files changed, 144 insertions(+), 20 deletions(-) create mode 100644 src/api/Inc/midi_utils.h diff --git a/src/api/Inc/midi_utils.h b/src/api/Inc/midi_utils.h new file mode 100644 index 000000000..227238caa --- /dev/null +++ b/src/api/Inc/midi_utils.h @@ -0,0 +1,94 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License +// ============================================================================ +// This is part of the Windows MIDI Services App API and should be used +// in your Windows application via an official binary distribution. +// Further information: https://aka.ms/midi +// ============================================================================ + +#pragma once + +#include +#include +#include +#include + +#define GUID_STRING_LENGTH 39 + +const wchar_t appModelPath[] = L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock"; +const wchar_t developerModeKey[] = L"AllowDevelopmentWithoutDevLicense"; + +namespace WindowsMidiServicesInternal +{ + inline bool IsDeveloperModeEnabled() + { + DWORD developerMode {0}; + DWORD dataSize = sizeof(DWORD); + + // Skip check if developer mode is enabled. + if (ERROR_SUCCESS == RegGetValue(HKEY_LOCAL_MACHINE, appModelPath, developerModeKey, RRF_RT_DWORD, NULL, &developerMode, &dataSize) && + developerMode == 1) + { + return true; + } + + return false; + } + + inline std::wstring GetFileNameFromCLSID(const std::wstring& clsidStr) + { + std::wstring subKey = L"CLSID\\" + clsidStr + L"\\InprocServer32"; + wchar_t filePath[MAX_PATH] {NULL}; + DWORD bufferSize = sizeof(filePath); + + if (ERROR_SUCCESS != RegGetValue(HKEY_CLASSES_ROOT, subKey.c_str(), NULL, RRF_RT_REG_SZ, NULL, &filePath, &bufferSize)) + { + return L""; + } + + return std::wstring(filePath); + } + + inline HRESULT IsFileDigitallySigned(const std::wstring& filePath) + { + LONG status; + WINTRUST_FILE_INFO fileData; + WINTRUST_DATA winTrustData; + GUID policyGUID = WINTRUST_ACTION_GENERIC_VERIFY_V2; + + memset(&fileData, 0, sizeof(fileData)); + fileData.cbStruct = sizeof(WINTRUST_FILE_INFO); + fileData.pcwszFilePath = filePath.c_str(); + + memset(&winTrustData, 0, sizeof(winTrustData)); + winTrustData.cbStruct = sizeof(WINTRUST_DATA); + winTrustData.dwUIChoice = WTD_UI_NONE; + winTrustData.fdwRevocationChecks = WTD_REVOKE_NONE; + winTrustData.dwUnionChoice = WTD_CHOICE_FILE; + winTrustData.dwStateAction = WTD_STATEACTION_VERIFY; + winTrustData.dwProvFlags = WTD_SAFER_FLAG; + winTrustData.pFile = &fileData; + + status = WinVerifyTrust(NULL, &policyGUID, &winTrustData); + + winTrustData.dwStateAction = WTD_STATEACTION_CLOSE; + WinVerifyTrust(NULL, &policyGUID, &winTrustData); + + return HRESULT_FROM_WIN32(status); + } + + inline HRESULT IsComponentPermitted(GUID guid) + { + // If we are in developer mode, we allow loading of untrusted components. + RETURN_HR_IF(S_OK, IsDeveloperModeEnabled()); + + wchar_t guidString[GUID_STRING_LENGTH] {NULL}; + RETURN_HR_IF(E_INVALIDARG, 0 == StringFromGUID2(guid, guidString, _countof(guidString))); + + // otherwise, confirm the component is trusted. + std::wstring fileName = GetFileNameFromCLSID(std::wstring(guidString)); + RETURN_IF_FAILED(IsFileDigitallySigned(fileName.c_str())); + + return S_OK; + } +} diff --git a/src/api/Service/Exe/MidiConfigurationManager.cpp b/src/api/Service/Exe/MidiConfigurationManager.cpp index d24edfe47..5b21317d1 100644 --- a/src/api/Service/Exe/MidiConfigurationManager.cpp +++ b/src/api/Service/Exe/MidiConfigurationManager.cpp @@ -327,33 +327,37 @@ std::vector CMidiConfigurationManager::GetAllEnabledTransport wil::com_ptr_nothrow midiTransport; wil::com_ptr_nothrow plugin; - if (SUCCEEDED(CoCreateInstance(transportId, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&midiTransport)))) + // Do not load any transports which are untrusted, unless in developer mode. + if (SUCCEEDED(internal::IsComponentPermitted(transportId))) { - if (SUCCEEDED(midiTransport->Activate(__uuidof(IMidiServiceTransportPluginMetadataProvider), (void**)&plugin))) + if (SUCCEEDED(CoCreateInstance(transportId, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&midiTransport)))) { - plugin->Initialize(); + if (SUCCEEDED(midiTransport->Activate(__uuidof(IMidiServiceTransportPluginMetadataProvider), (void**)&plugin))) + { + plugin->Initialize(); - TRANSPORTMETADATA metadata; + TRANSPORTMETADATA metadata; - LOG_IF_FAILED(plugin->GetMetadata(&metadata)); + LOG_IF_FAILED(plugin->GetMetadata(&metadata)); - results.push_back(std::move(metadata)); + results.push_back(std::move(metadata)); - plugin->Shutdown(); - } - else - { - // log that the interface isn't there, but don't terminate or anything + plugin->Shutdown(); + } + else + { + // log that the interface isn't there, but don't terminate or anything - TraceLoggingWrite( - MidiSrvTelemetryProvider::Provider(), - MIDI_TRACE_EVENT_ERROR, - TraceLoggingString(__FUNCTION__, MIDI_TRACE_EVENT_LOCATION_FIELD), - TraceLoggingLevel(WINEVENT_LEVEL_ERROR), - TraceLoggingWideString(L"Unable to activate IMidiServiceTransportPlugin", MIDI_TRACE_EVENT_MESSAGE_FIELD), - TraceLoggingGuid(transportId, "transport id"), - TraceLoggingPointer(this, "this") - ); + TraceLoggingWrite( + MidiSrvTelemetryProvider::Provider(), + MIDI_TRACE_EVENT_ERROR, + TraceLoggingString(__FUNCTION__, MIDI_TRACE_EVENT_LOCATION_FIELD), + TraceLoggingLevel(WINEVENT_LEVEL_ERROR), + TraceLoggingWideString(L"Unable to activate IMidiServiceTransportPlugin", MIDI_TRACE_EVENT_MESSAGE_FIELD), + TraceLoggingGuid(transportId, "transport id"), + TraceLoggingPointer(this, "this") + ); + } } } } diff --git a/src/api/Service/Exe/MidiDeviceManager.cpp b/src/api/Service/Exe/MidiDeviceManager.cpp index c58b7c945..5b2ccfe8c 100644 --- a/src/api/Service/Exe/MidiDeviceManager.cpp +++ b/src/api/Service/Exe/MidiDeviceManager.cpp @@ -119,6 +119,12 @@ CMidiDeviceManager::Initialize( TraceLoggingGuid(TransportLayer, "transport layer") ); + // Do not load any transports which are untrusted, unless in developer mode. + if (FAILED(internal::IsComponentPermitted(TransportLayer))) + { + continue; + } + // changed these from a return-on-fail to just log, so we don't prevent service startup // due to one bad transport diff --git a/src/api/Service/Exe/MidiDevicePipe.cpp b/src/api/Service/Exe/MidiDevicePipe.cpp index 4543538c2..b70888663 100644 --- a/src/api/Service/Exe/MidiDevicePipe.cpp +++ b/src/api/Service/Exe/MidiDevicePipe.cpp @@ -52,6 +52,10 @@ CMidiDevicePipe::Initialize( RETURN_HR_IF_NULL(E_INVALIDARG, prop); m_TransportGuid = winrt::unbox_value(prop); + // Confirm that this component is either signed, or we are in developer mode. + // Else, do not use it. + RETURN_IF_FAILED(internal::IsComponentPermitted(m_TransportGuid)); + GUID dummySessionId{}; if (MidiFlowBidirectional == creationParams->Flow) diff --git a/src/api/Service/Exe/MidiEndpointProtocolWorker.cpp b/src/api/Service/Exe/MidiEndpointProtocolWorker.cpp index ef1fb593a..0e258e852 100644 --- a/src/api/Service/Exe/MidiEndpointProtocolWorker.cpp +++ b/src/api/Service/Exe/MidiEndpointProtocolWorker.cpp @@ -181,6 +181,8 @@ CMidiEndpointProtocolWorker::Start( // this is not a good idea, but we don't have a reference to the COM lib here GUID midi2MidiSrvTransportIID = internal::StringToGuid(L"{2BA15E4E-5417-4A66-85B8-2B2260EFBC84}"); + + RETURN_IF_FAILED(internal::IsComponentPermitted(midi2MidiSrvTransportIID)); RETURN_IF_FAILED(CoCreateInstance((IID)midi2MidiSrvTransportIID, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&serviceTransport))); RETURN_IF_NULL_ALLOC(serviceTransport); diff --git a/src/api/Service/Exe/MidiTransformPipe.cpp b/src/api/Service/Exe/MidiTransformPipe.cpp index eed9e0af2..fd1c206ea 100644 --- a/src/api/Service/Exe/MidiTransformPipe.cpp +++ b/src/api/Service/Exe/MidiTransformPipe.cpp @@ -33,6 +33,10 @@ CMidiTransformPipe::Initialize( m_TransformGuid = pipeCreationParams->TransformGuid; + // Confirm that this component is either signed, or we are in developer mode. + // Else, do not use it. + RETURN_IF_FAILED(internal::IsComponentPermitted(m_TransformGuid)); + // Transforms are "bidirectional" from the midi pipes perspective, // so we always initialize the pipe as bidirectional. RETURN_IF_FAILED(CMidiPipe::Initialize(device, MidiFlowBidirectional)); diff --git a/src/api/Service/Exe/stdafx.h b/src/api/Service/Exe/stdafx.h index dc95a5992..e0c18d8c2 100644 --- a/src/api/Service/Exe/stdafx.h +++ b/src/api/Service/Exe/stdafx.h @@ -48,6 +48,7 @@ #include "devpkey.h" // Shared helpers +#include "midi_utils.h" #include "midi_ump.h" #include "midi_timestamp.h" #include "ump_helpers.h" diff --git a/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiInProxy.cpp b/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiInProxy.cpp index ad0113ef4..b30cedea7 100644 --- a/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiInProxy.cpp +++ b/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiInProxy.cpp @@ -83,6 +83,10 @@ CMidi2KSAggregateMidiInProxy::Initialize( auto transformId = __uuidof(Midi2BS2UMPTransform); + // Confirm that this component is either signed, or we are in developer mode. + // Else, do not use it. + RETURN_IF_FAILED(internal::IsComponentPermitted(transformId)); + RETURN_IF_FAILED(CoCreateInstance(transformId, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&transformPlugin))); RETURN_IF_FAILED(transformPlugin->Activate(__uuidof(IMidiDataTransform), (void**)(&m_bs2UmpTransform))); diff --git a/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiOutProxy.cpp b/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiOutProxy.cpp index 3c55c6cc7..28f9bf4d7 100644 --- a/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiOutProxy.cpp +++ b/src/api/Transport/KSAggregateTransport/Midi2.KSAggregateMidiOutProxy.cpp @@ -79,6 +79,10 @@ CMidi2KSAggregateMidiOutProxy::Initialize( auto transformId = __uuidof(Midi2UMP2BSTransform); + // Confirm that this component is either signed, or we are in developer mode. + // Else, do not use it. + RETURN_IF_FAILED(internal::IsComponentPermitted(transformId)); + RETURN_IF_FAILED(CoCreateInstance(transformId, nullptr, CLSCTX_ALL, IID_PPV_ARGS(&transformPlugin))); RETURN_IF_FAILED(transformPlugin->Activate(__uuidof(IMidiDataTransform), (void**)(&m_ump2BSTransform))); diff --git a/src/api/Transport/KSAggregateTransport/pch.h b/src/api/Transport/KSAggregateTransport/pch.h index 79b40541d..7057ebf5f 100644 --- a/src/api/Transport/KSAggregateTransport/pch.h +++ b/src/api/Transport/KSAggregateTransport/pch.h @@ -65,6 +65,7 @@ namespace internal = ::WindowsMidiServicesInternal; #include "MidiDefs.h" #include "WindowsMidiServices.h" #include "WindowsMidiServices_i.c" +#include "midi_utils.h" #undef GetObject