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 <[email protected]>
Co-authored-by: Lucas Nogueira <[email protected]>

* fix: create target binary directory before copying

* chore: update error doc

* fix: apply suggestion

Co-authored-by: Amr Bashir <[email protected]>

---------

Co-authored-by: Trey Smith <[email protected]>
Co-authored-by: Lucas Nogueira <[email protected]>
Co-authored-by: Amr Bashir <[email protected]>

.changes/codesign-frameworks-binaries.md

@@ -0,0 +1,5 @@
+---
+"cargo-packager": patch
+---
+
+Code sign binaries and frameworks on macOS.

.changes/remove-bundler-xattr.md

@@ -0,0 +1,5 @@
+---
+"cargo-packager": patch
+---
+
+Remove extended attributes on the macOS app bundle using `xattr -cr $PATH`.

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<SignTarget>, 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();

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<Vec<PathBuf>> {
+        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)
     }
 }
 

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.

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<Vec<PathBuf>> {
@@ -28,35 +37,69 @@ pub(crate) fn package(ctx: &Context) -> crate::Result<Vec<PathBuf>> {
 
     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)?;
 
     tracing::debug!("Creating Info.plist");
     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
         .macos()
         .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<Vec<PathBuf>> {
+    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)
 }

crates/packager/src/package/dmg/mod.rs

@@ -147,7 +147,14 @@ pub(crate) fn package(ctx: &Context) -> crate::Result<Vec<PathBuf>> {
         .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])