Bug#393283: RFC: change chown *not* to look up numeric user/group names

2007-01-19 Thread Jim Meyering
Jim Meyering <[EMAIL PROTECTED]> wrote:
...
> I propose to change GNU chown to perform that look-up of an all-numeric
> "user" or "group" string only when the POSIXLY_CORRECT envvar is set.
> Otherwise, (when POSIXLY_CORRECT is not set and a "name" is a valid user
> ID or group ID), chown would use the value obtained from converting the
> string with a function like strtoul.
>
> For consistency, the same policy would apply to chgrp.

Here's a postscript to the discussion that started above, aka,

http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/8682

Today I realized that there is a way to avoid the look-up
without resorting to a new option.
The trick is to realize that in commands like the following

   chown +0:+0 file
   chgrp +0 file

the getpwnam and getgrnam look-up operations must always
fail, since "+" is not a valid character in a user name.
Hence, chown and chgrp can skip those function calls when
the first byte is "+".  chown and chgrp have always accepted
a leading sign on those arguments.

So if you want to avoid the expense of what you deem to be a pointless
name-to-ID look-up of "0" (or any numeric value), just prepend a "+" --
if you're using the patch below.

Here's a demo:

The old way:

# strace -e open,chown ./chown 0:0 /t/Z
open("/etc/ld.so.cache", O_RDONLY)  = 3
open("/lib/libc.so.6", O_RDONLY)= 3
open("/etc/nsswitch.conf", O_RDONLY)= 3
open("/etc/ld.so.cache", O_RDONLY)  = 3
open("/lib/libnss_compat.so.2", O_RDONLY) = 3
open("/lib/libnsl.so.1", O_RDONLY)  = 3
open("/etc/ld.so.cache", O_RDONLY)  = 3
open("/lib/libnss_nis.so.2", O_RDONLY)  = 3
open("/lib/libnss_files.so.2", O_RDONLY) = 3
open("/etc/passwd", O_RDONLY)   = 3
open("/etc/group", O_RDONLY)= 3
open(".", O_RDONLY) = 3
chown("/t/Z", 0, 0) = 0
Process 17271 detached

New way: no look-up:

# strace -e open,chown ./chown +0:+0 /t/Z
open("/etc/ld.so.cache", O_RDONLY)  = 3
open("/lib/libc.so.6", O_RDONLY)= 3
open(".", O_RDONLY) = 3
chown("/t/Z", 0, 0) = 0
Process 17234 detached

However, there is a drawback: this syntax is not portable.
For example, Solaris 10's chgrp and chown reject it, e.g.,

$ /bin/chgrp +0 k
chgrp: unknown group: +0
[Exit 2]

Well, seeing that, I'm less enthusiastic about this change.
I'll sleep on it.  If I do apply it, I'll also document that
GNU chown and chgrp accept this syntax, and the semantics.

Here's the proposed gnulib patch:

2007-01-19  Jim Meyering  <[EMAIL PROTECTED]>

* lib/userspec.c (parse_with_separator): If a user or group string
starts with "+", skip the corresponding name-to-ID look-up, since
such a look-up must fail: user and group names may not include "+".

Index: lib/userspec.c
===
RCS file: /sources/gnulib/gnulib/lib/userspec.c,v
retrieving revision 1.53
diff -u -p -r1.53 userspec.c
--- lib/userspec.c  13 Sep 2006 22:38:14 -  1.53
+++ lib/userspec.c  19 Jan 2007 22:38:12 -
@@ -1,5 +1,5 @@
 /* userspec.c -- Parse a user and group string.
-   Copyright (C) 1989-1992, 1997-1998, 2000, 2002-2006 Free Software
+   Copyright (C) 1989-1992, 1997-1998, 2000, 2002-2007 Free Software
Foundation, Inc.

This program is free software; you can redistribute it and/or modify
@@ -156,7 +156,8 @@ parse_with_separator (char const *spec,

   if (u != NULL)
 {
-  pwd = getpwnam (u);
+  /* If it starts with "+", skip the look-up.  */
+  pwd = (*u == '+' ? NULL : getpwnam (u));
   if (pwd == NULL)
{
  bool use_login_group = (separator != NULL && g == NULL);
@@ -196,7 +197,8 @@ parse_with_separator (char const *spec,
   if (g != NULL && error_msg == NULL)
 {
   /* Explicit group.  */
-  grp = getgrnam (g);
+  /* If it starts with "+", skip the look-up.  */
+  grp = (*g == '+' ? NULL : getgrnam (g));
   if (grp == NULL)
{
  unsigned long int tmp;


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2007-01-20 Thread Jim Meyering
Paul Eggert <[EMAIL PROTECTED]> wrote:
> Jim Meyering <[EMAIL PROTECTED]> writes:
>
>>  * lib/userspec.c (parse_with_separator): If a user or group string
>>  starts with "+", skip the corresponding name-to-ID look-up, since
>>  such a look-up must fail: user and group names may not include "+".
>
> The usage is portable to OpenBSD 3.9 and FreeBSD current, so there is
> some precedent for it.  I like the idea myself, even if it doesn't
> happen to work on Solaris.

Thanks for checking.
I've confirmed that the syntax also portable to NetBSD 1.6
and MacOS X 7.9.0 systems.  That's good enough for me.
I've checked in that change.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2007-01-20 Thread Paul Eggert
Jim Meyering <[EMAIL PROTECTED]> writes:

>   * lib/userspec.c (parse_with_separator): If a user or group string
>   starts with "+", skip the corresponding name-to-ID look-up, since
>   such a look-up must fail: user and group names may not include "+".

The usage is portable to OpenBSD 3.9 and FreeBSD current, so there is
some precedent for it.  I like the idea myself, even if it doesn't
happen to work on Solaris.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-19 Thread Jim Meyering
Pádraig Brady <[EMAIL PROTECTED]> wrote:
> Jim Meyering wrote:
>> In , Helge Hafting objected to the fact
>> that GNU chown performs a DB look-up for a numeric "user name", e.g., in
>> "chown 0 FILE".  chown does this deliberately, in case "0" is an actual
>> user *name*, that is associated potentially, with some numeric user ID.
>> That is the historical behavior, and it is required for POSIX conformance.
>>
>> Yes, that does sound silly, if not downright wrong.  Who actually uses
>> numeric user or group names these days?  Of the systems that still allow
>> such names, how many actually require or even use that capability?
>
> I can see this as being quite common.
> Consider a university server.
> In my uni all our accounts were 8 digit numbers.

Hi Pádraig,

Do you know if they still do that?

If numeric user names are still common enough, a compromise would be
to add a configure-time option to enable one behavior or the other,
and new options:

  --lookup-numeric
  --no-lookup-numeric

Or maybe -- easiest of all -- just don't change anything :-)

Jim



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-19 Thread Pádraig Brady
Jim Meyering wrote:
> In , Helge Hafting objected to the fact
> that GNU chown performs a DB look-up for a numeric "user name", e.g., in
> "chown 0 FILE".  chown does this deliberately, in case "0" is an actual
> user *name*, that is associated potentially, with some numeric user ID.
> That is the historical behavior, and it is required for POSIX conformance.
> 
> Yes, that does sound silly, if not downright wrong.  Who actually uses
> numeric user or group names these days?  Of the systems that still allow
> such names, how many actually require or even use that capability?

I can see this as being quite common.
Consider a university server.
In my uni all our accounts were 8 digit numbers.

Pádraig.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-19 Thread Jim Meyering
In , Helge Hafting objected to the fact
that GNU chown performs a DB look-up for a numeric "user name", e.g., in
"chown 0 FILE".  chown does this deliberately, in case "0" is an actual
user *name*, that is associated potentially, with some numeric user ID.
That is the historical behavior, and it is required for POSIX conformance.

Yes, that does sound silly, if not downright wrong.  Who actually uses
numeric user or group names these days?  Of the systems that still allow
such names, how many actually require or even use that capability?

I propose to change GNU chown to perform that look-up of an all-numeric
"user" or "group" string only when the POSIXLY_CORRECT envvar is set.
Otherwise, (when POSIXLY_CORRECT is not set and a "name" is a valid user
ID or group ID), chown would use the value obtained from converting the
string with a function like strtoul.

For consistency, the same policy would apply to chgrp.

My motivation for making this change is mainly security.
The paranoid user of chown (usually root) should not have to imagine
that a numeric user name argument like "1000" might be interpreted as
a name and mapped to "0".

Can anyone present a case for *not* making this change?


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Pádraig Brady
Andreas Schwab wrote:
> Michael Stone <[EMAIL PROTECTED]> writes:
> 
> 
>>I guess it's a case of "numeric usernames are stupid" vs "will it break
>>something". I don't see much reason *not* to be posix compliant in this
>>case, though.
> 
> 
> Perhaps there should just be an option to force the numeric name to be
> interpreted as a number.

Yes I agree with this solution.
But the problem is a startup config error as far as I can see.
No point in adding an option to work around this.
What chown does at present is correct IMHO.

Pádraig.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Pádraig Brady
Jim Meyering wrote:
> Do you know if they still do that?

Just checked and yes they do.

Also it was mentioned on a local list that
mobile phone companies all over the world
that use Linux as a messaging platform,
use the mobile number as the username.

> 
> If numeric user names are still common enough, a compromise would be
> to add a configure-time option to enable one behavior or the other,
> and new options:
> 
>   --lookup-numeric
>   --no-lookup-numeric
> 
> Or maybe -- easiest of all -- just don't change anything :-)

I wouldn't change it.

Pádraig.


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Michael Stone

On Thu, Oct 19, 2006 at 11:29:23AM +0200, Jim Meyering wrote:

My motivation for making this change is mainly security.
The paranoid user of chown (usually root) should not have to imagine
that a numeric user name argument like "1000" might be interpreted as
a name and mapped to "0".

Can anyone present a case for *not* making this change?


I don't particularly care either way. I think that calling it a security 
concern is overstating it; if someone can create a user with uid 0 
you've got bigger problems than whether they can use that ability to 
fool root. (Similarly if your root user simply doesn't understand the 
system they're working on.)


I guess it's a case of "numeric usernames are stupid" vs "will it break 
something". I don't see much reason *not* to be posix compliant in this 
case, though.


Mike Stone


--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Andreas Schwab
Michael Stone <[EMAIL PROTECTED]> writes:

> I guess it's a case of "numeric usernames are stupid" vs "will it break
> something". I don't see much reason *not* to be posix compliant in this
> case, though.

Perhaps there should just be an option to force the numeric name to be
interpreted as a number.

Andreas.

-- 
Andreas Schwab, SuSE Labs, [EMAIL PROTECTED]
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Bob Proulx
Pádraig Brady wrote:
> Jim Meyering wrote:
> > Can anyone present a case for *not* making this change?

This is also controlled by /etc/nsswitch.conf.  A typical
configuration would always search local files first and then search
network configuration after failing to get a local answer.  (In the
case of 'chown 0:0' this typically searches the network after
searching local files.)

For example in file /etc/nsswitch.conf any of the next would be
fairly normal configurations.
  passwd: compat
Or:
  passwd: files nis
Or:
  passwd: files ldap

The only way to trigger the originally reported behavior of doing a
network lookup for 0:0 and causing a boot time problem is if the
system either a) does not have a zero entry in their local /etc/passwd
file or b) if the /etc/nsswitch.conf file is misconfigured to search
the network first as in this (bad) example.
  passwd: ldap files  # bad override of local with network

I strongly believe that not having a root entry in the local password
file is a wrong configuration.  I strongly believe that configuring a
network override of local files is a wrong configuration.

At boot time when the network is not yet configured the local file
will be searched first, then the network will be attempted but will
fail without delay.  The values of 0:0 will resolve to root:root from
the data in the local file.  The operation will proceed.

After boot with the networking fully configured and a network database
such as ldap or nis configured a the values of 0:0 will also search
the network database.  It must do this because a user name of 0 may be
configured in the network database.  If that is so then the
translation for user name zero must occur.  (However I would also
consider that at the least an insane configuration too.)

Unless you want to make user name 0 uniquely special as compared to
other user names such as user name 1423.  And if so then should user
names below a configurable system threshold designating system users
also be special?  How will this configuration value be configured into
chown?  It suddenly becomes a bigger issue.  Once you make the name 0
special then a lot of other things in that area suddenly also need to
be special too that never needed it before.  I would not go there.

Therefore I don't see how these steps can be avoided.  A script
wishing to be completely local should use 'chown root:root' and the
system should have a root /etc/passwd entry and /etc/nsswitch.conf
should search local files before network databases.

> > Or maybe -- easiest of all -- just don't change anything :-)
> 
> I wouldn't change it.

I also vote not to change it.

Bob



Bug#393283: RFC: change chown *not* to look up numeric user/group names

2006-10-20 Thread Helge Hafting

Bob Proulx wrote:

I strongly believe that not having a root entry in the local password
file is a wrong configuration.  I strongly believe that configuring a
network override of local files is a wrong configuration.

At boot time when the network is not yet configured the local file
will be searched first, then the network will be attempted but will
fail without delay.  The values of 0:0 will resolve to root:root from
the data in the local file.  The operation will proceed.
  

The problem is that I don't get this "failure without delay".
/etc/init.d/xorg-common does a couple of chown 0:0,
attempts to contact the ldap server on 127.0.0.1.  The interface
is up, but ldap is not yet running due to the startup order.
So xorg.common halts and no more startup scripts run.

[...]

Therefore I don't see how these steps can be avoided.  A script
wishing to be completely local should use 'chown root:root' and the
system should have a root /etc/passwd entry and /etc/nsswitch.conf
should search local files before network databases.
  


I now see that chown must work the way it does, with people actually
using numeric usernames.  Having nsswitch.conf searching local
files first is fine with me.

I think it'd be nice to fix this for future ldap users, what would be the
correct approach?

* report a bug against xorg-common, it should use "chown root:root"?
* report a bug against slapd, it should start before xorg-common?
* report a bug against libnss-ldap, it should advice the admin to create
  the "0" user and "0" group when making the change to nsswitch.conf?

* Or several of the above?

Helge Hafting


--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]