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

AdGuard for Safari doesn't work on OS Catalina #172

Closed
vbagirov opened this issue Jun 5, 2019 · 19 comments
Closed

AdGuard for Safari doesn't work on OS Catalina #172

vbagirov opened this issue Jun 5, 2019 · 19 comments
Assignees
Labels
Bug Something isn't working P3: Medium Medium priority
Milestone

Comments

@vbagirov
Copy link
Member

vbagirov commented Jun 5, 2019

Screenshot 1

image

Screenshot 2

image

@ameshkov ameshkov added this to the 1.4 milestone Jun 5, 2019
@choco
Copy link

choco commented Jun 5, 2019

Works for me? I'm using the latest beta

@JasSmiths
Copy link

@choco Latest beta of Catalina or AdGuard?

Fresh install of AdGuard and Catalina here and definitely not working

@choco
Copy link

choco commented Jun 5, 2019

Latest AdGuard beta. I didn't do a clean install, but upgraded from Mojave.

@JasSmiths
Copy link

@choco checking latest beta now

@JasSmiths
Copy link

Confirm latest beta works fine!

Thanks @choco

@NanoSector
Copy link

NanoSector commented Jul 1, 2019

I think this is a bug in Catalina itself because WhatsApp also fails to launch with the same Gatekeeper error:

image

Perhaps just something related to Electron apps. I'll see if I can find more to test.

@NanoSector
Copy link

What's weird is that apparently the created file is properly signed:

image

I've filed a bug with the Feedback Assistant on the 1st of July but there hasn't been any updates since.

@ameshkov ameshkov added Bug Something isn't working P3: Medium Medium priority labels Jul 3, 2019
@ameshkov
Copy link
Member

ameshkov commented Jul 3, 2019

@Yoshi2889 could you please check if there's any issues with the standalone build of AdGuard?

@NanoSector
Copy link

NanoSector commented Jul 3, 2019

@ameshkov The GitHub releases build seems to work just fine, it doesn't throw any errors.

Gatekeeper is behaving a bit weird however:

Screenshot 2019-07-03 at 14 55 54

Speaking of Gatekeeper, my idea of checking the signing status of the file came from this blog post: https://eclecticlight.co/2019/06/12/grokking-gatekeeper-in-catalina/
Apparently not only the binary that's being launched is checked now. But, as the file is properly signed, that should not be an issue. Might help with narrowing down this issue on your side though.

Looking through the Task Explorer app, none of the standalone app's open files list any file named similarly to what throws the error on the MAS version.

EDIT: Wrong image, oops.

@ameshkov
Copy link
Member

ameshkov commented Jul 3, 2019

I am starting to think that it requires the library to be notarized, and not just signed.

The question is why it is not notarized automatically when we upload the app to the appstore? After all, the standalone build is properly notarized.

@ameshkov
Copy link
Member

ameshkov commented Jul 4, 2019

@Mizzick let's try getting rid of dlopen and electron-rebuild to get ours included into the main binary:
https://github.com/electron/electron/blob/master/docs/tutorial/using-native-node-modules.md#installing-modules-and-rebuilding-for-electron

@zzebrum zzebrum modified the milestones: 1.4, 1.5 Jul 11, 2019
@glouel
Copy link

glouel commented Jul 19, 2019

I am starting to think that it requires the library to be notarized, and not just signed.

Just to clarify, it's now a requirement in Catalina, your whole app must be signed and notarized. Every file needs to be properly signed inside for notarization to work. Notarization will fail if one file is not signed, but at least it will give you a good error output on which files are not signed.

You can have a look here : https://developer.apple.com/documentation/security/notarizing_your_app_before_distribution/customizing_the_notarization_workflow?language=objc

This is mandatory for all distributions methods, so your builds in this repository should also be notarized or they won't work without disabling gatekeeper.

@ameshkov
Copy link
Member

ameshkov commented Jul 19, 2019

@glouel well, we notarize the standalone build, and it works just okay on Catalina.

But it's not that simple with the MAS build. In theory, we shouldn't do anything, it all must be done on the App Store side.

@NanoSector
Copy link

Is the non MAS version sandboxed? I’m thinking that there might be a difference that causes that file to only be generated under the MAS version.

@ameshkov
Copy link
Member

@Yoshi2889 nope, it's just with hardened runtime enabled.

I’m thinking that there might be a difference that causes that file to only be generated under the MAS version.

Hm, sounds interesting.

@NanoSector
Copy link

Hm, might be worth checking if the MAS is sandboxed.

I’ll check if the standalone WhatsApp client also works on Catalina tonight seeing how its MAS version is also broken in a similar way.

@Mizzick
Copy link
Contributor

Mizzick commented Jul 19, 2019

@Yoshi2889
The MAS version is sandboxed, this is the main difference comparing to standalone.

I’m thinking that there might be a difference that causes that file to only be generated under the MAS version.

Are you talking about that native node module file?

@glouel
Copy link

glouel commented Jul 19, 2019

@glouel well, we notarize the standalone build, and it works just okay on Catalina.

Apologies, I assumed you didn't. FYI for some reason 1.4.1 (this file : https://github.com/AdguardTeam/AdGuardForSafari/releases/download/v1.4.1/AdGuard.for.Safari.app.zip ) won't launch on my system on Catalina beta 4. I had previously installed the MAS version a few days ago (I think 1.3.3 ?) and had the same error as top post (I bypassed it by disabling gatekeeper temporarily).

But right now the standalone 1.4.1 file won't launch, a quick check show the file is quarantined :

glouel@iMac-de-Guillaume-2:/Applications$ xattr -v AdGuard\ for\ Safari.app/
AdGuard for Safari.app/: com.apple.quarantine

If I run codesign, looks like the Electron framework is indeed the culprit as you guessed earlier:

glouel@iMac-de-Guillaume-2:/$ codesign -vvv --deep --strict /Applications/AdGuard\ for\ Safari.app/
(...)
/Applications/AdGuard for Safari.app/: bundle format unrecognized, invalid, or unsuitable
In subcomponent: /Applications/AdGuard for Safari.app/Contents/Frameworks/Electron Framework.framework

Usually removing the quarantine bit should do the trick (xattr -d com.apple.quarantine filename). It doesn't do the trick for me so I assume there's something linked to having the MAS version, having removed it and being unable to install the standalone version in it's place. I doubt the issue is on your end, but the Electron Framework thing is probably relevant anyway.

But it's not that simple with the MAS build. In theory, we shouldn't do anything, it all must be done on the App Store side.

Since 1.4.1 is also on the MAS now, I grabbed it there and it's better, in the sense I have the "usual" gatekeeper error :

Capture d’écran 2019-07-19 à 12 01 05

Funnily enough, it looks like it's properly signed:

(...)
--prepared:/Applications/AdGuard for Safari.app/Contents/Frameworks/Electron Framework.framework/Versions/Current/.
--validated:/Applications/AdGuard for Safari.app/Contents/Frameworks/Electron Framework.framework/Versions/Current/.
AdGuard for Safari.app/: valid on disk
AdGuard for Safari.app/: satisfies its Designated Requirement

(so here Electron framework seems to be signed differently than in your standalone build chain).

System policy seems to find the file ok too :

$ spctl -a -v AdGuard\ for\ Safari.app/
AdGuard for Safari.app/: accepted
source=Mac App Store

My guess is you may be having some issues with the hardened runtime, possibly the JIT part but not only. Here's what I see in Console :

erreur	12:38:43.103533+0200	kernel	Sandbox: AdGuard for Safa(1131) deny(1) mach-lookup com.apple.GameController.gamecontrollerd

(GameController ?)

and

erreur	12:39:27.343628+0200	tccd	Prompting policy for hardened runtime; service: kTCCServiceAppleEvents requires entitlement com.apple.security.automation.apple-events but it is missing for ACC:{ID: com.adguard.safari.AdGuard, PID[1131], auid: 501, euid: 501, binary path: '/Applications/AdGuard for Safari.app/Contents/MacOS/AdGuard for Safari'}, REQ:{ID: com.apple.appleeventsd, PID[368], auid: 55, euid: 55, binary path: '/System/Library/CoreServices/appleeventsd'}

So you need entitlements (this happens just before you try to set it for relaunch):

par défaut	12:39:32.273323+0200	loginwindow	-[PersistentAppsSupport saveLogoutPersistentState:finalSnapshot:] |      checkAgainstApp is : AdGuard for Safari
par défaut	12:39:32.273339+0200	loginwindow	-[PersistentAppsSupport appShouldBeRelaunched:] | entered. checking app: AdGuard for Safari
par défaut	12:39:32.273365+0200	loginwindow	-[PersistentAppsSupport saveLogoutPersistentState:finalSnapshot:] |           Adding to relaunchArray: AdGuard for Safari

So at the very least you need com.apple.security.automation.apple-events. I would probably suggest you also check this just to be sure about the JIT issues with hardened runtime : electron-userland/electron-builder#4040

To get back to making the MAS version work, here's what I did and should work for everyone:

  • Close AdGuard For Safari
  • In terminal type sudo spctl --master-disable
  • Launch AdGuard For Safari (it will launch correctly)
  • In terminal type sudo spctl --master-enable

Don't forget the last step to reenable Gatekeeper.

Sorry for the long post but hopefully that will help you figure out the issues.

@NanoSector
Copy link

Hm, might be worth checking if the MAS is sandboxed.

I’ll check if the standalone WhatsApp client also works on Catalina tonight seeing how its MAS version is also broken in a similar way.

Apologies for my late reply, I totally forgot. I tried the official WhatsApp Desktop client and it also works if you download its standalone website build. It is also not sandboxed in that case.

Mizzick added a commit that referenced this issue Sep 10, 2019
…e-version to master

* commit 'd2f73ca18d9cba489ffc4a22419702f03c685f24':
  updated version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working P3: Medium Medium priority
Projects
None yet
Development

No branches or pull requests

9 participants