Hello sssd-devel & linux-nfs,
This is v2 of the patch set. It is now sent to linux-nfs as weel, following Steve Dickson request.
The code in the next 5 patches implements the design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
The changes in the code (compared to v1) are following the review done in sssd-devel. I believe that there are still two items in dispute (no final verdict was given in the sssd mailing list) but this new patch set will give a chance to have a cleaner look on things.
--
Noam Meltzer Linux Software Engineer PRIMARY DATA
P.O. Box 12650, Herzliya Pituach 4673300 9 Hamenofim St. Akerstein Towers, Tower A, 5th fl. Herzliya
Office: +972-77-8981888 | Fax: +972-3-7617140 | Mobile: +972-54-5873843
Email: noam@primarydata.com
--
Noam Meltzer (5): NEW CLIENT: plugin for NFSv4 rpc.idmapd NFSv4 client: (private) headers from libnfsidmap NFSv4 client: add to build system NFSv4 client: man page NFSv4 client: add to RPM spec
Makefile.am | 19 ++ configure.ac | 10 + contrib/sssd.spec.in | 8 + src/conf_macros.m4 | 30 ++ src/external/libnfsidmap.m4 | 17 + src/man/Makefile.am | 4 +- src/man/include/seealso.xml | 4 + src/man/sss_rpcidmapd.5.xml | 97 ++++++ src/sss_client/common.c | 5 + src/sss_client/nfs/nfsidmap_internal.h | 78 +++++ src/sss_client/nfs/sss_nfs_client.c | 569 +++++++++++++++++++++++++++++++++ src/sss_client/sss_cli.h | 2 + 12 files changed, 842 insertions(+), 1 deletion(-) create mode 100644 src/external/libnfsidmap.m4 create mode 100644 src/man/sss_rpcidmapd.5.xml create mode 100644 src/sss_client/nfs/nfsidmap_internal.h create mode 100644 src/sss_client/nfs/sss_nfs_client.c
Implementation of design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin --- src/sss_client/common.c | 5 + src/sss_client/nfs/sss_nfs_client.c | 569 ++++++++++++++++++++++++++++++++++++ src/sss_client/sss_cli.h | 2 + 3 files changed, 576 insertions(+) create mode 100644 src/sss_client/nfs/sss_nfs_client.c
diff --git a/src/sss_client/common.c b/src/sss_client/common.c index 6044af0..58a9eca 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -936,6 +936,11 @@ int sss_ssh_make_request(enum sss_cli_command cmd, return ret; }
+int sss_nfs_make_request(enum sss_cli_command cmd, struct sss_cli_req_data *rd, + uint8_t **rep, size_t *replen, int *errnop) +{ + return sss_nss_make_request(cmd, rd, rep, replen, errnop); +}
const char *ssscli_err2string(int err) { diff --git a/src/sss_client/nfs/sss_nfs_client.c b/src/sss_client/nfs/sss_nfs_client.c new file mode 100644 index 0000000..7b23eab --- /dev/null +++ b/src/sss_client/nfs/sss_nfs_client.c @@ -0,0 +1,569 @@ +/* + SSSD + + NFS Client + + Copyright (C) Noam Meltzer noam@primarydata.com 2013-2014 + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see http://www.gnu.org/licenses/. +*/ + +#define _GNU_SOURCE + +#include <stddef.h> +#include <stdlib.h> +#include <sys/types.h> +#include <errno.h> +#include <string.h> + +#include <nfsidmap.h> +#include "nfsidmap_internal.h" + +#include "sss_client/sss_cli.h" +#include "sss_client/nss_mc.h" + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +#define PLUGIN_NAME "sss_nfs" +#define CONF_SECTION "sss_nfs" +#define CONF_USE_MC "memcache" +#define REPLY_ID_OFFSET (8) +#define REPLY_NAME_OFFSET (REPLY_ID_OFFSET + 8) +#define MCBUF_LEN (4096) +#define USE_MC_DEFAULT true + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +static char sss_nfs_plugin_name[] = PLUGIN_NAME; +static char nfs_conf_sect[] = CONF_SECTION; +static char nfs_conf_use_mc[] = CONF_USE_MC; + +static bool nfs_use_mc = USE_MC_DEFAULT; + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* Forward declarations */ +static int send_recv(uint8_t **repp, size_t *rep_lenp, enum sss_cli_command cmd, + const void *req, size_t req_len); +static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len); +static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t rep_len); + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* get from memcache functions */ +static int get_uid_from_mc(id_t *uid, const char *name) +{ + int rc = 0; + struct passwd pwd; + char *buf = NULL; + char *p = NULL; + size_t buflen = 0; + size_t len = 0; + + if (!nfs_use_mc) { + return -1; + } + + sss_strnlen(name, SSS_NAME_MAX, &len); + + do { + buflen += MCBUF_LEN; + if ((p = realloc(buf, buflen)) == NULL) { + rc = ENOMEM; + goto done; + } + buf = p; + rc = sss_nss_mc_getpwnam(name, len, &pwd, buf, buflen); + } while (rc == ERANGE); + + if (rc == 0) { + IDMAP_LOG(1, ("found user %s in memcache", name)); + *uid = pwd.pw_uid; + } else { + IDMAP_LOG(1, ("user %s not in memcache", name)); + } + +done: + free(buf); + return rc; +} + +static int get_gid_from_mc(id_t *gid, const char *name) { + int rc = 0; + struct group grp; + char *buf = NULL; + char *p = NULL; + size_t buflen = 0; + size_t len; + + if (!nfs_use_mc) { + return -1; + } + + sss_strnlen(name, SSS_NAME_MAX, &len); + + do { + buflen += MCBUF_LEN; + if ((p = realloc(buf, buflen)) == NULL) { + rc = ENOMEM; + goto done; + } + buf = p; + rc = sss_nss_mc_getgrnam(name, len, &grp, buf, buflen); + } while (rc == ERANGE); + + if (rc == 0) { + IDMAP_LOG(1, ("found group %s in memcache", name)); + *gid = grp.gr_gid; + } else { + IDMAP_LOG(1, ("group %s not in memcache", name)); + } + +done: + free(buf); + return rc; +} + +static int get_user_from_mc(char *name, size_t len, uid_t uid) +{ + int rc; + struct passwd pwd; + char *buf = NULL; + char *p = NULL; + size_t buflen = 0; + size_t pw_name_len; + + if (!nfs_use_mc) { + return -1; + } + + do { + buflen += MCBUF_LEN; + if ((p = realloc(buf, buflen)) == NULL) { + rc = ENOMEM; + goto done; + } + buf = p; + rc = sss_nss_mc_getpwuid(uid, &pwd, buf, MCBUF_LEN); + } while (rc == ERANGE); + + if (rc == 0) { + pw_name_len = strlen(pwd.pw_name) + 1; + if (pw_name_len > len) { + IDMAP_LOG(0, ("%s: reply too long; pw_name_len=%lu, len=%lu", + __func__, pw_name_len, len)); + rc = ENOBUFS; + } + IDMAP_LOG(1, ("found uid %i in memcache", uid)); + memcpy(name, pwd.pw_name, pw_name_len); + } else { + IDMAP_LOG(1, ("uid %i not in memcache", uid)); + } + +done: + free(buf); + return rc; +} + +static int get_group_from_mc(char *name, size_t len, id_t gid) +{ + int rc; + struct group grp; + char *buf = NULL; + char *p = NULL; + size_t buflen = 0; + size_t gr_name_len; + + if (!nfs_use_mc) { + return -1; + } + + do { + buflen += MCBUF_LEN; + if ((p = realloc(buf, buflen)) == NULL) { + rc = ENOMEM; + goto done; + } + buf = p; + rc = sss_nss_mc_getgrgid(gid, &grp, buf, MCBUF_LEN); + } while (rc == ERANGE); + + if (rc == 0) { + gr_name_len = strlen(grp.gr_name) + 1; + if (gr_name_len > len) { + IDMAP_LOG(0, ("%s: reply too long; gr_name_len=%lu, len=%lu", + __func__, gr_name_len, len)); + rc = ENOBUFS; + } + IDMAP_LOG(1, ("found gid %i in memcache", gid)); + memcpy(name, grp.gr_name, gr_name_len); + } else { + IDMAP_LOG(1, ("gid %i not in memcache", gid)); + } + +done: + free(buf); + return rc; +} + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +static int name_to_id(const char *name, id_t *id, enum sss_cli_command cmd) +{ + int rc; + uint8_t *rep = NULL; + size_t rep_len = 0; + size_t name_len; + + sss_strnlen(name, SSS_NAME_MAX, &name_len); + + rc = send_recv(&rep, &rep_len, cmd, name, name_len + 1); + if (rc == 0) { + rc = reply_to_id(id, rep, rep_len); + } + + free(rep); + + return rc; +} + +static int id_to_name(char *name, size_t len, id_t id, + enum sss_cli_command cmd) +{ + int rc; + size_t rep_len = 0; + size_t req_len = sizeof(id_t); + uint8_t *rep = NULL; + uint8_t req[req_len]; + + memcpy(req, &id, req_len); + rc = send_recv(&rep, &rep_len, cmd, &req, req_len); + if (rc == 0) { + rc = reply_to_name(name, len, rep, rep_len); + } + + free(rep); + + return rc; +} + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +static int send_recv(uint8_t **rep, size_t *rep_len, enum sss_cli_command cmd, + const void *req, size_t req_len) +{ + int err = 0; + enum nss_status req_rc; + struct sss_cli_req_data rd; + + rd.data = req; + rd.len = req_len; + + sss_nss_lock(); + req_rc = sss_nfs_make_request(cmd, &rd, rep, rep_len, &err); + sss_nss_unlock(); + + if (req_rc == NSS_STATUS_NOTFOUND) { + return ENOENT; + } + if (req_rc != NSS_STATUS_SUCCESS) { + IDMAP_LOG(0, ("no-make-request; err=%i", err)); + return EPIPE; + } + + return 0; +} + +static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len) +{ + int rc = 0; + id_t id; + uint32_t num_results = 0; + + if (rep_len < sizeof(uint32_t)) { + IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__, rep_len)); + rc = EBADMSG; + goto done; + } + + SAFEALIGN_COPY_UINT32(&num_results, rep, NULL); + if (num_results > 1) { + IDMAP_LOG(0, ("%s: too many results (%lu)", __func__, num_results)); + rc = EBADMSG; + goto done; + } + if (num_results == 0) { + rc = ENOENT; + goto done; + } + if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) { + IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__, + rep_len)); + rc = EBADMSG; + goto done; + } + + SAFEALIGN_COPY_UINT32(&id, rep + REPLY_ID_OFFSET, NULL); + *idp = id; + +done: + return rc; +} + +static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t rep_len) +{ + int rc = 0; + uint32_t num_results = 0; + const char *buf; + size_t buf_len; + size_t offset; + + if (rep_len < sizeof(uint32_t)) { + IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__, rep_len)); + rc = EBADMSG; + goto done; + } + + SAFEALIGN_COPY_UINT32(&num_results, rep, NULL); + if (num_results > 1) { + IDMAP_LOG(0, ("%s: too many results (%lu)", __func__, num_results)); + rc = EBADMSG; + goto done; + } + if (num_results == 0) { + rc = ENOENT; + goto done; + } + if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) { + IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__, + rep_len)); + rc = EBADMSG; + goto done; + } + + buf = (const char *)(rep + REPLY_NAME_OFFSET); + buf_len = rep_len - REPLY_NAME_OFFSET; + offset = 0; + rc = sss_readrep_copy_string(buf, &offset, &buf_len, &len, &name, NULL); + if (rc != 0) { + rc = -rc; + } + +done: + return rc; +} + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* configuration parsing aids */ +static bool str_equal(const char *s1, const char *s2) +{ + bool res = false; + size_t len1; + size_t len2; + + len1 = strlen(s1); + len2 = strlen(s2); + + if (len1 == len2) { + res = (strncasecmp(s1, s2, len1) == 0); + } + + return res; +} + +static int nfs_conf_get_bool(char *sect, char *attr, int def) +{ + int res; + char *val; + + res = def; + val = conf_get_str(sect, attr); + if (val) { + res = (str_equal("1", val) || + str_equal("yes", val) || + str_equal("true", val) || + str_equal("on", val)); + } + + return res; +} + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* libnfsidmap return-code aids */ + +/* + * we only want to return 0 or ENOENT; otherwise libnfsidmap will stop + * translation instead of proceeding to the next translation plugin + */ +int normalise_rc(int rc) { + int res; + + res = rc; + if (res != 0 && res != ENOENT) { + res = ENOENT; + } + + return res; +} + +/* log the actual rc from our code (to be used before normalising the rc) */ +void log_actual_rc(const char *trans_name, int rc) { + char tmp[80]; + IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc, + strerror_r(rc, tmp, sizeof(tmp)))); +} + + +/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* The external interface */ +static int sss_nfs_init(void) +{ + nfs_use_mc = nfs_conf_get_bool(nfs_conf_sect, nfs_conf_use_mc, + USE_MC_DEFAULT); + IDMAP_LOG(1, ("%s: use memcache: %i", __func__, nfs_use_mc)); + + return 0; +} + +static int sss_nfs_princ_to_ids(char *secname, char *princ, uid_t *uid, + gid_t *gid, extra_mapping_params **ex) +{ + IDMAP_LOG(0, ("%s: not implemented", __func__)); + return -ENOENT; +} + +static int sss_nfs_name_to_uid(char *name, uid_t *uid) +{ + int rc; + size_t name_len = 0; + + if (name == NULL) { + IDMAP_LOG(0, ("%s: name is null", __func__)); + return -EINVAL; + } + if (uid == NULL) { + IDMAP_LOG(0, ("%s: uid is null", __func__)); + return -EINVAL; + } + + rc = sss_strnlen(name, SSS_NAME_MAX, &name_len); + if (rc != 0) { + IDMAP_LOG(0, ("%s: no-strnlen; rc=%i", __func__, rc)); + return -rc; + } + + rc = get_uid_from_mc(uid, name); + if (rc != 0) { + rc = name_to_id(name, uid, SSS_NSS_GETPWNAM); + } + + log_actual_rc(__func__, rc); + rc = normalise_rc(rc); + + return -rc; +} + +static int sss_nfs_name_to_gid(char *name, gid_t *gid) +{ + int rc; + size_t name_len = 0; + + if (name == NULL) { + IDMAP_LOG(0, ("%s: name is null", __func__)); + return -EINVAL; + } + if (gid == NULL) { + IDMAP_LOG(0, ("%s: gid is null", __func__)); + return -EINVAL; + } + + rc = sss_strnlen(name, SSS_NAME_MAX, &name_len); + if (rc != 0) { + IDMAP_LOG(0, ("%s: no-strnlen; rc=%i", __func__, rc)); + return -rc; + } + + rc = get_gid_from_mc(gid, name); + if (rc != 0) { + rc = name_to_id(name, gid, SSS_NSS_GETGRNAM); + } + + log_actual_rc(__func__, rc); + rc = normalise_rc(rc); + + return -rc; +} + +static int sss_nfs_uid_to_name(uid_t uid, char *domain, char *name, size_t len) +{ + int rc; + + if (name == NULL) { + IDMAP_LOG(0, ("%s: name is null", __func__)); + return -EINVAL; + } + + rc = get_user_from_mc(name, len, uid); + if (rc != 0) { + rc = id_to_name(name, len, uid, SSS_NSS_GETPWUID); + } + + log_actual_rc(__func__, rc); + rc = normalise_rc(rc); + + return -rc; +} + +static int sss_nfs_gid_to_name(gid_t gid, char *domain, char *name, size_t len) +{ + int rc; + + if (name == NULL) { + IDMAP_LOG(0, ("%s: name is null", __func__)); + return -EINVAL; + } + + rc = get_group_from_mc(name, len, gid); + if (rc != 0) { + rc = id_to_name(name, len, gid, SSS_NSS_GETGRGID); + } + + log_actual_rc(__func__, rc); + rc = normalise_rc(rc); + + return -rc; +} + +static int sss_nfs_gss_princ_to_grouplist( + char *secname, char *princ, gid_t *groups, int *ngroups, + extra_mapping_params **ex) +{ + IDMAP_LOG(0, ("%s: not implemented", __func__)); + return -ENOENT; +} + +static struct trans_func s_sss_nfs_trans = { + .name = sss_nfs_plugin_name, + .init = sss_nfs_init, + .princ_to_ids = sss_nfs_princ_to_ids, + .name_to_uid = sss_nfs_name_to_uid, + .name_to_gid = sss_nfs_name_to_gid, + .uid_to_name = sss_nfs_uid_to_name, + .gid_to_name = sss_nfs_gid_to_name, + .gss_princ_to_grouplist = sss_nfs_gss_princ_to_grouplist, +}; + +struct trans_func *libnfsidmap_plugin_init(void) +{ + return (&s_sss_nfs_trans); +} diff --git a/src/sss_client/sss_cli.h b/src/sss_client/sss_cli.h index 285a297..7e488e4 100644 --- a/src/sss_client/sss_cli.h +++ b/src/sss_client/sss_cli.h @@ -527,6 +527,8 @@ int sss_ssh_make_request(enum sss_cli_command cmd, struct sss_cli_req_data *rd, uint8_t **repbuf, size_t *replen, int *errnop); +int sss_nfs_make_request(enum sss_cli_command cmd, struct sss_cli_req_data *rd, + uint8_t **rep, size_t *replen, int *errnop);
#if 0
On Tue, Mar 04, 2014 at 09:37:52AM +0200, Noam Meltzer wrote:
Implementation of design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
src/sss_client/common.c | 5 + src/sss_client/nfs/sss_nfs_client.c | 569 ++++++++++++++++++++++++++++++++++++ src/sss_client/sss_cli.h | 2 + 3 files changed, 576 insertions(+) create mode 100644 src/sss_client/nfs/sss_nfs_client.c
diff --git a/src/sss_client/common.c b/src/sss_client/common.c index 6044af0..58a9eca 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -936,6 +936,11 @@ int sss_ssh_make_request(enum sss_cli_command cmd, return ret; }
+int sss_nfs_make_request(enum sss_cli_command cmd, struct sss_cli_req_data *rd,
uint8_t **rep, size_t *replen, int *errnop)+{
- return sss_nss_make_request(cmd, rd, rep, replen, errnop);
+}
I wonder if we need to add this call at all. Since the pipe we are talking to is the NSS pipe and we are locking the NSS mutex as well, maybe it's more readable to drop this call altogether and use sss_nss_make_request directly?
const char *ssscli_err2string(int err) { diff --git a/src/sss_client/nfs/sss_nfs_client.c b/src/sss_client/nfs/sss_nfs_client.c new file mode 100644 index 0000000..7b23eab --- /dev/null +++ b/src/sss_client/nfs/sss_nfs_client.c @@ -0,0 +1,569 @@ +/*
- SSSD
- NFS Client
- Copyright (C) Noam Meltzer noam@primarydata.com 2013-2014
This, and other places -- I guess you'd like to update the address to something that exists :-)
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see http://www.gnu.org/licenses/.
+*/
+#define _GNU_SOURCE
+#include <stddef.h> +#include <stdlib.h> +#include <sys/types.h> +#include <errno.h> +#include <string.h>
+#include <nfsidmap.h> +#include "nfsidmap_internal.h"
+#include "sss_client/sss_cli.h" +#include "sss_client/nss_mc.h"
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +#define PLUGIN_NAME "sss_nfs" +#define CONF_SECTION "sss_nfs" +#define CONF_USE_MC "memcache" +#define REPLY_ID_OFFSET (8) +#define REPLY_NAME_OFFSET (REPLY_ID_OFFSET + 8) +#define MCBUF_LEN (4096)
I think this constant is a bit misnamed. The MC prefix suggests that it's only used for memory cache results, which is not true.
+#define USE_MC_DEFAULT true
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +static char sss_nfs_plugin_name[] = PLUGIN_NAME; +static char nfs_conf_sect[] = CONF_SECTION; +static char nfs_conf_use_mc[] = CONF_USE_MC;
+static bool nfs_use_mc = USE_MC_DEFAULT;
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* Forward declarations */ +static int send_recv(uint8_t **repp, size_t *rep_lenp, enum sss_cli_command cmd,
const void *req, size_t req_len);+static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len); +static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t rep_len);
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* get from memcache functions */ +static int get_uid_from_mc(id_t *uid, const char *name) +{
- int rc = 0;
- struct passwd pwd;
- char *buf = NULL;
- char *p = NULL;
- size_t buflen = 0;
- size_t len = 0;
- if (!nfs_use_mc) {
return -1;- }
- sss_strnlen(name, SSS_NAME_MAX, &len);
- do {
buflen += MCBUF_LEN;if ((p = realloc(buf, buflen)) == NULL) {rc = ENOMEM;goto done;}buf = p;rc = sss_nss_mc_getpwnam(name, len, &pwd, buf, buflen);- } while (rc == ERANGE);
- if (rc == 0) {
IDMAP_LOG(1, ("found user %s in memcache", name));*uid = pwd.pw_uid;- } else {
IDMAP_LOG(1, ("user %s not in memcache", name));- }
+done:
- free(buf);
- return rc;
+}
+static int get_gid_from_mc(id_t *gid, const char *name) {
^^^^^^^^ We prefer to put the opening "{" after a newline for function declarations.
- int rc = 0;
- struct group grp;
- char *buf = NULL;
- char *p = NULL;
- size_t buflen = 0;
- size_t len;
- if (!nfs_use_mc) {
return -1;- }
- sss_strnlen(name, SSS_NAME_MAX, &len);
- do {
buflen += MCBUF_LEN;if ((p = realloc(buf, buflen)) == NULL) {rc = ENOMEM;goto done;}buf = p;rc = sss_nss_mc_getgrnam(name, len, &grp, buf, buflen);- } while (rc == ERANGE);
- if (rc == 0) {
IDMAP_LOG(1, ("found group %s in memcache", name));*gid = grp.gr_gid;- } else {
IDMAP_LOG(1, ("group %s not in memcache", name));- }
+done:
- free(buf);
- return rc;
+}
[snip]
+static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len) +{
- int rc = 0;
- id_t id;
- uint32_t num_results = 0;
- if (rep_len < sizeof(uint32_t)) {
IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__, rep_len));rc = EBADMSG;goto done;- }
- SAFEALIGN_COPY_UINT32(&num_results, rep, NULL);
- if (num_results > 1) {
IDMAP_LOG(0, ("%s: too many results (%lu)", __func__, num_results));rc = EBADMSG;goto done;- }
- if (num_results == 0) {
rc = ENOENT;goto done;- }
- if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) {
IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__,rep_len));rc = EBADMSG;goto done;- }
Could we use just this ^ check instead of the one at the beginning of the function? It's not like any partial results can be used if the buffer is > sizeof(uint32_t) but < sizeof(uint32_t) + REPLY_ID_OFFSET.
- SAFEALIGN_COPY_UINT32(&id, rep + REPLY_ID_OFFSET, NULL);
- *idp = id;
+done:
- return rc;
+}
+static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t rep_len) +{
- int rc = 0;
- uint32_t num_results = 0;
- const char *buf;
- size_t buf_len;
- size_t offset;
- if (rep_len < sizeof(uint32_t)) {
IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__, rep_len));rc = EBADMSG;goto done;- }
- SAFEALIGN_COPY_UINT32(&num_results, rep, NULL);
- if (num_results > 1) {
IDMAP_LOG(0, ("%s: too many results (%lu)", __func__, num_results));rc = EBADMSG;goto done;- }
- if (num_results == 0) {
rc = ENOENT;goto done;- }
- if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) {
I think this is a typo, shouldn't the length be checked against REPLY_NAME_OFFSET instead?
IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__,rep_len));rc = EBADMSG;goto done;- }
Similar comment about the length check.
- buf = (const char *)(rep + REPLY_NAME_OFFSET);
- buf_len = rep_len - REPLY_NAME_OFFSET;
- offset = 0;
- rc = sss_readrep_copy_string(buf, &offset, &buf_len, &len, &name, NULL);
- if (rc != 0) {
rc = -rc;- }
+done:
- return rc;
+}
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* configuration parsing aids */ +static bool str_equal(const char *s1, const char *s2) +{
- bool res = false;
- size_t len1;
- size_t len2;
- len1 = strlen(s1);
- len2 = strlen(s2);
- if (len1 == len2) {
res = (strncasecmp(s1, s2, len1) == 0);- }
- return res;
+}
+static int nfs_conf_get_bool(char *sect, char *attr, int def) +{
- int res;
- char *val;
- res = def;
- val = conf_get_str(sect, attr);
- if (val) {
res = (str_equal("1", val) ||str_equal("yes", val) ||str_equal("true", val) ||str_equal("on", val));- }
- return res;
+}
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .*/ +/* libnfsidmap return-code aids */
+/*
- we only want to return 0 or ENOENT; otherwise libnfsidmap will stop
- translation instead of proceeding to the next translation plugin
- */
+int normalise_rc(int rc) {
^^^^^^^ Same comment like get_gid_from_mc
- int res;
- res = rc;
- if (res != 0 && res != ENOENT) {
res = ENOENT;- }
- return res;
+}
+/* log the actual rc from our code (to be used before normalising the rc) */ +void log_actual_rc(const char *trans_name, int rc) {
^^^^^^^^^ And here.
- char tmp[80];
- IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc,
strerror_r(rc, tmp, sizeof(tmp))));+}
The rest of the file looks good to me.
On Fri, Jun 20, 2014 at 5:09 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, Mar 04, 2014 at 09:37:52AM +0200, Noam Meltzer wrote:
Implementation of design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
src/sss_client/common.c | 5 + src/sss_client/nfs/sss_nfs_client.c | 569
++++++++++++++++++++++++++++++++++++
src/sss_client/sss_cli.h | 2 + 3 files changed, 576 insertions(+) create mode 100644 src/sss_client/nfs/sss_nfs_client.c
diff --git a/src/sss_client/common.c b/src/sss_client/common.c index 6044af0..58a9eca 100644 --- a/src/sss_client/common.c +++ b/src/sss_client/common.c @@ -936,6 +936,11 @@ int sss_ssh_make_request(enum sss_cli_command cmd, return ret; }
+int sss_nfs_make_request(enum sss_cli_command cmd, struct
sss_cli_req_data *rd,
uint8_t **rep, size_t *replen, int *errnop)+{
- return sss_nss_make_request(cmd, rd, rep, replen, errnop);
+}
I wonder if we need to add this call at all. Since the pipe we are talking to is the NSS pipe and we are locking the NSS mutex as well, maybe it's more readable to drop this call altogether and use sss_nss_make_request directly?
I like this "simple" wrapper for a single reason - code readability. at the end, I believe the compiler will "optimise out" this function. (though, I haven't actually tested it) bottom line - let me know what you prefer, and I'll go with you.
const char *ssscli_err2string(int err) { diff --git a/src/sss_client/nfs/sss_nfs_client.c
b/src/sss_client/nfs/sss_nfs_client.c
new file mode 100644 index 0000000..7b23eab --- /dev/null +++ b/src/sss_client/nfs/sss_nfs_client.c @@ -0,0 +1,569 @@ +/*
- SSSD
- NFS Client
- Copyright (C) Noam Meltzer noam@primarydata.com 2013-2014
This, and other places -- I guess you'd like to update the address to something that exists :-)
PrimaryData deserves their credit. I'll add a 2nd line with my personal email :)
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <http://www.gnu.org/licenses/
. +*/
+#define _GNU_SOURCE
+#include <stddef.h> +#include <stdlib.h> +#include <sys/types.h> +#include <errno.h> +#include <string.h>
+#include <nfsidmap.h> +#include "nfsidmap_internal.h"
+#include "sss_client/sss_cli.h" +#include "sss_client/nss_mc.h"
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+#define PLUGIN_NAME "sss_nfs" +#define CONF_SECTION "sss_nfs" +#define CONF_USE_MC "memcache" +#define REPLY_ID_OFFSET (8) +#define REPLY_NAME_OFFSET (REPLY_ID_OFFSET + 8) +#define MCBUF_LEN (4096)
I think this constant is a bit misnamed. The MC prefix suggests that it's only used for memory cache results, which is not true.
Will "BUF_LEN" make more sense?
+#define USE_MC_DEFAULT true
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+static char sss_nfs_plugin_name[] = PLUGIN_NAME; +static char nfs_conf_sect[] = CONF_SECTION; +static char nfs_conf_use_mc[] = CONF_USE_MC;
+static bool nfs_use_mc = USE_MC_DEFAULT;
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+/* Forward declarations */ +static int send_recv(uint8_t **repp, size_t *rep_lenp, enum
sss_cli_command cmd,
const void *req, size_t req_len);+static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len); +static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t
rep_len);
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+/* get from memcache functions */ +static int get_uid_from_mc(id_t *uid, const char *name) +{
- int rc = 0;
- struct passwd pwd;
- char *buf = NULL;
- char *p = NULL;
- size_t buflen = 0;
- size_t len = 0;
- if (!nfs_use_mc) {
return -1;- }
- sss_strnlen(name, SSS_NAME_MAX, &len);
- do {
buflen += MCBUF_LEN;if ((p = realloc(buf, buflen)) == NULL) {rc = ENOMEM;goto done;}buf = p;rc = sss_nss_mc_getpwnam(name, len, &pwd, buf, buflen);- } while (rc == ERANGE);
- if (rc == 0) {
IDMAP_LOG(1, ("found user %s in memcache", name));*uid = pwd.pw_uid;- } else {
IDMAP_LOG(1, ("user %s not in memcache", name));- }
+done:
- free(buf);
- return rc;
+}
+static int get_gid_from_mc(id_t *gid, const char *name) {
^^^^^^^^We prefer to put the opening "{" after a newline for function declarations.
so do I. my bad :)
- int rc = 0;
- struct group grp;
- char *buf = NULL;
- char *p = NULL;
- size_t buflen = 0;
- size_t len;
- if (!nfs_use_mc) {
return -1;- }
- sss_strnlen(name, SSS_NAME_MAX, &len);
- do {
buflen += MCBUF_LEN;if ((p = realloc(buf, buflen)) == NULL) {rc = ENOMEM;goto done;}buf = p;rc = sss_nss_mc_getgrnam(name, len, &grp, buf, buflen);- } while (rc == ERANGE);
- if (rc == 0) {
IDMAP_LOG(1, ("found group %s in memcache", name));*gid = grp.gr_gid;- } else {
IDMAP_LOG(1, ("group %s not in memcache", name));- }
+done:
- free(buf);
- return rc;
+}
[snip]
+static int reply_to_id(id_t *idp, uint8_t *rep, size_t rep_len) +{
- int rc = 0;
- id_t id;
- uint32_t num_results = 0;
- if (rep_len < sizeof(uint32_t)) {
IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__,rep_len));
rc = EBADMSG;goto done;- }
- SAFEALIGN_COPY_UINT32(&num_results, rep, NULL);
- if (num_results > 1) {
IDMAP_LOG(0, ("%s: too many results (%lu)", __func__,num_results));
rc = EBADMSG;goto done;- }
- if (num_results == 0) {
rc = ENOENT;goto done;- }
- if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) {
IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__,rep_len));rc = EBADMSG;goto done;- }
Could we use just this ^ check instead of the one at the beginning of the function? It's not like any partial results can be used if the buffer is > sizeof(uint32_t) but < sizeof(uint32_t) + REPLY_ID_OFFSET.
the first check ( rep_len < sizeof(uint32_t) ) is mainly defensive
programming: what if rep_len is 0? if i'm not missing something here, in such a case, SAFEALIGN_COPY_UINT32 will copy garbage into num_results, resulting in unexpected behaviour.
- SAFEALIGN_COPY_UINT32(&id, rep + REPLY_ID_OFFSET, NULL);
- *idp = id;
+done:
- return rc;
+}
+static int reply_to_name(char *name, size_t len, uint8_t *rep, size_t
rep_len)
+{
- int rc = 0;
- uint32_t num_results = 0;
- const char *buf;
- size_t buf_len;
- size_t offset;
- if (rep_len < sizeof(uint32_t)) {
IDMAP_LOG(0, ("%s: reply too small; rep_len=%lu", __func__,rep_len));
rc = EBADMSG;goto done;- }
- SAFEALIGN_COPY_UINT32(&num_results, rep, NULL);
- if (num_results > 1) {
IDMAP_LOG(0, ("%s: too many results (%lu)", __func__,num_results));
rc = EBADMSG;goto done;- }
- if (num_results == 0) {
rc = ENOENT;goto done;- }
- if (rep_len < sizeof(uint32_t) + REPLY_ID_OFFSET) {
I think this is a typo, shouldn't the length be checked against REPLY_NAME_OFFSET instead?
indeed.
IDMAP_LOG(0, ("%s: reply too small(2); rep_len=%lu", __func__,rep_len));rc = EBADMSG;goto done;- }
Similar comment about the length check.
similar answer :)
- buf = (const char *)(rep + REPLY_NAME_OFFSET);
- buf_len = rep_len - REPLY_NAME_OFFSET;
- offset = 0;
- rc = sss_readrep_copy_string(buf, &offset, &buf_len, &len, &name,
NULL);
- if (rc != 0) {
rc = -rc;- }
+done:
- return rc;
+}
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+/* configuration parsing aids */ +static bool str_equal(const char *s1, const char *s2) +{
- bool res = false;
- size_t len1;
- size_t len2;
- len1 = strlen(s1);
- len2 = strlen(s2);
- if (len1 == len2) {
res = (strncasecmp(s1, s2, len1) == 0);- }
- return res;
+}
+static int nfs_conf_get_bool(char *sect, char *attr, int def) +{
- int res;
- char *val;
- res = def;
- val = conf_get_str(sect, attr);
- if (val) {
res = (str_equal("1", val) ||str_equal("yes", val) ||str_equal("true", val) ||str_equal("on", val));- }
- return res;
+}
+/*. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
. . .*/
+/* libnfsidmap return-code aids */
+/*
- we only want to return 0 or ENOENT; otherwise libnfsidmap will stop
- translation instead of proceeding to the next translation plugin
- */
+int normalise_rc(int rc) {
^^^^^^^ Same comment like get_gid_from_mc
- int res;
- res = rc;
- if (res != 0 && res != ENOENT) {
res = ENOENT;- }
- return res;
+}
+/* log the actual rc from our code (to be used before normalising the
rc) */
+void log_actual_rc(const char *trans_name, int rc) {
^^^^^^^^^ And here.
- char tmp[80];
- IDMAP_LOG(1, ("%s: rc=%i msg=%s", trans_name, rc,
strerror_r(rc, tmp, sizeof(tmp))));+}
The rest of the file looks good to me.
The private headers are needed in order to: nfsidmap_internal.h: * definition of struct trans_func * prototype for logger function cfg.h + queue.h: * prototype(s) for accessing rpc.idmpad configuration file --- src/sss_client/nfs/nfsidmap_internal.h | 78 ++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 src/sss_client/nfs/nfsidmap_internal.h
diff --git a/src/sss_client/nfs/nfsidmap_internal.h b/src/sss_client/nfs/nfsidmap_internal.h new file mode 100644 index 0000000..a598c10 --- /dev/null +++ b/src/sss_client/nfs/nfsidmap_internal.h @@ -0,0 +1,78 @@ +/* + * nfsidmap_internal.h + * + * nfs idmapping library, primarily for nfs4 client/server kernel idmapping + * and for userland nfs4 idmapping by acl libraries. + * + * Copyright (c) 2004 The Regents of the University of Michigan. + * All rights reserved. + * + * Andy Adamson andros@umich.edu + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the University nor the names of its + * contributors may be used to endorse or promote products derived + * from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +char *get_default_domain(void); +struct conf_list *get_local_realms(void); + +typedef struct trans_func * (*libnfsidmap_plugin_init_t)(void); + +struct trans_func { + char *name; + int (*init)(void); + int (*princ_to_ids)(char *secname, char *princ, uid_t *uid, gid_t *gid, + extra_mapping_params **ex); + int (*name_to_uid)(char *name, uid_t *uid); + int (*name_to_gid)(char *name, gid_t *gid); + int (*uid_to_name)(uid_t uid, char *domain, char *name, size_t len); + int (*gid_to_name)(gid_t gid, char *domain, char *name, size_t len); + int (*gss_princ_to_grouplist)(char *secname, char *princ, gid_t *groups, + int *ngroups, extra_mapping_params **ex); +}; + +struct mapping_plugin { + void *dl_handle; + struct trans_func *trans; +}; + +typedef enum { + IDTYPE_USER = 1, + IDTYPE_GROUP = 2 +} idtypes; + +extern int idmap_verbosity; +extern nfs4_idmap_log_function_t idmap_log_func; +/* Level zero always prints, others print depending on verbosity level */ +#define IDMAP_LOG(LVL, MSG) \ + do { if (LVL <= idmap_verbosity) (*idmap_log_func)MSG; } while (0) + + +/* + * from libnfsidmap's cfg.h (same license as above) + * Copyright (c) 1998, 1999, 2001 Niklas Hallqvist. All rights reserved. + * Copyright (c) 2000, 2003 H�kan Olsson. All rights reserved. + */ +extern char *conf_get_str(char *, char *);
On Tue, Mar 04, 2014 at 09:37:53AM +0200, Noam Meltzer wrote:
The private headers are needed in order to: nfsidmap_internal.h:
- definition of struct trans_func
- prototype for logger function
cfg.h + queue.h:
- prototype(s) for accessing rpc.idmpad configuration file
Looks OK, but if this file is needed for building the modules, shouldn't it be part of the NFS-idmap devel interface?
On Fri, Jun 20, 2014 at 5:10 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, Mar 04, 2014 at 09:37:53AM +0200, Noam Meltzer wrote:
The private headers are needed in order to: nfsidmap_internal.h:
- definition of struct trans_func
- prototype for logger function
cfg.h + queue.h:
- prototype(s) for accessing rpc.idmpad configuration file
Looks OK, but if this file is needed for building the modules, shouldn't it be part of the NFS-idmap devel interface?
I think that this is the first time such a plugin is developed outside of nfs-utils/libnfsidmap ecosystem. I can start a thread about it in nfs-devel requesting that these headers will become part of the public API , but I don't think that this should stop us here
--- Makefile.am | 19 +++++++++++++++++++ configure.ac | 10 ++++++++++ src/conf_macros.m4 | 30 ++++++++++++++++++++++++++++++ src/external/libnfsidmap.m4 | 17 +++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 src/external/libnfsidmap.m4
diff --git a/Makefile.am b/Makefile.am index 9f010d9..23f6093 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,6 +47,7 @@ localedir = @localedir@ nsslibdir = @nsslibdir@ pamlibdir = @pammoddir@ autofslibdir = @appmodpath@ +nfslibdir = @nfsidmaplibdir@
dbpath = @dbpath@ pluginpath = @pluginpath@ @@ -526,6 +527,7 @@ dist_noinst_HEADERS = \ src/tests/cmocka/common_mock_sysdb_objects.h \ src/sss_client/ssh/sss_ssh_client.h \ src/sss_client/sudo/sss_sudo.h \ + src/sss_client/nfs/nfsidmap_internal.h \ src/lib/idmap/sss_idmap_private.h
@@ -1666,6 +1668,23 @@ libnss_sss_la_LDFLAGS = \ -version-info 2:0:0 \ -Wl,--version-script,$(srcdir)/src/sss_client/sss_nss.exports
+ +if BUILD_NFS +nfslib_LTLIBRARIES = sss_nfs.la +sss_nfs_la_SOURCES = \ + src/sss_client/common.c \ + src/sss_client/nss_mc_common.c \ + src/util/io.c \ + src/util/murmurhash3.c \ + src/sss_client/nss_mc_passwd.c \ + src/sss_client/nss_mc_group.c \ + src/sss_client/nfs/sss_nfs_client.c \ + $(NULL) +sss_nfs_la_CFLAGS = $(AM_CFLAGS) +sss_nfs_la_LIBADD = $(CLIENT_LIBS) $(NFSIDMAP_LIBS) +sss_nfs_la_LDFLAGS = -module -avoid-version +endif + pamlib_LTLIBRARIES = pam_sss.la pam_sss_la_SOURCES = \ src/sss_client/pam_sss.c \ diff --git a/configure.ac b/configure.ac index 8295139..b440f92 100644 --- a/configure.ac +++ b/configure.ac @@ -91,6 +91,13 @@ AC_ARG_ENABLE([pammoddir], [AS_HELP_STRING([--enable-pammoddir], [pammoddir=$libdir/security]) AC_SUBST(pammoddir)
+#Set the NFSv4 idmapd library install path +AC_ARG_ENABLE([nfsidmaplibdir], [AS_HELP_STRING([--enable-nfsidmaplibdir], + [Where to install libnfsidmap libraries ($libdir/libnfsidmap)])], + [nfsidmaplibdir=$enableval], + [nfsidmaplibdir=$libdir/libnfsidmap]) +AC_SUBST(nfsidmaplibdir) + #Include here cause WITH_INIT_DIR requires $osname set in platform.m4 m4_include([src/external/platform.m4])
@@ -127,6 +134,8 @@ WITH_AUTOFS WITH_SSH WITH_CRYPTO WITH_SYSLOG +WITH_NFS +WITH_NFS_LIB_PATH
m4_include([src/external/pkg.m4]) m4_include([src/external/libpopt.m4]) @@ -159,6 +168,7 @@ m4_include([src/external/signal.m4]) m4_include([src/external/inotify.m4]) m4_include([src/external/libndr_nbt.m4]) m4_include([src/external/sasl.m4]) +m4_include([src/external/libnfsidmap.m4])
WITH_UNICODE_LIB if test x$unicode_lib = xlibunistring; then diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index 7111859..3f59b1f 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -646,3 +646,33 @@ AC_DEFUN([WITH_SSH], fi AM_CONDITIONAL([BUILD_SSH], [test x"$with_ssh" = xyes]) ]) + + AC_DEFUN([WITH_NFS], + [ AC_ARG_WITH([nfs], + [AC_HELP_STRING([--with-nfs], + [Whether to build with NFSv4 IDMAP support [yes]] + ) + ], + [with_nfs=$withval], + with_nfs=yes + ) + + if test x"$with_nfs" = xyes; then + AC_DEFINE(BUILD_NFS, 1, [whether to build with NFSv4 IDMAP support]) + fi + AM_CONDITIONAL([BUILD_NFS], [test x"$with_nfs" = xyes]) + ]) + +AC_DEFUN([WITH_NFS_LIB_PATH], + [ AC_ARG_WITH([nfs-lib-path], + [AC_HELP_STRING([--with-nfs-lib-path=<path>], + [Path to the nfs library [${libdir}]] + ) + ] + ) + nfslibpath="${libdir}" + if test x"$with_nfs_lib_path" != x; then + nfslibpath=$with_nfs_lib_path + fi + AC_SUBST(nfslibpath) + ]) diff --git a/src/external/libnfsidmap.m4 b/src/external/libnfsidmap.m4 new file mode 100644 index 0000000..5bb6d86 --- /dev/null +++ b/src/external/libnfsidmap.m4 @@ -0,0 +1,17 @@ +AC_SUBST(NFSIDMAP_OBJ) +AC_SUBST(NFSIDMAP_CFLAGS) +AC_SUBST(NFSIDMAP_LIBS) + +PKG_CHECK_MODULES([NFSIDMAP], [libnfsidmap], [found_nfsidmap=yes], + [found_nfsidmap=no]) + +SSS_AC_EXPAND_LIB_DIR() +AS_IF([test x"$with_nfs" = xyes -a x"$found_nfsidmap" != xyes], + [AC_CHECK_HEADER([nfsidmap.h], + [AC_CHECK_LIB([nfsidmap], + [nfs4_init_name_mapping], + [NFSIDMAP_LIBS="-L$sss_extra_libdir -lnfsidmap"], + [AC_MSG_ERROR([libnfsidmap missing nfs4_init_name_mapping])], + [-L$sss_extra_libdir])], + [AC_MSG_ERROR([libnfsidmap header files are not installed])])] +)
This patch needs rebasing on current master. For the sake of review, I rebased it locally -- mostly it was trivial "use both" case.
The functionality is fine, make, make check and make distcheck all pass.
On Tue, Mar 04, 2014 at 09:37:54AM +0200, Noam Meltzer wrote:
Makefile.am | 19 +++++++++++++++++++ configure.ac | 10 ++++++++++ src/conf_macros.m4 | 30 ++++++++++++++++++++++++++++++ src/external/libnfsidmap.m4 | 17 +++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 src/external/libnfsidmap.m4
diff --git a/Makefile.am b/Makefile.am index 9f010d9..23f6093 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,6 +47,7 @@ localedir = @localedir@ nsslibdir = @nsslibdir@ pamlibdir = @pammoddir@ autofslibdir = @appmodpath@ +nfslibdir = @nfsidmaplibdir@
dbpath = @dbpath@ pluginpath = @pluginpath@ @@ -526,6 +527,7 @@ dist_noinst_HEADERS = \ src/tests/cmocka/common_mock_sysdb_objects.h \ src/sss_client/ssh/sss_ssh_client.h \ src/sss_client/sudo/sss_sudo.h \
- src/sss_client/nfs/nfsidmap_internal.h \ src/lib/idmap/sss_idmap_private.h
@@ -1666,6 +1668,23 @@ libnss_sss_la_LDFLAGS = \ -version-info 2:0:0 \ -Wl,--version-script,$(srcdir)/src/sss_client/sss_nss.exports
+if BUILD_NFS +nfslib_LTLIBRARIES = sss_nfs.la +sss_nfs_la_SOURCES = \
- src/sss_client/common.c \
- src/sss_client/nss_mc_common.c \
- src/util/io.c \
- src/util/murmurhash3.c \
- src/sss_client/nss_mc_passwd.c \
- src/sss_client/nss_mc_group.c \
- src/sss_client/nfs/sss_nfs_client.c \
- $(NULL)
+sss_nfs_la_CFLAGS = $(AM_CFLAGS) +sss_nfs_la_LIBADD = $(CLIENT_LIBS) $(NFSIDMAP_LIBS) +sss_nfs_la_LDFLAGS = -module -avoid-version
I know this is serious bikeshedding, but With CFLAGS, LIBADD and LDFLAGS could you use the same format we use elsewhere?
sss_nfs_la_LIBADD = \ $(CLIENT_LIBS) \ $(NFSIDMAP_LIBS)
I'm not too keen about the format itself, but being consistent is good for maintenance :-)
The rest looks good to me.
On Fri, Jun 20, 2014 at 5:10 PM, Jakub Hrozek jhrozek@redhat.com wrote:
This patch needs rebasing on current master. For the sake of review, I rebased it locally -- mostly it was trivial "use both" case.
The functionality is fine, make, make check and make distcheck all pass.
On Tue, Mar 04, 2014 at 09:37:54AM +0200, Noam Meltzer wrote:
Makefile.am | 19 +++++++++++++++++++ configure.ac | 10 ++++++++++ src/conf_macros.m4 | 30 ++++++++++++++++++++++++++++++ src/external/libnfsidmap.m4 | 17 +++++++++++++++++ 4 files changed, 76 insertions(+) create mode 100644 src/external/libnfsidmap.m4
diff --git a/Makefile.am b/Makefile.am index 9f010d9..23f6093 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,6 +47,7 @@ localedir = @localedir@ nsslibdir = @nsslibdir@ pamlibdir = @pammoddir@ autofslibdir = @appmodpath@ +nfslibdir = @nfsidmaplibdir@
dbpath = @dbpath@ pluginpath = @pluginpath@ @@ -526,6 +527,7 @@ dist_noinst_HEADERS = \ src/tests/cmocka/common_mock_sysdb_objects.h \ src/sss_client/ssh/sss_ssh_client.h \ src/sss_client/sudo/sss_sudo.h \
- src/sss_client/nfs/nfsidmap_internal.h \ src/lib/idmap/sss_idmap_private.h
@@ -1666,6 +1668,23 @@ libnss_sss_la_LDFLAGS = \ -version-info 2:0:0 \ -Wl,--version-script,$(srcdir)/src/sss_client/sss_nss.exports
+if BUILD_NFS +nfslib_LTLIBRARIES = sss_nfs.la +sss_nfs_la_SOURCES = \
- src/sss_client/common.c \
- src/sss_client/nss_mc_common.c \
- src/util/io.c \
- src/util/murmurhash3.c \
- src/sss_client/nss_mc_passwd.c \
- src/sss_client/nss_mc_group.c \
- src/sss_client/nfs/sss_nfs_client.c \
- $(NULL)
+sss_nfs_la_CFLAGS = $(AM_CFLAGS) +sss_nfs_la_LIBADD = $(CLIENT_LIBS) $(NFSIDMAP_LIBS) +sss_nfs_la_LDFLAGS = -module -avoid-version
I know this is serious bikeshedding, but With CFLAGS, LIBADD and LDFLAGS could you use the same format we use elsewhere?
sss_nfs_la_LIBADD = \ $(CLIENT_LIBS) \ $(NFSIDMAP_LIBS)
I'm not too keen about the format itself, but being consistent is good for maintenance :-)
sure.
The rest looks good to me.
--- src/man/Makefile.am | 4 +- src/man/include/seealso.xml | 4 ++ src/man/sss_rpcidmapd.5.xml | 97 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/man/sss_rpcidmapd.5.xml
diff --git a/src/man/Makefile.am b/src/man/Makefile.am index eb87980..97a1b66 100644 --- a/src/man/Makefile.am +++ b/src/man/Makefile.am @@ -42,7 +42,9 @@ man_MANS = \ sssd.8 sssd.conf.5 sssd-ldap.5 \ sssd-krb5.5 sssd-ipa.5 sssd-simple.5 sssd-ad.5 \ sssd_krb5_locator_plugin.8 sss_groupshow.8 \ - pam_sss.8 sss_obfuscate.8 sss_cache.8 sss_debuglevel.8 sss_seed.8 + pam_sss.8 sss_obfuscate.8 sss_cache.8 sss_debuglevel.8 sss_seed.8 \ + sss_rpcidmapd.5 \ + $(NULL)
if BUILD_SSH man_MANS += sss_ssh_authorizedkeys.1 sss_ssh_knownhostsproxy.1 diff --git a/src/man/include/seealso.xml b/src/man/include/seealso.xml index 4f79431..ad6aaab 100644 --- a/src/man/include/seealso.xml +++ b/src/man/include/seealso.xml @@ -77,5 +77,9 @@ <citerefentry> <refentrytitle>pam_sss</refentrytitle><manvolnum>8</manvolnum> </citerefentry>. + <citerefentry> + <refentrytitle>sss_rpcidmapd</refentrytitle> + <manvolnum>5</manvolnum> + </citerefentry> </para> </refsect1> diff --git a/src/man/sss_rpcidmapd.5.xml b/src/man/sss_rpcidmapd.5.xml new file mode 100644 index 0000000..791870e --- /dev/null +++ b/src/man/sss_rpcidmapd.5.xml @@ -0,0 +1,97 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!DOCTYPE reference PUBLIC "-//OASIS//DTD DocBook V4.4//EN" +"http://www.oasis-open.org/docbook/xml/4.4/docbookx.dtd"> +<reference> +<title>SSSD Manual pages</title> +<refentry> + <refentryinfo> + <productname>sss_nfs</productname> + <orgname>Noam Meltzer, Primary Data Inc. - noam@primarydata.com</orgname> + </refentryinfo> + + <refmeta> + <refentrytitle>sss_rpcidmapd</refentrytitle> + <manvolnum>5</manvolnum> + <refmiscinfo class="manual">File Formats and Conventions</refmiscinfo> + </refmeta> + + <refnamediv id='name'> + <refname>sss_rpcidmapd</refname> + <refpurpose>sss_nfs plugin configuration directives for rpc.idmapd</refpurpose> + </refnamediv> + + <refsect1 id='conf-file'> + <title>CONFIGURATION FILE</title> + <para> + rpc.idmapd configuration file is usually found at + <emphasis>/etc/idmapd.conf</emphasis>. See + <citerefentry> + <refentrytitle>idmapd.conf</refentrytitle> + <manvolnum>5</manvolnum> + </citerefentry> for more information. + </para> + </refsect1> + + <refsect1 id='sss-conf-extension'> + <title>SSS CONFIGURATION EXTENSION</title> + <refsect2 id='enable-sss'> + <title>Enable SSS plugin</title> + <para> + In section <quote>[Translation]</quote>, modify/set + <quote>Method</quote> attribute to contain + <emphasis>sss_nfs</emphasis>. + </para> + </refsect2> + <refsect2 id='sss_nfs-conf-sect'> + <title>[sss_nfs] config section</title> + <para> + In order to change the default of one of the configuration + attributes of the <emphasis>sss_nfs</emphasis> plugin listed + below you will need to create a config section for it, named + <quote>[sss_nfs]</quote>. + </para> + <variablelist> + <title>Configuration attributes</title> + <varlistentry> + <term>memcache (bool)</term> + <listitem> + <para> + Indicates whether or not to use memcache + optimisation technique. + </para> + <para> + Default: True + </para> + </listitem> + </varlistentry> + </variablelist> + </refsect2> + </refsect1> + + <refsect1 id='sssd-integration'> + <title>SSSD INTEGRATION</title> + <para> + The sss_nfs plugin requires the <emphasis>NSS Responder</emphasis> + to be enabled in sssd. + </para> + <para> + The attribute <quote>use_fully_qualified_names</quote> must be + enabled on all domains (NFSv4 clients expect a fully qualified name + to be sent on the wire). + </para> + </refsect1> + + <refsect1 id='see_also'> + <title>SEE ALSO</title> + <para> + <citerefentry> + <refentrytitle>sssd</refentrytitle><manvolnum>8</manvolnum> + </citerefentry>, + <citerefentry> + <refentrytitle>idmapd.conf</refentrytitle> + <manvolnum>5</manvolnum> + </citerefentry> + </para> + </refsect1> +</refentry> +</reference>
Looks good to me, maybe except for the e-mail address.
On Tue, Mar 04, 2014 at 09:37:55AM +0200, Noam Meltzer wrote:
src/man/Makefile.am | 4 +- src/man/include/seealso.xml | 4 ++ src/man/sss_rpcidmapd.5.xml | 97 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 src/man/sss_rpcidmapd.5.xml
--- contrib/sssd.spec.in | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in index 055de4a..30ab9a5 100644 --- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -138,6 +138,11 @@ BuildRequires: systemd-devel %if (0%{?with_cifs_utils_plugin} == 1) BuildRequires: cifs-utils-devel %endif +%if ((0%{?fedora} >= 15) || (0%{?rhel} >= 7)) +BuildRequires: libnfsidmap-devel +%else +BuildRequires: nfs-utils-lib-devel +%endif
# RHEL 5 is too old to support samba4 and the PAC responder %if !0%{?is_rhel5} @@ -418,6 +423,7 @@ autoreconf -ivf --with-krb5-rcache-dir=%{_localstatedir}/cache/krb5rcache \ --enable-nsslibdir=/%{_lib} \ --enable-pammoddir=/%{_lib}/security \ + --enable-nfsidmaplibdir=%{_libdir}/libnfsidmap \ --disable-static \ --disable-rpath \ %{?with_ccache} \ @@ -567,6 +573,7 @@ rm -rf $RPM_BUILD_ROOT # 3rd party application libraries %{_libdir}/sssd/modules/libsss_autofs.so %{_libdir}/libsss_sudo.so +%{_libdir}/libnfsidmap/sss_nfs.so
%{ldb_modulesdir}/memberof.so %{_bindir}/sss_ssh_authorizedkeys @@ -597,6 +604,7 @@ rm -rf $RPM_BUILD_ROOT %{_mandir}/man5/sssd.conf.5* %{_mandir}/man5/sssd-simple.5* %{_mandir}/man5/sssd-sudo.5* +%{_mandir}/man5/sss_rpcidmapd.5* %{_mandir}/man8/sssd.8* %{_mandir}/man8/sss_cache.8* %{_mandir}/man1/sss_ssh_authorizedkeys.1*
So far I only tested on F-20 but there the RPMs built fine.
ACK
Hello,
On 03/04/2014 02:37 AM, Noam Meltzer wrote:
Hello sssd-devel & linux-nfs,
This is v2 of the patch set. It is now sent to linux-nfs as weel, following Steve Dickson request.
The code in the next 5 patches implements the design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
The changes in the code (compared to v1) are following the review done in sssd-devel. I believe that there are still two items in dispute (no final verdict was given in the sssd mailing list) but this new patch set will give a chance to have a cleaner look on things.
What exactly are you looking for from us to move this forward?
I would like to point out that the NFS client no longer uses the rpc.idmapd daemon to do its id mapping. Instead the kernel now does an upcall to the nfsidmap(5) command and the resulting uid/gid is stored in the kernel key chain, if a mapping is done at all...
V4 servers now have an option of returning a uid string (aka "3606") where the NFS client just converts that into the uid.
steved.
--
Noam Meltzer Linux Software Engineer PRIMARY DATA
P.O. Box 12650, Herzliya Pituach 4673300 9 Hamenofim St. Akerstein Towers, Tower A, 5th fl. Herzliya
Office: +972-77-8981888 | Fax: +972-3-7617140 | Mobile: +972-54-5873843
Email: noam@primarydata.com
--
Noam Meltzer (5): NEW CLIENT: plugin for NFSv4 rpc.idmapd NFSv4 client: (private) headers from libnfsidmap NFSv4 client: add to build system NFSv4 client: man page NFSv4 client: add to RPM spec
Makefile.am | 19 ++ configure.ac | 10 + contrib/sssd.spec.in | 8 + src/conf_macros.m4 | 30 ++ src/external/libnfsidmap.m4 | 17 + src/man/Makefile.am | 4 +- src/man/include/seealso.xml | 4 + src/man/sss_rpcidmapd.5.xml | 97 ++++++ src/sss_client/common.c | 5 + src/sss_client/nfs/nfsidmap_internal.h | 78 +++++ src/sss_client/nfs/sss_nfs_client.c | 569 +++++++++++++++++++++++++++++++++ src/sss_client/sss_cli.h | 2 + 12 files changed, 842 insertions(+), 1 deletion(-) create mode 100644 src/external/libnfsidmap.m4 create mode 100644 src/man/sss_rpcidmapd.5.xml create mode 100644 src/sss_client/nfs/nfsidmap_internal.h create mode 100644 src/sss_client/nfs/sss_nfs_client.c
On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote:
On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote:
V4 servers now have an option of returning a uid string (aka "3606") where the NFS client just converts that into the uid.
Can the client tell the server *not to do that* ?
The client can use kerberos, in which case the server won't do that.
Other than that, no, the behavior can only be controlled by server-side configuration.
--b.
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote:
On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote:
On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote:
V4 servers now have an option of returning a uid string (aka "3606") where the NFS client just converts that into the uid.
Can the client tell the server *not to do that* ?
The client can use kerberos, in which case the server won't do that.
This is sufficient, thanks.
Other than that, no, the behavior can only be controlled by server-side configuration.
A little sub-optimal if the server can do either but the client can have a choice, but ok.
Simo.
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce simo@redhat.com wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote:
On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote:
On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote:
V4 servers now have an option of returning a uid string (aka "3606") where the NFS client just converts that into the uid.
Can the client tell the server *not to do that* ?
The client can use kerberos, in which case the server won't do that.
This is sufficient, thanks.
Other than that, no, the behavior can only be controlled by server-side configuration.
A little sub-optimal if the server can do either but the client can have a choice, but ok.
Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
On 03/20/2014 03:33 AM, Noam Meltzer wrote:
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce <simo@redhat.com mailto:simo@redhat.com> wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote: > On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote: > > On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote: > > > > > V4 servers now have an option of returning a uid string (aka "3606") > > > where the NFS client just converts that into the uid. > > > > Can the client tell the server *not to do that* ? > > The client can use kerberos, in which case the server won't do that. This is sufficient, thanks. > Other than that, no, the behavior can only be controlled by server-side > configuration. A little sub-optimal if the server can do either but the client can have a choice, but ok.Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
Not sure since I don't maintain any of the code these patches change...
steved.
On Thu, Mar 20, 2014 at 09:49:05AM -0400, Steve Dickson wrote:
On 03/20/2014 03:33 AM, Noam Meltzer wrote:
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce <simo@redhat.com mailto:simo@redhat.com> wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote: > On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote: > > On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote: > > > > > V4 servers now have an option of returning a uid string (aka "3606") > > > where the NFS client just converts that into the uid. > > > > Can the client tell the server *not to do that* ? > > The client can use kerberos, in which case the server won't do that. This is sufficient, thanks. > Other than that, no, the behavior can only be controlled by server-side > configuration. A little sub-optimal if the server can do either but the client can have a choice, but ok.Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
Not sure since I don't maintain any of the code these patches change...
steved.
Does the code work with the current NFS idmap implementation or are there any changes on either side needed?
Simo, are there still any issues that would prevent us from merging the code into SSSD?
On Tue, 2014-05-27 at 21:46 +0200, Jakub Hrozek wrote:
On Thu, Mar 20, 2014 at 09:49:05AM -0400, Steve Dickson wrote:
On 03/20/2014 03:33 AM, Noam Meltzer wrote:
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce <simo@redhat.com mailto:simo@redhat.com> wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote: > On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote: > > On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote: > > > > > V4 servers now have an option of returning a uid string (aka "3606") > > > where the NFS client just converts that into the uid. > > > > Can the client tell the server *not to do that* ? > > The client can use kerberos, in which case the server won't do that. This is sufficient, thanks. > Other than that, no, the behavior can only be controlled by server-side > configuration. A little sub-optimal if the server can do either but the client can have a choice, but ok.Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
Not sure since I don't maintain any of the code these patches change...
steved.
Does the code work with the current NFS idmap implementation or are there any changes on either side needed?
Simo, are there still any issues that would prevent us from merging the code into SSSD?
I am not aware of any issue, but I have not found any time to do any testing, sorry :-/
Simo.
On Tue, May 27, 2014 at 04:02:36PM -0400, Simo Sorce wrote:
On Tue, 2014-05-27 at 21:46 +0200, Jakub Hrozek wrote:
On Thu, Mar 20, 2014 at 09:49:05AM -0400, Steve Dickson wrote:
On 03/20/2014 03:33 AM, Noam Meltzer wrote:
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce <simo@redhat.com mailto:simo@redhat.com> wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote: > On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote: > > On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote: > > > > > V4 servers now have an option of returning a uid string (aka "3606") > > > where the NFS client just converts that into the uid. > > > > Can the client tell the server *not to do that* ? > > The client can use kerberos, in which case the server won't do that. This is sufficient, thanks. > Other than that, no, the behavior can only be controlled by server-side > configuration. A little sub-optimal if the server can do either but the client can have a choice, but ok.Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
Not sure since I don't maintain any of the code these patches change...
steved.
Does the code work with the current NFS idmap implementation or are there any changes on either side needed?
Simo, are there still any issues that would prevent us from merging the code into SSSD?
I am not aware of any issue, but I have not found any time to do any testing, sorry :-/
Simo.
That's fine, some of us might help with the testing. But what worries me more is that mail to Noah Meltzer bounced back and I'd prefer if the original author was there at least for some time to fix issues..
I added one other person from primarydata to the CC list -- are you still interested in adding this code to SSSD?
On Thu, Mar 13, 2014 at 7:41 PM, Simo Sorce simo@redhat.com wrote:
On Thu, 2014-03-13 at 13:25 -0400, J. Bruce Fields wrote:
On Thu, Mar 13, 2014 at 12:58:59PM -0400, Simo Sorce wrote:
On Thu, 2014-03-13 at 11:24 -0400, Steve Dickson wrote:
V4 servers now have an option of returning a uid string (aka "3606") where the NFS client just converts that into the uid.
Can the client tell the server *not to do that* ?
The client can use kerberos, in which case the server won't do that.
This is sufficient, thanks.
Other than that, no, the behavior can only be controlled by server-side configuration.
A little sub-optimal if the server can do either but the client can have a choice, but ok.
Sorry for the late response. A misplaced email filtering rule had moved it away from my inbox. How can we push this forward?
On Tue, Mar 04, 2014 at 09:37:51AM +0200, Noam Meltzer wrote:
Hello sssd-devel & linux-nfs,
This is v2 of the patch set. It is now sent to linux-nfs as weel, following Steve Dickson request.
The code in the next 5 patches implements the design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
The changes in the code (compared to v1) are following the review done in sssd-devel. I believe that there are still two items in dispute (no final verdict was given in the sssd mailing list) but this new patch set will give a chance to have a cleaner look on things.
I have a couple of minor suggestions about the patches (mostly style issues), I'm going to reply to the patches themselves. I haven't yet done any testing, just visual review and sanity testing of the patches (make, make check, make distcheck, make rpms)
But more importantly, I wonder what the benefit for an administrator would be over using libnfsidmap/nsswitch.so ? Since the names must be fully qualified the raw getpwnam() interface would yield the same usernames as the sssd module, right? Is it perhaps better multi-domain support ?
On Fri, Jun 20, 2014 at 5:09 PM, Jakub Hrozek jhrozek@redhat.com wrote:
On Tue, Mar 04, 2014 at 09:37:51AM +0200, Noam Meltzer wrote:
Hello sssd-devel & linux-nfs,
This is v2 of the patch set. It is now sent to linux-nfs as weel,
following
Steve Dickson request.
The code in the next 5 patches implements the design document: https://fedorahosted.org/sssd/wiki/DesignDocs/rpc.idmapd%20plugin
The changes in the code (compared to v1) are following the review done in sssd-devel. I believe that there are still two items in dispute (no final verdict
was given
in the sssd mailing list) but this new patch set will give a chance to
have a
cleaner look on things.
I have a couple of minor suggestions about the patches (mostly style issues), I'm going to reply to the patches themselves. I haven't yet done any testing, just visual review and sanity testing of the patches (make, make check, make distcheck, make rpms)
But more importantly, I wonder what the benefit for an administrator would be over using libnfsidmap/nsswitch.so ? Since the names must be fully qualified the raw getpwnam() interface would yield the same usernames as the sssd module, right? Is it perhaps better multi-domain support ?
The main advantage of this new plugin would be in a NFS server which is not intended for users to log into. Allowing the NSS layer (i.e. the OS, in the common man language) to be aware of users is a potential security risk, and many times not desirable. The direct interface between rpc.idmapd and sssd solves this issue. It allows an administrator *not* to configure the OS to be aware of LDAP/AD users but rather only the NFS component.
- Noam
sssd-devel@lists.fedorahosted.org
-
J. Bruce Fields -
Jakub Hrozek -
Noam Meltzer -
Noam Meltzer -
Simo Sorce -
Steve Dickson