auriamg / macdylibbundler

Utility to ease bundling libraries into executables for OSX
MIT License
546 stars 83 forks source link

Fix binary signatures, as valid signatures are required on ARM Macs #71

Closed Kreeblah closed 2 years ago

Kreeblah commented 2 years ago

I noticed that dylibbundler was failing for me on my new M1 Mac, whereas it works fine for the same code on my old Intel Mac. I found this Github issue which shed some light on it, along with the error specifically referring to an invalid signature on one of the libraries in the app bundle, so it seems that adding a step to replace the signatures on the libraries and binary is required for the output of this to run when used on ARM code.

I did test this on my Intel mac as well, and the output still works fine there, too, so this doesn't break those use cases.

As far as why I went with an ad hoc signature, frankly, it was easiest, and anybody who cares about which signing identity is applied to their code can pretty easily go through and update the signatures afterwards.

P.S. If this pull request is accepted, could you please also make another release tag so I can have the Homebrew folks update their copy?

Kreeblah commented 2 years ago

Yep, these changes still resolve the issues I was having.

Kreeblah commented 2 years ago

For context around my last commit, I found this Apple document which outlines the code signing changes for ARM Macs. They recommend including --preserve-metadata, though the specific flags used by other projects (such as Homebrew's approach) include different options passed to it than Apple's document suggests. Namely, they're replacing the identifier option with requirements.

SCG82 commented 2 years ago

@Kreeblah Please check out https://github.com/Kreeblah/macdylibbundler/pull/1

Kreeblah commented 2 years ago

It seems to compile and run, and the logic makes sense. I've yet to have any signing failures, though, so I haven't been able to test the workaround.

I did, however, change the temp directory to be created in the directory indicated by the TMPDIR variable, since that's limited to only the running user, and is going to be better from a security perspective.

SCG82 commented 2 years ago

I tested it by commenting out line 225 in Utils.cpp:

* Processing ./OBS.app/Contents/Plugins/obs-x264.so
    chmod +w "./OBS.app/Contents/Plugins/obs-x264.so"
  * Fixing dependencies on ./OBS.app/Contents/Plugins/obs-x264.so
    install_name_tool -change "/Users/administrator/src/obsdeps/lib/libx264.161.dylib" "@executable_path/../libs/libx264.161.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -change "/Users/administrator/src/obs-studio-202111/xAuto/OBS.app/Contents/MacOS/libobs.0.dylib" "@executable_path/../libs/libobs.0.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -change "@rpath/libobs.0.dylib" "@executable_path/../libs/libobs.0.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -rpath "@loader_path/" "@executable_path/../libs/" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -rpath "@executable_path/" "@executable_path/../libs/" "./OBS.app/Contents/Plugins/obs-x264.so"
  * Error : An error occurred while applying ad-hoc signature to ./OBS.app/Contents/Plugins/obs-x264.so. Attempting workaround
    mkdir -p "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler"
    cp -p "./OBS.app/Contents/Plugins/obs-x264.so" "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler/obs-x264.so"
    mv -f "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler/obs-x264.so" "./OBS.app/Contents/Plugins/obs-x264.so"
    rm -rf "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler"
    codesign --force --deep --preserve-metadata=entitlements,requirements,flags,runtime --sign - "./OBS.app/Contents/Plugins/obs-x264.so"
SCG82 commented 2 years ago

@auriamg This PR looks good to me. Nice job, @Kreeblah!

Kreeblah commented 2 years ago

@SCG82 - Actually, could I get one more review? I was thinking about how the same directory name is being used for runs, and while it's unlikely, it is possible that somebody running multiple dylibbundler processes at the same time could have issues (the temp directory gets erased right after another dylibbunder creates it, for example). So, I created https://github.com/Kreeblah/macdylibbundler/tree/use_random_temp_dir

It works as I intended it to for the workaround logic (using the same method you did, with commenting out the if statement), so I can merge it into the branch I have in this PR if it looks good.

SCG82 commented 2 years ago

Perhaps change

const auto runCommand = [](const std::string& command, const std::string& errMsg, const bool& isArm)

to

const auto runCommand = [isArm](const std::string& command, const std::string& errMsg)

in line 247 and remove the isArm arguments from the runCommand calls.

Looks good otherwise (tested).

Kreeblah commented 2 years ago

Done. Thank you!

SCG82 commented 2 years ago

Looks good!

* Processing ./OBS.app/Contents/Plugins/obs-x264.so
    chmod +w "./OBS.app/Contents/Plugins/obs-x264.so"
  * Fixing dependencies on ./OBS.app/Contents/Plugins/obs-x264.so
    install_name_tool -change "/Users/administrator/src/obsdeps/lib/libx264.161.dylib" "@executable_path/../libs/libx264.161.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -change "/Users/administrator/src/obs-studio-202111/xAuto/OBS.app/Contents/MacOS/libobs.0.dylib" "@executable_path/../libs/libobs.0.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -change "@rpath/libobs.0.dylib" "@executable_path/../libs/libobs.0.dylib" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -rpath "@loader_path/" "@executable_path/../libs/" "./OBS.app/Contents/Plugins/obs-x264.so"
    install_name_tool -rpath "@executable_path/" "@executable_path/../libs/" "./OBS.app/Contents/Plugins/obs-x264.so"
  * Error : An error occurred while applying ad-hoc signature to ./OBS.app/Contents/Plugins/obs-x264.so. Attempting workaround
    cp -p "./OBS.app/Contents/Plugins/obs-x264.so" "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler.ebNb7JtA/obs-x264.so"
    mv -f "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler.ebNb7JtA/obs-x264.so" "./OBS.app/Contents/Plugins/obs-x264.so"
    rm -rf "/var/folders/jv/r2yktcxj7d35v3fk696zkq7w0000gn/T/dylibbundler.ebNb7JtA"
    codesign --force --deep --preserve-metadata=entitlements,requirements,flags,runtime --sign - "./OBS.app/Contents/Plugins/obs-x264.so"
SCG82 commented 2 years ago

@Kreeblah I'm going to squash these into 1 commit and force-push to your branch. @auriamg This PR is ready for review.

SCG82 commented 2 years ago

@Kreeblah merged & tagged