On Wed, Oct 09, 2013 at 03:50:11PM +0200, Benjamin Franzke wrote:
Hi Sumit,
Sorry for the delay, got sidetracked by trying to find an appropriate way
to handle the well known sids etc..
But thanks to your reminder I got over to do the integration into sssd's
autoconf/make now :)
So here is the patch as an attachment.
(as recommended by the contrib page - but if you like it better for review
l'll git send-email it as well..)
Thanks a lot for the patch. It works well in my test but I have some
comments, see below.
This patch does not include an update for the rpm specs.
Could you do that integration? For me as a gentoo user that is currently an
unknown-area :)
Sure no problem.
Regards, Ben
2013/10/9 Sumit Bose <sbose(a)redhat.com>
> On Mon, Oct 07, 2013 at 12:22:13PM +0200, Benjamin Franzke wrote:
> > 2013/10/7 Sumit Bose <sbose(a)redhat.com>
> >
> > > On Thu, Oct 03, 2013 at 11:57:07PM +0200, Benjamin Franzke wrote:
> > > > >
> > > > > Hi Ben,
> > > > >
> > > > > this is really great, thank you very much for the contribution!
As
> a
> > > > > matter of fact, we've been planning on delivering the CIFS
plugin
> as
> > > > > part of the next release:
> > > > >
https://fedorahosted.org/sssd/ticket/1534
> > > > >
> > > > > Have you found the design page we created?
> > > > >
> > >
>
https://fedorahosted.org/sssd/wiki/DesignDocs/IntegrateSSSDWithCIFSClient
> > > >
> > > >
> > > > Yeah, I've read those pages before writing the plugin..
> > > >
> > > >
> > > > >
> > > > > Mostly the work on our side was done by Sumit Bose (I've
CC-ed
> him). I
> > > > > would propose to get in touch with him to avoid duplicating
some
> > > effort.
> > > > >
> > > > Unfortunately Sumit is in Germany and there is a holiday so he
won't
> be
> > > > > available until Monday.
> > > > >
> > > >
> > > > > In general, I would suggest to merge this code into the SSSD
tree
> if
> > > you
> > > > > wouldn't mind. I think it would makes sense to have the code
in the
> > > same
> > > > > repo as the ID mapping library.
> > > > >
> > > >
> > > > I'd really appreciate having it integrated in sssd tree,
> > > > to have centralized maintainance and adoption by distributions..
> > >
> > > Thanks a lot for the patch. I would suggest to put your code into
> > > src/lib/cifs_idmap_sss/ and modify SSSD's Makefile.am to build the
> > > plugin. Would you like to do the integration and send the result for
> > > review to this list again?
> > >
> >
> > I'd like to do that integration, will send a patch..
>
> I'd like to add the plugin to the next Fedora release which
> unfortunately has its deadline next week. I don't want to rush you, but
> do you have a chance to send the patch today or tomorrow? Otherwise
> would you mind if I prepare the patch to integrate your code into the
> SSSD tree?
>
> Thanks for your help.
>
> bye,
> Sumit
> >
> >
> > > About the well-know SIDs. They should not be handled inside the plugin
> > > but by SSSD itself. If you are interested in adding them I'd be happy
> to
> > > help you how to start (if my help is needed at all :-).
> > >
> >
> > Yup, sounds better to handle the SIDs by SSSD core. I'd like to try..
> > I'll see whether time permits me to do that soon.. (doing this as a part
> of
> > my thesis)
> >
> >
> >
> > >
> > > bye,
> > > Sumit
> > >
> > > >
> > > >
> > > > > Thanks again for the contribution, much appreciated!
> > > > > _______________________________________________
> > > > > sssd-devel mailing list
> > > > > sssd-devel(a)lists.fedorahosted.org
> > > > >
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > > > >
> > >
> > > > _______________________________________________
> > > > sssd-devel mailing list
> > > > sssd-devel(a)lists.fedorahosted.org
> > > >
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
> > > _______________________________________________
> > > sssd-devel mailing list
> > > sssd-devel(a)lists.fedorahosted.org
> > >
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> > >
>
> > _______________________________________________
> > sssd-devel mailing list
> > sssd-devel(a)lists.fedorahosted.org
> >
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
>
...
diff --git a/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
new file mode 100644
index 0000000000000000000000000000000000000000..f0599cb6b943145a685dbfd57bf5f8f48bfbfde6
--- /dev/null
+++ b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
@@ -0,0 +1,338 @@
+/*
+ Authors:
+ Benjamin Franzke <benjaminfranzke(a)googlemail.com>
+
+ Copyright (C) 2013 Benjamin Franzke
+
+ 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/>.
+*/
+
+/* TODO: Support well known SIDs as in samba's
+ * - librpc/idl/security.idl or
+ * - source4/rpc_server/lsa/lsa_lookup.c?
+ */
+
+/* TODO: Support of [all] samba's Unix SIDs:
+ * Users: S-1-22-1-%UID
+ * Groups: S-1-22-2-%GID
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
+#include <limits.h>
+#include <stdarg.h>
+
+#include <cifsidmap.h>
+
+#include "lib/idmap/sss_idmap.h"
+#include "sss_client/idmap/sss_nss_idmap.h"
+
+#define WORLD_SID "S-1-1-0"
+
+#ifdef DEBUG
+#include <syslog.h>
+#define debug(str, ...) \
+ syslog(0, "%s: " str "\n", \
+ __FUNCTION__, ##__VA_ARGS__)
+#else
+#define debug(...) do { } while(0)
+#endif
+
+struct sssd_ctx {
+ struct sss_idmap_ctx *idmap;
+ const char **errmsg;
+};
+
+#define ctx_set_error(ctx, error) \
+ do { \
+ *ctx->errmsg = error; \
+ debug("%s", error ? error : ""); \
+ } while (0);
+
+int cifs_idmap_init_plugin(void **handle, const char **errmsg)
+{
+ struct sssd_ctx *ctx;
+ enum idmap_error_code err;
it should be checked if handle or errmsg are NULL.
+
+ ctx = malloc(sizeof *ctx);
+ if (!ctx) {
+ *errmsg = "Failed to allocate context";
+ return -1;
+ }
+ ctx->errmsg = errmsg;
+ ctx_set_error(ctx, NULL);
+
+ err = sss_idmap_init(NULL, NULL, NULL, &ctx->idmap);
+ if (err != IDMAP_SUCCESS) {
+ ctx_set_error(ctx, idmap_error_string(err));
ctx should be freed to not leak memory.
+ return -1;
+ }
+
+ *handle = ctx;
+ return 0;
+}
+
+void cifs_idmap_exit_plugin(void *handle)
+{
+ struct sssd_ctx *ctx = handle;
+
+ debug("exit");
+
it should be check if ctx is NULL before using it.
+ sss_idmap_free(ctx->idmap);
+
+ free(ctx);
+}
+
+
+/* Test with `getcifsacl file` on client. */
+int cifs_idmap_sid_to_str(void *handle, const struct cifs_sid *csid,
+ char **name)
+{
+ struct sssd_ctx *ctx = handle;
+ enum idmap_error_code iderr;
+ char *sid;
+ enum sss_id_type id_type;
+ int err;
+
+ iderr = sss_idmap_bin_sid_to_sid(ctx->idmap,
+ (uint8_t *) csid, sizeof(*csid), &sid);
please use const uint8_t. Using sss_idmap_bin_sid_to_sid() works as expected
because struct cifs_sid is defined in the right order and is packed.
Nevertheless I think in the long run we should add something like
sss_idmap_cifs_sid_to_sid() to make the code more robust. But this does not
need to happen now.
+ if (iderr != IDMAP_SUCCESS) {
+ ctx_set_error(ctx, idmap_error_string(iderr));
+ *name = NULL;
+ return -1;
+ }
+
+ debug("sid: %s", sid);
+
+ if (strcmp(sid, WORLD_SID) == 0) {
+ *name = strdup("\\Everyone");
+ if (!*name) {
+ ctx_set_error(ctx, strerror(ENOMEM));
+ return -ENOMEM;
+ }
+ return 0;
+ }
+
+ err = sss_nss_getnamebysid(sid, name, &id_type);
+ if (err != 0) {
+ ctx_set_error(ctx, strerror(err));
+ *name = NULL;
+ return -err;
+ }
+
+ /* FIXME: Map Samba Unix SIDs? (sid->id and use getpwuid)? */
+
+ debug("name: %s", *name);
+
+ return 0;
+}
+
+static int sid_to_cifs_sid(struct sssd_ctx *ctx, const char *sid,
+ struct cifs_sid *csid)
+{
+ uint8_t *bsid = NULL;
+ enum idmap_error_code err;
+ size_t length;
+
+ err = sss_idmap_sid_to_bin_sid(ctx->idmap,
+ sid, &bsid, &length);
+ if (err != IDMAP_SUCCESS) {
+ ctx_set_error(ctx, idmap_error_string(err));
+ return -1;
+ }
+ if (length > sizeof(struct cifs_sid)) {
+ ctx_set_error(ctx, "too large sid length");
+ free(bsid);
+ return -1;
+ }
+
+ memcpy(csid, bsid, length);
+ free(bsid);
+
+ return 0;
+}
+
+/* Test with setcifsacl -a */
+int cifs_idmap_str_to_sid(void *handle, const char *name,
+ struct cifs_sid *csid)
+{
+ struct sssd_ctx *ctx = handle;
+ int err;
+ enum sss_id_type id_type;
+ const char *sid;
+ char *sss_sid = NULL;
+ int success = 0;
+
+ debug("%s", name);
+
+ if (strncmp("S-", name, 2) == 0) {
I think this check is not needed and puts an unnecessary limitation to the user
names.
+ debug("%s: name is sid string representation",
__FUNCTION__);
+ sid = name;
+ if (!sid) {
+ }
+ } else {
+ err = sss_nss_getsidbyname(name, &sss_sid, &id_type);
+ if (err != 0) {
+ ctx_set_error(ctx, strerror(err));
+ /* TODO: Map name==Everyone to WOLD_SID? */
+ return -err;
+ }
+ sid = sss_sid;
+ }
+
+ if (sid_to_cifs_sid(ctx, sid, csid) != 0)
+ success = -1;
+
+ if (sss_sid)
the if here is not needed, free() can handle NULL.
+ free(sss_sid);
+
+ return success;
+}
+
+static int samba_unix_sid_to_id(const char *sid, struct cifs_uxid *cuxid)
+{
+ id_t id;
+ uint8_t type;
+
+ if (sscanf(sid, "S-1-22-%hhu-%u", &type, &id) != 2)
+ return -1;
+
+ switch (type) {
+ case 1:
+ cuxid->type = CIFS_UXID_TYPE_UID;
+ cuxid->id.uid = id;
+ break;
+ case 2:
+ cuxid->type = CIFS_UXID_TYPE_GID;
+ cuxid->id.gid = id;
+ default:
+ cuxid->type = CIFS_UXID_TYPE_UNKNOWN;
+ return -1;
+ }
+
+ return 0;
+}
+
+static int sss_sid_to_id(struct sssd_ctx *ctx, const char *sid,
+ struct cifs_uxid *cuxid)
+{
+ int err;
+ enum sss_id_type id_type;
+
+ err = sss_nss_getidbysid(sid, (uint32_t *)&cuxid->id.uid, &id_type);
+ if (err != 0) {
+ ctx_set_error(ctx, strerror(err));
+ return -1;
+ }
+
+ switch (id_type) {
+ case SSS_ID_TYPE_UID:
+ cuxid->type = CIFS_UXID_TYPE_UID;
+ break;
+ case SSS_ID_TYPE_GID:
+ cuxid->type = CIFS_UXID_TYPE_GID;
+ break;
+ case SSS_ID_TYPE_BOTH:
+ cuxid->type = CIFS_UXID_TYPE_BOTH;
+ break;
+ case SSS_ID_TYPE_NOT_SPECIFIED:
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+/**
+ * cifs_idmap_sids_to_ids - convert struct cifs_sids to struct cifs_uxids
+ * usecase: mount.cifs -o sec=krb5,multiuser,cifsacl,nounix
there is a trailing whitespace ^
+ * test: ls -n on mounted share
+ */
+int cifs_idmap_sids_to_ids(void *handle, const struct cifs_sid *csid,
+ const size_t num, struct cifs_uxid *cuxid)
+{
+ struct sssd_ctx *ctx = handle;
+ enum idmap_error_code err;
+ int success = -1;
+ size_t i;
+ char *sid;
+
+ debug("num: %zd", num);
+
+ if (num > UINT_MAX) {
+ ctx_set_error(ctx, "num is too large.");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; ++i) {
+ err = sss_idmap_bin_sid_to_sid(ctx->idmap, (uint8_t *) &csid[i],
+ sizeof(csid[i]), &sid);
please use const uint8_t here as well.
+ if (err != IDMAP_SUCCESS) {
+ ctx_set_error(ctx, idmap_error_string(err));
+ continue;
+ }
+
+ cuxid[i].type = CIFS_UXID_TYPE_UNKNOWN;
+
+ if (sss_sid_to_id(ctx, sid, &cuxid[i]) == 0 ||
there is a trailing whitespace ^
+ samba_unix_sid_to_id(sid, &cuxid[i]) == 0) {
+
+ debug("setting uid of %s to %d", sid, cuxid[i].id.uid);
+ success = 0;
+ }
+
+ free(sid);
+ }
+
+ return success;
+}
+
+
+int cifs_idmap_ids_to_sids(void *handle, const struct cifs_uxid *cuxid,
+ const size_t num, struct cifs_sid *csid)
+{
+ struct sssd_ctx *ctx = handle;
+ int err, success = -1;
+ char *sid;
+ enum sss_id_type id_type;
+ size_t i;
+
+ debug("num ids: %zd", num);
+
+ if (num > UINT_MAX) {
+ ctx_set_error(ctx, "num is too large.");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; ++i) {
+ err = sss_nss_getsidbyid((uint32_t)cuxid[i].id.uid, &sid, &id_type);
+ if (err != 0) {
+ ctx_set_error(ctx, strerror(err));
+ csid[i].revision = 0;
+ /* FIXME: would it be safe to map *any* uid/gids unknown by sssd to
+ * SAMBA's UNIX SIDs? */
+ continue;
+ }
+
+ if (sid_to_cifs_sid(ctx, sid, csid) == 0)
+ success = 0;
+ else
+ csid[i].revision = 0;
+ free(sid);
+ }
+
+ return success;
+}
--
1.8.1.5