Re: svn commit: r318262 - head/usr.sbin/mountd

2017-05-14 Thread Rick Macklem
Oh, and if the commit doesn't get reverted, I do plan on committing a
change to the exports.5 man page. I just held off on that until the dust
settles.

I had expected more discussion on freebsd-current@ w.r.t. this, but after
several days of no messages, I went ahead with what the two responders
seemed to support.

rick

From: Bruce Evans <b...@optusnet.com.au>
Sent: Saturday, May 13, 2017 11:35:00 PM
To: Rick Macklem
Cc: src-committ...@freebsd.org; svn-src-all@freebsd.org; 
svn-src-h...@freebsd.org
Subject: Re: svn commit: r318262 - head/usr.sbin/mountd

On Sun, 14 May 2017, Rick Macklem wrote:

> Log:
>  Change the default uid/gid values for nobody/nogroup to 65534/65533.
>
>  The default values found in /etc/passwd and /etc/group are 65534, 65533.
>  In mountd.c, the defaults were -2, which was 65534 back when uid_t was 
> 16bits.
>  Without this patch, a file created by root on an NFS exported volume without
>  the "-root=" export option will end up owned by uid 4**32 - 2.
>  When discussed on freebsd-current@, it seemed that users preferred the
>  values being changed to 65534/65533.

I got used to 4294967294.  The large number makes it easy to see files
created by root on another system.  I mostly use nfs without maproot, and
create such files often using tmp directories to transfer files.

>  I have not added code to acquire these values from the databases, since
>  the mountd daemon might get "stuck" during startup waiting for a 
> non-responsive
>  password database server.
>
>  Discussed on:freebsd-current
>
> Modified:
>  head/usr.sbin/mountd/mountd.c

exports(5) is not modified, so still documents -2:-2 but not the actual
value of 4294967294:4294967294.  It seems dangerous to change the documented
default.

What happens if the server only supports 16-bit (or 15-bit, or 8-bit) uids?

> Modified: head/usr.sbin/mountd/mountd.c
> ==
> --- head/usr.sbin/mountd/mountd.c Sun May 14 00:23:27 2017
> (r318261)
> +++ head/usr.sbin/mountd/mountd.c Sun May 14 00:38:41 2017
> (r318262)
> @@ -230,9 +230,9 @@ static char **exnames;
> static char **hosts = NULL;
> static struct xucred def_anon = {
>   XUCRED_VERSION,
> - (uid_t)-2,
> + (uid_t)65534,
>   1,
> - { (gid_t)-2 },
> + { (gid_t)65533 },
>   NULL
> };
> static int force_v2 = 0;

The casts are now bogus.  They might have been needed to avoid warnings
about possible sign extension bugs...

> @@ -2893,8 +2893,8 @@ parsecred(char *namelist, struct xucred
>   /*
>* Set up the unprivileged user.
>*/
> - cr->cr_uid = -2;
> - cr->cr_groups[0] = -2;
> + cr->cr_uid = 65534;
> + cr->cr_groups[0] = 65533;
>   cr->cr_ngroups = 1;
>   /*
>* Get the user's password table entry.

But there were no casts here, and the warnings should be the same.

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r318262 - head/usr.sbin/mountd

2017-05-13 Thread Bruce Evans

On Sun, 14 May 2017, Rick Macklem wrote:


Log:
 Change the default uid/gid values for nobody/nogroup to 65534/65533.

 The default values found in /etc/passwd and /etc/group are 65534, 65533.
 In mountd.c, the defaults were -2, which was 65534 back when uid_t was 16bits.
 Without this patch, a file created by root on an NFS exported volume without
 the "-root=" export option will end up owned by uid 4**32 - 2.
 When discussed on freebsd-current@, it seemed that users preferred the
 values being changed to 65534/65533.


I got used to 4294967294.  The large number makes it easy to see files
created by root on another system.  I mostly use nfs without maproot, and
create such files often using tmp directories to transfer files.


 I have not added code to acquire these values from the databases, since
 the mountd daemon might get "stuck" during startup waiting for a non-responsive
 password database server.

 Discussed on:  freebsd-current

Modified:
 head/usr.sbin/mountd/mountd.c


exports(5) is not modified, so still documents -2:-2 but not the actual
value of 4294967294:4294967294.  It seems dangerous to change the documented
default.

What happens if the server only supports 16-bit (or 15-bit, or 8-bit) uids?


Modified: head/usr.sbin/mountd/mountd.c
==
--- head/usr.sbin/mountd/mountd.c   Sun May 14 00:23:27 2017
(r318261)
+++ head/usr.sbin/mountd/mountd.c   Sun May 14 00:38:41 2017
(r318262)
@@ -230,9 +230,9 @@ static char **exnames;
static char **hosts = NULL;
static struct xucred def_anon = {
XUCRED_VERSION,
-   (uid_t)-2,
+   (uid_t)65534,
1,
-   { (gid_t)-2 },
+   { (gid_t)65533 },
NULL
};
static int force_v2 = 0;


The casts are now bogus.  They might have been needed to avoid warnings
about possible sign extension bugs...


@@ -2893,8 +2893,8 @@ parsecred(char *namelist, struct xucred
/*
 * Set up the unprivileged user.
 */
-   cr->cr_uid = -2;
-   cr->cr_groups[0] = -2;
+   cr->cr_uid = 65534;
+   cr->cr_groups[0] = 65533;
cr->cr_ngroups = 1;
/*
 * Get the user's password table entry.


But there were no casts here, and the warnings should be the same.

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


svn commit: r318262 - head/usr.sbin/mountd

2017-05-13 Thread Rick Macklem
Author: rmacklem
Date: Sun May 14 00:38:41 2017
New Revision: 318262
URL: https://svnweb.freebsd.org/changeset/base/318262

Log:
  Change the default uid/gid values for nobody/nogroup to 65534/65533.
  
  The default values found in /etc/passwd and /etc/group are 65534, 65533.
  In mountd.c, the defaults were -2, which was 65534 back when uid_t was 16bits.
  Without this patch, a file created by root on an NFS exported volume without
  the "-root=" export option will end up owned by uid 4**32 - 2.
  When discussed on freebsd-current@, it seemed that users preferred the
  values being changed to 65534/65533.
  I have not added code to acquire these values from the databases, since
  the mountd daemon might get "stuck" during startup waiting for a 
non-responsive
  password database server.
  
  Discussed on: freebsd-current

Modified:
  head/usr.sbin/mountd/mountd.c

Modified: head/usr.sbin/mountd/mountd.c
==
--- head/usr.sbin/mountd/mountd.c   Sun May 14 00:23:27 2017
(r318261)
+++ head/usr.sbin/mountd/mountd.c   Sun May 14 00:38:41 2017
(r318262)
@@ -230,9 +230,9 @@ static char **exnames;
 static char **hosts = NULL;
 static struct xucred def_anon = {
XUCRED_VERSION,
-   (uid_t)-2,
+   (uid_t)65534,
1,
-   { (gid_t)-2 },
+   { (gid_t)65533 },
NULL
 };
 static int force_v2 = 0;
@@ -2893,8 +2893,8 @@ parsecred(char *namelist, struct xucred 
/*
 * Set up the unprivileged user.
 */
-   cr->cr_uid = -2;
-   cr->cr_groups[0] = -2;
+   cr->cr_uid = 65534;
+   cr->cr_groups[0] = 65533;
cr->cr_ngroups = 1;
/*
 * Get the user's password table entry.
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"