From 65b8c20a96877038daa4907b80cd96f96e0bfe33 Mon Sep 17 00:00:00 2001 From: Jason Tsai Date: Mon, 23 Oct 2023 06:32:58 -0500 Subject: [PATCH] fix(codesign): code signing frameworks and binaries on macOS (#54) * fix: code sign frameworks and binaries Port of tauri-apps/tauri#7774 Co-authored-by: Trey Smith Co-authored-by: Lucas Nogueira * fix: create target binary directory before copying * chore: update error doc * fix: apply suggestion Co-authored-by: Amr Bashir --------- Co-authored-by: Trey Smith Co-authored-by: Lucas Nogueira Co-authored-by: Amr Bashir --- .changes/codesign-frameworks-binaries.md | 5 ++ .changes/remove-bundler-xattr.md | 5 ++ crates/packager/src/codesign/macos.rs | 52 +++++++++------ crates/packager/src/config/mod.rs | 8 ++- crates/packager/src/error.rs | 4 ++ crates/packager/src/package/app/mod.rs | 85 ++++++++++++++++++++---- crates/packager/src/package/dmg/mod.rs | 9 ++- 7 files changed, 131 insertions(+), 37 deletions(-) create mode 100644 .changes/codesign-frameworks-binaries.md create mode 100644 .changes/remove-bundler-xattr.md diff --git a/.changes/codesign-frameworks-binaries.md b/.changes/codesign-frameworks-binaries.md new file mode 100644 index 00000000..52786eb8 --- /dev/null +++ b/.changes/codesign-frameworks-binaries.md @@ -0,0 +1,5 @@ +--- +"cargo-packager": patch +--- + +Code sign binaries and frameworks on macOS. diff --git a/.changes/remove-bundler-xattr.md b/.changes/remove-bundler-xattr.md new file mode 100644 index 00000000..9f08e10e --- /dev/null +++ b/.changes/remove-bundler-xattr.md @@ -0,0 +1,5 @@ +--- +"cargo-packager": patch +--- + +Remove extended attributes on the macOS app bundle using `xattr -cr $PATH`. diff --git a/crates/packager/src/codesign/macos.rs b/crates/packager/src/codesign/macos.rs index 0d959e0b..7df1fb2d 100644 --- a/crates/packager/src/codesign/macos.rs +++ b/crates/packager/src/codesign/macos.rs @@ -145,19 +145,14 @@ pub fn delete_keychain() { .output_ok(); } -#[tracing::instrument(level = "trace")] -pub fn try_sign( - path_to_sign: &Path, - identity: &str, - config: &Config, - is_an_executable: bool, -) -> crate::Result<()> { - tracing::info!( - "Signing {} with identity \"{}\"", - path_to_sign.display(), - identity - ); +#[derive(Debug)] +pub struct SignTarget { + pub path: PathBuf, + pub is_an_executable: bool, +} +#[tracing::instrument(level = "trace")] +pub fn try_sign(targets: Vec, identity: &str, config: &Config) -> crate::Result<()> { let packager_keychain = if let (Some(certificate_encoded), Some(certificate_password)) = ( std::env::var_os("APPLE_CERTIFICATE"), std::env::var_os("APPLE_CERTIFICATE_PASSWORD"), @@ -170,20 +165,22 @@ pub fn try_sign( false }; - let res = sign( - path_to_sign, - identity, - config, - is_an_executable, - packager_keychain, - ); + for target in targets { + sign( + &target.path, + identity, + config, + target.is_an_executable, + packager_keychain, + )?; + } if packager_keychain { // delete the keychain again after signing delete_keychain(); } - res + Ok(()) } #[tracing::instrument(level = "trace")] @@ -194,6 +191,12 @@ fn sign( is_an_executable: bool, pcakger_keychain: bool, ) -> crate::Result<()> { + tracing::info!( + "Signing {} with identity \"{}\"", + path_to_sign.display(), + identity + ); + let mut args = vec!["--force", "-s", identity]; if pcakger_keychain { @@ -269,7 +272,14 @@ pub fn notarize( .macos() .and_then(|macos| macos.signing_identity.as_ref()) { - try_sign(&zip_path, identity, config, false)?; + try_sign( + vec![SignTarget { + path: zip_path.clone(), + is_an_executable: false, + }], + identity, + config, + )?; }; let zip_path_str = zip_path.to_string_lossy().to_string(); diff --git a/crates/packager/src/config/mod.rs b/crates/packager/src/config/mod.rs index 48568756..b3a270b7 100644 --- a/crates/packager/src/config/mod.rs +++ b/crates/packager/src/config/mod.rs @@ -1016,7 +1016,8 @@ impl Config { } #[allow(unused)] - pub(crate) fn copy_external_binaries(&self, path: &Path) -> crate::Result<()> { + pub(crate) fn copy_external_binaries(&self, path: &Path) -> crate::Result> { + let mut paths = Vec::new(); if let Some(external_binaries) = &self.external_binaries { for src in external_binaries { let src = dunce::canonicalize(PathBuf::from(src))?; @@ -1026,11 +1027,12 @@ impl Config { .to_string_lossy() .replace(&format!("-{}", self.target_triple()), ""); let dest = path.join(file_name_no_triple); - std::fs::copy(src, dest)?; + std::fs::copy(src, &dest)?; + paths.push(dest); } } - Ok(()) + Ok(paths) } } diff --git a/crates/packager/src/error.rs b/crates/packager/src/error.rs index 3e8d7876..3d537742 100644 --- a/crates/packager/src/error.rs +++ b/crates/packager/src/error.rs @@ -232,6 +232,10 @@ pub enum Error { /// Failed to extract external binary filename #[error("Failed to extract filename from {0}")] FailedToExtractFilename(PathBuf), + /// Failed to remove extended attributes from app bundle + #[error("Failed to remove extended attributes from app bundle: {0}")] + #[cfg(target_os = "macos")] + FailedToRemoveExtendedAttributes(std::io::Error), } /// Convenient type alias of Result type for cargo-packager. diff --git a/crates/packager/src/package/app/mod.rs b/crates/packager/src/package/app/mod.rs index cf143e83..10790c71 100644 --- a/crates/packager/src/package/app/mod.rs +++ b/crates/packager/src/package/app/mod.rs @@ -4,10 +4,19 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: MIT -use std::path::{Path, PathBuf}; +use std::{ + ffi::OsStr, + path::{Path, PathBuf}, + process::Command, +}; use super::Context; -use crate::{codesign, config::Config, util}; +use crate::{ + codesign::{self, SignTarget}, + config::Config, + shell::CommandExt, + util, +}; #[tracing::instrument(level = "trace")] pub(crate) fn package(ctx: &Context) -> crate::Result> { @@ -28,6 +37,9 @@ pub(crate) fn package(ctx: &Context) -> crate::Result> { let resources_dir = contents_directory.join("Resources"); let bin_dir = contents_directory.join("MacOS"); + std::fs::create_dir_all(&bin_dir)?; + + let mut sign_paths = Vec::new(); let bundle_icon_file = util::create_icns_file(&resources_dir, config)?; @@ -35,20 +47,39 @@ pub(crate) fn package(ctx: &Context) -> crate::Result> { create_info_plist(&contents_directory, bundle_icon_file, config)?; tracing::debug!("Copying frameworks"); - copy_frameworks_to_bundle(&contents_directory, config)?; + let framework_paths = copy_frameworks_to_bundle(&contents_directory, config)?; + sign_paths.extend( + framework_paths + .into_iter() + .filter(|p| { + let ext = p.extension(); + ext == Some(OsStr::new("framework")) || ext == Some(OsStr::new("dylib")) + }) + .map(|path| SignTarget { + path, + is_an_executable: false, + }), + ); tracing::debug!("Copying resources"); config.copy_resources(&resources_dir)?; tracing::debug!("Copying external binaries"); - config.copy_external_binaries(&bin_dir)?; + let bin_paths = config.copy_external_binaries(&bin_dir)?; + sign_paths.extend(bin_paths.into_iter().map(|path| SignTarget { + path, + is_an_executable: true, + })); tracing::debug!("Copying binaries"); - let bin_dir = contents_directory.join("MacOS"); - std::fs::create_dir_all(&bin_dir)?; for bin in &config.binaries { let bin_path = config.binary_path(bin); - std::fs::copy(&bin_path, bin_dir.join(&bin.filename))?; + let dest_path = bin_dir.join(&bin.filename); + std::fs::copy(&bin_path, &dest_path)?; + sign_paths.push(SignTarget { + path: dest_path, + is_an_executable: true, + }); } if let Some(identity) = config @@ -56,7 +87,19 @@ pub(crate) fn package(ctx: &Context) -> crate::Result> { .and_then(|macos| macos.signing_identity.as_ref()) { tracing::debug!("Codesigning {}", app_bundle_path.display()); - codesign::try_sign(&app_bundle_path, identity, config, true)?; + // Sign frameworks and sidecar binaries first, per apple, signing must be done inside out + // https://developer.apple.com/forums/thread/701514 + sign_paths.push(SignTarget { + path: app_bundle_path.clone(), + is_an_executable: true, + }); + + // Remove extra attributes, which could cause codesign to fail + // https://developer.apple.com/library/archive/qa/qa1940/_index.html + remove_extra_attr(&app_bundle_path)?; + + // sign application + codesign::try_sign(sign_paths, identity, config)?; // notarization is required for distribution match codesign::notarize_auth() { @@ -253,7 +296,12 @@ fn copy_framework_from(dest_dir: &Path, framework: &str, src_dir: &Path) -> crat // Copies the macOS application bundle frameworks to the .app #[tracing::instrument(level = "trace")] -fn copy_frameworks_to_bundle(contents_directory: &Path, config: &Config) -> crate::Result<()> { +fn copy_frameworks_to_bundle( + contents_directory: &Path, + config: &Config, +) -> crate::Result> { + let mut paths = Vec::new(); + if let Some(frameworks) = config.macos().and_then(|m| m.frameworks.as_ref()) { let dest_dir = contents_directory.join("Frameworks"); std::fs::create_dir_all(contents_directory)?; @@ -264,7 +312,9 @@ fn copy_frameworks_to_bundle(contents_directory: &Path, config: &Config) -> crat let src_name = src_path .file_name() .ok_or_else(|| crate::Error::FailedToExtractFilename(src_path.clone()))?; - copy_dir(&src_path, &dest_dir.join(src_name))?; + let dest_path = dest_dir.join(src_name); + copy_dir(&src_path, &dest_path)?; + paths.push(dest_path); continue; } else if framework.ends_with(".dylib") { let src_path = PathBuf::from(&framework); @@ -275,7 +325,9 @@ fn copy_frameworks_to_bundle(contents_directory: &Path, config: &Config) -> crat .file_name() .ok_or_else(|| crate::Error::FailedToExtractFilename(src_path.clone()))?; std::fs::create_dir_all(&dest_dir)?; - std::fs::copy(&src_path, dest_dir.join(src_name))?; + let dest_path = dest_dir.join(src_name); + std::fs::copy(&src_path, &dest_path)?; + paths.push(dest_path); continue; } else if framework.contains('/') { return Err(crate::Error::InvalidFramework { @@ -303,5 +355,14 @@ fn copy_frameworks_to_bundle(contents_directory: &Path, config: &Config) -> crat } } - Ok(()) + Ok(paths) +} + +fn remove_extra_attr(app_bundle_path: &Path) -> crate::Result<()> { + Command::new("xattr") + .arg("-cr") + .arg(app_bundle_path) + .output_ok() + .map(|_| ()) + .map_err(crate::Error::FailedToRemoveExtendedAttributes) } diff --git a/crates/packager/src/package/dmg/mod.rs b/crates/packager/src/package/dmg/mod.rs index a053263c..e98b4008 100644 --- a/crates/packager/src/package/dmg/mod.rs +++ b/crates/packager/src/package/dmg/mod.rs @@ -147,7 +147,14 @@ pub(crate) fn package(ctx: &Context) -> crate::Result> { .and_then(|macos| macos.signing_identity.as_ref()) { tracing::debug!("Codesigning {}", dmg_path.display()); - codesign::try_sign(&dmg_path, identity, config, false)?; + codesign::try_sign( + vec![codesign::SignTarget { + path: dmg_path.clone(), + is_an_executable: false, + }], + identity, + config, + )?; } Ok(vec![dmg_path])