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

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

Closed
thisiscam opened this issue Nov 14, 2021 · 30 comments
Closed

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

thisiscam opened this issue Nov 14, 2021 · 30 comments

Comments

@thisiscam
Copy link
Owner

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
Copy link
Owner Author

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
Copy link

@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
Copy link

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

@thisiscam
Copy link
Owner Author

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

I should be able to fix this one.

@thisiscam
Copy link
Owner Author

@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
Copy link
Owner Author

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
Copy link
Owner Author

@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
Copy link
Owner Author

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
Copy link
Owner Author

thisiscam commented Nov 22, 2021

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
Copy link

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
Copy link
Owner Author

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:

  • in theory, there's no way to prevent code injection like our script if the script has sudo access
  • but code signing at least protects against threats coming lower than the Keychain level -- the effect is that if the binary is modified, the user must re-login manually to avoid the possibility of a malicious software directly reading off the old credentials.
  • I'm not sure what I described above is done intentionally by the Slack team, but this does seem to be a pretty nice security feature.

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
Copy link
Owner Author

#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
Copy link

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
Copy link
Owner Author

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

@Szepi
Copy link

Szepi commented Dec 16, 2021

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
Copy link

motiwari commented Dec 16, 2021

@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
Copy link
Owner Author

@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
Copy link
Owner Author

thisiscam commented Dec 16, 2021

@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.

@thisiscam thisiscam reopened this Dec 17, 2021
@Szepi
Copy link

Szepi commented Dec 17, 2021

@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
Copy link
Owner Author

thisiscam commented Dec 17, 2021

@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
Copy link

Szepi commented Dec 18, 2021

@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
Copy link
Owner Author

thisiscam commented Dec 18, 2021

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
Copy link

Szepi commented Dec 19, 2021

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
Copy link
Owner Author

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
Copy link

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
Copy link
Owner Author

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

@ilan-gold
Copy link

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

@dpo
Copy link

dpo commented Feb 27, 2022

Codesigning never succeeds for me either on Monterey.

@thisiscam
Copy link
Owner Author

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

@thisiscam
Copy link
Owner Author

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

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

No branches or pull requests

5 participants