mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.33k stars 1.53k forks source link

'ninja install' attempts to gain elevated privileges #7345

Closed dankegel closed 1 year ago

dankegel commented 3 years ago

It is disturbing for 'ninja install' to prompt for a sudo password with polkit.

This breaks user expectations, and introduces a security risk.

This has evidently been meson's behavior for a very long time, but it's distressing, and it's surprising because meson is usually so keen on simplicity; the last thing I expect when invoking a build tool is for it to take over the screen and request a password.

(It would be equally distressing for 'sudo ninja install' to clutter the local directory with root-owned files.)

dreamer-coding-555 commented 3 years ago

What system are you using? Asking because it helps use determine how to fix this. Also would help to say what version of Meson and Ninja you’re using.

dankegel commented 3 years ago

This is on Linux (ubuntu 20.04), and using meson from either the system or up-to-date git.

It's not a bug; it's a feature. It just happens to be a feature that gives me the shivers.

Here's an example meson.build to reproduce with:

project('blah', 'c')
configure_file(
   input: 'foo.h.in',
   output: 'foo.h',
   configuration : {},
   install_dir: 'include',
   install: true)

Create a foo.h.in with anything in it, then do

$ meson . btmp
$ cd btmp
$ ninja install

This will trigger the prompt for a sudo password.

dreamer-coding-555 commented 3 years ago

The sudo command is used to temporarily give members of the sudo group privileges similar to the one root users have. When setting up Ubuntu you are asked to provide a password to proceed with installation. That is the password needed here.

Note it won't be seen on the terminal when typing as this prevents any one around from looking over your shoulder and seeing it.

That is the password that will be asked of you whenever you want to carry out any tasks that required elevated user privileges, like in this particular case.

It’s possible to disable sudo but I don’t recommend doing it.

dankegel commented 3 years ago

That is all true, and I knew that already.

The bug I am reporting is a design bug; build utilities have traditionally never attempted to gain elevated privileges, and it is highly surprising that meson does so.

eli-schwartz commented 3 years ago

@michaelbadcrumble This issue was opened with the rationale "asking for sudo is a disturbing design and security violation because build tools should not ask for elevated privileges". It's very out of place to try explaining that sudo is a tool to gain elevated privileges, because that was never in doubt...

...

This was added in #3567 with the rationale:

dankegel commented 3 years ago

Yes, it solves a real problem (sudo ninja install accidentally touching files in build directory).

But alternate solutions (e.g. abort rather than build if something needs building during install, or dropping privs while building) are possible.

nirbheek commented 3 years ago

We should probably just print what we need elevated privileges for. Then the user can make a decision about whether to provide it or not.

orbea commented 3 years ago

We should probably just print what we need elevated privileges for.

Why not just let to fall back to "Permission denied" when it fails to install files without a the right permissions? Anyone that knows how to use meson should be able to figure out what that means.

dankegel commented 3 years ago

It's hard to come up with something both secure and convenient...

smurfix commented 3 years ago

There is a simple solution to this. Don't elevate privileges. If the user wants root, tell them to use "sudo meson …". This is very easy to implement and it's what every other build system does.

EDIT: Workaround for all of this: export PKEXEC_UID=99999. Meson will now raise a permission error instead of trying to grab elevated privileges.

eli-schwartz commented 3 years ago

I recently helped debug someone's distro build script using meson install. By accident, DESTDIR was not set. Instead of either visibly trying pkexec or failing with normal permission errors, meson simply crashed because LD_PRELOAD was automatically exported by fakeroot, which the entire distro build script runs under, and hence pkexec could not be executed at all.

That was a pretty annoying and silly thing to debug. It would have been a whole lot clearer to just get a permission error. :p

Volker-Weissmann commented 3 years ago

Similar problem here. On my machine, sudo does not ask for a password if its executed from my user account. I was certainly surprised when it printed Installing *.so to /usr/local/lib.

Good things the libraries have a timestamp, so that I can see which one was just installed and delete them.

If there were some libraries in this project that are called the same like an existing library, I would have accidentally broken my machine.

IMHO, an explicit --try-to-grab-root-rights flag would be ok, but meson install just taking root rights is very unexpected.

dabrain34 commented 2 years ago

meson is not taking any root rights without user's permission but user, through pkexec, is, when the user let meson manage the DESTDIR, /usr/local.

pkexec's popup allows you to cancel the privilege grant anytime but sudo is keeping on until a timeout where you can execute root command without entering the password again.

For me the pkexec is safer and give more control to the user including newbies.

dabrain34 commented 2 years ago

polkit is an issue in a non X env such as a docker image setup with a user

eli-schwartz commented 2 years ago

This ticket is not discussing whether polkit or sudo is more secure or user friendly than the other.

It is discussing whether "automatically try a privilege elevation command of any sort" is a good/bad idea.

dabrain34 commented 2 years ago

To answer your question I think it's a good idea to grant pivilege but using polkit limits to a graphical environment.

dnicolodi commented 2 years ago

I think it is very unexpected too. I haven't looked at the code (and I didn't see this behavior mentioned int he documentation, but I didn't go looking for it) but how does meson decide whether it needs elevated privileges? I can imagine situations where the current user is not root yet it can write to the destination directory just fine.

smurfix commented 2 years ago

IMHO an install command is supposed to be started with privileges appropriate for installing to DESTDIR, not try to grab them itself. This is how most if not all other equivalents of "make install" work; the Principle of Least Surprise should apply to meson too.

I can imagine situations where the current user is not root yet it can write to the destination directory just fine.

Exactly. Worse, the way some home directories and/or test environments are set up I can easily imagine an install to fail because it's done by root.

dabrain34 commented 2 years ago

The pkexec is used only if a permission error is detected by meson (if DESTDIR is a safe user place, no pkexec). In any case the sudoer's password is requested.

This solution avoids ninja install to build missing dependency (file modified and ninja install called to fast the process) as a sudoers and create root files in the build folder.

We could use sudo -k which is always requesting the sudoer password and is console compatible.

Volker-Weissmann commented 2 years ago

In any case the sudoer's password is requested.

If sudo is configured to work without password, there is no password request, even with the -k option

xclaesse commented 2 years ago

If sudo is configured to work without password

Don't do that. And if you do, I don't see how it's meson problem's if you shoot yourself in the feet...

polkit is an issue in a non X env such as a docker image setup with a user To answer your question I think it's a good idea to grant pivilege but using polkit limits to a graphical environment.

Why is it a problem? It will just fail, but I guess meson would raise a backtrace instead of printing a nice error message.

pkexec true
Error getting authority: Error initializing authority: Could not connect: No such file or directory

I can easily imagine an install to fail because it's done by root.

Actually no, because the pkexec case is run ONLY in a case it would have failed anyway. If you expected the install to work without priviledge and that prompt popup, just cancel it and fix your setup.

I can imagine situations where the current user is not root yet it can write to the destination directory just fine.

I'm doing that all the time, meson won't use pkexec in that case.

The bug I am reporting is a design bug; build utilities have traditionally never attempted to gain elevated privileges, and it is highly surprising that meson does so.

I would not take traditional build utilities as example, they all sucks, that's why we have meson in the first place... It is surprising only because it's a new thing to lots of people, but it's not unique neither. systemctl command line does it too, I think more and more CLI does it because it's a nicer user experience. The more CLI does it, the less surprising it will become.

eli-schwartz commented 2 years ago

I recently helped debug someone's distro build script using meson install. By accident, DESTDIR was not set. Instead of either visibly trying pkexec or failing with normal permission errors, meson simply crashed because LD_PRELOAD was automatically exported by fakeroot, which the entire distro build script runs under, and hence pkexec could not be executed at all.

That was a pretty annoying and silly thing to debug. It would have been a whole lot clearer to just get a permission error. :p

eli-schwartz commented 2 years ago

"it's a nice user experience, the more CLI tools do it the more people will expect it".

That was never really my argument.

No, it's a bad user experience, I don't want meson to emulate other build tools because other build tools don't do it -- I want meson to not do it, because it's a bad thing to do, and "just for the record everyone else realized it's a bad idea, too".

dabrain34 commented 2 years ago

"it's a nice user experience, the more CLI tools do it the more people will expect it".

No, it's a bad user experience, I don't want meson to emulate other build tools because other build tools don't do it -- I want meson to not do it, because it's a bad thing to do, and "just for the record everyone else realized it's a bad idea, too".

Could you please elaborate a bit more about because it's a bad thing to do ?

xclaesse commented 2 years ago

I recently helped debug someone's distro build script using meson install. By accident, DESTDIR was not set. Instead of either visibly trying pkexec or failing with normal permission errors, meson simply crashed because LD_PRELOAD was automatically exported by fakeroot, which the entire distro build script runs under, and hence pkexec could not be executed at all. That was a pretty annoying and silly thing to debug. It would have been a whole lot clearer to just get a permission error. :p

That seems a very specific case and easy to fix. We can detect if we are in fakeroot and not use pkexec: https://stackoverflow.com/questions/33446353/bash-check-if-user-is-root

eli-schwartz commented 2 years ago

You have taken my objection "this is a fragile pile of 💩 and fails in surprising ways whenever you step out of the narrow-minded box of the polkit developers" and extracted from that, that "hey, we can make this a special case, everything will be fine". :p

The essential problem here is that under a number of circumstances meson decides it needs root permission, and it's wrong because I actually wanted meson to error out with permission errors and I absolutely under no circumstances want it to be successful, I'd rather fix meson.build or the configure options. And meson gives no insight into why it's automatically doing this nonsense.

eli-schwartz commented 2 years ago

Also polkit was a horrible choice if anything should be done :p it pops up a GUI window when running cli commands. I'm extremely annoyed at systemctl for the same reason, I assure you.

mensinda commented 2 years ago

The current situation might not be perfect, but most people will then just call doas/sudo ninja install which is really bad because you can easily end up with files owned by root in your build directory. In this case, there is not much Meson can do (dropping privileges, etc.) because ninja is in charge of compiling at this point.

So, the solution here is to use the least privilege principle. The easiest way of achieving this is to only elevate privileges when needed.

Whether polkit is the right tool (I am also not a fan of GUI popups in my CLI), stability, bugs, etc. is a different question.

dnicolodi commented 2 years ago

I think that ninja install requiring to create files in the build directory is a bug in Meson or a deficiency of the build definition. Fixing this may be harder than wrapping the "real" install part of ninja install into a wrapper that elevates privileges, but IMHO is the correct solution.

eli-schwartz commented 2 years ago

Whether polkit is the right tool (I am also not a fan of GUI popups in my CLI), stability, bugs, etc. is a different question.

Indeed, and that's why I think polkit prompts don't solve the problem.

The current state of affairs:

The previous state of affairs:

The "solution" merged doesn't solve anything, it merely shifts the brokenness one step over to the left. And people building stuff on the command line are statistically speaking more likely than any other linux users to simply... not have polkit installed at all (but they probably do have sudo or doas).

These people are still getting root owned files.

I guess in the year 2018 I'd object fiercely to #3567 being merged on the grounds "this solves nothing". I still believe it should be reverted, but I do understand it is harder to convince other people to move back from one brokenness to the original brokenness because I think the original brokenness is less bad.

Inertia, "let's just do nothing if we cannot solve it properly", cuts both ways.

xclaesse commented 2 years ago

The current state of affairs:

* meson is broken

What's broken? If it's just the fakeroot case, it's easy to fix without throwing the baby with the bathwater.

xclaesse commented 2 years ago

And people building stuff on the command line are statistically speaking more likely than any other linux users to simply... not have polkit installed at all

[citation needed]

Not sure who does not have pkexec installed, is there any distro not having it by default? And if you don't have it then it's all good for you, it will just not use it and you'll get your permission denied error.

Volker-Weissmann commented 2 years ago

If sudo is configured to work without password

Don't do that. And if you do, I don't see how it's meson problem's if you shoot yourself in the feet...

Why shouldn't I do that? The password prompt is just security theater, if a malicious program has user rights it can just run

echo "alias sudo=keyloggedsudo" >> ~/.bashrc 
alegra8611 commented 2 years ago

There is a simple solution to this. Don't elevate privileges. If the user wants root, tell them to use "sudo meson …". This is very easy to implement and it's what every other build system does.

EDIT: Workaround for all of this: export PKEXEC_UID=99999. Meson will now raise a permission error instead of trying to grab elevated privileges.

This was a good work around for me as I was building with meson on an NFS

richfelker commented 2 years ago

Just here to chime in that this is wrong and should be removed. The proper way to do this is not prompting for privilege elevation (that's always wrong and trains users to get phished) nor sudo ninja install (which as noted can give root-owned files in build directory) but performing an unprivileged install into a DESTDIR then manually elevating privileges to copy the contents of the staged install to the main filesystem after confirming it's not writing places you don't want it to.

Volker-Weissmann commented 2 years ago

and trains users to get phished

A program with user privileges can run

echo "alias sudo=keyloggedsudo" >> ~/.bashrc 

So that argument is invalid.

richfelker commented 2 years ago

You're missing the point which is about the kind of reaction it trains, not the particular privilege domains involved here. It's training the pattern "something asks for credentials and presents a prompt" as valid, which is how phishing works.

Volker-Weissmann commented 2 years ago

Yeah ok. To be fair, phishing is usually browser based and not console based.

dabrain34 commented 2 years ago

You're missing the point which is about the kind of reaction it trains, not the particular privilege domains involved here. It's training the pattern "something asks for credentials and presents a prompt" as valid, which is how phishing works.

IMHO it's user's problem. If It asks for privilege, we should respect people's intelligence to be responsible of putting their credentials.

smurfix commented 2 years ago

There are better mechanisms (cf. DESTDIR) than asking for privileges in the first place.

Respecting people's intelligence is not the point. You can train perfectly intelligent people to habitually ack away any and all auth-request dialog windows if you annoy them enough with them.

The point is motor memory, unexpected interactions, and anti-patterns (e.g. an install method that behaves differently when started non-interactively).

dabrain34 commented 2 years ago

The point here is to install by default your software (no DESTDIR) on most linux distro dest dir, /usr/local which is a system restricted area.

So user needs to grant privilege to install his library (he chose to do it). Train him to use sudo ninja install sounds a bit the same as training him to enter the password in pkexec.

And it avoids to generate files which will be root owned by a rebuild of a modified source file.

mensinda commented 2 years ago

@Volker-Weissmann I would also argue that if you have a malicious user program with access to your home directory that you have far bigger problems to worry about (like it encrypting or uploading everything it has access to, keylogging your entire desktop https://github.com/anko/xkbcat).

Volker-Weissmann commented 2 years ago

@Volker-Weissmann I would also argue that if you have a malicious user program with access to your home directory that you have far bigger problems to worry about (like it encrypting or uploading everything it has access to, keylogging your entire desktop https://github.com/anko/xkbcat).

Yes, of course, but keylogging everything doesn't mean you could not also keylog sudo. Even if your data is only readible/writable as root, that won't save you.

xclaesse commented 2 years ago

It's training the pattern "something asks for credentials and presents a prompt" as valid, which is how phishing works.

Right, we should stop training people to prepend "sudo" to all commands. Better only request elevated permission only if needed.

eli-schwartz commented 1 year ago

I guess in the year 2018 I'd object fiercely to #3567 being merged on the grounds "this solves nothing". I still believe it should be reverted, but I do understand it is harder to convince other people to move back from one brokenness to the original brokenness because I think the original brokenness is less bad.

A couple years later, I return to this topic, this time with patches and the confidence of a commit bit. :stuck_out_tongue_winking_eye:

I'm still not a fan of automatically doing this, but that code exists and presumably people want it (???) and it's hard to argue against that on principled grounds (though if pressed I can make a good try at it), so I took the path of least resistance and kept that code but made it better. Hopefully this makes everyone happy. It certainly makes me happy -- now it will never ever ever run things I don't expect it to, and the worst case scenario is it hangs for 30 seconds and then reports the original permission error.

Now: