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

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
code-nodeadmin
major
enhancement
1.4.1
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
Author
Owner

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?

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. :)

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. :)
tahoe-lafs modified the milestone from 1.5.0 to eventually 2009-06-30 12:40:46 +00:00
davidsarah commented 2009-11-08 05:09:50 +00:00
Author
Owner

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?
davidsarah commented 2009-11-08 05:13:52 +00:00
Author
Owner

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

No, should be `os.geteuid()` (being setuid root is just as bad).
tahoe-lafs modified the milestone from eventually to 1.6.0 2009-11-21 06:32:30 +00:00
tahoe-lafs modified the milestone from 1.6.0 to eventually 2009-12-13 05:33:20 +00:00
tahoe-lafs modified the milestone from eventually to 1.7.0 2010-02-02 02:56:58 +00:00
tahoe-lafs modified the milestone from 1.7.0 to 1.7.1 2010-06-12 20:56:26 +00:00
tahoe-lafs 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
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.

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.
davidsarah commented 2011-05-05 18:17:16 +00:00
Author
Owner

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.

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
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?

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

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.
davidsarah commented 2011-08-18 03:56:20 +00:00
Author
Owner

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

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

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

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.

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.
davidsarah commented 2012-07-31 16:39:51 +00:00
Author
Owner

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

  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?

[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?
tahoe-lafs modified the milestone from soon to 1.10.0 2012-07-31 16:39:51 +00:00
tahoe-lafs modified the milestone from 1.10.0 to soon 2012-09-04 15:47:27 +00:00
davidsarah commented 2012-09-04 15:55:14 +00:00
Author
Owner

The error message needs to be printed to stderr.

The error message needs to be printed to stderr.
Lcstyle commented 2014-09-27 02:22:56 +00:00
Author
Owner

i like making it an error, makes sense for unprivileged users.

i like making it an error, makes sense for unprivileged users.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/trac-2024-07-25#725
No description provided.