From bdfe0d28ee4c23dec311423a34b2a3ec65b5b318 Mon Sep 17 00:00:00 2001 From: "J. Bruce Fields" Date: Fri, 13 Oct 2006 15:35:52 -0400 Subject: [PATCH] Don't always use default mapping to "nobody". Signed-off-by: Kevin Coffman We've been ignoring all mapping errors and instead mapping to a "nobody" user or group. This is arguably OK for the cases where we're returning a value to the user (so, id->name mapping on the server or name->id mapping on the client). But it's a disaster in the other direction (id->name on the server or id->name on the client): for example, a chown to an unknown user should *not* automatically be translated into a succesful chown to "nobody". This patch fixes that problem on the server side. Signed-off-by: J. Bruce Fields --- utils/idmapd/idmapd.c | 52 +++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/utils/idmapd/idmapd.c b/utils/idmapd/idmapd.c index 21a1916..19bf7a6 100644 --- a/utils/idmapd/idmapd.c +++ b/utils/idmapd/idmapd.c @@ -626,10 +626,18 @@ nfsdcb(int fd, short which, void *data) snprintf(buf1, sizeof(buf1), "%lu", time(NULL) + cache_entry_expiration); addfield(&bp, &bsiz, buf1); - /* ID */ - snprintf(buf1, sizeof(buf1), "%u", im.im_id); - addfield(&bp, &bsiz, buf1); + /* Note that we don't want to write the id if the mapping + * failed; instead, by leaving it off, we write a negative + * cache entry which will result in an error returned to + * the client. We don't want a chown or setacl referring + * to an unknown user to result in giving permissions to + * "nobody"! */ + if (im.im_status == IDMAP_STATUS_SUCCESS) { + /* ID */ + snprintf(buf1, sizeof(buf1), "%u", im.im_id); + addfield(&bp, &bsiz, buf1); + } //if (bsiz == sizeof(buf)) /* XXX */ bp[-1] = '\n'; @@ -646,6 +654,8 @@ nfsdcb(int fd, short which, void *data) snprintf(buf1, sizeof(buf1), "%lu", time(NULL) + cache_entry_expiration); addfield(&bp, &bsiz, buf1); + /* Note we're ignoring the status field in this case; we'll + * just map to nobody instead. */ /* Name */ addfield(&bp, &bsiz, im.im_name); @@ -718,6 +728,13 @@ nfscb(int fd, short which, void *data) imconv(ic, &im); + /* XXX: I don't like ignoring this error in the id->name case, + * but we've never returned it, and I need to check that the client + * can handle it gracefully before starting to return it now. */ + + if (im.im_status == IDMAP_STATUS_LOOKUPFAIL) + im.im_status = IDMAP_STATUS_SUCCESS; + if (atomicio(write, ic->ic_fd, &im, sizeof(im)) != sizeof(im)) idmapd_warn("nfscb: write(%s)", ic->ic_path); out: @@ -837,8 +854,10 @@ idtonameres(struct idmap_msg *im) } break; } - /* XXX Hack? */ - im->im_status = IDMAP_STATUS_SUCCESS; + if (ret) + im->im_status = IDMAP_STATUS_LOOKUPFAIL; + else + im->im_status = IDMAP_STATUS_SUCCESS; } static void @@ -848,30 +867,29 @@ nametoidres(struct idmap_msg *im) gid_t gid; int ret = 0; - /* XXX: nobody fallbacks shouldn't always happen: - * server id -> name should be OK - * client name -> id should be OK - * but not otherwise */ /* XXX: move nobody stuff to library calls * (nfs4_get_nobody_user(domain), nfs4_get_nobody_group(domain)) */ - /* XXX: should make this call higher up in the call chain (so we'd - * have a chance on looking up server/whatever. */ + + im->im_status = IDMAP_STATUS_SUCCESS; + switch (im->im_type) { case IDMAP_TYPE_USER: ret = nfs4_name_to_uid(im->im_name, &uid); im->im_id = (u_int32_t) uid; - if (ret) + if (ret) { + im->im_status = IDMAP_STATUS_LOOKUPFAIL; im->im_id = nobodyuid; - break; + } + return; case IDMAP_TYPE_GROUP: ret = nfs4_name_to_gid(im->im_name, &gid); im->im_id = (u_int32_t) gid; - if (ret) + if (ret) { + im->im_status = IDMAP_STATUS_LOOKUPFAIL; im->im_id = nobodygid; - break; + } + return; } - /* XXX? */ - im->im_status = IDMAP_STATUS_SUCCESS; } static int -- 2.39.5