We should whine if we're running as root. #725

New issue

Open

opened 2009-06-01 07:07:27 +00:00 by zandr · 19 comments

zandr commented

2009-06-01 07:07:27 +00:00

Owner

Just spent some time unravelling a problem at ScanCafe where nodes were refusing to start because they couldn't open storage/shares/incoming.

It turns out that directory was owned by root. I'm guessing that someone tried to run the nodes as root, thus hosing things up. Not sure why that was the only thing affected, but it seemed to be.

We should complain if we think we're running as root, possibly with a commandline switch or config entry to force it... or possibly not. :)

Just spent some time unravelling a problem at [ScanCafe](wiki/ScanCafe) where nodes were refusing to start because they couldn't open storage/shares/incoming. It turns out that directory was owned by root. I'm guessing that someone tried to run the nodes as root, thus hosing things up. Not sure why that was the only thing affected, but it seemed to be. We should complain if we think we're running as root, possibly with a commandline switch or config entry to force it... or possibly not. :)

tahoe-lafs added the

labels 2009-06-01 07:07:27 +00:00

tahoe-lafs added this to the 1.5.0 milestone 2009-06-01 07:07:27 +00:00

zooko commented

2009-06-01 14:45:45 +00:00

Good idea. Also, should storage servers be more loud and explicit about the fact that they are having a problem with "storage/shares/incoming", or are they already good on that?

zandr commented

2009-06-01 19:32:41 +00:00

Author

Owner

No, that error is pretty explicit. The slient failure that annoys me is if we can't bind to one of our many ports (manhole seems to get me most often). That's a different, ticket, though. :)

zooko modified the milestone from 1.5.0 to eventually

2009-06-30 12:40:46 +00:00

daira commented

2009-11-08 05:09:50 +00:00

So what is the criterion for runnning as root? os.getuid() == 0 and does not throw an exception?

So what is the criterion for runnning as root? `os.getuid() == 0` and does not throw an exception?

daira commented

2009-11-08 05:13:52 +00:00

No, should be os.geteuid() (being setuid root is just as bad).

No, should be `os.geteuid()` (being setuid root is just as bad).

daira modified the milestone from eventually to 1.6.0

2009-11-21 06:32:30 +00:00

zooko modified the milestone from 1.6.0 to eventually

2009-12-13 05:33:20 +00:00

daira modified the milestone from eventually to 1.7.0

2010-02-02 02:56:58 +00:00

daira self-assigned this 2010-02-02 02:56:58 +00:00

daira modified the milestone from 1.7.0 to 1.7.1

2010-06-12 20:56:26 +00:00

daira modified the milestone from 1.7.1 to soon

2010-07-17 03:53:00 +00:00

mcoppola commented

2011-05-04 07:17:51 +00:00

Author

Owner

Attachment 725.patch (1093 bytes) added

**Attachment** 725.patch (1093 bytes) added

725.patch

1.1 KiB

mcoppola commented

2011-05-04 07:18:54 +00:00

Author

Owner

I think that alerting the user about running as root should instead be addressed as a security concern. It makes sense that initially running as root would mess with the file permissions, but I believe the issue is generalized to first running as X user and then as Y user, not just root. Anyways, here is the patch I wrote.

daira commented

2011-05-05 18:17:16 +00:00

os.geteuid isn't available in native-Windows Python:

Python 2.7.1 (r271:86832, Nov 27 2010, 18:30:46) [MSC v.1500 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.geteuid()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'geteuid'

Perhaps "if hasattr(os, 'geteuid') and os.geteuid() == 0:".

In principle you also shouldn't run as Administrator on Windows, but that's probably too common (and Windows security is beyond fixing anyway) to be worth warning about.

`os.geteuid` isn't available in native-Windows Python: ``` Python 2.7.1 (r271:86832, Nov 27 2010, 18:30:46) [MSC v.1500 32 bit (Intel)] on win32 Type "help", "copyright", "credits" or "license" for more information. >>> import os >>> os.geteuid() Traceback (most recent call last): File "<stdin>", line 1, in <module> AttributeError: 'module' object has no attribute 'geteuid' ``` Perhaps "`if hasattr(os, 'geteuid') and os.geteuid() == 0:`". In principle you also shouldn't run as Administrator on Windows, but that's probably too common (and Windows security is beyond fixing anyway) to be worth warning about.

mcoppola commented

2011-05-05 18:50:27 +00:00

Author

Owner

That's a very good suggestion - I forgot to consider if the user is running on a Windows system. I also agree with not warning the user on Windows systems. I've found that non-Administrator users still have quite enough access to cause significant harm anyways, so it's probably a moot point to restrict it. I updated the patch.

mcoppola commented

2011-05-05 18:57:20 +00:00

Author

Owner

Attachment 725.2.2.patch (1092 bytes) added

Updated patch, considers Windows users

**Attachment** 725.2.2.patch (1092 bytes) added Updated patch, considers Windows users

725.2.2.patch

1.1 KiB

mcoppola commented

2011-05-10 05:55:24 +00:00

Author

Owner

I'm in the process of writing tests, and zooko and I came across the question of whether or not the '--quiet' flag should suppress the "running as root" error. It should also be known that the error is going to be printed out to STDERR, not STDOUT. What do you guys think?

zooko commented

2011-05-10 20:38:05 +00:00

Hi! I think that the --quiet flag should suppress the warning, and also that the warning should go to stderr, not stdout. Want more help writing tests? Let's get together on IRC for that.

Hi! I think that the `--quiet` flag should suppress the warning, and also that the warning should go to `stderr`, not `stdout`. Want more help writing tests? Let's get together on IRC for that.

daira commented

2011-08-18 03:56:20 +00:00

We also need a test that mocks os.geteuid to return 0, and checks that the warning is printed (also, that it is not printed when --quiet is used).

We also need a test that mocks os.geteuid to return 0, and checks that the warning is printed (also, that it is not printed when `--quiet` is used).

zooko commented

2011-08-18 20:43:03 +00:00

Hey mcoppola: still interested? If you don't indicate interest on this ticket soon, I'll feel free to let someone else do it.

mcoppola commented

2011-08-19 17:33:21 +00:00

Author

Owner

Hey, sorry about the lapse. Thanks for giving me a kick in the behind. I'll try to finish it up.

EDIT: Some guidance with how to navigate and add units to the testing utility in Tahoe would be appreciated, though.

Hey, sorry about the lapse. Thanks for giving me a kick in the behind. I'll try to finish it up. EDIT: Some guidance with how to navigate and add units to the testing utility in Tahoe would be appreciated, though.

zooko commented

2011-08-19 18:41:58 +00:00

mcoppola: great!

Let's get together on IRC sometime to walk you through how to write these tests. In the meantime, you can see other tests which do similar things, such as [test_error_on_old_config_files]source:trunk/src/allmydata/test/test_client.py?rev=5143#L33.

mcoppola: great! Let's get together on IRC sometime to walk you through how to write these tests. In the meantime, you can see other tests which do similar things, such as [test_error_on_old_config_files]source:trunk/src/allmydata/test/test_client.py?rev=5143#L33.

mcoppola commented

2011-09-01 06:34:29 +00:00

Author

Owner

Looks like I'm busier than I thought. I'd still love to help out with the project, but for the sake of this bug someone else should take care of it. Sorry 'bout that.

daira commented

2012-07-31 16:39:51 +00:00

On tahoe-dev, davidsarah wrote:

On 31/07/12 07:59, Two Spirit wrote:

And people do what they are expected to do? I can't speak for the rest of the world, but
yea, I guess there are a lot of "users" like myself who run as root and have no clue what
we are doing. My experience with file systems is that you have to run as root for any
file system stuff. I'm sure there are a lot of people who share my background.

My idea was a one sentance, standard WARNING disclaimer indicating

this should be done as a non-root user or

this doesn't need to be done as root
somewhere in the running.rst maybe before the first command 'To construct a client node,
run "tahoe create-client"....'

"We should whine if we're running as root."
https://tahoe-lafs.org/trac/tahoe-lafs/ticket/725

There's a patch, and I see the ticket is assigned to me; it just needs tests.
I'll put it in the 1.10 milestone.
What would your idea of said short warning look like?
The one in the current patch says:

###############################################################
WARNING: You should not be running Tahoe-LAFS as root!
This poses an unnecessary security risk and is NOT recommended.
###############################################################

There's an argument for saying that this shouldn't just be a warning; it should
be an error, because running as root once may already do things that need to be
undone (e.g. creating files owned by root, as in the case that motivated the ticket).
If we made it an error then we could add an --allow-root option to suppress it;
is that necessary, or overcomplicated?

[On tahoe-dev](https://tahoe-lafs.org/pipermail/tahoe-dev/2012-July/007613.html), davidsarah wrote: > On 31/07/12 07:59, Two Spirit wrote: > > And people do what they are expected to do? I can't speak for the rest of the world, but > > yea, I guess there are a lot of "users" like myself who run as root and have no clue what > > we are doing. My experience with file systems is that you have to run as root for any > > file system stuff. I'm sure there are a lot of people who share my background. > > > > My idea was a one sentance, standard WARNING disclaimer indicating > > 1) this should be done as a non-root user or > > 2) this doesn't need to be done as root > > somewhere in the running.rst maybe before the first command 'To construct a client node, > > run "tahoe create-client"....' > > "We should whine if we're running as root." > <https://tahoe-lafs.org/trac/tahoe-lafs/ticket/725> > > There's a patch, and I see the ticket is assigned to me; it just needs tests. > I'll put it in the 1.10 milestone. > > > What would your idea of said short warning look like? > > The one in the current patch says: ``` ############################################################### WARNING: You should not be running Tahoe-LAFS as root! This poses an unnecessary security risk and is NOT recommended. ############################################################### ``` > There's an argument for saying that this shouldn't just be a warning; it should > be an error, because running as root once may already do things that need to be > undone (e.g. creating files owned by root, as in the case that motivated the ticket). > If we made it an error then we could add an `--allow-root` option to suppress it; > is that necessary, or overcomplicated?