From 23b7ae869ca66cf84d03897afce180dcb86deb0b Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Tue, 2 Apr 2024 16:22:40 -0700 Subject: [PATCH 01/21] Add windows_signing_tool keyword --- CONSTRUCT.md | 14 ++++++++++++-- constructor/construct.py | 23 +++++++++++++++++++++-- constructor/main.py | 3 +++ constructor/winexe.py | 4 ++-- docs/source/construct-yaml.md | 14 ++++++++++++-- 5 files changed, 50 insertions(+), 8 deletions(-) diff --git a/CONSTRUCT.md b/CONSTRUCT.md index 6497acd5d..bc2cb8bd0 100644 --- a/CONSTRUCT.md +++ b/CONSTRUCT.md @@ -332,13 +332,23 @@ to sign `conda.exe`. For this, you need an "Application certificate" (different "Installer certificate" mentioned above). Common values for this option follow the format `Developer ID Application: Name of the owner (XXXXXX)`. +### `windows_signing_tool` + +_required:_ no
+_type:_ string
+ +The tool used to sign Windows installers. Must be one of: signtool. +Some tools require `signing_certificate` to be set. +Defaults to `signtool` if `signing_certificate` is set. + ### `signing_certificate` _required:_ no
_type:_ string
-On Windows only, set this key to the path of a PFX certificate to be used with `signtool`. -Additional environment variables can be used to configure this step, namely: +On Windows only, set this key to the path of the certificate file to be used +with the `windows_signing_tool`. Additional environment variables may need to +be used to configure signing, namely: - `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) - `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) diff --git a/constructor/construct.py b/constructor/construct.py index 6a1d6fb5a..3b070aa92 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -15,6 +15,10 @@ from constructor.exceptions import UnableToParse, UnableToParseMissingJinja2, YamlParsingError from constructor.utils import yaml +WIN_SIGNTOOLS = [ + "signtool", +] + # list of tuples (key name, required, type, description) KEYS = [ ('name', True, str, ''' @@ -240,11 +244,18 @@ to sign `conda.exe`. For this, you need an "Application certificate" (different from the "Installer certificate" mentioned above). Common values for this option follow the format `Developer ID Application: Name of the owner (XXXXXX)`. +'''), + + ('windows_signing_tool', False, str, f''' +The tool used to sign Windows installers. Must be one of: {", ".join(WIN_SIGNTOOLS)}. +Some tools require `signing_certificate` to be set. +Defaults to `signtool` if `signing_certificate` is set. '''), ('signing_certificate', False, str, ''' -On Windows only, set this key to the path of a PFX certificate to be used with `signtool`. -Additional environment variables can be used to configure this step, namely: +On Windows only, set this key to the path of the certificate file to be used +with the `windows_signing_tool`. Additional environment variables may need to +be used to configure signing, namely: - `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) - `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) @@ -792,6 +803,14 @@ def verify(info): types_str = " or ".join([type_.__name__ for type_ in types]) sys.exit(f"Value for 'extra_envs.{env_name}.{key}' " f"must be an instance of {types_str}") + if signtool := info.get("windows_signing_tool"): + if signtool not in WIN_SIGNTOOLS: + sys.exit("Value for 'windows_signing_tool' must be one of: " + f"{', '.join(WIN_SIGNTOOL)}. You tried to use: {signtool}." + ) + need_cert_file = ["signtool"] + if signtool in need_cert_file and not info.get("signing_certificate"): + sys.exit(f"The signing tool {signtool} requires 'signing_certificate' to be set.") def generate_doc(): diff --git a/constructor/main.py b/constructor/main.py index 76f483aea..891c43b40 100644 --- a/constructor/main.py +++ b/constructor/main.py @@ -109,6 +109,9 @@ def main_build(dir_path, output_dir='.', platform=cc_platform, if info.get(key): # only join if there's a truthy value set info[key] = abspath(join(dir_path, info[key])) + if info.get("signing_certificate") and not info.get("windows_signing_tool"): + info["windows_signing_tool"] = "signtool" + for key in 'specs', 'packages': if key not in info: continue diff --git a/constructor/winexe.py b/constructor/winexe.py index 93a6d6a1b..876dd6198 100644 --- a/constructor/winexe.py +++ b/constructor/winexe.py @@ -438,7 +438,7 @@ def verify_nsis_install(): def verify_signtool_is_available(info): - if not info.get("signing_certificate"): + if not info.get("windows_signing_tool"): return signtool = os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "signtool") logger.info("Checking for '%s'...", signtool) @@ -507,7 +507,7 @@ def create(info, verbose=False): logger.debug("makensis stderr:\n'%s'", process.stderr) process.check_returncode() - if info.get("signing_certificate"): + if info.get("windows_signing_tool"): verify_installer_signature(info['_outpath']) shutil.rmtree(tmp_dir) diff --git a/docs/source/construct-yaml.md b/docs/source/construct-yaml.md index 6497acd5d..bc2cb8bd0 100644 --- a/docs/source/construct-yaml.md +++ b/docs/source/construct-yaml.md @@ -332,13 +332,23 @@ to sign `conda.exe`. For this, you need an "Application certificate" (different "Installer certificate" mentioned above). Common values for this option follow the format `Developer ID Application: Name of the owner (XXXXXX)`. +### `windows_signing_tool` + +_required:_ no
+_type:_ string
+ +The tool used to sign Windows installers. Must be one of: signtool. +Some tools require `signing_certificate` to be set. +Defaults to `signtool` if `signing_certificate` is set. + ### `signing_certificate` _required:_ no
_type:_ string
-On Windows only, set this key to the path of a PFX certificate to be used with `signtool`. -Additional environment variables can be used to configure this step, namely: +On Windows only, set this key to the path of the certificate file to be used +with the `windows_signing_tool`. Additional environment variables may need to +be used to configure signing, namely: - `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) - `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) From 03cc07c820b9ecfe7e3bdacfaadca97ad390a65e Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Mon, 8 Apr 2024 11:41:23 -0700 Subject: [PATCH 02/21] Refactor to creates classes for signing Windows installers --- constructor/construct.py | 5 ++- constructor/main.py | 5 ++- constructor/signing.py | 93 ++++++++++++++++++++++++++++++++++++++++ constructor/utils.py | 9 ++++ constructor/winexe.py | 84 ++++++++---------------------------- 5 files changed, 128 insertions(+), 68 deletions(-) create mode 100644 constructor/signing.py diff --git a/constructor/construct.py b/constructor/construct.py index 3b070aa92..2f73b3567 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -805,8 +805,9 @@ def verify(info): f"must be an instance of {types_str}") if signtool := info.get("windows_signing_tool"): if signtool not in WIN_SIGNTOOLS: - sys.exit("Value for 'windows_signing_tool' must be one of: " - f"{', '.join(WIN_SIGNTOOL)}. You tried to use: {signtool}." + sys.exit( + "Value for 'windows_signing_tool' must be one of: " + f"{', '.join(WIN_SIGNTOOLS)}. You tried to use: {signtool}." ) need_cert_file = ["signtool"] if signtool in need_cert_file and not info.get("signing_certificate"): diff --git a/constructor/main.py b/constructor/main.py index 891c43b40..c2223a6bf 100644 --- a/constructor/main.py +++ b/constructor/main.py @@ -109,7 +109,10 @@ def main_build(dir_path, output_dir='.', platform=cc_platform, if info.get(key): # only join if there's a truthy value set info[key] = abspath(join(dir_path, info[key])) - if info.get("signing_certificate") and not info.get("windows_signing_tool"): + # Normalize name and set default value + if info.get("windows_signing_tool"): + info["windows_signing_tool"] = info["windows_signing_tool"].lower().replace(".exe", "") + elif info.get("signing_certificate"): info["windows_signing_tool"] = "signtool" for key in 'specs', 'packages': diff --git a/constructor/signing.py b/constructor/signing.py new file mode 100644 index 000000000..aa58cb728 --- /dev/null +++ b/constructor/signing.py @@ -0,0 +1,93 @@ +import logging +import os +import shutil +from pathlib import Path +from subprocess import PIPE, STDOUT, check_call, run +from typing import Union + +from .utils import win_str_esc + +logger = logging.getLogger(__name__) + + +class SigningTool: + def __init__( + self, + executable: Union[str, Path], + certificate_file: Union[str, Path] = None, + ): + self.executable = str(executable) + if certificate_file and not Path(certificate_file).exists(): + raise FileNotFoundError(f"Certificate file {certificate_file} does not exist.") + self.certificate_file = certificate_file + + def _verify_tool_is_available(self): + logger.info(f"Checking for {self.executable}...") + if not shutil.which(self.executable): + raise FileNotFoundError( + f"Could not find {self.executable}. Verify that the file exists or is in PATH." + ) + + def verify_signing_tool(self): + self._verify_tool_is_available() + + def get_signing_command(self): + return self.executable + + def verify_signature(self): + raise NotImplementedError("Signature verification not implemented for base class.") + + +class WindowsSignTool(SigningTool): + def __init__(self, certificate_file=None): + super().__init__( + os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "signtool"), + certificate_file=certificate_file, + ) + + def get_signing_command(self) -> str: + timestamp_server = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL", + "http://timestamp.sectigo.com" + ) + timestamp_digest = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST", + "sha256" + ) + file_digest = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_FILE_DIGEST", + "sha256" + ) + command = ( + f"{win_str_esc(self.executable)} sign /f {win_str_esc(self.certificate_file)} " + f"/tr {win_str_esc(timestamp_server)} /td {timestamp_digest} /fd {file_digest}" + ) + if "CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD" in os.environ: + # signtool can get the password from the env var on its own + command += ' /p "%CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD%"' + return command + + def verify_signing_tool(self): + self._verify_tool_is_available() + if not Path(self.certificate_file).exists(): + raise FileNotFoundError(f"Could not find certificate file {self.certificate_file}.") + check_call([self.executable, "/?"], stdout=PIPE, stderr=PIPE) + + def verify_signature(self, installer_file: Union[str, Path]): + proc = run( + [self.executable, "verify", "/v", str(installer_file)], + stdout=PIPE, + stderr=STDOUT, + text=True, + ) + logger.info(proc.stdout) + if "SignTool Error: No signature found" in proc.stdout: + # This is a signing error! + proc.check_returncode() + elif proc.returncode: + # we had errors but maybe not critical ones + logger.error( + f"SignTool could find a signature in {installer_file} but detected errors. " + "This is expected for untrusted (development) certificates. " + "If it is supposed to be trusted, please check your certificate!" + ) diff --git a/constructor/utils.py b/constructor/utils.py index 395fa0841..9b10292e2 100644 --- a/constructor/utils.py +++ b/constructor/utils.py @@ -265,3 +265,12 @@ def identify_conda_exe(conda_exe=None): name = "micromamba" version = output.strip() return name, version + + +def win_str_esc(s, newlines=True): + maps = [('$', '$$'), ('"', '$\\"'), ('\t', '$\\t')] + if newlines: + maps.extend([('\n', '$\\n'), ('\r', '$\\r')]) + for a, b in maps: + s = s.replace(a, b) + return '"%s"' % s diff --git a/constructor/winexe.py b/constructor/winexe.py index 876dd6198..26d1bd0dc 100644 --- a/constructor/winexe.py +++ b/constructor/winexe.py @@ -11,13 +11,14 @@ import tempfile from os.path import abspath, dirname, isfile, join from pathlib import Path -from subprocess import PIPE, STDOUT, check_call, check_output, run +from subprocess import check_output, run from typing import List from .construct import ns_platform from .imaging import write_images from .preconda import copy_extra_files from .preconda import write_files as preconda_write_files +from .signing import WindowsSignTool from .utils import ( add_condarc, approx_size_kb, @@ -27,6 +28,7 @@ make_VIProductVersion, preprocess, shortcuts_flags, + win_str_esc, ) NSIS_DIR = join(abspath(dirname(__file__)), 'nsis') @@ -35,15 +37,6 @@ logger = logging.getLogger(__name__) -def str_esc(s, newlines=True): - maps = [('$', '$$'), ('"', '$\\"'), ('\t', '$\\t')] - if newlines: - maps.extend([('\n', '$\\n'), ('\r', '$\\r')]) - for a, b in maps: - s = s.replace(a, b) - return '"%s"' % s - - def read_nsi_tmpl(info) -> str: path = abspath(info.get('nsis_template', join(NSIS_DIR, 'main.nsi.tmpl'))) logger.info('Reading: %s', path) @@ -53,7 +46,7 @@ def read_nsi_tmpl(info) -> str: def pkg_commands(download_dir, dists): for fn in dists: - yield 'File %s' % str_esc(join(download_dir, fn)) + yield 'File %s' % win_str_esc(join(download_dir, fn)) def extra_files_commands(paths, common_parent): @@ -101,7 +94,7 @@ def setup_script_env_variables(info) -> List[str]: for name, value in info.get('script_env_variables', {}).items(): lines.append( "System::Call 'kernel32::SetEnvironmentVariable(t,t)i" - + f"""("{name}", {str_esc(value)}).r0'""") + + f"""("{name}", {win_str_esc(value)}).r0'""") return lines @@ -217,27 +210,7 @@ def uninstall_menus_commands(info): return [line.strip() for line in lines] -def signtool_command(info): - "Generates a signtool command to be used in the NSIS template" - pfx_certificate = info.get("signing_certificate") - if pfx_certificate: - signtool = os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "signtool") - timestamp_server = os.environ.get( - "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL", - "http://timestamp.sectigo.com" - ) - command = ( - f'{str_esc(signtool)} sign /f {str_esc(pfx_certificate)} ' - f'/tr {str_esc(timestamp_server)} /td sha256 /fd sha256' - ) - if "CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD" in os.environ: - # signtool can get the password from the env var on its own - command += ' /p "%CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD%"' - return command - return "" - - -def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None): +def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None, signing_tool=None): "Creates the tmp/main.nsi from the template file" if extra_files is None: @@ -335,7 +308,7 @@ def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None): for key, value in replace.items(): if value.startswith('@'): value = join(dir_path, value[1:]) - replace[key] = str_esc(value) + replace[key] = win_str_esc(value) data = read_nsi_tmpl(info) ppd = ns_platform(info['_platform']) @@ -372,7 +345,7 @@ def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None): ('@NSIS_DIR@', NSIS_DIR), ('@BITS@', str(arch)), ('@PKG_COMMANDS@', '\n '.join(pkg_commands(download_dir, dists))), - ('@SIGNTOOL_COMMAND@', signtool_command(info)), + ('@SIGNTOOL_COMMAND@', signing_tool.get_signing_command()), ('@SETUP_ENVS@', '\n '.join(setup_envs_commands(info, dir_path))), ('@WRITE_CONDARC@', '\n '.join(add_condarc(info))), ('@SIZE@', str(approx_pkgs_size_kb)), @@ -437,36 +410,17 @@ def verify_nsis_install(): sys.exit("Error: no file untgz.dll") -def verify_signtool_is_available(info): - if not info.get("windows_signing_tool"): - return - signtool = os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "signtool") - logger.info("Checking for '%s'...", signtool) - check_call([signtool, "/?"], stdout=PIPE, stderr=PIPE) - - -def verify_installer_signature(path): - """ - Verify installer was properly signed by NSIS - We'll assume that the uninstaller was handled in the same way - """ - signtool = os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "signtool") - p = run([signtool, "verify", "/v", path], stdout=PIPE, stderr=STDOUT, text=True) - logger.info(p.stdout) - if "SignTool Error: No signature found" in p.stdout: - # This is a signing error! - p.check_returncode() - elif p.returncode: - # we had errors but maybe not critical ones - logger.error( - "SignTool could find a signature in %s but detected errors. " - "Please check your certificate!", path - ) - - def create(info, verbose=False): verify_nsis_install() - verify_signtool_is_available(info) + signing_tool = None + if signing_tool_name := info.get("windows_signing_tool"): + if signing_tool_name == "signtool": + signing_tool = WindowsSignTool( + certificate_file=info.get("signing_certificate") + ) + else: + raise ValueError(f"Unknown signing tool: {signing_tool_name}") + signing_tool.verify_signing_tool() tmp_dir = tempfile.mkdtemp() preconda_write_files(info, tmp_dir) copied_extra_files = copy_extra_files(info.get("extra_files", []), tmp_dir) @@ -507,8 +461,8 @@ def create(info, verbose=False): logger.debug("makensis stderr:\n'%s'", process.stderr) process.check_returncode() - if info.get("windows_signing_tool"): - verify_installer_signature(info['_outpath']) + if signing_tool: + signing_tool.verify_signature(info['_outpath']) shutil.rmtree(tmp_dir) From 972f32415dc649223ebad2e7bac5174e9c697784 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Mon, 8 Apr 2024 15:11:50 -0700 Subject: [PATCH 03/21] Add AzureSignTool support --- constructor/construct.py | 1 + constructor/signing.py | 107 +++++++++++++++++++++++++++++++++++++++ constructor/winexe.py | 7 ++- 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/constructor/construct.py b/constructor/construct.py index 2f73b3567..0d2003d42 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -16,6 +16,7 @@ from constructor.utils import yaml WIN_SIGNTOOLS = [ + "azuresigntool", "signtool", ] diff --git a/constructor/signing.py b/constructor/signing.py index aa58cb728..bbcee4661 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -91,3 +91,110 @@ def verify_signature(self, installer_file: Union[str, Path]): "This is expected for untrusted (development) certificates. " "If it is supposed to be trusted, please check your certificate!" ) + + +class AzureSignTool(SigningTool): + def __init__(self): + super().__init__(os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "AzureSignTool")) + + def get_signing_command(self) -> str: + def _check_required_env_vars(env_vars): + missing_vars = {var for var in env_vars if var not in os.environ} + if missing_vars: + raise RuntimeError( + f"Missing required environment variables {', '.join(missing_vars)}." + ) + + required_env_vars = ( + "AZURE_SIGNTOOL_KEY_VAULT_URL", + "AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE", + ) + _check_required_env_vars(required_env_vars) + timestamp_server = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL", + "http://timestamp.sectigo.com" + ) + timestamp_digest = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST", + "sha256" + ) + file_digest = os.environ.get( + "CONSTRUCTOR_SIGNTOOL_FILE_DIGEST", + "sha256" + ) + + command = ( + f"{self.executable} sign -v" + ' -kvu %AZURE_SIGNTOOL_KEY_VAULT_URL%' + ' -kvc %AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE%' + f" -tr {timestamp_server}" + f" -td {timestamp_digest}" + f" -fd {file_digest}" + ) + # There are three ways to sign: + # 1. Access token + # 2. Secret (requires tenant ID) + # 3. Managed identity (requires prior login to Azure) + if "AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN" in os.environ: + command += ' -kva "%AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN%"' + elif "AZURE_SIGNTOOL_KEY_VAULT_SECRET" in os.environ: + # Authentication via secret required client and tenant ID + required_env_vars = ( + "AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID", + "AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID", + ) + _check_required_env_vars(required_env_vars) + command += ( + " -kvi %AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID%" + " -kvt %AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID%" + " -kvs %AZURE_SIGNTOOL_KEY_VAULT_SECRET%" + ) + else: + # No token or secret found, assume managed identity + command += " -kvm" + return command + + def verify_signing_tool(self): + self._verify_tool_is_available() + check_call([self.executable, "--help"], stdout=PIPE, stderr=PIPE) + + def verify_signature(self, installer_file: Union[str, Path]): + """ + ` https://learn.microsoft.com/en-us/dotnet/api/system.management.automation.signaturestatus + """ + if shutil.which("powershell") is None: + logger.error("Could not verify signature: PowerShell not found.") + return + command = ( + f"$sig = Get-AuthenticodeSignature -LiteralPath {installer_file};" + "$sig.Status.value__;" + "$sig.StatusMessage" + ) + proc = run([ + "powershell", + "-c", + command, + ], + capture_output=True, + text=True, + ) + # The return code will always be 0, + # but stderr will be non-empty on errors + if proc.stderr: + logger.error(proc.stderr) + return + try: + status, status_message = proc.stdout.strip().split("\n") + status = int(status) + if status > 1: + # Includes missing signature + raise RuntimeError(f"Error signing {installer_file}: {status_message}") + elif status == 1: + logger.error( + f"{installer_file} contains a signature that is either invalid or not trusted. " + "This is expected with development certificates. " + "If it is supposed to be trusted, please check your certificate!" + ) + except ValueError: + # Something else is in the output + raise RuntimeError(f"Unexpected signature verification output: {proc.stdout}") diff --git a/constructor/winexe.py b/constructor/winexe.py index 26d1bd0dc..dab273b44 100644 --- a/constructor/winexe.py +++ b/constructor/winexe.py @@ -18,7 +18,7 @@ from .imaging import write_images from .preconda import copy_extra_files from .preconda import write_files as preconda_write_files -from .signing import WindowsSignTool +from .signing import AzureSignTool, WindowsSignTool from .utils import ( add_condarc, approx_size_kb, @@ -345,7 +345,7 @@ def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None, signing_to ('@NSIS_DIR@', NSIS_DIR), ('@BITS@', str(arch)), ('@PKG_COMMANDS@', '\n '.join(pkg_commands(download_dir, dists))), - ('@SIGNTOOL_COMMAND@', signing_tool.get_signing_command()), + ('@SIGNTOOL_COMMAND@', signing_tool.get_signing_command() if signing_tool else ""), ('@SETUP_ENVS@', '\n '.join(setup_envs_commands(info, dir_path))), ('@WRITE_CONDARC@', '\n '.join(add_condarc(info))), ('@SIZE@', str(approx_pkgs_size_kb)), @@ -418,6 +418,8 @@ def create(info, verbose=False): signing_tool = WindowsSignTool( certificate_file=info.get("signing_certificate") ) + elif signing_tool_name == "azuresigntool": + signing_tool = AzureSignTool() else: raise ValueError(f"Unknown signing tool: {signing_tool_name}") signing_tool.verify_signing_tool() @@ -452,6 +454,7 @@ def create(info, verbose=False): tmp_dir, extra_files=copied_extra_files, temp_extra_files=copied_temp_extra_files, + signing_tool=signing_tool, ) verbosity = f"{'/' if sys.platform == 'win32' else '-'}V{4 if verbose else 2}" args = [MAKENSIS_EXE, verbosity, nsi] From fff427fc0e63bfd990c2cbe11f44fafaf08eff21 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Mon, 8 Apr 2024 15:13:36 -0700 Subject: [PATCH 04/21] Update documentation --- CONSTRUCT.md | 13 +++----- constructor/construct.py | 11 +++---- docs/source/construct-yaml.md | 13 +++----- docs/source/howto.md | 60 +++++++++++++++++++++++++++++------ 4 files changed, 65 insertions(+), 32 deletions(-) diff --git a/CONSTRUCT.md b/CONSTRUCT.md index bc2cb8bd0..9e02ff8e2 100644 --- a/CONSTRUCT.md +++ b/CONSTRUCT.md @@ -337,9 +337,12 @@ to sign `conda.exe`. For this, you need an "Application certificate" (different _required:_ no
_type:_ string
-The tool used to sign Windows installers. Must be one of: signtool. +The tool used to sign Windows installers. Must be one of: azuresigntool, signtool. Some tools require `signing_certificate` to be set. Defaults to `signtool` if `signing_certificate` is set. +Additional environment variables may need to be used to configure signing. +See the documentation for details: +https://conda.github.io/constructor/howto/#signing-exe-installers ### `signing_certificate` @@ -347,13 +350,7 @@ _required:_ no
_type:_ string
On Windows only, set this key to the path of the certificate file to be used -with the `windows_signing_tool`. Additional environment variables may need to -be used to configure signing, namely: - -- `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) -- `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) -- `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` (custom RFC 3161 timestamping server, default is -http://timestamp.sectigo.com) +with the `windows_signing_tool`. ### `attempt_hardlinks` diff --git a/constructor/construct.py b/constructor/construct.py index 0d2003d42..53ff52fee 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -251,17 +251,14 @@ The tool used to sign Windows installers. Must be one of: {", ".join(WIN_SIGNTOOLS)}. Some tools require `signing_certificate` to be set. Defaults to `signtool` if `signing_certificate` is set. +Additional environment variables may need to be used to configure signing. +See the documentation for details: +https://conda.github.io/constructor/howto/#signing-exe-installers '''), ('signing_certificate', False, str, ''' On Windows only, set this key to the path of the certificate file to be used -with the `windows_signing_tool`. Additional environment variables may need to -be used to configure signing, namely: - -- `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) -- `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) -- `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` (custom RFC 3161 timestamping server, default is -http://timestamp.sectigo.com) +with the `windows_signing_tool`. '''), ('attempt_hardlinks', False, (bool, str), ''' diff --git a/docs/source/construct-yaml.md b/docs/source/construct-yaml.md index bc2cb8bd0..9e02ff8e2 100644 --- a/docs/source/construct-yaml.md +++ b/docs/source/construct-yaml.md @@ -337,9 +337,12 @@ to sign `conda.exe`. For this, you need an "Application certificate" (different _required:_ no
_type:_ string
-The tool used to sign Windows installers. Must be one of: signtool. +The tool used to sign Windows installers. Must be one of: azuresigntool, signtool. Some tools require `signing_certificate` to be set. Defaults to `signtool` if `signing_certificate` is set. +Additional environment variables may need to be used to configure signing. +See the documentation for details: +https://conda.github.io/constructor/howto/#signing-exe-installers ### `signing_certificate` @@ -347,13 +350,7 @@ _required:_ no
_type:_ string
On Windows only, set this key to the path of the certificate file to be used -with the `windows_signing_tool`. Additional environment variables may need to -be used to configure signing, namely: - -- `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` (password to unlock the certificate, if needed) -- `CONSTRUCTOR_SIGNTOOL_PATH` (absolute path to `signtool.exe`, in case is not in `PATH`) -- `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` (custom RFC 3161 timestamping server, default is -http://timestamp.sectigo.com) +with the `windows_signing_tool`. ### `attempt_hardlinks` diff --git a/docs/source/howto.md b/docs/source/howto.md index 2481439a0..08be76766 100644 --- a/docs/source/howto.md +++ b/docs/source/howto.md @@ -38,21 +38,63 @@ On Windows, you can also add extra pages to the installer. This is an advanced o ## Signing and notarization -Windows can trigger SmartScreen alerts for EXE installers, signed or not. It does help when they are signed, though. [Read this SO answer about SmartScreen reputation for more details](https://stackoverflow.com/questions/48946680/how-to-avoid-the-windows-defender-smartscreen-prevented-an-unrecognized-app-fro/66582477#66582477). -In the case of macOS, users might get similar warnings for PKGs if the installers are not signed _and_ notarized. However, once these two requirements are fulfilled, the warnings disappear instantly. - -`constructor` offers some configuration options to help you in this process: - -- For Windows, you will need to provide the path to your code signing certificate (PFX format) in [`signing_certificate`](construct-yaml.md#signing_certificate). -- For macOS, you will need to provide two identity names. One for the PKG signature (via [`signing_identity_name`](construct-yaml.md#signing_identity_name)), and one to pass the notarization (via [`notarization_identity_name`](construct-yaml.md#notarization_identity_name)). These can be obtained in the [Apple Developer portal](https://developer.apple.com/account/). -Once signed, you can notarize your PKG with Apple's `notarytool`. - ```{seealso} Example of a CI pipeline implementing: - [Signing on Windows](https://github.com/napari/packaging/blob/6f5fcfaf7b/.github/workflows/make_bundle_conda.yml#L390) - [Signing](https://github.com/napari/packaging/blob/6f5fcfaf7b/.github/workflows/make_bundle_conda.yml#L349) and [notarization](https://github.com/napari/packaging/blob/6f5fcfaf7b/.github/workflows/make_bundle_conda.yml#L459) on macOS ``` +### Signing exe installers + +Windows can trigger SmartScreen alerts for EXE installers, signed or not. It does help when they are signed, though. [Read this SO answer about SmartScreen reputation for more details](https://stackoverflow.com/questions/48946680/how-to-avoid-the-windows-defender-smartscreen-prevented-an-unrecognized-app-fro/66582477#66582477). + +`constructor` supports the following tools to sign installers: + +* [SignTool](https://learn.microsoft.com/en-us/windows/win32/seccrypto/signtool) +* [AzureSignTool](https://github.com/vcsjones/AzureSignTool) + +The signtool that is used can be set in the `construct.yaml` file via the [`windows_signing_tool`](construct-yaml.md#windows_signing_tool) key. +If the [`signing_certificate`](construct-yaml.md#signing_certificate) key is set, `windows_signing_tool` defaults to `signtool`. + +For each tool, there are environment variables that may need to be set to configure signing. + +#### Environment variables for SignTool + +| Variable | Description | CLI flag | Default | +|---------------------------------------------|----------------------------------------------------------------|----------|------------------------------| +| `CONSTRUCTOR_PFX_CERTIFICATE_PASSWORD` | Password for the `pfx` certificate file. | `/p` | Empty | +| `CONSTRUCTOR_SIGNTOOL_PATH` | Path to `signtool.exe`. Needed if `signtool` is not in `PATH`. | N/A | `signtool` | +| `CONSTRUCTOR_SIGNTOOL_FILE_DIGEST` | Digest algorithm for creating the file signatures. | `/fd` | `sha256` | +| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` | URL to the RFC 3161 timestamp server. | `/tr` | http://timestamp.sectigo.com | +| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST` | Digest algorithm for the RFC 3161 time stamp. | `/td` | `sha256` | + +#### Environment variables for AzureSignTool + +| Variable | Description | CLI flag | Default | +|---------------------------------------------|---------------------------------------------------------------------------------------------|----------|------------------------------| +| `AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN` | An access token used to authenticate to Azure. | `-kva` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE` | The name of the certificate in the key vault. | `-kvc` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID` | The client ID used to authenticate to Azure. Required for authentication with a secret. | `-kvi` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_SECRET` | The client secret used to authenticate to Azure. Required for authentication with a secret. | `-kvs` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID` | The tenant ID used to authenticate to Azure. Required for authentication with a secret. | `-kvt` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_URL` | The URL of the key vault with the certificate. | `-kvu` | Empty | +| `CONSTRUCTOR_SIGNTOOL_PATH` | Path to `AzureSignTool.exe`. Needed if `azuresigntool` is not in `PATH`. | N/A | `azuresigntool` | +| `CONSTRUCTOR_SIGNTOOL_FILE_DIGEST` | Digest algorithm for creating the file signatures. | `-fd` | `sha256` | +| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` | URL to the RFC 3161 timestamp server. | `-tr` | http://timestamp.sectigo.com | +| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST` | Digest algorithm for the RFC 3161 time stamp. | `-td` | `sha256` | + +:::{note} + +If neither `AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN` nor `AZURE_SIGNTOOL_KEY_VAULT_SECRET` are set, `constructor` will use a Managed Identity (`-kvm` CLI option). + +### Signing and notarizing pkg installers + +In the case of macOS, users might get warnings for PKGs if the installers are not signed _and_ notarized. However, once these two requirements are fulfilled, the warnings disappear instantly. +`constructor` offers some configuration options to help you in this process: + +You will need to provide two identity names. One for the PKG signature (via [`signing_identity_name`](construct-yaml.md#signing_identity_name)), and one to pass the notarization (via [`notarization_identity_name`](construct-yaml.md#notarization_identity_name)). These can be obtained in the [Apple Developer portal](https://developer.apple.com/account/). +Once signed, you can notarize your PKG with Apple's `notarytool`. + ## Create shortcuts On Windows, `conda` supports `menuinst 1.x` shortcuts. If a package provides a certain JSON file From 621f6c733f3f95d4c68c6e6b1e81e3c674f2f9d0 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Tue, 9 Apr 2024 09:36:46 -0700 Subject: [PATCH 05/21] Add news item --- news/771-add-azure-signtool-support | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 news/771-add-azure-signtool-support diff --git a/news/771-add-azure-signtool-support b/news/771-add-azure-signtool-support new file mode 100644 index 000000000..7758fe263 --- /dev/null +++ b/news/771-add-azure-signtool-support @@ -0,0 +1,19 @@ +### Enhancements + +* Add support for AzureSignTool to sign Windows installers. + +### Bug fixes + +* + +### Deprecations + +* + +### Docs + +* + +### Other + +* From d791cd8afeb9d9b98126bad7a048bf42cd2c896f Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Tue, 9 Apr 2024 10:20:02 -0700 Subject: [PATCH 06/21] Add docs strings --- constructor/signing.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/constructor/signing.py b/constructor/signing.py index bbcee4661..6492bd3ea 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -11,6 +11,15 @@ class SigningTool: + """Base class to sign installers. + + Attributes + ---------- + executable: str | Path + Path to the signing tool binary. + certificate_file: str | Path + Path to the certificate file + """ def __init__( self, executable: Union[str, Path], @@ -22,6 +31,13 @@ def __init__( self.certificate_file = certificate_file def _verify_tool_is_available(self): + """Helper function to verify that the signing tool executable exists. + + This is a minimum verification step and should be done even if other steps are performed + to verify the signing tool (e.g., signtool.exe /?) to receive better error messages. + For example, using `signtool.exe /?` when the path does not exist, results in a misleading + Permission Denied error. + """ logger.info(f"Checking for {self.executable}...") if not shutil.which(self.executable): raise FileNotFoundError( @@ -29,12 +45,18 @@ def _verify_tool_is_available(self): ) def verify_signing_tool(self): + """Verify that the signing tool is usable.""" self._verify_tool_is_available() def get_signing_command(self): + """Get the string of the signing command to be executed. + + For Windows, this command is inserted into the NSIS template. + """ return self.executable def verify_signature(self): + """Verify the signed installer.""" raise NotImplementedError("Signature verification not implemented for base class.") From b2b032522a355a096d12f37bc565a9164a934e95 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 07:38:19 -0700 Subject: [PATCH 07/21] Apply suggestions from code review Co-authored-by: jaimergp --- constructor/signing.py | 4 ++-- docs/source/howto.md | 6 +++--- news/771-add-azure-signtool-support | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/constructor/signing.py b/constructor/signing.py index 6492bd3ea..1c456e7d0 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -90,7 +90,7 @@ def get_signing_command(self) -> str: return command def verify_signing_tool(self): - self._verify_tool_is_available() + super()._verify_tool_is_available() if not Path(self.certificate_file).exists(): raise FileNotFoundError(f"Could not find certificate file {self.certificate_file}.") check_call([self.executable, "/?"], stdout=PIPE, stderr=PIPE) @@ -182,7 +182,7 @@ def verify_signing_tool(self): def verify_signature(self, installer_file: Union[str, Path]): """ - ` https://learn.microsoft.com/en-us/dotnet/api/system.management.automation.signaturestatus + https://learn.microsoft.com/en-us/dotnet/api/system.management.automation.signaturestatus """ if shutil.which("powershell") is None: logger.error("Could not verify signature: PowerShell not found.") diff --git a/docs/source/howto.md b/docs/source/howto.md index 08be76766..9e900f31e 100644 --- a/docs/source/howto.md +++ b/docs/source/howto.md @@ -44,7 +44,7 @@ Example of a CI pipeline implementing: - [Signing](https://github.com/napari/packaging/blob/6f5fcfaf7b/.github/workflows/make_bundle_conda.yml#L349) and [notarization](https://github.com/napari/packaging/blob/6f5fcfaf7b/.github/workflows/make_bundle_conda.yml#L459) on macOS ``` -### Signing exe installers +### Signing EXE installers Windows can trigger SmartScreen alerts for EXE installers, signed or not. It does help when they are signed, though. [Read this SO answer about SmartScreen reputation for more details](https://stackoverflow.com/questions/48946680/how-to-avoid-the-windows-defender-smartscreen-prevented-an-unrecognized-app-fro/66582477#66582477). @@ -86,8 +86,8 @@ For each tool, there are environment variables that may need to be set to config :::{note} If neither `AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN` nor `AZURE_SIGNTOOL_KEY_VAULT_SECRET` are set, `constructor` will use a Managed Identity (`-kvm` CLI option). - -### Signing and notarizing pkg installers +::: +### Signing and notarizing PKG installers In the case of macOS, users might get warnings for PKGs if the installers are not signed _and_ notarized. However, once these two requirements are fulfilled, the warnings disappear instantly. `constructor` offers some configuration options to help you in this process: diff --git a/news/771-add-azure-signtool-support b/news/771-add-azure-signtool-support index 7758fe263..eb7a47aaf 100644 --- a/news/771-add-azure-signtool-support +++ b/news/771-add-azure-signtool-support @@ -1,6 +1,6 @@ ### Enhancements -* Add support for AzureSignTool to sign Windows installers. +* Add support for AzureSignTool to sign Windows installers. (#767 via #771) ### Bug fixes From 6f088ace5a7ed1a7c454d215bbda359cca012e62 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 08:24:25 -0700 Subject: [PATCH 08/21] Normalize signing tool name in validation function --- constructor/construct.py | 2 +- constructor/signing.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/constructor/construct.py b/constructor/construct.py index 53ff52fee..2fe4c8978 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -802,7 +802,7 @@ def verify(info): sys.exit(f"Value for 'extra_envs.{env_name}.{key}' " f"must be an instance of {types_str}") if signtool := info.get("windows_signing_tool"): - if signtool not in WIN_SIGNTOOLS: + if signtool.lower().replace(".exe", "") not in WIN_SIGNTOOLS: sys.exit( "Value for 'windows_signing_tool' must be one of: " f"{', '.join(WIN_SIGNTOOLS)}. You tried to use: {signtool}." diff --git a/constructor/signing.py b/constructor/signing.py index 1c456e7d0..4e3d75263 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -181,7 +181,9 @@ def verify_signing_tool(self): check_call([self.executable, "--help"], stdout=PIPE, stderr=PIPE) def verify_signature(self, installer_file: Union[str, Path]): - """ + """Use Powershell to verify signature. + + For available statuses, see the Microsoft documentation: https://learn.microsoft.com/en-us/dotnet/api/system.management.automation.signaturestatus """ if shutil.which("powershell") is None: From 86f118a5a77bc249971a92e03386203ac966212a Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 08:34:21 -0700 Subject: [PATCH 09/21] Add typing to make_nsi --- constructor/winexe.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/constructor/winexe.py b/constructor/winexe.py index dab273b44..3afbd8f9a 100644 --- a/constructor/winexe.py +++ b/constructor/winexe.py @@ -12,7 +12,7 @@ from os.path import abspath, dirname, isfile, join from pathlib import Path from subprocess import check_output, run -from typing import List +from typing import List, Union from .construct import ns_platform from .imaging import write_images @@ -210,7 +210,13 @@ def uninstall_menus_commands(info): return [line.strip() for line in lines] -def make_nsi(info, dir_path, extra_files=None, temp_extra_files=None, signing_tool=None): +def make_nsi( + info: dict, + dir_path: str, + extra_files: List = None, + temp_extra_files: List = None, + signing_tool: Union[AzureSignTool, WindowsSignTool] = None, +): "Creates the tmp/main.nsi from the template file" if extra_files is None: From 11b5dcb8f1baf41ff8f8f74f954be0b63838bc2a Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 08:43:17 -0700 Subject: [PATCH 10/21] Raise on stderr with AzureSignTool --- constructor/signing.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/constructor/signing.py b/constructor/signing.py index 4e3d75263..b928bdc34 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -205,8 +205,7 @@ def verify_signature(self, installer_file: Union[str, Path]): # The return code will always be 0, # but stderr will be non-empty on errors if proc.stderr: - logger.error(proc.stderr) - return + raise RuntimeError(f"Signature verification failed.\n{proc.stderr}") try: status, status_message = proc.stdout.strip().split("\n") status = int(status) From cca1ce4efcdc4eb0b8168ee03373486fbe8e5b88 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 08:45:57 -0700 Subject: [PATCH 11/21] Log AzureSignTool authentication method --- constructor/signing.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/constructor/signing.py b/constructor/signing.py index b928bdc34..ed5f29285 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -158,9 +158,11 @@ def _check_required_env_vars(env_vars): # 2. Secret (requires tenant ID) # 3. Managed identity (requires prior login to Azure) if "AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN" in os.environ: + logger.info("AzureSignTool: signing binary using access token.") command += ' -kva "%AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN%"' elif "AZURE_SIGNTOOL_KEY_VAULT_SECRET" in os.environ: # Authentication via secret required client and tenant ID + logger.info("AzureSignTool: signing binary using secret.") required_env_vars = ( "AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID", "AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID", @@ -173,6 +175,7 @@ def _check_required_env_vars(env_vars): ) else: # No token or secret found, assume managed identity + logger.info("AzureSignTool: signing binary using managed identity.") command += " -kvm" return command From e6565c77f5738ef550775245e4c580d72d67af6d Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 09:00:28 -0700 Subject: [PATCH 12/21] Move check_required_env_vars to utils --- constructor/signing.py | 12 +++--------- constructor/utils.py | 10 +++++++++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/constructor/signing.py b/constructor/signing.py index ed5f29285..b95978591 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -5,7 +5,7 @@ from subprocess import PIPE, STDOUT, check_call, run from typing import Union -from .utils import win_str_esc +from .utils import check_required_env_vars, win_str_esc logger = logging.getLogger(__name__) @@ -120,18 +120,12 @@ def __init__(self): super().__init__(os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "AzureSignTool")) def get_signing_command(self) -> str: - def _check_required_env_vars(env_vars): - missing_vars = {var for var in env_vars if var not in os.environ} - if missing_vars: - raise RuntimeError( - f"Missing required environment variables {', '.join(missing_vars)}." - ) required_env_vars = ( "AZURE_SIGNTOOL_KEY_VAULT_URL", "AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE", ) - _check_required_env_vars(required_env_vars) + check_required_env_vars(required_env_vars) timestamp_server = os.environ.get( "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL", "http://timestamp.sectigo.com" @@ -167,7 +161,7 @@ def _check_required_env_vars(env_vars): "AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID", "AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID", ) - _check_required_env_vars(required_env_vars) + check_required_env_vars(required_env_vars) command += ( " -kvi %AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID%" " -kvt %AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID%" diff --git a/constructor/utils.py b/constructor/utils.py index 9b10292e2..90c4b8cea 100644 --- a/constructor/utils.py +++ b/constructor/utils.py @@ -10,7 +10,7 @@ import re import sys from io import StringIO -from os import sep, unlink +from os import environ, sep, unlink from os.path import basename, isdir, isfile, islink, join, normpath from shutil import rmtree from subprocess import check_call, check_output @@ -274,3 +274,11 @@ def win_str_esc(s, newlines=True): for a, b in maps: s = s.replace(a, b) return '"%s"' % s + + +def check_required_env_vars(env_vars): + missing_vars = {var for var in env_vars if var not in environ} + if missing_vars: + raise RuntimeError( + f"Missing required environment variables {', '.join(missing_vars)}." + ) From 01011752a03bdffbb3cef6f6ab5cd2bb6d2092a7 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Wed, 10 Apr 2024 11:07:53 -0700 Subject: [PATCH 13/21] Add missing debugging section to TOC tree --- docs/source/index.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/index.md b/docs/source/index.md index d3586a53b..9c909768b 100644 --- a/docs/source/index.md +++ b/docs/source/index.md @@ -11,4 +11,5 @@ getting-started howto construct-yaml cli-options +debugging ``` From 352209f9bc1a84644d992e665ad1f4e139a89bf4 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Fri, 12 Apr 2024 14:22:42 -0700 Subject: [PATCH 14/21] Add integration test for AzureSignTool signing --- examples/azure_signtool/construct.yaml | 8 ++++++ tests/test_examples.py | 35 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 examples/azure_signtool/construct.yaml diff --git a/examples/azure_signtool/construct.yaml b/examples/azure_signtool/construct.yaml new file mode 100644 index 000000000..37b3c1744 --- /dev/null +++ b/examples/azure_signtool/construct.yaml @@ -0,0 +1,8 @@ +name: Signed_AzureSignTool +version: X +installer_type: exe +channels: + - http://repo.anaconda.com/pkgs/main/ +specs: + - python +windows_signing_tool: azuresigntool # [win] diff --git a/tests/test_examples.py b/tests/test_examples.py index 90ea18c75..bfa4dd94e 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -484,6 +484,41 @@ def test_example_signing(tmp_path, request): _run_installer(input_path, installer, install_dir, request=request) +@pytest.mark.skipif(sys.platform != "win32", reason="Windows only") +@pytest.mark.skipif(not shutil.which("azuresigntool"), reason="AzureSignTool not available") +@pytest.mark.parametrize( + "auth_method", + os.environ.get("AZURE_SIGNTOOL_TEST_AUTH_METHODS", "token,secret").split(","), +) +def test_azure_signtool(tmp_path, request, monkeypatch, auth_method): + """Test signing installers with AzureSignTool. + + There are three ways to authenticate with Azure: tokens, secrets, and managed identities. + There is no good sentinel environment for manged identities, so an environment variable + is used to determine which authentication methods to test. + """ + monkeypatch.delenv("CONSTRUCTOR_SIGNTOOL_PATH", raising=False) + if auth_method == "token": + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) + if "AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN" not in os.environ: + pytest.skip("No AzureSignTool token in environment.") + elif auth_method == "secret": + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN", raising=False) + if "AZURE_SIGNTOOL_KEY_VAULT_SECRET" not in os.environ: + pytest.skip("No AzureSignTool secret in environment.") + elif auth_method == "managed": + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN", raising=False) + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) + else: + pytest.skip(f"Unknown authentication method {auth_method}.") + input_path = _example_path("azure_signtool") + for installer, install_dir in create_installer( + input_path, + tmp_path, + ): + _run_installer(input_path, installer, install_dir, request=request) + + def test_example_use_channel_remap(tmp_path, request): input_path = _example_path("use_channel_remap") for installer, install_dir in create_installer(input_path, tmp_path): From c049abe55ba3b20a0d84c0a7ad21a2f9360f6a1a Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Mon, 15 Apr 2024 08:27:12 -0700 Subject: [PATCH 15/21] Do not overload signtool variables for AzureSignTool to facilitate testing --- constructor/signing.py | 8 ++++---- docs/source/howto.md | 24 ++++++++++++------------ tests/test_examples.py | 6 ++++-- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/constructor/signing.py b/constructor/signing.py index b95978591..4a5f33031 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -117,7 +117,7 @@ def verify_signature(self, installer_file: Union[str, Path]): class AzureSignTool(SigningTool): def __init__(self): - super().__init__(os.environ.get("CONSTRUCTOR_SIGNTOOL_PATH", "AzureSignTool")) + super().__init__(os.environ.get("AZURE_SIGNTOOL_PATH", "AzureSignTool")) def get_signing_command(self) -> str: @@ -127,15 +127,15 @@ def get_signing_command(self) -> str: ) check_required_env_vars(required_env_vars) timestamp_server = os.environ.get( - "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL", + "AZURE_SIGNTOOL_TIMESTAMP_SERVER_URL", "http://timestamp.sectigo.com" ) timestamp_digest = os.environ.get( - "CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST", + "AZURE_SIGNTOOL_TIMESTAMP_DIGEST", "sha256" ) file_digest = os.environ.get( - "CONSTRUCTOR_SIGNTOOL_FILE_DIGEST", + "AZURE_SIGNTOOL_FILE_DIGEST", "sha256" ) diff --git a/docs/source/howto.md b/docs/source/howto.md index 9e900f31e..c8701f00b 100644 --- a/docs/source/howto.md +++ b/docs/source/howto.md @@ -70,18 +70,18 @@ For each tool, there are environment variables that may need to be set to config #### Environment variables for AzureSignTool -| Variable | Description | CLI flag | Default | -|---------------------------------------------|---------------------------------------------------------------------------------------------|----------|------------------------------| -| `AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN` | An access token used to authenticate to Azure. | `-kva` | Empty | -| `AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE` | The name of the certificate in the key vault. | `-kvc` | Empty | -| `AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID` | The client ID used to authenticate to Azure. Required for authentication with a secret. | `-kvi` | Empty | -| `AZURE_SIGNTOOL_KEY_VAULT_SECRET` | The client secret used to authenticate to Azure. Required for authentication with a secret. | `-kvs` | Empty | -| `AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID` | The tenant ID used to authenticate to Azure. Required for authentication with a secret. | `-kvt` | Empty | -| `AZURE_SIGNTOOL_KEY_VAULT_URL` | The URL of the key vault with the certificate. | `-kvu` | Empty | -| `CONSTRUCTOR_SIGNTOOL_PATH` | Path to `AzureSignTool.exe`. Needed if `azuresigntool` is not in `PATH`. | N/A | `azuresigntool` | -| `CONSTRUCTOR_SIGNTOOL_FILE_DIGEST` | Digest algorithm for creating the file signatures. | `-fd` | `sha256` | -| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_SERVER_URL` | URL to the RFC 3161 timestamp server. | `-tr` | http://timestamp.sectigo.com | -| `CONSTRUCTOR_SIGNTOOL_TIMESTAMP_DIGEST` | Digest algorithm for the RFC 3161 time stamp. | `-td` | `sha256` | +| Variable | Description | CLI flag | Default | +|----------------------------------------|---------------------------------------------------------------------------------------------|----------|------------------------------| +| `AZURE_SIGNTOOL_FILE_DIGEST` | Digest algorithm for creating the file signatures. | `-fd` | `sha256` | +| `AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN` | An access token used to authenticate to Azure. | `-kva` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE` | The name of the certificate in the key vault. | `-kvc` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID` | The client ID used to authenticate to Azure. Required for authentication with a secret. | `-kvi` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_SECRET` | The client secret used to authenticate to Azure. Required for authentication with a secret. | `-kvs` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID` | The tenant ID used to authenticate to Azure. Required for authentication with a secret. | `-kvt` | Empty | +| `AZURE_SIGNTOOL_KEY_VAULT_URL` | The URL of the key vault with the certificate. | `-kvu` | Empty | +| `AZURE_SIGNTOOL_PATH` | Path to `AzureSignTool.exe`. Needed if `azuresigntool` is not in `PATH`. | N/A | `azuresigntool` | +| `AZURE_SIGNTOOL_TIMESTAMP_SERVER_URL` | URL to the RFC 3161 timestamp server. | `-tr` | http://timestamp.sectigo.com | +| `AZURE_SIGNTOOL_TIMESTAMP_DIGEST` | Digest algorithm for the RFC 3161 time stamp. | `-td` | `sha256` | :::{note} diff --git a/tests/test_examples.py b/tests/test_examples.py index bfa4dd94e..59bc420ed 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -485,7 +485,10 @@ def test_example_signing(tmp_path, request): @pytest.mark.skipif(sys.platform != "win32", reason="Windows only") -@pytest.mark.skipif(not shutil.which("azuresigntool"), reason="AzureSignTool not available") +@pytest.mark.skipif( + not shutil.which("azuresigntool") and not os.environ.get("AZURE_SIGNTOOL_PATH"), + reason="AzureSignTool not available" +) @pytest.mark.parametrize( "auth_method", os.environ.get("AZURE_SIGNTOOL_TEST_AUTH_METHODS", "token,secret").split(","), @@ -497,7 +500,6 @@ def test_azure_signtool(tmp_path, request, monkeypatch, auth_method): There is no good sentinel environment for manged identities, so an environment variable is used to determine which authentication methods to test. """ - monkeypatch.delenv("CONSTRUCTOR_SIGNTOOL_PATH", raising=False) if auth_method == "token": monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) if "AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN" not in os.environ: From 31110f5494738d9ab6d2933e1d611bbbd0edbd1e Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Thu, 18 Apr 2024 13:18:46 -0700 Subject: [PATCH 16/21] Add AzureSignTool to CI tests --- .github/workflows/main.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index eba3db1d7..ab6e6ec5d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -94,6 +94,10 @@ jobs: activate-environment: constructor-dev environment-file: dev/environment.yml python-version: ${{ matrix.python-version }} + - name: Install AzureSignTool + if: ${{ matrix.os == 'windows' }} + run: dotnet.exe tool install --global AzureSignTool + shell: pwsh - name: Supply extra dependencies and install constructor run: | files=(--file "tests/requirements.txt") @@ -129,6 +133,11 @@ jobs: flags: unit - name: Run examples env: + AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE }} + AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID }} + AZURE_SIGNTOOL_KEY_VAULT_SECRET: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_SECRET }} + AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID }} + AZURE_SIGNTOOL_KEY_VAULT_URL: ${{ secrets.AZURE_SIGNTOOL_KEY_VAULT_URL }} CONSTRUCTOR_EXAMPLES_KEEP_ARTIFACTS: "${{ runner.temp }}/examples_artifacts" CONSTRUCTOR_SIGNTOOL_PATH: "C:/Program Files (x86)/Windows Kits/10/bin/10.0.17763.0/x86/signtool.exe" run: | From b6e7aa34a6fa7fe16934d11ee39e9286a9d887d6 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Fri, 19 Apr 2024 10:01:41 -0700 Subject: [PATCH 17/21] Always explicitly copy environment --- tests/test_examples.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_examples.py b/tests/test_examples.py index 59bc420ed..4600d619e 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -45,11 +45,9 @@ def _execute( cmd: Iterable[str], installer_input=None, check=True, timeout=420, **env_vars ) -> subprocess.CompletedProcess: t0 = time.time() + env = os.environ.copy() if env_vars: - env = os.environ.copy() env.update({k: v for (k, v) in env_vars.items() if v is not None}) - else: - env = None p = subprocess.Popen( cmd, stdin=subprocess.PIPE if installer_input else None, From 9e8dff810413da09f73853c64ae84368281c2e5b Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Fri, 19 Apr 2024 10:33:18 -0700 Subject: [PATCH 18/21] Add comment to explain why environment is always copied in _execute --- tests/test_examples.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_examples.py b/tests/test_examples.py index 4600d619e..e3b759344 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -45,6 +45,7 @@ def _execute( cmd: Iterable[str], installer_input=None, check=True, timeout=420, **env_vars ) -> subprocess.CompletedProcess: t0 = time.time() + # The environment is not copied on Windows, so copy here to get consistent behavior env = os.environ.copy() if env_vars: env.update({k: v for (k, v) in env_vars.items() if v is not None}) From 477d7e04e442f28460ddb9a95ea2fa0404b632c3 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Mon, 22 Apr 2024 09:10:26 -0700 Subject: [PATCH 19/21] Use get to check for environment variables in AzureSignTool test --- tests/test_examples.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_examples.py b/tests/test_examples.py index e3b759344..d06b9e39c 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -500,13 +500,13 @@ def test_azure_signtool(tmp_path, request, monkeypatch, auth_method): is used to determine which authentication methods to test. """ if auth_method == "token": - monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) - if "AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN" not in os.environ: + if not os.environ.get("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN"): pytest.skip("No AzureSignTool token in environment.") + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) elif auth_method == "secret": - monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN", raising=False) - if "AZURE_SIGNTOOL_KEY_VAULT_SECRET" not in os.environ: + if not os.environ.get("AZURE_SIGNTOOL_KEY_VAULT_SECRET"): pytest.skip("No AzureSignTool secret in environment.") + monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN", raising=False) elif auth_method == "managed": monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_ACCESSTOKEN", raising=False) monkeypatch.delenv("AZURE_SIGNTOOL_KEY_VAULT_SECRET", raising=False) From be5b259db46803169f60ee414aaced6f08551496 Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Tue, 23 Apr 2024 07:46:37 -0700 Subject: [PATCH 20/21] Update .github/workflows/main.yml Co-authored-by: jaimergp --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ab6e6ec5d..62549f124 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -95,7 +95,7 @@ jobs: environment-file: dev/environment.yml python-version: ${{ matrix.python-version }} - name: Install AzureSignTool - if: ${{ matrix.os == 'windows' }} + if: matrix.os == 'windows' run: dotnet.exe tool install --global AzureSignTool shell: pwsh - name: Supply extra dependencies and install constructor From 55458a4ccc997a9d882b6f9a7912ff8a54bc1f9f Mon Sep 17 00:00:00 2001 From: Marco Esters Date: Tue, 23 Apr 2024 14:56:50 -0700 Subject: [PATCH 21/21] Quote environment variables --- constructor/signing.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/constructor/signing.py b/constructor/signing.py index 4a5f33031..31e4e488f 100644 --- a/constructor/signing.py +++ b/constructor/signing.py @@ -140,10 +140,10 @@ def get_signing_command(self) -> str: ) command = ( - f"{self.executable} sign -v" - ' -kvu %AZURE_SIGNTOOL_KEY_VAULT_URL%' - ' -kvc %AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE%' - f" -tr {timestamp_server}" + f"{win_str_esc(self.executable)} sign -v" + ' -kvu "%AZURE_SIGNTOOL_KEY_VAULT_URL%"' + ' -kvc "%AZURE_SIGNTOOL_KEY_VAULT_CERTIFICATE%"' + f' -tr "{timestamp_server}"' f" -td {timestamp_digest}" f" -fd {file_digest}" ) @@ -163,9 +163,9 @@ def get_signing_command(self) -> str: ) check_required_env_vars(required_env_vars) command += ( - " -kvi %AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID%" - " -kvt %AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID%" - " -kvs %AZURE_SIGNTOOL_KEY_VAULT_SECRET%" + ' -kvi "%AZURE_SIGNTOOL_KEY_VAULT_CLIENT_ID%"' + ' -kvt "%AZURE_SIGNTOOL_KEY_VAULT_TENANT_ID%"' + ' -kvs "%AZURE_SIGNTOOL_KEY_VAULT_SECRET%"' ) else: # No token or secret found, assume managed identity