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

Better integration with macOS 10.15 Catalina security features #94

Closed
jamesquilty opened this issue May 4, 2020 · 8 comments
Closed

Comments

@jamesquilty
Copy link

In order for Emacs.app to access files when running under macOS 10.15 Catalina, it appears to be necessary to manually grant Full Disk Access permissions to not only Emacs.app but also to /usr/bin/ruby, as detailed at https://spin.atomicobject.com/2019/12/12/fixing-emacs-macos-catalina/. That seems unsafe, as that grants those permissions to any arbitrary Ruby script, not just the launcher code for Emacs. The Ruby launch script appears to require the user to degrade their system security in order to use Emacs.

Would it be possible for there to be better integration between Emacs and the security features of Mac OS 10.15 so this was not necessary?

@caldwell
Copy link
Owner

caldwell commented Apr 5, 2022

I think this should be improved in the latest nightly builds. I re-wrote the launcher in rust so it compiles to a native binary. This should show up and integrate better. I haven't been able to test it because my system never complains about this for some reason I don't understand.

@caldwell caldwell closed this as completed Apr 5, 2022
@dougharris
Copy link

To be clear, this change is not in the version 28.1 released today, right? Just nightlies?

(I downloaded the 28.1 release and it still has the problem)

@caldwell
Copy link
Owner

caldwell commented Apr 5, 2022

Correct. I was hoping to get some feedback from the nightly builds (where any unforseen breakage is mostly ok) before rebuilding the stable version with the change.

Specifically, the nightly build from 2022-04-05 has the (alleged) fix.

@DanLipsitt
Copy link

DanLipsitt commented Apr 5, 2022

Version 29.0.50 installed from the 22-04-05 build using brew install emacs-nightly solves the issue for me.
Looking forward to having this in the standard build!

@johnhawkinson
Copy link

Correct. I was hoping to get some feedback from the nightly builds (where any unforseen breakage is mostly ok) before rebuilding the stable version with the change.

@caldwell Is this the right forum for such a discussion? I'm loathe to open a new Issue for discussion; I also considered a comment on bfe57e0 but this seems like it's a bit better.

First of all, thanks for porting the Ruby launcher to Rust. This definitely seems like the right way to go!

I'm not a Rust developer, so please bear with me, but I'm concerned that the above change seems to invoke the login shell (usually /bin/zsh) as sh --login -c $launcher with EMACS_LAUNCHER_PLEASE_DUMP_YOUR_ENV=1 for the purpose of dumping the login shell's environment, and then imports it into Emacs.

That does not seem correct behavior if I run emacs -nw from a Terminal window. Emacs should not go through crazy machinations to figure out the environment it is not invoked from. Maybe if I was running M-x shell (I don't recall how it initializes its environment), but not typically. It should inherit the parent environment.

I can understand why you might want to do this when Emacs is invoked from launchd or Dock or whatever the Finder mechanism is. If that's the case, I'm not sure this is the right mechanism to do so, but maybe it is. But maybe not? Historically I have mucked about with /etc/paths and /etc/paths.d and somehow it never all worked right and for a while I used (setq exec-path …) in .emacs until that stopped working with some OS upgrade and then shifted to (setenv PATH …) in .emacs and honestly that never felt great, so maybe this comment is off-base. If we can avoid people suffering and getting confused, that's helpful.

So anyhow…this code has me wary. I feel like it will lead to subtle problems and a lot of corner-cases and most users won't notice it (until they do), so I wanted to raise it now, early in the process.

Also, I tend to think that this is a very strange practice to use the environment to alter the behavior of the launcher, and that it would be better to use a command-line argument, i.e. $launcher --dump-environment. That's the model we normally use for this kind of thing, and I'm not sure there is good reason to deviate from it. (On the other hand, then there would be a command-line argument that was not enumerated when you ran emacs --help which isn't great, but the environment variable just hides it in another way.)

Also, do I read the Rust code properly that it sets argv[0] to sh? That doesn't seem like it is correct, although I doubt it matters?

Thanks.

@caldwell
Copy link
Owner

caldwell commented Apr 6, 2022

That does not seem correct behavior if I run emacs -nw from a Terminal window. Emacs should not go through crazy machinations to figure out the environment it is not invoked from. Maybe if I was running M-x shell (I don't recall how it initializes its environment), but not typically. It should inherit the parent environment.

I agree. That is an oversight on my part—too fixated on "my way" of launching emacs—I use emacsclient from the cli. I think a simple enough change is to detect when an app is launched from finder and only do the env stealing there. I'll try to do this before tonight's nightly gets built.

I can understand why you might want to do this when Emacs is invoked from launchd or Dock or whatever the Finder mechanism is. If that's the case, I'm not sure this is the right mechanism to do so, but maybe it is. But maybe not?

The problem is that Apple has been changing and restricting what env vars can be set for all processes over time. I've gone through all the steps you listed. At the moment I think the only way is to replicate everything in emacs lisp, or some other lispy solution but those all feel like terrible hacks. This is also terrible but slightly less so, I think—I think a programmer's editor inheriting your .bashrc nonsense nonsense automatically is what most people expect (there have been a lot of bug reports and emails over the years asking me about this).

Honestly I've had the equivalent changes for the old Ruby launcher sitting around uncommitted in my local dev directory for literally years because I have the same basic misgivings as you. I finally gave up and just did it. I think it'll be ok.

Also, I tend to think that this is a very strange practice to use the environment to alter the behavior of the launcher, and that it would be better to use a command-line argument, i.e. $launcher --dump-environment.

I thought about that of course, but the env var was easier to implement, only shows up in one place, and is completely internal.

Also, do I read the Rust code properly that it sets argv[0] to sh? That doesn't seem like it is correct, although I doubt it matters?

Do you mean in this line?

   let env_raw = Command::new(std::env::var_os("SHELL").unwrap_or(osstr("sh"))).args([osstr("--login"), osstr("-c"), std::env::current_exe()?.into_os_string()])

In that line it's using "sh" as a fallback only if the SHELL env var isn't set (which I don't think can actually ever happen).

Thanks

@johnhawkinson
Copy link

Thanks. A fix to detect the Finder sounds pretty good. I can't shake the feeling that something has gone horribly wrong that this is necessary, but I don't have anything better to offer, and I'm not suggesting it's your fault.

Do you mean in this line?

In that line it's using "sh" as a fallback only if the SHELL env var isn't set (which I don't think can actually ever happen).

I did, and thanks for clarifying. It can certainly happen! It's not crazy-rare for people to run things like env - /path/to/Emacs-or-something-else, almost always when debugging something. It's good that absence of a $SHELL doesn't make Emacs go do something crazy since it's easy to see how that might turn a long painful debugging session into something doubly long and painful for no good reason. Although I then wonder if the fallback shouldn't be /bin/sh rather than sh.

As a footnote, I got around to downloading the nightly binary (it shrunk from 85MB to 68MB, did something go wrong?), and I definitely did not expect this degree of obfuscation:

jhawk@lrr /Applications % env - AAA=A EMACS_LAUNCHER_PLEASE_DUMP_YOUR_ENV=1 ./Emacs-2022-04-05_14-51-13-575c3beb4c001687ce7a4581de005a16d6f2e081-universal.app/Contents/MacOS/Emacs
[[{"Unix":[65,65,65]},{"Unix":[65]}],[{"Unix":[69,77,65,67,83,95,76,65,85,78,67,72,69,82,95,80,76,69,65,83,69,95,68,85,77,80,95,89,79,85,82,95,69,78,86]},{"Unix":[49]}],[{"Unix":[95,95,67,70,95,85,83,69,82,95,84,69,88,84,95,69,78,67,79,68,73,78,71]},{"Unix":[48,120,49,70,53,58,48,120,48,58,48,120,48]}]]
jhawk@lrr /Applications % 

I'm not saying this is a real problem, but…I can imagine someone hating you in the future for it.

@caldwell
Copy link
Owner

caldwell commented Apr 6, 2022

It can certainly happen! It's not crazy-rare for people to run things like env - /path/to/Emacs-or-something-else, almost always when debugging something.

True, but I was already thinking in terms of detecting being opened from Finder and I don't think it's possible there. It certainly seems very difficult to get Finder to do that on purpose.

[The nightly binary] shrunk from 85MB to 68MB, did something go wrong?

No, that's the result of switching the .dmg format back to HFS+. I had to switch away because Big Sur's hdiutil was flat out broken and threw errors when trying to create HFS+ images.

{"Unix":[69,77,65,67,83,95,76,65,85,78 …

I hate that too—that's Rust being picky since technically there's no guarantee that the values aren't almost pure random binary (aside from 0x00 of course) and so it serializes it in a way that's guaranteed to be unscathed when deserialized. Part of me really wanted to add a 2nd case where if it was valid utf8 it would just encode as a string, but the other part of me didn't want to bloat the code just for a format that no one will ever see…

caldwell added a commit that referenced this issue Apr 19, 2022
…nched from Finder

We detect this by checking the parent process id, which is 1 when launched via launch services.

Thanks to johnhawkinson for pointing this out here: #94 (comment)
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