search

thisiscam / math-with-slack

Rendered math (MathJax) with Slack's desktop client
MIT License
306 stars 28 forks source link

App routinely logs user out on macOS after Slack 4.22 #30

Closed thisiscam closed 2 years ago

thisiscam commented 2 years ago

This is a separate issue created from information in #27. Currently, the script is fixed to handle Slack's new mechanism of verifying the app's integrity. However, it appears that during login Slack attempts to read from Apple's keychain for user authentication information. Because the fix requires modifications to the code, Apple gives an error:

[ERROR:keychain_password_mac.mm(103)] Keychain lookup failed: Error Domain=NSOSStatusErrorDomain Code=-25293 "errKCAuthFailed / errSecAuthFailed:  / Authorization/Authentication failed." (-25293)

And Slack will log the user off due to the failed attempt to fetch auth info.

thisiscam commented 2 years ago

FWIW, my intuition here is that there's very little we can do, since we don't really want to tamper with Apple's code signing process. But this makes me wonder if Slack might refuse a user to login if the code sign fails one day. If that happens, the script cannot work anymore for sure.

motiwari commented 2 years ago

@thisiscam do you think it's possible to do something akin to what users need to do for gdb? https://sourceware.org/gdb/wiki/PermissionsDarwin

motiwari commented 2 years ago

Also @thisiscam actually the issue occurs in Slack <4.22 as well :(

thisiscam commented 2 years ago

Also @thisiscam actually the issue occurs in Slack <4.22 as well :(

I should be able to fix this one.

thisiscam commented 2 years ago

@thisiscam do you think it's possible to do something akin to what users need to do for gdb? https://sourceware.org/gdb/wiki/PermissionsDarwin

I will in investigate

thisiscam commented 2 years ago

Also @thisiscam actually the issue occurs in Slack <4.22 as well :(

I should be able to fix this one.

@motiwari Could you try the branch https://github.com/thisiscam/math-with-slack/tree/fix_logout_for_old_version? I don't have access to an older Slack client (the SlackNoAutoUpdates trick doesn't work for me for some reason)

thisiscam commented 2 years ago

@thisiscam do you think it's possible to do something akin to what users need to do for gdb? https://sourceware.org/gdb/wiki/PermissionsDarwin

I will in investigate

Oh wow. I think I got this to work. Basically I had to remove the existing code sign, and sign it myself with the same entitlements.

thisiscam commented 2 years ago

Running the scripts here: https://github.com/thisiscam/math-with-slack/tree/master/macos-codesign solve this problem.

These are modified based on the above linked gdb information:

sh macos-codesign/macos-setup-codesign.sh  # Installs a certificate/identity for you to codesigning (only need to be run once)
sh macos-codesign/macos-codesign-mws.sh   # Swaps out Slack's signature by yours
thisiscam commented 2 years ago

Now, one question is if we should include these scripts as part of the main math-with-slack.py. The upside is that we get back a one-liner patch. My main concern, however, is that not everyone would want to the main script to contain a piece of code that might modify the code signatures.

motiwari commented 2 years ago

Nice @thisiscam !! I'm glad this is working again!!

My opinion is that they should be run as part of math-with-slack.py, since they are now a necessity and nothing would work without them. Perhaps prompt the user / provide some sort of interstitial like the following?

You are about create a new certificate to codesign the modified Slack app. Are you sure you want to continue (y/n):

thisiscam commented 2 years ago

since they are now a necessity and nothing would work without them

On my end the scripts works, but it logs me out every time I quick Slack. So the script is working, to some extent.

For some background, Slack might do this log-off thing because:

Now clearly by replacing the code sign, the user opt-in to not use such security feature. My hypothesis is that not everyone will like that.

One thing we can do is to keep an optional opt-in command line flag that performs the code signing if enabled.

thisiscam commented 2 years ago

32 implements optional code signing on MacOS. I will keep it open for now, and users are encouraged to try it.

It would be nice if people can some feedback.

motiwari commented 2 years ago

Thanks @thisiscam ! Yes, you are correct that I informally equated "being signed out every time you quit Slack" with "nothing working at all" -- since I have dozens of Slack instances and re-signing into them every time is infeasible

32 works great -- thanks for the fix! This is critical Slack addition that we can't live without!

thisiscam commented 2 years ago

I merged #32 into master. Closing this for now!

Szepi commented 2 years ago

It seems not everything is working yet:

$ sudo python math-with-slack-fromweb.py --macos-codesign
Using Slack installation at: /Applications/Slack.app/Contents/Resources/app-arm64.asar
Downloading MathJax...100%, 4.5 MB / 4.5 MB, 16380.8 KB/s, 0.3 sec
Generating and installing math-with-slack-codesign certificate
Geneterated new certificate math-with-slack-codesign
/tmp/tmpew2jvo84/slack-entitlements.xml: unrecognized blob type (accepting blindly)
/tmp/tmpew2jvo84/slack-entitlements.xml: invalid length in entitlement blob
Warning: code signing failed. Math-with-slack is likely functional; however, you might experience log-offs if you quit Slack. See github.com/thisiscam/math-with-slack/issues/30 for more info.
Install successful. Please restart Slack.

Am I doing something wrong? The version of Slack I am using is: Version 4.23.0 9895565dda2453a9e5a82b561a75d3af0f471355@1639681045 (Production) My Mac is running Monterey version 12.0.1

motiwari commented 2 years ago

@Szepi could you put your output in a 

code block

with triple backtick to preserve newlines?

Interestingly, my version of Slack 4.22.0 refuses to update to 4.23.0 once math-with-slack is installed:

image

Maybe 4.23.0 broke something. Could you try with Slack 4.22.0?

thisiscam commented 2 years ago

@Szepi I think I know what your error is about. The issue is that Apple is using a new plist format (for the code signing) since Monterey. I will work on this some time soon.

thisiscam commented 2 years ago

@motiwari

Interestingly, my version of Slack 4.22.0 refuses to update to 4.23.0 once math-with-slack is installed:

Unfortunately, this is a side-effect due to custom code-signing. It's not safe for Slack to do hot-patch updates once we've modified its binary. Manually updating (by downloading from Slack website) should work.

Szepi commented 2 years ago

@Szepi could you put your output in a

code block

with triple backtick to preserve newlines?

Interestingly, my version of Slack 4.22.0 refuses to update to 4.23.0 once math-with-slack is installed:

image

Maybe 4.23.0 broke something. Could you try with Slack 4.22.0?

Triple backtip: Done. I think I'll wait for @thisiscam 's patch; given his response it is likely that the problem has nothing to do with slack but the way plists are handled.

thisiscam commented 2 years ago

@Szepi the raised issue should be addressed by #33.

I had to upgrade my system to implement this, and hopefully this didn't break old Mac versions. Feedbacks on old MacOS system versions are welcomed.

Szepi commented 2 years ago

@Szepi the raised issue should be addressed by #33.

I had to upgrade my system to implement this, and hopefully this didn't break old Mac versions. Feedbacks on old MacOS system versions are welcomed.

I downloaded it and tested it. Two things:

  1. After a clean install of Slack, without starting Slack first, I applied the patch. Then I tried starting Slack. This did not work as the app was probably checking for its integrity. The moral is that one perhaps needs to start Slack first, then apply the patch. Maybe the doc/FAQ could mentioned something to this effect.
  2. The next time after a clean install of Slack, I started Slack, logged in into my workspaces. Then I quit Slack, applied the patch. But now I am hitting a wall: I got a dialog box saying that Slack wants to use .. in your keychain. To allow this, enter the "login" keychain password. See the dialog box below. Sadly, I don't recall ever setting up a password for the "login" keychain. So all I can do is to escape this dialog box. The effect is that every time I log in to slack, I have to log into all my workspaces.

image

Also, if I do not apply the patch, slack starts without asking for the keychain "login" password and everything works as normal.

thisiscam commented 2 years ago

After a clean install of Slack, without starting Slack first, I applied the patch. Then I tried starting Slack. This did not work as the app was probably checking for its integrity. The moral is that one perhaps needs to start Slack first, then apply the patch. Maybe the doc/FAQ could mentioned something to this effect.

Yes. This is indeed the behavior since the beginning. I will accept PRs to the doc/FAQ if anyone is willing to contribute. Another option is to try to detect if the Slack App had been opened at least once, if not, warn the user or open it directly.

See the dialog box below. Sadly, I don't recall ever setting up a password for the "login" keychain. So all I can do is to escape this dialog box. The effect is that every time I log in to slack, I have to log into all my workspaces.

This should be the keyword when you do sudo.

FWIW, I will not encourage people to use the --macos-codesign option if they do not understand the consequences. So a golden CAVEAT (hinted by that dialog box) if you ever want to use it :)

Szepi commented 2 years ago

After a clean install of Slack, without starting Slack first, I applied the patch. Then I tried starting Slack. This did not work as the app was probably checking for its integrity. The moral is that one perhaps needs to start Slack first, then apply the patch. Maybe the doc/FAQ could mentioned something to this effect.

Yes. This is indeed the behavior since the beginning. I will accept PRs to the doc/FAQ if anyone is willing to contribute. Another option is to try to detect if the Slack App had been opened at least once, if not, warn the user or open it directly.

I guess this is superior though probably more work:)

See the dialog box below. Sadly, I don't recall ever setting up a password for the "login" keychain. So all I can do is to escape this dialog box. The effect is that every time I log in to slack, I have to log into all my workspaces.

This should be the keyword when you do sudo.

Unfortunately, not.

FWIW, I will not encourage people to use the --macos-codesign option if they do not understand the consequences. So a golden CAVEAT (hinted by that dialog box) if you ever want to use it :)

The problem is that the password is not the same so I am locked off from using the option --macos-codedesign. A quick internet search confirms that it is somewhat common that people just have separate passwords for the keychain service even though they have no recollection of creating a separate password.

thisiscam commented 2 years ago

I guess this is superior though probably more work:)

I might try to see if I can implement that some time today since that actually seems pretty useful.

Unfortunately, not.

Oops, very true, my bad.

A quick internet search confirms that it is somewhat common that people just have separate passwords for the keychain service even though they have no recollection of creating a separate password.

Thanks for noticing this! My google search suggest just resetting the keychain password to sync with the user password if one forgets it... I cannot advise people on these security related decisions, so please use any of those methods with care!

ilan-gold commented 2 years ago

Is there a set of steps one can follow to not be signed out but also use this plugin? I'm a little lost in the discussion here.

thisiscam commented 2 years ago

@ilan-gold If you use the latest master branch, there should be a --macos-codesign option to the script.

ilan-gold commented 2 years ago

I have tried that but unfortunately it fails every time. I have tried a clean install of slack.

dpo commented 2 years ago

Codesigning never succeeds for me either on Monterey.

thisiscam commented 2 years ago

@ilan-gold @dpo What is your error message when it "fails"? Or does it just fail silently (formula does not render)?

thisiscam commented 2 years ago

I'm closing this for now since there is no further information. People should try the lastest master branch since there was a fix of a bug for Apple M1 chips, which was likely the issue for at least some people