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

code signature invalid for running process #4994

Closed
tessus opened this issue Jan 8, 2019 · 27 comments
Closed

code signature invalid for running process #4994

tessus opened this issue Jan 8, 2019 · 27 comments

Comments

@tessus
Copy link
Contributor

tessus commented Jan 8, 2019

I've noticed some disconcerting behavior when starting Electrum (3.3.2).

Even though the signature is valid on disk

$ codesign --verify --verbose=4 /Applications/Electrum.app
/Applications/Electrum.app: valid on disk
/Applications/Electrum.app: satisfies its Designated Requirement

I get the following warning from Little Snitch that the code signature of the running process is not valid:

screen shot 2019-01-08 at 17 01 17

I downloaded Electrum from the official place and verified its signature.

This definitely raises some red flags and I kindly ask for an explanation.

@ecdsa
Copy link
Member

ecdsa commented Jan 9, 2019

The 3.3.x releases of Electrum are the first that we sign with a MacOS developer key.
The warning you get is probably related to that.

@tessus
Copy link
Contributor Author

tessus commented Jan 10, 2019

@ecdsa Unfortunately the problem is somewhat related to signing the app with a developer id.

Electrum changes the running process, which is a big no-no. I have already contacted the developers of LS and I got a very detailed explanation of why this is happening.

Suffice it to say, if you won't change how Electrum is run and the way you sign the app, Electrum will stop running on future versions of macOS - maybe even on 10.14.x.

I asked the LS developer for permission to copy and paste the explanation here. Until then, please send me an email, if you want to know why this is happening and what the problem is.

@ecdsa
Copy link
Member

ecdsa commented Jan 10, 2019

please send to the email address on my profile

@tessus
Copy link
Contributor Author

tessus commented Jan 10, 2019

Here's a very detailed explanation what is happening. Thanks to @marcomasser

There’s a difference between the code signature of an executable on disk and the code signature of a running process. Using the following command, I can verify that the Electrum app has a valid code signature on disk:
codesign --verify --verbose=4 /Volumes/Electrum/Electrum.app

But when I launch it, Little Snitch shows the same warning that you see. We can now substitute the app path in the codesign command with the process’ PID and check the running process’ code signature (you can get the PID from the details in Little Snitch’s connection alert by pressing the D key):
codesign --verify --verbose=4 <PID of Electrum>

This command then shows the message:
<PID>: code identity has been invalidated

This means that something has turned the running process’ code signature invalid. This usually happens when a validly signed executable is launched and the resulting process dynamically links against libraries (or plugins) that do not have a valid code signature. We can investigate a bit further and see if we find anything in the case of Electrum.

First, let’s see what the executable is linked against:

$ otool -L /Volumes/Electrum/Electrum.app/Contents/MacOS/Electrum
/Volumes/Electrum/Electrum.app/Contents/MacOS/Electrum:
	/System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 157.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1226.10.1)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
	/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 48.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1258.1.0)
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 728.9.0)

That is not much. Specifically, those are only libraries that come with macOS and I wouldn’t expect them to have issues with the code signature (they are protected by the System Integrity Protection, after all).

Let’s see what the running process links against:

$ vmmap -w <PID of Electrum> | grep "__TEXT"
__TEXT                 0000000105e6c000-0000000105e73000 [   28K    28K     0K     0K] r-x/rwx SM=COW          /Volumes/Electrum/Electrum.app/Contents/MacOS/Electrum
__TEXT                 0000000108f43000-0000000109109000 [ 1816K  1816K     0K     0K] r-x/rwx SM=COW          /var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/Python
__TEXT                 0000000109438000-000000010943d000 [   20K    20K     0K     0K] r-x/rwx SM=COW          /var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/_struct.cpython-36m-darwin.so
<and many more…>

It looks like the running process links against libraries in a temporary directory. The first of those libraries is weirdly called just “Python”. It is a dynamic library, though:

$ file /var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/Python
/var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/Python: Mach-O 64-bit dynamically linked shared library x86_64

Let’s check its code signature:

$ codesign --verify --verbose=4 /var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/Python
/var/folders/w0/g3nlt5f14w9cy_shkddr9l800000gn/T/_MEINg34e8/Python: code object is not signed at all
In architecture: x86_64

And there you have it. A process that starts out with a valid code signature links against an unsigned library. This makes the whole process untrustworthy and therefore macOS takes away the flag that indicates a valid code signature, which Little Snitch then sees and reports to you.

Electrum seems to be a weird beast. How can it link against libraries in a temporary directory? It must copy them there itself. But to do that, they must come with the app. I find it rather strange, then, that the whole of the app bundle does not contain any of these files:

$ find /Volumes/Electrum/Electrum.app/
/Volumes/Electrum/Electrum.app/
/Volumes/Electrum/Electrum.app//Contents
/Volumes/Electrum/Electrum.app//Contents/_CodeSignature
/Volumes/Electrum/Electrum.app//Contents/_CodeSignature/CodeResources
/Volumes/Electrum/Electrum.app//Contents/Frameworks
/Volumes/Electrum/Electrum.app//Contents/Info.plist
/Volumes/Electrum/Electrum.app//Contents/MacOS
/Volumes/Electrum/Electrum.app//Contents/MacOS/Electrum
/Volumes/Electrum/Electrum.app//Contents/Resources
/Volumes/Electrum/Electrum.app//Contents/Resources/electrum.icns

That’s all of it, none of the files appear anywhere in the file system here. The executable itself is almost 50MB, though, so all that stuff is probably packed in there somewhere:

$ ls -lh /Volumes/Electrum/Electrum.app/Contents/MacOS/Electrum 
-rwxr-xr-x  1 marco  staff    49M Dec 21 22:52 /Volumes/Electrum/Electrum.app/Contents/MacOS/Electrum

I don’t know what the intent behind this approach is to get unsigned code onto a user’s computer, but I wouldn’t trust that app as far as I can throw it. Speaking of which, I’m throwing it in the trash right now. I’d suggest you remain at least sceptical about what this app does.

All that being said, the app on disk does have a valid code signature and most developers think that is enough. I assume they simply did not think through what it means to have an app that links against dynamic libraries in this weird way. And since there are almost no consequences on macOS in cases such as this where the code signature becomes invalid, it’s likely they don’t know they have a problem here.
macOS 10.14 introduced the concept of the Hardened Runtime that at some point in the future all apps must enable this feature. If I understand it correctly, one of its features is that apps that break their code signature will simply be killed. I can’t wait until that is in effect because dubious practices like the one used by Electrum will then have to be fixed.

@SomberNight
Copy link
Member

SomberNight commented Jan 10, 2019 via email

@marcomasser
Copy link

I’m not familiar with PyInstaller, but I assumed that Electrum uses some kind of thing that allows it to be packaged up in such a way. Since it seems to use a bunch of libraries that are packaged up into a single file (the executable) I think it should be possible to sign those libraries before packaging them up. I’ll have a look at it and see if I can find anything.

@ecdsa
Copy link
Member

ecdsa commented Jan 14, 2019

maybe related pyinstaller/pyinstaller#3550

@cculianu
Copy link
Collaborator

cculianu commented Jan 15, 2019

Ugh. Python and Apple products so rarely play nice.

Yeah basically what's happening is the generated PyInstaller app is basically a wrapper app with an embedded zip file. It unzips the zip file then runs a subprocess with a python interpreter.

The subprocess runs an executable and/or links against that Python lib that is not codesigned -- hence the Little Snitch warning.

Strangely macOS gatekeeper doesn't complain and it lets you get away with it (even though internally it marks the app as no longer properly signed, I guess).

There are lots of extant apps in the wild that do this (obviously all PyInstaller based apps do). Many apps run subprograms and do other weirdness. LS is particularly conservative about this as it's a tool designed for the paranoid.

I'm wondering if/when Apple will actually restrict apps in the wild to not do this. Perhaps soon.. perhaps in 5 years? Who knows!

I do know that already App Store apps are very restricted. They all run in a sandbox. It's impossible to submit an app like Electrum or Elecrtron Cash to the app store as a result because of PyInstaller issues. :)

and yes pyinstaller/pyinstaller#3550 is related but solving that wouldn't 100% solve everything as there needs to be a mechanism in place to also sign the python interpreter that pyinstaller bundles... perhaps some "hooks" or something can be written (no idea about how extensible pyinstaller is).

sigh.

EDIT: You'd have to codesign a PLETHORA of libs to get LS to not complain. Every .so embedded with python (python has lots of .so's) as well as all the 'dylibs' associated with Qt and the libs Qt calls. I don't see this problem being solved unless someone figures out how to hook into pyinstaller before it packages the .zip and execute some shell code (maybe that's possible?!). I'm not familiar enough with pyinstaller (yet) to know how to do this. I'm curious what @marcomasser comes up with!

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

Update: I think I figured out a fix for this in Electron Cash: Electron-Cash#1110

It's a bit hackish, but I basically sign all the binaries before they get zipped into the final .app bundle. It fixes the issue 100%, even if it is a bit ugly.

@ecdsa let me know if you would be interested in this for Electrum and I can put together a PR that incorporates this change for Electrum.

@marcomasser
Copy link

@cculianu: What you’re describing is how I would interpret the situation and what you’re suggesting is exactly what I wanted to try: sign the libraries before they are zipped up.
I didn’t have time yet to take a closer look at the issue, but it seems to me that you figured it out. I’ll have time to test your latest commit tomorrow and I’ll report back with what Little Snitch has to say about it. Thank you very much for digging into this!

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

@marcomasser Awesome! I'd love to know. Note: that I did this on Electron Cash first. I can put together a PR for Electrum for you to try. Are you set up to build? Want me to put up a built (and signed, by me) binary of Electrum on my website so you can try it? It does need to be built and signed for this problem to appear. I actual'y don't have Little Snitch installed -- so it would be a huge help if you can verify that this satisfies Little Snitch. I did however go in and verify each binary was signed (in that _MEI directory that pyinstaller creates at runtime), as well as run codesign -vvv PID on all the PIDs of the running program.

Let me know!

cculianu added a commit to Electron-Cash/Electron-Cash that referenced this issue Jan 16, 2019
Related: spesmilo#4994

OSX building wasn't code signing the bundled binaries that PyInstaller adds to the .zip embedded in the app executable.

This could potentially cause issues in the future if MacOS versions in the future require fully signed apps.
cculianu added a commit to simpleledger/Electron-Cash-SLP that referenced this issue Jan 16, 2019
Related: spesmilo#4994

OSX building wasn't code signing the bundled binaries that PyInstaller adds to the .zip embedded in the app executable.

This could potentially cause issues in the future if MacOS versions in the future require fully signed apps.
@cculianu
Copy link
Collaborator

@marcomasser : Ok, I added the PR to Electrum here: #5007 .

You can build it yourself and sign it -- or, if you want to try the built, code-signed binary (I promise no viruses!), you can download it here: https://www.c3-soft.com/downloads/Bitcoin/Electrum/electrum-3.3.2-17-g5ec330680.dmg

Please do let me know if Little Snitch is satisfied now! (It should be, I verified the dynamic process is indeed code sign valid).

@marcomasser
Copy link

@cculianu: You rock! 🎉
I downloaded your build and everything checks out fine. According to codesign the code signature on disk is fine, the code signature of all dynamically linked libraries on disk in the temporary directory is fine, and the code signature of the running process is fine. As a result, Little Snitch shows that everything is as it should be.

In short: If your changes can be upstreamed into Electrum, I would consider this issue fixed.

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

@marcomasser Ah man I love you. Thanks for responding so quickly. I was dying to find out if Little Snitch was satisfied! :D I already merged this into Electron Cash -- hopefully Electrum will also be ok with it and merge it soon.

Thanks again!!

@marcomasser
Copy link

@cculianu: Perfect! As a side question: Is this a solution that is general enough to be considered for inclusion with PyInstall? Or is this Electrum-specific?

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

@marcomasser The high level view of what's happening is not Electrum (nor Electron Cash) specific. It's a fairly generic "sign binaries as a final step before packaging".

Right now that's accomplished by hot-patching PyInstaller to provide a "home made" hook to do that by allowing it to call our code.

I imagine a "real pyinstaller patch" would either:

  1. Expose an API where calling code could do its own final touch-ups before packaging. (Very generic)
  2. Or pyinstaller could be made codesigning-aware enough so that you pass it a signing identity and it does the codesigning for you (probably the most developer-friendly approach is this one).

So yes -- this could potentially go upstream to pyinstaller in some form.

Now my two cents: My own experience with contributing to PyInstaller has been one where they are not very responsive or very quick to integrate patches. I have a patch sitting with them for quite some time now: pyinstaller/pyinstaller#3832. So I'm rather lazy/hesitant to even submit another patch to them -- for fear it may prove an equally unsatisfying experience.

That being said -- perhaps I can add this to pyinstaller as a patch and maybe they might merge it eventually, one would hope.

I'm pretty sure we aren't the only people wanting this feature.

What do you think?

@marcomasser
Copy link

@cculianu: Got it, thanks for the explanation. Good to know this is rather generic.

As for the likelihood of such a change being merged: I can tell even less than you. But I would hope that this kind of thing would be something that they are interested in. Especially considering Apple‘s announcements that:

  • The Hardened Runtime introduced in macOS Mojave kills apps whose code signature gets invalidated (as far as I understand).
  • Notarized apps must use the Hardened Runtime.
  • In the future, all Developer ID signed apps must be notarized.

In short: There will be a point in the future where PyInstall either must implement this or be only usable for developers who do not sign their code at all.

Don‘t get me wrong, I don‘t want to belittle your other pull request, but the impact of this seems greater than not being able to open URLs and accept drag&drop. Although a good Mac app should do these things properly, of course!

@cculianu
Copy link
Collaborator

@marcomasser — you are absolutely right on all fronts. You made me laugh: good point that this is much more critical that the other patch. (The other patch was also all C based which may be intimidating/time-consuming to review).

You’ve convinced me.

I’ll submit a patch to PyInstaller. Also thanks for the bullet point list. I’ll be sure to use that as arguments along with the PR write up to scare them to pay attention. Ha ha.

(also this patch is probably going to be tiny and 100% python so it stands a stronger shot on that front too).

Thanks!

@marcomasser
Copy link

@cculianu: 😀 Thank you so much for investing your time into this! If there’s anything I can help, just let me know.

Also, thanks again @tessus for bringing this to my attention!

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

Cool. When I do submit the PR I’ll let you know so you can go in there and lobby for it, ha.

Thanks! :)

@tessus
Copy link
Contributor Author

tessus commented Jan 16, 2019

This is great news. Thanks for looking into this and having found a solution.

The following 3 points kind of make me a bit uncomfortable:

  • The Hardened Runtime introduced in macOS Mojave kills apps whose code signature gets invalidated (as far as I understand).
  • Notarized apps must use the Hardened Runtime.
  • In the future, all Developer ID signed apps must be notarized.

A while back I wanted to subscribe to the developer program, but Apple forces you to use 2FA. Generally not a bad idea, but their 2FA is just a facade for upselling additional HW. It's also less secure than TOTP. Their 2FA requires a trusted device, which uses an iCloud account. I only have my MacBook and I refuse to open it up to another attack vector and I do not know what Apple syncs in the background with iCloud. Fun fact, the policy profile on my work Mac deactivated iCloud altogether.
As a backup solution Apple allows you to use your phone to receive a text message. Wow, seriously? What will happen, if I travel and use a different SIM card? I'd have to take my other SIM card and pay up to $5 roaming charges to receive a text. (Yes, Canada is the 5th most expensive country for wireless.)
Basically Apple's 2FA is unusable for me.

Now, coming back to the 3 points above. While I like the fact that the hardened runtime kills an app that modifies its signature, I have a serious problem with notarized apps.
Apple now thinks their signatures are not safe enough? They want to have additional say in what an app can do, even, if it is not in their AppStore. A lot of developers cannot put their apps in the AppStore, because of its restrictions. Not because the app does something harmful, but Apple thinks it might or just because Apple doesn't like you to use a certain API. Take a torrent app, e.g. Transmission. So now, this app can't be notorized, because Apple thinks that all torrenting apps are bad, ignoring the fact that companies distribute their large downloads via torrents (e.g. Fedora).
And believe me, Apple will apply the same rules to notarize an app as they do for apps in the AppStore - if not now, they will for sure in the future.

Also, now I have to wait on Apple again to get an app notorized? Does this also mean they will get my source code? The 3rd item in the list above will once again kill a lot of awesome apps.

While I love macOS, I always had reservations about their app landscape and the way they force people to use a device a certain way. That's why I don't own an iPad or iPhone. Now Apple is starting to make the same mistake with macOS.

@cculianu
Copy link
Collaborator

cculianu commented Jan 16, 2019

Man, I could not agree with you more.

Take a torrent app, e.g. Transmission. So now, this app can't be notorized, because Apple thinks that all torrenting apps are bad,

Yes, this is where I fear it will go. I am going to personally resist notarization for as long as I can -- basically for this reason. I think once they have that power they will start using it pretty quickly. They may start banning certain app categories entirely -- such as torrent apps.

So basically I won't notarize any apps with them unless I absolutely have to. I urge other developers to do the same.

Hopefully the market won't support them banning apps and locking down the platform to a point where nothing can happen without Apple's stamp of approval -- ever. As you say the App Store is a great example of certain apps being absolutely impossible to release. I have a screen recorder app in the store that can't even properly record system audio because Apple bans apps from the store that use custom kernel-space drivers (!!). Their sandbox mode is very restrictive. And guess what? It's just a control scheme, ultimately, which restricts competition. All of Apple's apps (such as Xcode) violate the sandboxing rules. As do a handful of other apps from key players in the industry that are good friends with Apple. The sandboxing rules only apply to small-time developers like me. Big companies that play nice with Apple get a pass. Us smaller developers are at a severe disadvantage to compete with these apps when they can install kernel drivers and do anything to a machine and we are stuck in sandbox-land.

We can only hope competition remains and they don't become some sort of de-facto monopoly where we stop having a choice. Currently we do have a choice.. so they can't get away with most of what they would naturally be inclined to do should they become a full monopoly.

But yes, well said. There is lots of room for abuse there and we as technical people pretty much should call them out on in whenever we can.

EDIT: I agree that Apple sucks but maybe other people in this thread wouldn't want to hear our gripes with them, ha. We can agree and move on perhaps? (I don't want to spam Electrum's issue tracker with this stuff.. as much as I like talking about it)... :)

@ecdsa
Copy link
Member

ecdsa commented Jan 17, 2019

@cculianu How far are you from submitting a PR to pyinstaller? Even if your PR takes a lot of time to be merged, we could use a custom PyInstaller in our build process, instead of merging #5007

@ecdsa
Copy link
Member

ecdsa commented Jan 17, 2019

merged. thanks again

@ecdsa ecdsa closed this as completed Jan 17, 2019
@cculianu
Copy link
Collaborator

@ecdsa We already use a custom pyinstaller in the build process for Electron Cash (one that supports URL events after the app is running) -- it's not hard to do -- as I'm sure you know since you used to do it for Electrum on Windows. :)

I am a few days away from that patch as I have some other stuff to finish up.

I see you merged this already -- you can always unmerge or delete it (it's only those small changes) once that's done and you can go that route.

I'm going to submit the patch against 3.5-dev branch though of PyInstaller but I'll also make one against 3.4 as well. We shall see how that goes. I'll let you know!

@tessus
Copy link
Contributor Author

tessus commented Jan 18, 2019

@ecdsa can you please post a comment in this thread, when you have an Electrum release where all binaries are signed (even the ones from the PyInstaller)?

@cculianu
Copy link
Collaborator

cculianu commented Jan 19, 2019

@marcomasser @ecdsa Hey! I did it! I created the PR for PyInstaller. It is here. pyinstaller/pyinstaller#3991

Feel free to go into the PR conversation and lobby for it. Perhaps the squaky wheel gets more grease..

@ecdsa : Note this is against their 3.5 'develop' branch. I want to also make this for our private use against the current release 3.4 branch, and package it into a .zip (along with my 'AppleEvents' changes that will allow Electrum to open URLs once it's running -- currently it can't do that -- URLs only work if Electrum wasn't running!). So I will be doing this regardless for Electron Cash.. let me know if you are interested and you can use the exact same PyInstaller.zip file in your Electrum project and it can be easily added to the make_osx script... I'll let you know when that's done.

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

No branches or pull requests

6 participants