From 5e9b982ddf5a205752604a85332946bc39b1a31a Mon Sep 17 00:00:00 2001 From: Marek Roszko Date: Sat, 12 Mar 2022 19:12:34 -0500 Subject: [PATCH] Plant the ability to verify code signing signatures when trying to load kifaces Off by default and intended for use in released builds only --- CMakeLists.txt | 10 +++++ common/kiway.cpp | 10 +++++ libs/kiplatform/CMakeLists.txt | 1 + libs/kiplatform/gtk/environment.cpp | 6 +++ .../include/kiplatform/environment.h | 8 ++++ libs/kiplatform/msw/environment.cpp | 39 +++++++++++++++++++ libs/kiplatform/osx/environment.mm | 6 +++ 7 files changed, 80 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cd76e8db5..98a042bd36 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -156,6 +156,12 @@ option( KICAD_STEP_EXPORT_LIB "Build and use kicad2step as a library, meant for debugging" OFF ) +cmake_dependent_option( KICAD_WIN32_VERIFY_CODESIGN + "When enabled, verifies the code signing signature of certain DLLs loaded during runtime." + OFF "WIN32" + OFF ) +endif() + # Global setting: exports are explicit set( CMAKE_CXX_VISIBILITY_PRESET "hidden" ) set( CMAKE_VISIBILITY_INLINES_HIDDEN ON ) @@ -177,6 +183,10 @@ if( KICAD_GAL_PROFILE ) add_definitions( -DKICAD_GAL_PROFILE ) endif() +if( KICAD_WIN32_VERIFY_CODESIGN ) + add_definitions( -DKICAD_WIN32_VERIFY_CODESIGN ) +endif() + # Ensure DEBUG is defined for all platforms in Debug builds # change to add_compile_definitions() after minimum required CMake version is 3.12 set_property( DIRECTORY APPEND PROPERTY COMPILE_DEFINITIONS $<$:DEBUG> ) diff --git a/common/kiway.cpp b/common/kiway.cpp index ab84801f1d..7803a1829b 100644 --- a/common/kiway.cpp +++ b/common/kiway.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -230,6 +231,15 @@ KIFACE* KIWAY::KiFACE( FACE_T aFaceId, bool doLoad ) } } +#ifdef KICAD_WIN32_VERIFY_CODESIGN + bool codeSignOk = KIPLATFORM::ENV::VerifyFileSignature( dname ); + if( !codeSignOk ) + { + msg.Printf( _( "Failed to verify kiface library '%s' signature." ), dname ); + THROW_IO_ERROR( msg ); + } +#endif + wxDynamicLibrary dso; void* addr = nullptr; diff --git a/libs/kiplatform/CMakeLists.txt b/libs/kiplatform/CMakeLists.txt index 844edbaeef..23716f6fc2 100644 --- a/libs/kiplatform/CMakeLists.txt +++ b/libs/kiplatform/CMakeLists.txt @@ -30,6 +30,7 @@ elseif( WIN32 ) set( PLATFORM_LIBS "Shlwapi" "winhttp" + "wintrust" ) elseif( UNIX ) set( PLATFORM_SRCS diff --git a/libs/kiplatform/gtk/environment.cpp b/libs/kiplatform/gtk/environment.cpp index 21bfbdb17b..8c7e288b2a 100644 --- a/libs/kiplatform/gtk/environment.cpp +++ b/libs/kiplatform/gtk/environment.cpp @@ -107,3 +107,9 @@ bool KIPLATFORM::ENV::GetSystemProxyConfig( const wxString& aURL, PROXY_CONFIG& { return false; } + + +bool KIPLATFORM::ENV::VerifyFileSignature( const wxString& aPath ) +{ + return true; +} \ No newline at end of file diff --git a/libs/kiplatform/include/kiplatform/environment.h b/libs/kiplatform/include/kiplatform/environment.h index 98bba721cd..8cfc7e22bc 100644 --- a/libs/kiplatform/include/kiplatform/environment.h +++ b/libs/kiplatform/include/kiplatform/environment.h @@ -88,5 +88,13 @@ namespace KIPLATFORM * @return True if successful fetched proxy info */ bool GetSystemProxyConfig( const wxString& aURL, PROXY_CONFIG& aCfg ); + + /** + * Validates the code signing signature of a given file + * This is most likely only ever going to be applicable to Windows + * + * @return True if file signature passes + */ + bool VerifyFileSignature( const wxString& aPath ); } } diff --git a/libs/kiplatform/msw/environment.cpp b/libs/kiplatform/msw/environment.cpp index 17c0e1fafe..4c00cb3a68 100644 --- a/libs/kiplatform/msw/environment.cpp +++ b/libs/kiplatform/msw/environment.cpp @@ -31,6 +31,10 @@ #include #include +#include +#include +#include + void KIPLATFORM::ENV::Init() { @@ -258,3 +262,38 @@ bool KIPLATFORM::ENV::GetSystemProxyConfig( const wxString& aURL, PROXY_CONFIG& return success; } + + +bool KIPLATFORM::ENV::VerifyFileSignature( const wxString& aPath ) +{ + WINTRUST_FILE_INFO fileData; + memset( &fileData, 0, sizeof( fileData ) ); + fileData.cbStruct = sizeof( WINTRUST_FILE_INFO ); + fileData.pcwszFilePath = aPath.wc_str(); + + // verifies entire certificate chain + GUID policy = WINTRUST_ACTION_GENERIC_VERIFY_V2; + + WINTRUST_DATA trustData; + memset( &trustData, 0, sizeof( trustData ) ); + + trustData.cbStruct = sizeof( trustData ); + trustData.dwUIChoice = WTD_UI_NONE; + // revocation checking incurs latency penalities due to need for online queries + trustData.fdwRevocationChecks = WTD_REVOKE_NONE; + trustData.dwUnionChoice = WTD_CHOICE_FILE; + trustData.dwStateAction = WTD_STATEACTION_VERIFY; + trustData.pFile = &fileData; + + + bool verified = false; + LONG status = WinVerifyTrust( NULL, &policy, &trustData ); + + verified = ( status == ERROR_SUCCESS ); + + // Cleanup/release (yes its weird looking) + trustData.dwStateAction = WTD_STATEACTION_CLOSE; + WinVerifyTrust( NULL, &policy, &trustData ); + + return verified; +} \ No newline at end of file diff --git a/libs/kiplatform/osx/environment.mm b/libs/kiplatform/osx/environment.mm index 5fb3b4477f..8483de812d 100644 --- a/libs/kiplatform/osx/environment.mm +++ b/libs/kiplatform/osx/environment.mm @@ -99,4 +99,10 @@ wxString KIPLATFORM::ENV::GetUserCachePath() bool KIPLATFORM::ENV::GetSystemProxyConfig( const wxString& aURL, PROXY_CONFIG& aCfg ) { return false; +} + + +bool KIPLATFORM::ENV::VerifyFileSignature( const wxString& aPath ) +{ + return true; } \ No newline at end of file