-
-
Notifications
You must be signed in to change notification settings - Fork 539
feat: sign Linux system packages when building with Docker #2987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 16 commits
8f0473b
5928c59
7e223a6
2d48aee
054b2f7
9c7f3ca
041d388
fd313be
ba9a6a5
563ca87
e7e1658
07a9a25
df02366
f9ce71f
5810c02
39eae63
7c9e1d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| Linux system packages (`.deb`, `.rpm` and `.pkg.tar.zst`) can now be signed with a GPG signing identity, using the `--identity` option to `briefcase package`. Signing is not currently supported when building with Docker. | ||
| Linux system packages (`.deb`, `.rpm` and `.pkg.tar.zst`) can now be signed with a GPG signing identity, using the `--identity` option to `briefcase package`. This includes packages built inside a Docker container. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,14 @@ | |
|
|
||
| import gzip | ||
| import re | ||
| import shlex | ||
| import subprocess | ||
| import tarfile | ||
| from collections.abc import Collection | ||
| from pathlib import Path | ||
| from typing import cast | ||
| from typing import Any, cast | ||
|
|
||
| from briefcase.channels.base import BasePublicationChannel | ||
| from briefcase.commands import ( | ||
| BuildCommand, | ||
| CreateCommand, | ||
|
|
@@ -42,6 +44,33 @@ | |
| parse_freedesktop_os_release, | ||
| ) | ||
|
|
||
| # The packaging format implied by each system vendor base. | ||
| _SYSTEM_PACKAGING_FORMATS = { | ||
| DEBIAN: "deb", | ||
| RHEL: "rpm", | ||
| ARCH: "pkg", | ||
| SUSE: "rpm", | ||
| } | ||
|
|
||
|
|
||
| def _resolve_system_packaging_format( | ||
| app: AppConfig | FinalizedAppConfig, | ||
| packaging_format: str, | ||
| ) -> str: | ||
| """Resolve the "system" packaging format alias to a concrete format. | ||
|
|
||
| When the user hasn't explicitly selected a packaging format, the "system" alias is | ||
| used. The concrete format implied by the app's target is determined during app | ||
| finalization; this returns that value. | ||
|
|
||
| :param app: The app configuration | ||
| :param packaging_format: The packaging format requested on the command line | ||
| :returns: The concrete packaging format to use | ||
| """ | ||
| if packaging_format == "system": | ||
| return getattr(app, "packaging_format", "system") | ||
| return packaging_format | ||
|
|
||
|
|
||
| class LinuxSystemAppConfig(FinalizedAppConfig): | ||
| """A FinalizedAppConfig with Linux system packaging attributes. | ||
|
|
@@ -273,6 +302,19 @@ def finalize_app_config( | |
|
|
||
| self.console.verbose(f"Targeting Python{app.python_version_tag}") | ||
|
|
||
| # If no packaging format was selected (or the "system" alias was used), | ||
| # determine what that means. This must be done before the app tools are | ||
| # verified, so the Docker image can be built with the tools needed to | ||
| # package and sign the app. | ||
| if getattr(app, "packaging_format", None) in (None, "system"): | ||
| app.packaging_format = _SYSTEM_PACKAGING_FORMATS.get(app.target_vendor_base) | ||
| if app.packaging_format is None: | ||
| raise BriefcaseCommandError( | ||
| "Briefcase doesn't know the system packaging format for " | ||
| f"{app.target_vendor}. You may be able to build a package " | ||
| "by manually specifying a format with -p/--packaging-format" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok - this has exposed an interesting gap.
It also means that it's no longer possible to even create a Linux app for an unidentified Linux distribution without modifying the app configuration to add I think this might also be the cause of the roundabout logic you've added for So - there's a workflow issue we need to resolve here. At the very least, this needs to be only enforced for Docker builds; not supporting Docker builds for unknown linux distros would also be acceptable. |
||
| ) | ||
|
|
||
| return LinuxSystemAppConfig(super().finalize_app_config(app, **kwargs)) | ||
|
|
||
| def _deb_devirtualize(self, package: str) -> str: | ||
|
|
@@ -388,6 +430,27 @@ def _system_requirement_tools(self, app: LinuxSystemAppConfig): | |
| system_installer, | ||
| ) | ||
|
|
||
| def _signing_tool(self, app: LinuxSystemAppConfig) -> tuple[str, str, str]: | ||
| """Utility method returning the tool used to sign a package. | ||
|
|
||
| :param app: The app being packaged | ||
| :returns: A triple of (tool name, executable name, package name) for the tool | ||
| used to sign the package. | ||
| :raises KeyError: If the packaging format cannot be determined. | ||
| """ | ||
| # The packaging format may not be set on a draft app config. | ||
| packaging_format = getattr(app, "packaging_format", None) | ||
| tool_name, executable_name, package_name = { | ||
| "deb": ("debsigs", "debsigs", "debsigs"), | ||
| "rpm": ("rpmsign", "rpmsign", "rpm-sign"), | ||
| "pkg": ("gpg", "gpg", "gnupg"), | ||
| }[packaging_format] | ||
| if packaging_format == "rpm" and app.target_vendor_base == SUSE: | ||
| # On SUSE, rpmsign is provided by rpm-build; there is no separate | ||
| # `rpm-sign` package. | ||
| package_name = "rpm-build" | ||
| return tool_name, executable_name, package_name | ||
|
|
||
| def verify_system_packages(self, app: LinuxSystemAppConfig): | ||
| """Verify that the required system packages are installed. | ||
|
|
||
|
|
@@ -742,6 +805,22 @@ def verify_app_tools(self, app: FinalizedAppConfig): | |
| verify_python = not hasattr(self.tools[app], "app_context") | ||
|
|
||
| if self.use_docker: | ||
| # The Docker image is built before the signing identity is selected, | ||
| # so the signing tool must be installed in the image for the signing | ||
| # step to be able to run inside the container. | ||
| system_requires = getattr(app, "system_requires", None) | ||
| if system_requires is None: | ||
| system_requires = [] | ||
| app.system_requires = system_requires | ||
| try: | ||
| _, _, package_name = self._signing_tool(app) | ||
| except KeyError: | ||
| # An unknown packaging format has no signing tool that can be | ||
| # identified. | ||
| package_name = None | ||
| if package_name is not None and package_name not in system_requires: | ||
| system_requires.append(package_name) | ||
|
|
||
| DockerAppContext.verify( | ||
| tools=self.tools, | ||
| app=app, | ||
|
|
@@ -1057,19 +1136,6 @@ class LinuxSystemSigningMixin(_MixinBase): | |
| "available on the system." | ||
| ) | ||
|
|
||
| def _signing_tool(self, app: LinuxSystemAppConfig) -> tuple[str, str, str]: | ||
| """Utility method returning the tool used to sign a package. | ||
|
|
||
| :param app: The app being packaged | ||
| :returns: A triple of (tool name, executable name, package name) for the tool | ||
| used to sign the package. | ||
| """ | ||
| return { | ||
| "deb": ("debsigs", "debsigs", "debsigs"), | ||
| "rpm": ("rpmsign", "rpmsign", "rpm-sign"), | ||
| "pkg": ("gpg", "gpg", "gnupg"), | ||
| }[app.packaging_format] | ||
|
|
||
| def _verify_signing_tool(self, app: LinuxSystemAppConfig): | ||
| """Verify that the app environment contains the signing tool. | ||
|
|
||
|
|
@@ -1090,12 +1156,14 @@ def _verify_signing_tool(self, app: LinuxSystemAppConfig): | |
| if install_cmd := self._system_requirement_tools(app)[3]: | ||
| raise BriefcaseCommandError( | ||
| f"Can't find the {tool_name} tools. " | ||
| f"Try running `sudo {' '.join(install_cmd)} {package_name}`." | ||
| f"Try running `sudo {' '.join(install_cmd)} {package_name}`. " | ||
| "Alternatively, use `--adhoc-sign` to skip signing the package." | ||
| ) from None | ||
| else: | ||
| raise BriefcaseCommandError( | ||
| f"Can't find the {executable_name} tool. " | ||
| f"Install this first to sign the {app.packaging_format}." | ||
| f"Install this first to sign the {app.packaging_format}. " | ||
| "Alternatively, use `--adhoc-sign` to skip signing the package." | ||
| ) from None | ||
|
|
||
| def signature_path(self, app: LinuxSystemAppConfig) -> Path: | ||
|
|
@@ -1202,13 +1270,48 @@ def sign_package(self, app: LinuxSystemAppConfig, identity: str): | |
| ], | ||
| }[app.packaging_format] | ||
|
|
||
| subprocess_kwargs: dict[str, Any] = {} | ||
| key_file_path: Path | None = None | ||
|
|
||
| try: | ||
| self.tools[app].app_context.run(sign_command, check=True) | ||
| if self.use_docker: | ||
| # When packaging inside Docker, the secret key must be made available | ||
| # to the container. Export the key to the bundle path (which is mounted | ||
| # in to the container), then import it and sign the package in a single | ||
| # container run, so the key is not retained in the image or container. | ||
| # | ||
| # The bundle and dist folders are mounted in to the container, and the | ||
| # Docker layer rewrites the host paths in the commands to their | ||
| # container equivalents. | ||
| key_file_path = self.bundle_path(app) / "signing-key.gpg" | ||
| subprocess_kwargs["mounts"] = [(self.dist_path, "/dist")] | ||
| self.tools.gnupg.export_secret_key(identity, key_file_path) | ||
|
freakboy3742 marked this conversation as resolved.
|
||
| self.tools.os.chmod(key_file_path, 0o600) | ||
| sign_command = [ | ||
| "sh", | ||
| "-c", | ||
| " && ".join( | ||
| " ".join(shlex.quote(arg) for arg in command) | ||
| for command in [ | ||
| ["gpg", "--batch", "--import", str(key_file_path)], | ||
| sign_command, | ||
| ] | ||
| ), | ||
| ] | ||
|
|
||
| self.tools[app].app_context.run( | ||
| sign_command, | ||
| check=True, | ||
| **subprocess_kwargs, | ||
| ) | ||
| except subprocess.CalledProcessError as e: | ||
| raise BriefcaseCommandError( | ||
| f"Error while signing .{app.packaging_format} package for " | ||
| f"{app.app_name}." | ||
| ) from e | ||
| finally: | ||
| if self.use_docker: | ||
| key_file_path.unlink(missing_ok=True) | ||
|
|
||
| def clean_dist_folder(self, app, **options): | ||
| super().clean_dist_folder(app, **options) | ||
|
|
@@ -1237,12 +1340,6 @@ def package_app(self, app, identity=None, adhoc_sign=False, **kwargs): | |
| else: | ||
| identity = self.select_identity(identity=identity) | ||
| if identity: | ||
| if self.use_docker: | ||
| raise BriefcaseCommandError( | ||
| "Signing system packages is not supported when using " | ||
| "Docker. Re-run the package command without the " | ||
| "`--target` option, or select `Don't sign`." | ||
| ) | ||
| # Signing is required; verify the signing tool is available. | ||
| self._verify_signing_tool(app) | ||
| else: | ||
|
|
@@ -1285,6 +1382,29 @@ class LinuxSystemPackageCommand( | |
| def packaging_formats(self): | ||
| return ["deb", "rpm", "pkg", "system"] | ||
|
|
||
| def _package_app( | ||
| self, | ||
| app: FinalizedAppConfig, | ||
| update: bool, | ||
| packaging_format: str, | ||
| **options, | ||
| ) -> dict | None: | ||
| """Internal method to invoke packaging on a single app. | ||
|
|
||
| If the user hasn't specified a concrete packaging format, the format determined | ||
| during app finalization is used, rather than the raw "system" alias. | ||
|
|
||
| :param app: The application to package | ||
| :param update: Should the application be updated (and rebuilt) first? | ||
| :param packaging_format: The format of the packaging artefact to create. | ||
| """ | ||
| return super()._package_app( | ||
| app, | ||
| update, | ||
| _resolve_system_packaging_format(app, packaging_format), | ||
| **options, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see why this abstraction is needed. At the point |
||
|
|
||
| def _verify_packaging_tools(self, app: LinuxSystemAppConfig): | ||
| """Verify that the local environment contains the packaging tools.""" | ||
| tool_name, executable_name, package_name = { | ||
|
|
@@ -1308,21 +1428,6 @@ def _verify_packaging_tools(self, app: LinuxSystemAppConfig): | |
| def verify_app_tools(self, app: FinalizedAppConfig): | ||
| app = cast(LinuxSystemAppConfig, app) | ||
| super().verify_app_tools(app) | ||
| # If "system" packaging format was selected, determine what that means. | ||
| if app.packaging_format == "system": | ||
| app.packaging_format = { | ||
| DEBIAN: "deb", | ||
| RHEL: "rpm", | ||
| ARCH: "pkg", | ||
| SUSE: "rpm", | ||
| }.get(app.target_vendor_base) | ||
|
|
||
| if app.packaging_format is None: | ||
| raise BriefcaseCommandError( | ||
| "Briefcase doesn't know the system packaging format for " | ||
| f"{app.target_vendor}. You may be able to build a package " | ||
| "by manually specifying a format with -p/--packaging-format" | ||
| ) | ||
|
|
||
| if not self.use_docker: | ||
| self._verify_packaging_tools(app) | ||
|
|
@@ -1697,6 +1802,32 @@ def _package_pkg( | |
| class LinuxSystemPublishCommand(LinuxSystemDockerMixin, PublishCommand): | ||
| description = "Publish a Linux system project." | ||
|
|
||
| def _publish_app( | ||
| self, | ||
| app: FinalizedAppConfig, | ||
| update: bool, | ||
| packaging_format: str, | ||
| channel: BasePublicationChannel, | ||
| **options, | ||
| ) -> dict | None: | ||
| """Internal method to publish a single app. | ||
|
|
||
| If the user hasn't specified a concrete packaging format, the format determined | ||
| during app finalization is used, rather than the raw "system" alias. | ||
|
|
||
| :param app: The application to publish | ||
| :param update: Should the application be updated (and rebuilt) first? | ||
| :param packaging_format: The format of the packaging artefact to create. | ||
| :param channel: The resolved BasePublicationChannel instance | ||
| """ | ||
| return super()._publish_app( | ||
| app, | ||
| update, | ||
| _resolve_system_packaging_format(app, packaging_format), | ||
| channel, | ||
| **options, | ||
| ) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As with |
||
|
|
||
|
|
||
| # Declare the briefcase command bindings | ||
| create = LinuxSystemCreateCommand | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As flagged in my previous review - this is an example of a constant that is used in one location. There's no reason to have it pulled out here.