2010-08-17 19:38:43

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

In recent NFS v2/v3 to v4 transitions, one of the sticking
points have been that fact v4 uses strings in the format
of "user@domain" instead of 32bit integers for uids and
gids.

When the string can not be mapped, its mapped to the 'nobody'
user which is not optimal for things like backup servers and
such where the ids will not be know by both sides.

So this patch series enables the server to send out numeric
string of uids and gids that do not have the '@domain' part.
The series also adds functionality to the client that parse these
type of strings and will use the numeric representation
of the ids iff the id exists on the client, which is
sightly different that Solaris. Solaris dose not have that
"id must exist" restriction.

Steve Dickson (2):
Teach clients to map numeric strings into valid uids and gids.
Add server support to use of numeric strings for uid and gids.

utils/idmapd/idmapd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 51 insertions(+), 1 deletions(-)



2010-08-17 21:35:54

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.



On 08/17/2010 04:27 PM, Trond Myklebust wrote:
> On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
>> When a server can not map a uid or gid they can send
>> an numeric string (one that does not include the @domain)
>> of the given id. When this occurs the client will can use
>> that numeric representation iff that id exists on the client.
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>> ---
>> utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
>> 1 files changed, 38 insertions(+), 0 deletions(-)
>>
>> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
>> index 9ecab66..d6799c9 100644
>> --- a/utils/idmapd/idmapd.c
>> +++ b/utils/idmapd/idmapd.c
>> @@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
>> im->im_status = IDMAP_STATUS_SUCCESS;
>> }
>>
>> +static int
>> +alldigits(char *name)
>> +{
>> + char *p = name;
>> +
>> + if (name == NULL)
>> + return 0;
>> +
>> + while (*p != '\0') {
>> + if (!isdigit(*p++))
>> + return 0;
>> + }
>> + return 1;
>> +}
>> +
>> static void
>> nametoidres(struct idmap_msg *im)
>> {
>> @@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
>> ret = nfs4_name_to_uid(im->im_name, &uid);
>> im->im_id = (u_int32_t) uid;
>> if (ret) {
>> + /*
>> + * When a server sends numeric string that's
>> + * lacking the @domain part, and the uid exists
>> + * use the numeric string as the uid.
>> + */
>> + if (alldigits(im->im_name)) {
>> + sscanf(im->im_name, "%u", &uid);
>> + if (getpwuid(uid) != NULL) {
>> + im->im_id = uid;
>> + break;
>> + }
>> + }
>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
>> im->im_id = nobodyuid;
>> }
>> @@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
>> ret = nfs4_name_to_gid(im->im_name, &gid);
>> im->im_id = (u_int32_t) gid;
>> if (ret) {
>> + /*
>> + * Do the same type of mapping of a numeric string
>> + * sent as the gid.
>> + */
>> + if (alldigits(im->im_name)) {
>> + sscanf(im->im_name, "%u", &gid);
>> + if (getgrgid(gid) != NULL) {
>> + im->im_id = gid;
>> + break;
>> + }
>> + }
>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
>> im->im_id = nobodygid;
>> }
>
> Wouldn't it make more sense to put this into libnfsidmap instead?
I did think of that but since the nobody ids are being set here I
figured I made the most sense to do here... plus this is pretty
simple code why hide a way in the library and make it more
difficult to debug...

steved.

2010-08-17 22:07:27

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.



On 08/17/2010 05:47 PM, Trond Myklebust wrote:
> On Tue, 2010-08-17 at 17:35 -0400, Steve Dickson wrote:
>>
>> On 08/17/2010 04:27 PM, Trond Myklebust wrote:
>>> On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
>>>> When a server can not map a uid or gid they can send
>>>> an numeric string (one that does not include the @domain)
>>>> of the given id. When this occurs the client will can use
>>>> that numeric representation iff that id exists on the client.
>>>>
>>>> Signed-off-by: Steve Dickson <[email protected]>
>>>> ---
>>>> utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
>>>> 1 files changed, 38 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
>>>> index 9ecab66..d6799c9 100644
>>>> --- a/utils/idmapd/idmapd.c
>>>> +++ b/utils/idmapd/idmapd.c
>>>> @@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
>>>> im->im_status = IDMAP_STATUS_SUCCESS;
>>>> }
>>>>
>>>> +static int
>>>> +alldigits(char *name)
>>>> +{
>>>> + char *p = name;
>>>> +
>>>> + if (name == NULL)
>>>> + return 0;
>>>> +
>>>> + while (*p != '\0') {
>>>> + if (!isdigit(*p++))
>>>> + return 0;
>>>> + }
>>>> + return 1;
>>>> +}
>>>> +
>>>> static void
>>>> nametoidres(struct idmap_msg *im)
>>>> {
>>>> @@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
>>>> ret = nfs4_name_to_uid(im->im_name, &uid);
>>>> im->im_id = (u_int32_t) uid;
>>>> if (ret) {
>>>> + /*
>>>> + * When a server sends numeric string that's
>>>> + * lacking the @domain part, and the uid exists
>>>> + * use the numeric string as the uid.
>>>> + */
>>>> + if (alldigits(im->im_name)) {
>>>> + sscanf(im->im_name, "%u", &uid);
>>>> + if (getpwuid(uid) != NULL) {
>>>> + im->im_id = uid;
>>>> + break;
>>>> + }
>>>> + }
>>>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
>>>> im->im_id = nobodyuid;
>>>> }
>>>> @@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
>>>> ret = nfs4_name_to_gid(im->im_name, &gid);
>>>> im->im_id = (u_int32_t) gid;
>>>> if (ret) {
>>>> + /*
>>>> + * Do the same type of mapping of a numeric string
>>>> + * sent as the gid.
>>>> + */
>>>> + if (alldigits(im->im_name)) {
>>>> + sscanf(im->im_name, "%u", &gid);
>>>> + if (getgrgid(gid) != NULL) {
>>>> + im->im_id = gid;
>>>> + break;
>>>> + }
>>>> + }
>>>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
>>>> im->im_id = nobodygid;
>>>> }
>>>
>>> Wouldn't it make more sense to put this into libnfsidmap instead?
>> I did think of that but since the nobody ids are being set here I
>> figured I made the most sense to do here... plus this is pretty
>> simple code why hide a way in the library and make it more
>> difficult to debug...
>
> It means that Bryan is going to have to duplicate it all in the new
> highly scalable idmapper.
Hmm... I guess I didn't realize there was a "highly scalable idmapper" on
drawing board... Cool... Any details... Did I miss some email??

>
> Ditto for the nobody ids.
>
> What's the point of having the library when it doesn't do the full NFSv4
> idmapping for you?
At this point we don't do the nobody id mapping in the library... I'm just
going with what is there now.. I do see and understand your point about doing
all id mapping in the library... and with the new scalable idmapper, move the
code... I strongly suggest you do...

But at this point in time we have people doing the v4 transitioning now and
I can get the fix out much quicker, making their transition less painful,
if I put these few lines of code in nfs-utils verses the library...

steved.


2010-08-18 19:23:18

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

On Wed, 2010-08-18 at 15:09 -0400, Steve Dickson wrote:
>
> On 08/18/2010 02:20 PM, J. Bruce Fields wrote:
> > On Tue, Aug 17, 2010 at 03:38:43PM -0400, Steve Dickson wrote:
> >> In recent NFS v2/v3 to v4 transitions, one of the sticking
> >> points have been that fact v4 uses strings in the format
> >> of "user@domain" instead of 32bit integers for uids and
> >> gids.
> >>
> >> When the string can not be mapped, its mapped to the 'nobody'
> >> user which is not optimal for things like backup servers and
> >> such where the ids will not be know by both sides.
> >>
> >> So this patch series enables the server to send out numeric
> >> string of uids and gids that do not have the '@domain' part.
> >> The series also adds functionality to the client that parse these
> >> type of strings and will use the numeric representation
> >> of the ids iff the id exists on the client, which is
> >> sightly different that Solaris. Solaris dose not have that
> >> "id must exist" restriction.
> >
> > Why did you decide to impose that restriction?
> I just thought it made sense, from a security standpoint to make sure the
> ids were at least valid on the client... if they are not valid the id
> becomes 'nobody' which how it works today... but is different than how
> OpenSolaris does it... they just use whatever the server tells to...

As I read RFC3530, the recommendation is that the server SHOULD reject
an attempt by the client to use numeric ids if it knows of a valid
name@domain mapping for that uid or gid.

The client has no such restriction. It probably should just accept the
numeric uid or gid if that is what the server supplies.

Cheers
Trond


2010-08-18 19:09:57

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.



On 08/18/2010 02:20 PM, J. Bruce Fields wrote:
> On Tue, Aug 17, 2010 at 03:38:43PM -0400, Steve Dickson wrote:
>> In recent NFS v2/v3 to v4 transitions, one of the sticking
>> points have been that fact v4 uses strings in the format
>> of "user@domain" instead of 32bit integers for uids and
>> gids.
>>
>> When the string can not be mapped, its mapped to the 'nobody'
>> user which is not optimal for things like backup servers and
>> such where the ids will not be know by both sides.
>>
>> So this patch series enables the server to send out numeric
>> string of uids and gids that do not have the '@domain' part.
>> The series also adds functionality to the client that parse these
>> type of strings and will use the numeric representation
>> of the ids iff the id exists on the client, which is
>> sightly different that Solaris. Solaris dose not have that
>> "id must exist" restriction.
>
> Why did you decide to impose that restriction?
I just thought it made sense, from a security standpoint to make sure the
ids were at least valid on the client... if they are not valid the id
becomes 'nobody' which how it works today... but is different than how
OpenSolaris does it... they just use whatever the server tells to...

steved.


2010-08-17 20:04:25

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.



On 08/17/2010 03:51 PM, Tom Haynes wrote:
> Steve Dickson wrote:
>> In recent NFS v2/v3 to v4 transitions, one of the sticking points have been that fact v4 uses strings in the format
>> of "user@domain" instead of 32bit integers for uids and gids.
>>
>> When the string can not be mapped, its mapped to the 'nobody'
>> user which is not optimal for things like backup servers and
>> such where the ids will not be know by both sides.
>>
>> So this patch series enables the server to send out numeric string of uids and gids that do not have the '@domain' part.
>> The series also adds functionality to the client that parse these
>> type of strings and will use the numeric representation
>> of the ids iff the id exists on the client, which is sightly different that Solaris. Solaris dose not have that
>> "id must exist" restriction.
>>
>
> No, Solaris does have that restriction.
I thought so too.... but when I had an Open Solaris client
access an directory, on a Linux server, that was owned by a
user that was non-existent on either the server or client,
I got the numeric representation as the owner....
Basically:

osol# mount Linux-server:/home /mnt/home
osol# ls -ld /mnt/home/noid
drwxr-xr-x+ 2 111 111 4096 Aug 17 11:37 /mnt/home/noid/
osol# grep 111 /etc/passwd
osol#

steved.

2010-08-18 19:19:31

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

On Wed, Aug 18, 2010 at 03:09:52PM -0400, Steve Dickson wrote:
>
>
> On 08/18/2010 02:20 PM, J. Bruce Fields wrote:
> > On Tue, Aug 17, 2010 at 03:38:43PM -0400, Steve Dickson wrote:
> >> In recent NFS v2/v3 to v4 transitions, one of the sticking
> >> points have been that fact v4 uses strings in the format
> >> of "user@domain" instead of 32bit integers for uids and
> >> gids.
> >>
> >> When the string can not be mapped, its mapped to the 'nobody'
> >> user which is not optimal for things like backup servers and
> >> such where the ids will not be know by both sides.
> >>
> >> So this patch series enables the server to send out numeric
> >> string of uids and gids that do not have the '@domain' part.
> >> The series also adds functionality to the client that parse these
> >> type of strings and will use the numeric representation
> >> of the ids iff the id exists on the client, which is
> >> sightly different that Solaris. Solaris dose not have that
> >> "id must exist" restriction.
> >
> > Why did you decide to impose that restriction?
> I just thought it made sense, from a security standpoint to make sure the
> ids were at least valid on the client... if they are not valid the id
> becomes 'nobody' which how it works today... but is different than how
> OpenSolaris does it... they just use whatever the server tells to...

If we don't have a strong reason to do something different, let's just
do the same as OpenSolaris and save any restrictions for the
client-to-server (acl/owner-setting) path.

--b.

2010-08-17 19:38:44

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 2/2] Add server support to use of numeric strings for uid and gids.

When a uid or gid does not map into the a domain and the
new 'NumericIDs' variable is enabled in idmapd.conf, the server
will send out numeric representations, strings without the
@domain part, of the given id.

Signed-off-by: Steve Dickson <[email protected]>
---
utils/idmapd/idmapd.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index d6799c9..7f1291d 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -794,15 +794,22 @@ nfsopen(struct idmap_client *ic)
static void
idtonameres(struct idmap_msg *im)
{
- char domain[NFS4_MAX_DOMAIN_LEN];
+ char domain[NFS4_MAX_DOMAIN_LEN], *numids;
int ret = 0;

ret = nfs4_get_default_domain(NULL, domain, sizeof(domain));
+ numids = conf_get_str("Mapping", "NumericIDs");
+
switch (im->im_type) {
case IDMAP_TYPE_USER:
ret = nfs4_uid_to_name(im->im_id, domain, im->im_name,
sizeof(im->im_name));
if (ret) {
+ if (numids && strcasecmp(numids, "yes") == 0) {
+ sprintf(im->im_name, "%d", im->im_id);
+ ret=0;
+ break;
+ }
if (strlen(nobodyuser) < sizeof(im->im_name))
strcpy(im->im_name, nobodyuser);
else
@@ -813,6 +820,11 @@ idtonameres(struct idmap_msg *im)
ret = nfs4_gid_to_name(im->im_id, domain, im->im_name,
sizeof(im->im_name));
if (ret) {
+ if (numids && strcasecmp(numids, "yes") == 0) {
+ sprintf(im->im_name, "%d", im->im_id);
+ ret=0;
+ break;
+ }
if (strlen(nobodygroup) < sizeof(im->im_name))
strcpy(im->im_name, nobodygroup);
else
--
1.7.1


2010-08-17 19:38:44

by Steve Dickson

[permalink] [raw]
Subject: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.

When a server can not map a uid or gid they can send
an numeric string (one that does not include the @domain)
of the given id. When this occurs the client will can use
that numeric representation iff that id exists on the client.

Signed-off-by: Steve Dickson <[email protected]>
---
utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
index 9ecab66..d6799c9 100644
--- a/utils/idmapd/idmapd.c
+++ b/utils/idmapd/idmapd.c
@@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
im->im_status = IDMAP_STATUS_SUCCESS;
}

+static int
+alldigits(char *name)
+{
+ char *p = name;
+
+ if (name == NULL)
+ return 0;
+
+ while (*p != '\0') {
+ if (!isdigit(*p++))
+ return 0;
+ }
+ return 1;
+}
+
static void
nametoidres(struct idmap_msg *im)
{
@@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
ret = nfs4_name_to_uid(im->im_name, &uid);
im->im_id = (u_int32_t) uid;
if (ret) {
+ /*
+ * When a server sends numeric string that's
+ * lacking the @domain part, and the uid exists
+ * use the numeric string as the uid.
+ */
+ if (alldigits(im->im_name)) {
+ sscanf(im->im_name, "%u", &uid);
+ if (getpwuid(uid) != NULL) {
+ im->im_id = uid;
+ break;
+ }
+ }
im->im_status = IDMAP_STATUS_LOOKUPFAIL;
im->im_id = nobodyuid;
}
@@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
ret = nfs4_name_to_gid(im->im_name, &gid);
im->im_id = (u_int32_t) gid;
if (ret) {
+ /*
+ * Do the same type of mapping of a numeric string
+ * sent as the gid.
+ */
+ if (alldigits(im->im_name)) {
+ sscanf(im->im_name, "%u", &gid);
+ if (getgrgid(gid) != NULL) {
+ im->im_id = gid;
+ break;
+ }
+ }
im->im_status = IDMAP_STATUS_LOOKUPFAIL;
im->im_id = nobodygid;
}
--
1.7.1


2010-08-17 22:13:24

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.

On Tue, 2010-08-17 at 18:07 -0400, Steve Dickson wrote:
>
> On 08/17/2010 05:47 PM, Trond Myklebust wrote:
> > On Tue, 2010-08-17 at 17:35 -0400, Steve Dickson wrote:
> >>
> >> On 08/17/2010 04:27 PM, Trond Myklebust wrote:
> >>> On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
> >>>> When a server can not map a uid or gid they can send
> >>>> an numeric string (one that does not include the @domain)
> >>>> of the given id. When this occurs the client will can use
> >>>> that numeric representation iff that id exists on the client.
> >>>>
> >>>> Signed-off-by: Steve Dickson <[email protected]>
> >>>> ---
> >>>> utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
> >>>> 1 files changed, 38 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> >>>> index 9ecab66..d6799c9 100644
> >>>> --- a/utils/idmapd/idmapd.c
> >>>> +++ b/utils/idmapd/idmapd.c
> >>>> @@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
> >>>> im->im_status = IDMAP_STATUS_SUCCESS;
> >>>> }
> >>>>
> >>>> +static int
> >>>> +alldigits(char *name)
> >>>> +{
> >>>> + char *p = name;
> >>>> +
> >>>> + if (name == NULL)
> >>>> + return 0;
> >>>> +
> >>>> + while (*p != '\0') {
> >>>> + if (!isdigit(*p++))
> >>>> + return 0;
> >>>> + }
> >>>> + return 1;
> >>>> +}
> >>>> +
> >>>> static void
> >>>> nametoidres(struct idmap_msg *im)
> >>>> {
> >>>> @@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
> >>>> ret = nfs4_name_to_uid(im->im_name, &uid);
> >>>> im->im_id = (u_int32_t) uid;
> >>>> if (ret) {
> >>>> + /*
> >>>> + * When a server sends numeric string that's
> >>>> + * lacking the @domain part, and the uid exists
> >>>> + * use the numeric string as the uid.
> >>>> + */
> >>>> + if (alldigits(im->im_name)) {
> >>>> + sscanf(im->im_name, "%u", &uid);
> >>>> + if (getpwuid(uid) != NULL) {
> >>>> + im->im_id = uid;
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> >>>> im->im_id = nobodyuid;
> >>>> }
> >>>> @@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
> >>>> ret = nfs4_name_to_gid(im->im_name, &gid);
> >>>> im->im_id = (u_int32_t) gid;
> >>>> if (ret) {
> >>>> + /*
> >>>> + * Do the same type of mapping of a numeric string
> >>>> + * sent as the gid.
> >>>> + */
> >>>> + if (alldigits(im->im_name)) {
> >>>> + sscanf(im->im_name, "%u", &gid);
> >>>> + if (getgrgid(gid) != NULL) {
> >>>> + im->im_id = gid;
> >>>> + break;
> >>>> + }
> >>>> + }
> >>>> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> >>>> im->im_id = nobodygid;
> >>>> }
> >>>
> >>> Wouldn't it make more sense to put this into libnfsidmap instead?
> >> I did think of that but since the nobody ids are being set here I
> >> figured I made the most sense to do here... plus this is pretty
> >> simple code why hide a way in the library and make it more
> >> difficult to debug...
> >
> > It means that Bryan is going to have to duplicate it all in the new
> > highly scalable idmapper.
> Hmm... I guess I didn't realize there was a "highly scalable idmapper" on
> drawing board... Cool... Any details... Did I miss some email??

Not yet. He just finished the first draft today.

Bryan is basically reusing the keyring based design for the dns resolver
and applying it to idmapping.

> >
> > Ditto for the nobody ids.
> >
> > What's the point of having the library when it doesn't do the full NFSv4
> > idmapping for you?
> At this point we don't do the nobody id mapping in the library... I'm just
> going with what is there now.. I do see and understand your point about doing
> all id mapping in the library... and with the new scalable idmapper, move the
> code... I strongly suggest you do...

Ok...

> But at this point in time we have people doing the v4 transitioning now and
> I can get the fix out much quicker, making their transition less painful,
> if I put these few lines of code in nfs-utils verses the library...

I don't see why getting it into nfs-utils is faster. Is CITI being slow
taking patches for libnfsidmap?



2010-08-17 22:25:43

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.



On 08/17/2010 06:13 PM, Trond Myklebust wrote:
>
>> But at this point in time we have people doing the v4 transitioning now and
>> I can get the fix out much quicker, making their transition less painful,
>> if I put these few lines of code in nfs-utils verses the library...
>
> I don't see why getting it into nfs-utils is faster. Is CITI being slow
> taking patches for libnfsidmap?
No... That is not the case WRT CITI... But please remember...
as soon as you create a new interface you also create a new dependency...
which means we would have to push out two packages (simultaneously, which
can be painful) verses just pushing out one package... that's were the
quickness comes from...

steved.


2010-08-17 21:47:18

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.

On Tue, 2010-08-17 at 17:35 -0400, Steve Dickson wrote:
>
> On 08/17/2010 04:27 PM, Trond Myklebust wrote:
> > On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
> >> When a server can not map a uid or gid they can send
> >> an numeric string (one that does not include the @domain)
> >> of the given id. When this occurs the client will can use
> >> that numeric representation iff that id exists on the client.
> >>
> >> Signed-off-by: Steve Dickson <[email protected]>
> >> ---
> >> utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
> >> 1 files changed, 38 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> >> index 9ecab66..d6799c9 100644
> >> --- a/utils/idmapd/idmapd.c
> >> +++ b/utils/idmapd/idmapd.c
> >> @@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
> >> im->im_status = IDMAP_STATUS_SUCCESS;
> >> }
> >>
> >> +static int
> >> +alldigits(char *name)
> >> +{
> >> + char *p = name;
> >> +
> >> + if (name == NULL)
> >> + return 0;
> >> +
> >> + while (*p != '\0') {
> >> + if (!isdigit(*p++))
> >> + return 0;
> >> + }
> >> + return 1;
> >> +}
> >> +
> >> static void
> >> nametoidres(struct idmap_msg *im)
> >> {
> >> @@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
> >> ret = nfs4_name_to_uid(im->im_name, &uid);
> >> im->im_id = (u_int32_t) uid;
> >> if (ret) {
> >> + /*
> >> + * When a server sends numeric string that's
> >> + * lacking the @domain part, and the uid exists
> >> + * use the numeric string as the uid.
> >> + */
> >> + if (alldigits(im->im_name)) {
> >> + sscanf(im->im_name, "%u", &uid);
> >> + if (getpwuid(uid) != NULL) {
> >> + im->im_id = uid;
> >> + break;
> >> + }
> >> + }
> >> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> >> im->im_id = nobodyuid;
> >> }
> >> @@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
> >> ret = nfs4_name_to_gid(im->im_name, &gid);
> >> im->im_id = (u_int32_t) gid;
> >> if (ret) {
> >> + /*
> >> + * Do the same type of mapping of a numeric string
> >> + * sent as the gid.
> >> + */
> >> + if (alldigits(im->im_name)) {
> >> + sscanf(im->im_name, "%u", &gid);
> >> + if (getgrgid(gid) != NULL) {
> >> + im->im_id = gid;
> >> + break;
> >> + }
> >> + }
> >> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> >> im->im_id = nobodygid;
> >> }
> >
> > Wouldn't it make more sense to put this into libnfsidmap instead?
> I did think of that but since the nobody ids are being set here I
> figured I made the most sense to do here... plus this is pretty
> simple code why hide a way in the library and make it more
> difficult to debug...

It means that Bryan is going to have to duplicate it all in the new
highly scalable idmapper.

Ditto for the nobody ids.

What's the point of having the library when it doesn't do the full NFSv4
idmapping for you?

Cheers
Trond


2010-08-17 19:52:28

by Tom Haynes

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

Steve Dickson wrote:
> In recent NFS v2/v3 to v4 transitions, one of the sticking
> points have been that fact v4 uses strings in the format
> of "user@domain" instead of 32bit integers for uids and
> gids.
>
> When the string can not be mapped, its mapped to the 'nobody'
> user which is not optimal for things like backup servers and
> such where the ids will not be know by both sides.
>
> So this patch series enables the server to send out numeric
> string of uids and gids that do not have the '@domain' part.
> The series also adds functionality to the client that parse these
> type of strings and will use the numeric representation
> of the ids iff the id exists on the client, which is
> sightly different that Solaris. Solaris dose not have that
> "id must exist" restriction.
>

No, Solaris does have that restriction.


> Steve Dickson (2):
> Teach clients to map numeric strings into valid uids and gids.
> Add server support to use of numeric strings for uid and gids.
>
> utils/idmapd/idmapd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 51 insertions(+), 1 deletions(-)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


2010-08-17 20:27:41

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 1/2] Teach clients to map numeric strings into valid uids and gids.

On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
> When a server can not map a uid or gid they can send
> an numeric string (one that does not include the @domain)
> of the given id. When this occurs the client will can use
> that numeric representation iff that id exists on the client.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/idmapd/idmapd.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index 9ecab66..d6799c9 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -826,6 +826,21 @@ idtonameres(struct idmap_msg *im)
> im->im_status = IDMAP_STATUS_SUCCESS;
> }
>
> +static int
> +alldigits(char *name)
> +{
> + char *p = name;
> +
> + if (name == NULL)
> + return 0;
> +
> + while (*p != '\0') {
> + if (!isdigit(*p++))
> + return 0;
> + }
> + return 1;
> +}
> +
> static void
> nametoidres(struct idmap_msg *im)
> {
> @@ -843,6 +858,18 @@ nametoidres(struct idmap_msg *im)
> ret = nfs4_name_to_uid(im->im_name, &uid);
> im->im_id = (u_int32_t) uid;
> if (ret) {
> + /*
> + * When a server sends numeric string that's
> + * lacking the @domain part, and the uid exists
> + * use the numeric string as the uid.
> + */
> + if (alldigits(im->im_name)) {
> + sscanf(im->im_name, "%u", &uid);
> + if (getpwuid(uid) != NULL) {
> + im->im_id = uid;
> + break;
> + }
> + }
> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> im->im_id = nobodyuid;
> }
> @@ -851,6 +878,17 @@ nametoidres(struct idmap_msg *im)
> ret = nfs4_name_to_gid(im->im_name, &gid);
> im->im_id = (u_int32_t) gid;
> if (ret) {
> + /*
> + * Do the same type of mapping of a numeric string
> + * sent as the gid.
> + */
> + if (alldigits(im->im_name)) {
> + sscanf(im->im_name, "%u", &gid);
> + if (getgrgid(gid) != NULL) {
> + im->im_id = gid;
> + break;
> + }
> + }
> im->im_status = IDMAP_STATUS_LOOKUPFAIL;
> im->im_id = nobodygid;
> }

Wouldn't it make more sense to put this into libnfsidmap instead?

Cheers
Trond


2010-08-17 20:28:22

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2/2] Add server support to use of numeric strings for uid and gids.

On Tue, 2010-08-17 at 15:38 -0400, Steve Dickson wrote:
> When a uid or gid does not map into the a domain and the
> new 'NumericIDs' variable is enabled in idmapd.conf, the server
> will send out numeric representations, strings without the
> @domain part, of the given id.
>
> Signed-off-by: Steve Dickson <[email protected]>
> ---
> utils/idmapd/idmapd.c | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c
> index d6799c9..7f1291d 100644
> --- a/utils/idmapd/idmapd.c
> +++ b/utils/idmapd/idmapd.c
> @@ -794,15 +794,22 @@ nfsopen(struct idmap_client *ic)
> static void
> idtonameres(struct idmap_msg *im)
> {
> - char domain[NFS4_MAX_DOMAIN_LEN];
> + char domain[NFS4_MAX_DOMAIN_LEN], *numids;
> int ret = 0;
>
> ret = nfs4_get_default_domain(NULL, domain, sizeof(domain));
> + numids = conf_get_str("Mapping", "NumericIDs");
> +
> switch (im->im_type) {
> case IDMAP_TYPE_USER:
> ret = nfs4_uid_to_name(im->im_id, domain, im->im_name,
> sizeof(im->im_name));
> if (ret) {
> + if (numids && strcasecmp(numids, "yes") == 0) {
> + sprintf(im->im_name, "%d", im->im_id);
> + ret=0;
> + break;
> + }
> if (strlen(nobodyuser) < sizeof(im->im_name))
> strcpy(im->im_name, nobodyuser);
> else
> @@ -813,6 +820,11 @@ idtonameres(struct idmap_msg *im)
> ret = nfs4_gid_to_name(im->im_id, domain, im->im_name,
> sizeof(im->im_name));
> if (ret) {
> + if (numids && strcasecmp(numids, "yes") == 0) {
> + sprintf(im->im_name, "%d", im->im_id);
> + ret=0;
> + break;
> + }
> if (strlen(nobodygroup) < sizeof(im->im_name))
> strcpy(im->im_name, nobodygroup);
> else

Ditto...

Cheers
Trond


2010-08-17 20:31:29

by Tom Haynes

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

Steve Dickson wrote:
> On 08/17/2010 03:51 PM, Tom Haynes wrote:
>
>> Steve Dickson wrote:
>>
>>> In recent NFS v2/v3 to v4 transitions, one of the sticking points have been that fact v4 uses strings in the format
>>> of "user@domain" instead of 32bit integers for uids and gids.
>>>
>>> When the string can not be mapped, its mapped to the 'nobody'
>>> user which is not optimal for things like backup servers and
>>> such where the ids will not be know by both sides.
>>>
>>> So this patch series enables the server to send out numeric string of uids and gids that do not have the '@domain' part.
>>> The series also adds functionality to the client that parse these
>>> type of strings and will use the numeric representation
>>> of the ids iff the id exists on the client, which is sightly different that Solaris. Solaris dose not have that
>>> "id must exist" restriction.
>>>
>>>
>> No, Solaris does have that restriction.
>>
> I thought so too.... but when I had an Open Solaris client
> access an directory, on a Linux server, that was owned by a
> user that was non-existent on either the server or client,
> I got the numeric representation as the owner....
> Basically:
>
> osol# mount Linux-server:/home /mnt/home
> osol# ls -ld /mnt/home/noid
> drwxr-xr-x+ 2 111 111 4096 Aug 17 11:37 /mnt/home/noid/
> osol# grep 111 /etc/passwd
> osol#
>
> steved.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

That is what I get for reading code instead of running it. :->

Actually, it might be that the server acts that way and not the client.

2010-08-18 18:22:57

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.

On Tue, Aug 17, 2010 at 03:38:43PM -0400, Steve Dickson wrote:
> In recent NFS v2/v3 to v4 transitions, one of the sticking
> points have been that fact v4 uses strings in the format
> of "user@domain" instead of 32bit integers for uids and
> gids.
>
> When the string can not be mapped, its mapped to the 'nobody'
> user which is not optimal for things like backup servers and
> such where the ids will not be know by both sides.
>
> So this patch series enables the server to send out numeric
> string of uids and gids that do not have the '@domain' part.
> The series also adds functionality to the client that parse these
> type of strings and will use the numeric representation
> of the ids iff the id exists on the client, which is
> sightly different that Solaris. Solaris dose not have that
> "id must exist" restriction.

Why did you decide to impose that restriction?

--b.

2010-08-18 19:33:22

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH 0/2] Support for Numeric Representations of UIDs and GIDs.



On 08/18/2010 03:23 PM, Trond Myklebust wrote:
> On Wed, 2010-08-18 at 15:09 -0400, Steve Dickson wrote:
>>
>> On 08/18/2010 02:20 PM, J. Bruce Fields wrote:
>>> On Tue, Aug 17, 2010 at 03:38:43PM -0400, Steve Dickson wrote:
>>>> In recent NFS v2/v3 to v4 transitions, one of the sticking
>>>> points have been that fact v4 uses strings in the format
>>>> of "user@domain" instead of 32bit integers for uids and
>>>> gids.
>>>>
>>>> When the string can not be mapped, its mapped to the 'nobody'
>>>> user which is not optimal for things like backup servers and
>>>> such where the ids will not be know by both sides.
>>>>
>>>> So this patch series enables the server to send out numeric
>>>> string of uids and gids that do not have the '@domain' part.
>>>> The series also adds functionality to the client that parse these
>>>> type of strings and will use the numeric representation
>>>> of the ids iff the id exists on the client, which is
>>>> sightly different that Solaris. Solaris dose not have that
>>>> "id must exist" restriction.
>>>
>>> Why did you decide to impose that restriction?
>> I just thought it made sense, from a security standpoint to make sure the
>> ids were at least valid on the client... if they are not valid the id
>> becomes 'nobody' which how it works today... but is different than how
>> OpenSolaris does it... they just use whatever the server tells to...
>
> As I read RFC3530, the recommendation is that the server SHOULD reject
> an attempt by the client to use numeric ids if it knows of a valid
> name@domain mapping for that uid or gid.
>
> The client has no such restriction. It probably should just accept the
> numeric uid or gid if that is what the server supplies.
Fine... I will commit the patches without that check...

steved.