fix(windows): re-enable signature check

Fixes #9692. Signature checking was skipped because we missed a ".virtual" to force nmake to build the test and test_i3633 targets. This opened up a small cascade of related formatting issues on Makefiles, and the fact that the test_i3633 (has there ever been a more poorly named project?) Makefile did not even work. Refactored significantly, added same tests to Developer Makefile, and also now verifying the .msi and installer executable. We can improve this further but I'd like to get this in to avoid further critical issues with code signing given the current broken signing configuration.
keymanapp · Oct 6, 2023 · eeb755f · eeb755f
1 parent 77dc490
commit eeb755f
Show file tree

Hide file tree

Showing 21 changed files with 100 additions and 52 deletions.
diff --git a/common/windows/delphi/ext/sentry/test/Makefile b/common/windows/delphi/ext/sentry/test/Makefile
@@ -4,7 +4,8 @@
 
 !include ..\..\..\Defines.mak
 
-build: dirs # version.res manifest.res
+build: dirs
+# version.res manifest.res
     $(DELPHI_MSBUILD) SentryClientTest.dproj "/p:Platform=Win32"
     $(DELPHI_MSBUILD) SentryClientVclTest.dproj "/p:Platform=Win32"
 

diff --git a/common/windows/delphi/tools/Makefile b/common/windows/delphi/tools/Makefile
@@ -7,7 +7,7 @@ NOTARGET_SIGNCODE=yes
 !ifdef NODELPHI
 TARGETS=.virtual
 !else
-TARGETS=build_standards_data buildunidata devtools sentrytool test-klog
+TARGETS=build_standards_data buildunidata devtools sentrytool test-klog verify_signatures
 !endif
 
 CLEANS=clean-tools
@@ -28,6 +28,10 @@ buildunidata: .virtual
     cd $(COMMON_ROOT)\tools\buildunidata
     $(MAKE) $(TARGET)
 
+verify_signatures: .virtual
+    cd $(COMMON_ROOT)\tools\verify_signatures
+    $(MAKE) $(TARGET)
+
 devtools: .virtual
 !ifdef NODELPHI
     echo Skipping devtools

diff --git a/common/windows/delphi/tools/verify_signatures/Makefile b/common/windows/delphi/tools/verify_signatures/Makefile
@@ -0,0 +1,13 @@
+#
+# test for signatures and version information being correct in ?install directory or ?bin directory
+#
+
+!include ..\..\Defines.mak
+
+build:
+    $(DELPHI_MSBUILD) verify_signatures.dproj
+    copy sigcheck.bin $(WIN32_TARGET_PATH)\sigcheck.exe
+
+clean: def-clean
+
+!include ..\..\Target.mak
diff --git a/windows/src/test/test_i3633/sigcheck.bin → ...lphi/tools/verify_signatures/sigcheck.bin b/windows/src/test/test_i3633/sigcheck.bin → ...lphi/tools/verify_signatures/sigcheck.bin
diff --git a/windows/src/test/test_i3633/verify.dpr → ...s/verify_signatures/verify_signatures.dpr b/windows/src/test/test_i3633/verify.dpr → ...s/verify_signatures/verify_signatures.dpr
@@ -1,4 +1,4 @@
-program verify;
+program verify_signatures;
 
 {$APPTYPE CONSOLE}
 
@@ -90,7 +90,7 @@ begin
 
     if (ParamStr(1) = '-?') or (ParamCount < 1) then
     begin
-      writeln('verify [-d] VERSION.md: Verify the output of sigcheck to ensure all executables are signed and have proper version.');
+      writeln('verify_signatures [-d] VERSION.md: Verify the output of sigcheck to ensure all executables are signed and have proper version.');
       writeln('  -d: Check the timestamp on the signature is less than 2 days old.');
       writeln('  VERSION.md: path to the version to verify against');
       Halt(2);

diff --git a/windows/src/test/test_i3633/verify.dproj → ...verify_signatures/verify_signatures.dproj b/windows/src/test/test_i3633/verify.dproj → ...verify_signatures/verify_signatures.dproj
@@ -3,7 +3,7 @@
         <ProjectGuid>{A6DCA558-8DD0-4FFF-8DAE-F305AB8D2AF4}</ProjectGuid>
         <ProjectVersion>18.8</ProjectVersion>
         <FrameworkType>None</FrameworkType>
-        <MainSource>verify.dpr</MainSource>
+        <MainSource>verify_signatures.dpr</MainSource>
         <Base>True</Base>
         <Config Condition="'$(Config)'==''">Debug</Config>
         <Platform Condition="'$(Platform)'==''">Win32</Platform>
@@ -45,7 +45,7 @@
         <Base>true</Base>
     </PropertyGroup>
     <PropertyGroup Condition="'$(Base)'!=''">
-        <SanitizedProjectName>verify</SanitizedProjectName>
+        <SanitizedProjectName>verify_signatures</SanitizedProjectName>
         <Icns_MainIcns>$(BDS)\bin\delphi_PROJECTICNS.icns</Icns_MainIcns>
         <Icon_MainIcon>$(BDS)\bin\delphi_PROJECTICON.ico</Icon_MainIcon>
         <DCC_UsePackage>bindcompfmx;fmx;rtl;dbrtl;DbxClientDriver;bindcomp;inetdb;DBXInterBaseDriver;xmlrtl;DbxCommonDriver;DBXMySQLDriver;dbxcds;soaprtl;bindengine;CustomIPTransport;dsnap;fmxase;inet;fmxobj;inetdbxpress;fmxdae;dbexpress;$(DCC_UsePackage)</DCC_UsePackage>
@@ -161,7 +161,7 @@
                     <VersionInfoKeys Name="Comments"/>
                 </VersionInfoKeys>
                 <Source>
-                    <Source Name="MainSource">verify.dpr</Source>
+                    <Source Name="MainSource">verify_signatures.dpr</Source>
                 </Source>
                 <Excluded_Packages>
                     <Excluded_Packages Name="$(BDSBIN)\dcloffice2k260.bpl">Microsoft Office 2000 Sample Automation Server Wrapper Components</Excluded_Packages>

diff --git a/windows/src/test/test_i3633/verify.res → ...s/verify_signatures/verify_signatures.res b/windows/src/test/test_i3633/verify.res → ...s/verify_signatures/verify_signatures.res
diff --git a/developer/src/Makefile b/developer/src/Makefile
@@ -153,6 +153,8 @@ do-build-release:
     $(MAKE) build-release-dependencies
     $(MAKE) "SIGNCODE_BUILD=SIGNCODE_BUILD" build test
     $(MAKE) signcode
+    cd $(DEVELOPER_ROOT)\src\tools\verify_signatures
+    $(MAKE) test
 
     cd $(DEVELOPER_ROOT)\src\inst
     $(MAKE)

diff --git a/developer/src/inst/download.in.mak b/developer/src/inst/download.in.mak
@@ -26,6 +26,9 @@ copykmdev: makeinstaller make-kmcomp-install-zip
     -mkdir $(DEVELOPER_ROOT)\release\$Version
     copy /Y $(DEVELOPER_ROOT)\src\inst\keymandeveloper.msi $(DEVELOPER_ROOT)\release\$Version\keymandeveloper.msi
     copy /Y $(DEVELOPER_ROOT)\src\inst\keymandeveloper-$Version.exe $(DEVELOPER_ROOT)\release\$Version\keymandeveloper-$Version.exe
+    $(SIGCHECK) $(DEVELOPER_ROOT)\release\$Version\* > sig1
+    $(VERIFY_SIGNATURES) < sig1
+    -del sig1
 
 test-releaseexists:
     if exist $(DEVELOPER_ROOT)\release\$Version\keymandeveloper*.msi echo. & echo Release $Version already exists. Delete it or update VERSION.md and try again & exit 1

diff --git a/developer/src/tools/verify_signatures/Makefile b/developer/src/tools/verify_signatures/Makefile
@@ -0,0 +1,19 @@
+#
+# test for signatures and version information being correct in bin folder
+#
+
+!include ..\..\Defines.mak
+
+test: prereq
+  $(SIGCHECK) $(DEVELOPER_PROGRAM)\* > sig1
+  $(VERIFY_SIGNATURES) < sig1
+
+# prereq may not be needed?
+prereq:
+  cd $(VERIFY_SIGNATURES_PATH)
+  $(MAKE)
+
+clean: def-clean
+  -del sig1
+
+!include ..\..\Target.mak
diff --git a/windows/src/Defines.mak b/windows/src/Defines.mak
@@ -322,3 +322,12 @@ SYMSTORE="C:\Program Files (x86)\Windows Kits\10\Debuggers\x64\symstore.exe" add
     /compress /f
 
 CLEAN=-del /S /Q
+
+#
+# Signature checks
+#
+
+VERIFY_SIGNATURES_PATH=$(KEYMAN_ROOT)\common\windows\delphi\tools\verify_signatures
+# Note: hyphen prefix is important to ignore return value from sigcheck
+SIGCHECK=-$(VERIFY_SIGNATURES_PATH)\$(WIN32_TARGET_PATH)\sigcheck -q -s -e -v -accepteula
+VERIFY_SIGNATURES=$(VERIFY_SIGNATURES_PATH)\$(WIN32_TARGET_PATH)\verify_signatures -d $(KEYMAN_ROOT)\VERSION.md
diff --git a/windows/src/Makefile b/windows/src/Makefile
@@ -81,7 +81,7 @@ support: global
     $(MAKE) $(TARGET)
     cd $(ROOT)\src
 
-test:
+test: .virtual
     cd $(ROOT)\src\test
     $(MAKE) $(TARGET)
     cd $(ROOT)\src

diff --git a/windows/src/desktop/inst/download.in b/windows/src/desktop/inst/download.in
@@ -17,6 +17,10 @@ copyredist-desktop:
   copy /Y keymandesktop.exe $(ROOT)\release\$Version\keyman-$Version.exe
   copy /Y $(ROOT)\bin\desktop\setup.exe $(ROOT)\release\$Version\setup.exe
 
+  $(SIGCHECK) $(ROOT)\release\$Version\* > sig1
+  $(VERIFY_SIGNATURES) < sig1
+  -del sig1
+
 # Copy the unsigned setup.exe for use in bundling scenarios; zip it up for clarity
   $(WZZIP) $(ROOT)\release\$Version\setup-redist.zip $(ROOT)\bin\desktop\setup-redist.exe
 

diff --git a/windows/src/desktop/insthelp/Makefile b/windows/src/desktop/insthelp/Makefile
@@ -11,9 +11,9 @@ build: version.res dirs
     $(COPY) $(WIN32_TARGET_PATH)\insthelp.exe $(ROOT)\bin\desktop\insthelp.exe
 
 test-manifest:
-    # test that (a) linked manifest exists and correct, and (b) has uiAccess=true
+# test that (a) linked manifest exists and correct, and (b) has uiAccess=true
     @rem $(MT) -nologo -inputresource:$(PROGRAM)\desktop\insthelp.exe -validate_manifest
-    # TODO: Investigate why no manifest included?
+# TODO: Investigate why no manifest included?
 
 clean: def-clean
 

diff --git a/windows/src/desktop/kmconfig/Makefile b/windows/src/desktop/kmconfig/Makefile
@@ -21,7 +21,7 @@ wrap-symbols:
     $(SYMSTORE) $(DEBUGPATH)\desktop\kmconfig.dbg /t keyman-windows
 
 test-manifest:
-    # test that linked manifest exists and correct
+# test that linked manifest exists and correct
     $(MT) -nologo -inputresource:$(PROGRAM)\desktop\kmconfig.exe -validate_manifest
 
 install:

diff --git a/windows/src/desktop/kmshell/Makefile b/windows/src/desktop/kmshell/Makefile
@@ -39,7 +39,7 @@ wrap-symbols:
     $(SYMSTORE) $(DEBUGPATH)\desktop\kmshell.dbg /t keyman-windows
 
 test-manifest:
-    # test that (a) linked manifest exists and correct, and (b) has uiAccess=true
+# test that (a) linked manifest exists and correct, and (b) has uiAccess=true
     $(MT) -nologo -inputresource:$(PROGRAM)\desktop\kmshell.exe -validate_manifest
 
 install: .virtual

diff --git a/windows/src/support/charident/Makefile b/windows/src/support/charident/Makefile
@@ -15,6 +15,6 @@ signcode:
 
 wrap-symbols:
     $(SYMSTORE) $(PROGRAM)\support\charident.exe /t keyman-windows
-    #TODO: $(SYMSTORE) $(DEBUGPATH)\support\charident.dbg /t keyman-windows
+#TODO: $(SYMSTORE) $(DEBUGPATH)\support\charident.dbg /t keyman-windows
 
 !include ..\..\Target.mak
diff --git a/windows/src/support/km_yim/Makefile b/windows/src/support/km_yim/Makefile
@@ -4,12 +4,13 @@
 
 !include ..\..\Defines.mak
 
-build: # version.res
+build:
+# version.res
     $(DCC32) km_yim.dpr
     rem $(TDSPACK) $(PROGRAM)\desktop\km_yim.exe km_yim.tds
     rem $(TDS2DBG) $(PROGRAM)\desktop\km_yim.exe
     $(WZZIP) inst_km_yim.zip km_yim.exe
-    # $(WZSE) inst_km_yim -setup -t inst_km_yim.dialog.txt -st "Tavultesoft Keyman Desktop Yahoo Messenger Addin" -c km_yim.exe
+# $(WZSE) inst_km_yim -setup -t inst_km_yim.dialog.txt -st "Tavultesoft Keyman Desktop Yahoo Messenger Addin" -c km_yim.exe
 
 clean: def-clean
     if exist inst_km_yim.zip del inst_km_yim.zip

diff --git a/windows/src/test/Makefile b/windows/src/test/Makefile
@@ -4,12 +4,14 @@
 
 # ----------------------------------------------------------------------
 
+# TODO: both test-manifest-exec and verify_signatures are really part of buildtools
+
 !ifdef NODELPHI
 TARGETS=.virtual
 !else
-TARGETS=test_i3633
+TARGETS=verify_signatures
 !endif
-CLEAN=test_i3633
+CLEAN=verify_signatures
 
 test: test-manifest-exec
     $(MAKE) "TARGET=test" $(TARGETS)
@@ -22,10 +24,10 @@ test-manifest-exec:
     $(MAKE) test-manifest
     cd test
 
-# test_i3633: validate certificates and binary metadata on executables
-# TODO: Move this to buildtools
-test_i3633:
-    cd $(ROOT)\src\test\test_i3633
+# validate certificates and binary metadata on executables
+# TODO: move to buildtools?
+verify_signatures: .virtual
+    cd $(ROOT)\src\buildtools\verify_signatures
     $(MAKE) $(TARGET)
 
 # ----------------------------------------------------------------------

diff --git a/windows/src/test/test_i3633/Makefile b/windows/src/test/test_i3633/Makefile
diff --git a/windows/src/test/verify_signatures/Makefile b/windows/src/test/verify_signatures/Makefile
@@ -0,0 +1,21 @@
+#
+# test for signatures and version information being correct in bin folder
+#
+
+!include ..\..\Defines.mak
+
+test: prereq
+  $(SIGCHECK) $(ROOT)\bin\desktop\* > sig1
+  $(SIGCHECK) $(ROOT)\bin\engine\* >> sig1
+  $(SIGCHECK) $(ROOT)\bin\inst\* >> sig1
+  $(VERIFY_SIGNATURES) < sig1
+
+# prereq may not be needed?
+prereq:
+  cd $(VERIFY_SIGNATURES)
+  $(MAKE)
+
+clean: def-clean
+  -del sig1
+
+!include ..\..\Target.mak