Merge e30e00e into dff28c7

gerald-hartig · web-flow · commit c50fe0ece3ce · 2024-06-10T00:06:10.000Z
diff --git a/.gitignore b/.gitignore
@@ -51,3 +51,4 @@ source/locale/*/cldr.dic
 .venv
 nvdaHelper/docs/
 *.pfx
+appveyor-tools/
diff --git a/appveyor.yml b/appveyor.yml
@@ -15,8 +15,13 @@ branches:
 
 environment:
  PY_PYTHON: 3.11-32
+ # TODO: remove key
  secure_authenticode_pass:
   secure: Way+hJyhbiLG/cmCo4+dHHzS5DiSvk/45o6frnIQ27GBX6nVDsh7jwQ7fSnqxBRP
+ # TODO use appveyor encrypt YAML to change this from test cert to prod cert
+ apiSigningToken:
+  secure: pTgq98Ewdmg+NgPNoaskkeG6UoPNw1BvIKPAHP/v1PqvD7ihPaVc0TbvZ3qpTgRE
+ # TODO: check where this is used and remove key
  secure_ssh_pass:
   secure: Iql/RhSathGacONacsyr6gis+rjL75UFZ/R+nPAJpo3asAzQSQQd8hfxq0iv8+Th
  mozillaSymsAuthToken:
diff --git a/appveyor/scripts/decryptFilesForSigning.ps1 b/appveyor/scripts/decryptFilesForSigning.ps1
@@ -1,9 +1,11 @@
+# TODO update this for apiSigningToken - is any of this still required?
 if(!$env:APPVEYOR_PULL_REQUEST_NUMBER -and $env:feature_signing) {
 	openssl enc -d -md sha256 -aes-256-cbc -pbkdf2 -salt -pass pass:$env:secure_authenticode_pass -in appveyor\authenticode.pfx.enc -out appveyor\authenticode.pfx
 	if($LastExitCode -ne 0) {
 		$errorCode=$LastExitCode
 		Add-AppveyorMessage "Unable to decrypt authenticode certificate"
 	}
+	# TODO why was a ssh key used?
 	openssl enc -d -md sha256 -aes-256-cbc -pbkdf2 -salt -pass pass:$env:secure_ssh_pass -in appveyor\ssh_id_rsa.enc -out appveyor\ssh_id_rsa
 	if($LastExitCode -ne 0) {
 		$errorCode=$LastExitCode
diff --git a/appveyor/scripts/setSconsArgs.ps1 b/appveyor/scripts/setSconsArgs.ps1
@@ -12,7 +12,9 @@ if ($env:versionType) {
 }
 $sconsArgs += " publisher=`"$env:scons_publisher`""
 if (!$env:APPVEYOR_PULL_REQUEST_NUMBER -and $env:feature_signing) {
-	$sconsArgs += " certFile=appveyor\authenticode.pfx certTimestampServer=http://timestamp.digicert.com"
+	# TODO make sure this doesn't expose the decrypted API key in the appveyor log
+	# TODO Is this script run in local builds? if so we need to add back the certFile and certPassword since people still need to be able to make self-signed builds. 
+	$sconsArgs += " apiSigningToken=$env:apiSigningToken"
 }
 $sconsArgs += " version_build=$env:APPVEYOR_BUILD_NUMBER"
 # We use cmd to run scons because PowerShell throws exceptions if warnings get dumped to stderr.
diff --git a/appveyor/scripts/sign.ps1 b/appveyor/scripts/sign.ps1
@@ -0,0 +1,19 @@
+# A part of NonVisual Desktop Access (NVDA)
+# Copyright (C) 2010-2024 NV Access Limited
+# This file may be used under the terms of the GNU General Public License, version 2 or later.
+# For more details see: https://www.gnu.org/licenses/gpl-2.0.html
+
+param(
+    [string]$ApiToken,
+    [string]$FileToSign
+)
+
+# Check if the Submit-SigningRequest command is available
+if (-not (Get-Command -Name Submit-SigningRequest -ErrorAction SilentlyContinue)) {
+    # If the command is not available, install the SignPath module
+    Install-Module -Name SignPath -Scope CurrentUser -Force
+}
+
+# Execute Submit-SigningRequest command from the SignPath module
+# TODO replace test_signing_policy with prod policy slug
+Submit-SigningRequest -ApiToken $ApiToken -InputArtifactPath $FileToSign -OutputArtifactPath $FileToSign -OrganizationId "12147e94-bba9-4fef-b29b-300398e90c5a" -ProjectSlug "NVDA"  -SigningPolicySlug "test_signing_policy" -WaitForCompletion -Force
diff --git a/appx/sconscript b/appx/sconscript
@@ -58,7 +58,7 @@ else: # not for submission, just side-loadable
 	packagePublisherDisplayName=env['publisher']
 	productName="NVDA Screen Reader (Windows Desktop Bridge Edition)" 
 
-signExec=env['signExec'] if env['certFile'] else None
+signExec=env['signExec'] if  (bool(env['certFile']) ^ bool(env['apiSigningToken'])) else None
 
 # Files from NVDA's distribution that cannot be included in the appx due to policy or security restrictions
 excludedDistFiles = [
diff --git a/nvdaHelper/archBuild_sconscript b/nvdaHelper/archBuild_sconscript
@@ -70,7 +70,7 @@ if not env.get('MSVC_VERSION') or tuple(map(int, env.get('MSVC_VERSION').split("
 TARGET_ARCH=env['TARGET_ARCH']
 debug=env['nvdaHelperDebugFlags']
 release=env['release']
-signExec=env['signExec'] if env['certFile'] else None
+signExec=env['signExec'] if  (bool(env['certFile']) ^ bool(env['apiSigningToken'])) else None
 
 #Some defines and includes for the environment
 env.Append(
diff --git a/nvdaHelper/liblouis/sconscript b/nvdaHelper/liblouis/sconscript
@@ -33,7 +33,7 @@ louisSourceDir = louisRootDir.Dir("liblouis")
 louisTableDir = louisRootDir.Dir("tables")
 outDir = sourceDir.Dir("louis")
 unitTestTablesDir = env.Dir("#tests/unit/brailleTables")
-signExec=env['signExec'] if env['certFile'] else None
+signExec=env['signExec'] if  (bool(env['certFile']) ^ bool(env['apiSigningToken'])) else None
 
 RE_AC_INIT = re.compile(r"^AC_INIT\(\[(?P<package>.*)\], \[(?P<version>.*)\], \[(?P<bugReport>.*)\], \[(?P<tarName>.*)\], \[(?P<url>.*)\]\)")
 def getLouisVersion():
diff --git a/projectDocs/dev/buildingNVDA.md b/projectDocs/dev/buildingNVDA.md
@@ -76,6 +76,7 @@ This is useful when [creating a self signed build](./selfSignedBuild.md).
 	* This enables various C++ compiler optimizations such as /O2 and whole-program optimization.
 	* It also instructs Python to generate optimized byte code.
 * publisher: The publisher of this build.
+# TODO update documentation for apiSigningToken
 * certFile: The certificate file with which to sign executables. The certificate must be in pfx format and contain the private key.
 * certPassword: The password for the private key in the signing certificate. If omitted, no password will be assumed.
 * certTimestampServer: The URL of the timestamping server to use to timestamp authenticode signatures. If omitted, signatures will not be timestamped.
diff --git a/sconstruct b/sconstruct
@@ -1,5 +1,5 @@
 # A part of NonVisual Desktop Access (NVDA)
-# Copyright (C) 2010-2023 NV Access Limited, James Teh, Michael Curran, Peter Vágner, Joseph Lee,
+# Copyright (C) 2010-2024 NV Access Limited, James Teh, Michael Curran, Peter Vágner, Joseph Lee,
 # Reef Turner, Babbage B.V., Leonard de Ruijter, Łukasz Golonka, Accessolutions, Julien Cochuyt,
 # Cyrille Bougot
 # This file may be used under the terms of the GNU General Public License, version 2 or later.
@@ -95,6 +95,7 @@ vars.Add(PathVariable("certFile", "The certificate file with which to sign execu
 	lambda key, val, env: not val or PathVariable.PathIsFile(key, val, env)))
 vars.Add("certPassword", "The password for the private key in the signing certificate", "")
 vars.Add("certTimestampServer", "The URL of the timestamping server to use to timestamp authenticode signatures", "")
+vars.Add("apiSigningToken", "The API key for the signing service", "")
 vars.Add(PathVariable("outputDir", "The directory where the final built archives and such will be placed", "output",PathVariable.PathIsDirCreate))
 vars.Add(ListVariable("nvdaHelperDebugFlags", "a list of debugging features you require", 'none', ["debugCRT","RTC","analyze"]))
 vars.Add(EnumVariable('nvdaHelperLogLevel','The level of logging you wish to see, lower is more verbose','15',allowed_values=[str(x) for x in range(60)]))
@@ -146,6 +147,9 @@ publisher = env["publisher"]
 certFile = env["certFile"]
 certPassword = env["certPassword"]
 certTimestampServer = env["certTimestampServer"]
+# TODO remove local testing code when using secure Appveyor store
+# TODO make sure that the unencrypted API key isn't logged anywhere eg AppVeyor
+apiSigningToken = env["apiSigningToken"] or os.environ.get("apiSigningToken") # workaround for local testing
 userDocsDir=Dir('user_docs')
 sourceDir = env.Dir("source")
 Export('sourceDir')
@@ -165,24 +169,33 @@ Export('outFilePrefix')
 outputDir=Dir(env['outputDir'])
 Export('outputDir')
 
-# An action to sign an executable with certFile.
-# we encrypt with SHA256 as this is the minimum required by the Windows Store for appx packages 
-signExecCmd = ["signtool", "sign", "/fd", "SHA256", "/f", certFile]
-if certPassword:
-	signExecCmd.extend(("/p", certPassword))
-if certTimestampServer:
-	signExecCmd.extend(("/tr", certTimestampServer, "/td", "SHA256"))
-def signExec(target,source,env):
-	print([str(x) for x in target])
-	#sys.exit(1)
-	# #3795: signtool can quite commonly fail with timestamping, so allow it to try up to 3 times with a 1 second delay between each try. 
-	res=0
-	for count in range(3):
-		res=env.Execute([signExecCmd+[target[0].abspath]])
-		if not res:
-			return 0 # success
-		time.sleep(1)
-	return res # failed
+assert not (apiSigningToken and certFile), "Cannot specify signing with both API token (cloud) and local certificate"
+if apiSigningToken:
+	# Code signing with SignPath HSM 
+	def signExec(target, source, env):
+		# TODO see if we have to pass the apiSigningToken or whether the sign.ps1 shell can find the variable
+		ps_cmd = f"powershell -File appveyor\scripts\sign.ps1 -ApiToken {apiSigningToken} -FileToSign {target[0].abspath}"
+		return env.Execute(ps_cmd, shell=True)
+if certFile:
+	# Local code signing with a certFile
+	def signExec(target,source,env):
+		# we encrypt with SHA256 as this is the minimum required by the Windows Store for appx packages 
+		signExecCmd = ["signtool", "sign", "/fd", "SHA256", "/f", certFile]
+		if certPassword:
+			signExecCmd.extend(("/p", certPassword))
+		if certTimestampServer:
+			signExecCmd.extend(("/tr", certTimestampServer, "/td", "SHA256"))
+		def signExec(target,source,env):
+			print([str(x) for x in target])
+			#sys.exit(1)
+			# #3795: signtool can quite commonly fail with timestamping, so allow it to try up to 3 times with a 1 second delay between each try. 
+			res=0
+			for count in range(3):
+				res=env.Execute([signExecCmd+[target[0].abspath]])
+				if not res:
+					return 0 # success
+				time.sleep(1)
+			return res # failed
 #Export via scons environment so other libraries can be signed
 env['signExec']=signExec
 
@@ -313,7 +326,7 @@ def NVDADistGenerator(target, source, env, for_signature):
 			if file.name.endswith(".dll") and file.is_file():
 				action.append(Copy(target[0], file.path))
 
-	if certFile:
+	if certFile or apiSigningToken:
 		for prog in "nvda_noUIAccess.exe", "nvda_uiAccess.exe", "nvda_slave.exe":
 			action.append(lambda target,source,env, progByVal=prog: signExec([target[0].File(progByVal)],source,env))
 
@@ -376,10 +389,10 @@ uninstGen = env.Command(File("uninstaller/uninstGen.exe"), "uninstaller/uninst.n
 	"/DINSTEXE=${TARGET.abspath}",
 	"$SOURCE"]])
 uninstaller=env.Command(uninstFile,uninstGen,[uninstGen])
-if certFile:
+if certFile or apiSigningToken:
 	env.AddPostAction(uninstaller, [signExec])
 
-dist = env.NVDADist("dist", [sourceDir,userDocsDir], uiAccess=bool(certFile))
+dist = env.NVDADist("dist", [sourceDir,userDocsDir], uiAccess=bool(certFile) or bool(apiSigningToken))
 env.Depends(dist,uninstaller)
 # dist will always be considered obsolete
 AlwaysBuild(dist)
@@ -393,7 +406,7 @@ launcher = env.Command(outputDir.File("%s.exe" % outFilePrefix), ["launcher/nvda
 	"/DVERSION=$version", '/DPUBLISHER="$publisher"','/DCOPYRIGHT="$copyright"','/DVERSION_YEAR="$version_year"','/DVERSION_MAJOR="$version_major"','/DVERSION_MINOR="$version_minor"','/DVERSION_BUILD="$version_build"',
 	"/DNVDADistDir=${SOURCES[1].abspath}", "/DLAUNCHEREXE=${TARGET.abspath}",
 	"$SOURCE"]])
-if certFile:
+if certFile or apiSigningToken:
 	env.AddPostAction(launcher, [signExec])
 env.Alias("launcher", launcher)
 

Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,9 @@ if ($env:versionType) {`
`12`	`12`	`}`
`13`	`13`	$sconsArgs += " publisher=`"$env:scons_publisher`""
`14`	`14`	`if (!$env:APPVEYOR_PULL_REQUEST_NUMBER -and $env:feature_signing) {`
`15`		`- $sconsArgs += " certFile=appveyor\authenticode.pfx certTimestampServer=http://timestamp.digicert.com"`
	`15`	`+ # TODO make sure this doesn't expose the decrypted API key in the appveyor log`
	`16`	`+ # TODO Is this script run in local builds? if so we need to add back the certFile and certPassword since people still need to be able to make self-signed builds.`
	`17`	`+ $sconsArgs += " apiSigningToken=$env:apiSigningToken"`
`16`	`18`	`}`
`17`	`19`	`$sconsArgs += " version_build=$env:APPVEYOR_BUILD_NUMBER"`
`18`	`20`	`# We use cmd to run scons because PowerShell throws exceptions if warnings get dumped to stderr.`