Skip to content
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

macOS packaging: Perform ad-hoc signing of macOS bundle by default #4774

Merged
merged 2 commits into from
Jun 23, 2022

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented May 30, 2022

Starting with arm64 macOS Apple will require ad-hoc signatures, completely unsigned app bundles will no longer launch:

Screen Shot 2022-05-29 at 17 06 37

This branch fixes the issue by setting an ad-hoc signature, which only includes a checksum, by default if no other code signing identity is provided. With this patch, Mixxx launches normally again:

Screen Shot 2022-05-30 at 14 16 29

While arm64 macOS isn't officially supported by Mixxx yet, setting an ad-hoc signature on x86 too probably cannot hurt and fixing discrepancies between the x86 and arm64 builds in in advance is IMO always a good idea.

Marked as a draft PR for now since I haven't tested it with the x86 build yet.

@github-actions github-actions bot added the build label May 30, 2022
@fwcd fwcd changed the title [macOS packaging] Perform ad-hoc signing of macOS bundle by default macOS packaging: Perform ad-hoc signing of macOS bundle by default May 30, 2022
Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Thank you for the detailed and helpful comments!

Could be merged after confirmed to work as expected.

@fwcd
Copy link
Member Author

fwcd commented May 31, 2022

Hm, so there seems to be an issue with dynamic linking here, Mixxx crashes at startup and notes that one of the libraries didn't pass the validation check:

Exception Type:        EXC_CRASH (SIGABRT)
Exception Codes:       0x0000000000000000, 0x0000000000000000
Exception Note:        EXC_CORPSE_NOTIFY

Termination Reason:    Namespace DYLD, Code 1 Library missing
Library not loaded: @executable_path/../Frameworks/libchromaprint.1.5.0.dylib
Referenced from: /Users/USER/Downloads/mixxx.app/Contents/MacOS/mixxx
Reason: tried: '/Users/USER/Downloads/mixxx.app/Contents/MacOS/../Frameworks/libchromaprint.1.5.0.dylib' (code signature in <0D1C9E79-A9D4-3E1B-8B54-6F67437E93C0> '/Users/USER/Downloads/mixxx.app/Contents/Frameworks/libchromaprint.1.5.0.dylib' not valid for use in process: mapped file has no Team ID and is not a platform binary (signed with custom identity or adhoc?)), '/usr/lib/libchromaprint.1.5.0.dylib' (no such file)
(terminated at launch; ignore backtrace)

The reason why I didn't encounter this issue on ARM was presumably that I linked the dependencies statically, circumventing the issue. I am not entirely sure why this happens with ad-hoc signatures, Team ID-based library validation doesn't make much sense there (at least I would think so). According to a thread on the Apple developer forums disabling library validation might be an option (since we're making an unsigned build this shouldn't be much of a security issue), perhaps the easiest way would be not passing entitlements at all if the signing identity is set to -. Thoughts?

@daschuer
Copy link
Member

Maybe we have messed up something when building the environment. Can you check if this is also an issue with the main branch and using this patch?

@fwcd
Copy link
Member Author

fwcd commented May 31, 2022

I am using the GitHub Actions build from this repo of this PR, so that shouldn't be the issue

@fwcd
Copy link
Member Author

fwcd commented May 31, 2022

Not entirely sure why I was statically linking previously either, apparently the newer 2.4+ buildenvs produce static libraries and the older ones contain dylibs, but I haven't figured out why...

@daschuer
Copy link
Member

We have moved form our Jenkins server with custom scripts and dynamic libraries to a vcpkg based solution which is using static builds by default.

@Be-ing
Copy link
Contributor

Be-ing commented May 31, 2022

Not entirely sure why I was statically linking previously either, apparently the newer 2.4+ buildenvs produce static libraries and the older ones contain dylibs, but I haven't figured out why...

vcpkg builds static libraries by default on macOS (and Linux) (microsoft/vcpkg#19127). The 2.3 dependencies were not built with vcpkg (https://github.com/mixxxdj/buildserver)

@daschuer
Copy link
Member

daschuer commented Jun 2, 2022

@fwcd

Thoughts?

So it sounds like we have too options, disabling library validation for these private PR builds or merge this PR to main.
Is this correct? Both solution work for me.

I am not sure if the extra mile for a solution in the dynamic case will pay back. What do you think?

@fwcd fwcd marked this pull request as ready for review June 16, 2022 20:13
@fwcd
Copy link
Member Author

fwcd commented Jun 16, 2022

I have solved the issue now by disabling the hardened runtime (which includes library validation) if and only if an ad-hoc code signing identity is used. This should now yield the correct result in all cases, regardless of whether dynamic or static linking is used.

Before merging, I would like to verify that it works with the ARM build, otherwise this should be ready for review!

@fwcd
Copy link
Member Author

fwcd commented Jun 16, 2022

Seems to work well with the ARM build too.

cc @daschuer

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants