On Fri, Oct 11, 2013 at 03:36:13PM +0200, Lukas Slebodnik wrote:
On (11/10/13 12:53), Benjamin Franzke wrote:
>2013/10/11 Sumit Bose <sbose(a)redhat.com>
>
>> On Thu, Oct 10, 2013 at 02:41:54PM +0200, Benjamin Franzke wrote:
>> > Thanks for your review!
>> >
>> <snip>
>> >
>> > The patch with your concers fixed is attached.
>>
>> Thank you for the new version. I only have two comments left:
>>
>> + if (handle == NULL || errmsg == NULL) {
>> + *errmsg = strerror(EINVAL);
>>
>> you cannot return an error message if errmsg == NULL.
>>
>
>yes, of course not :)
>had to leave house and wanted to get the patch out before, that dumb
>mistake was the result :\
>
>>
>> + return -EINVAL;
>> + }
>>
>>
>> + err = sss_nss_getsidbyname(name, &sid, &id_type);
>> + if (err != 0) {
>> + /* Might be a raw string representation of SID,
>> + * try converting that before returning an error. */
>> + if (sid_to_cifs_sid(ctx, name, csid) == 0)
>> + return 0;
>> +
>>
>> ^^^^^^^^ trailing whitespace
>>
>> Oops, again.. have them highlighted in my editor now ;)
>
>> + ctx_set_error(ctx, strerror(err));
>> + return -err;
>> + }
>>
>>
>> bye,
>> Sumit
>>
>
>New patch attached, with these two things fixed.
Inline.
From c548048ad7f7db530de2194d9a71ba7a265a34c4 Mon Sep 17 00:00:00 2001
From: Benjamin Franzke <benjaminfranzke(a)googlemail.com>
Date: Thu, 26 Sep 2013 10:27:33 +0200
Subject: [PATCH] Add CIFS idmap plugin
---
Makefile.am | 21 ++
configure.ac | 2 +
src/conf_macros.m4 | 14 ++
src/external/cifsidmap.m4 | 13 ++
src/lib/cifs_idmap_sss/cifs_idmap_sss.c | 339 ++++++++++++++++++++++++++++++++
5 files changed, 389 insertions(+)
create mode 100644 src/external/cifsidmap.m4
create mode 100644 src/lib/cifs_idmap_sss/cifs_idmap_sss.c
diff --git a/src/external/cifsidmap.m4 b/src/external/cifsidmap.m4
new file mode 100644
index 0000000000000000000000000000000000000000..546c373b0e66dd4136bca8d92cf220102b2f7830
--- /dev/null
+++ b/src/external/cifsidmap.m4
@@ -0,0 +1,13 @@
+AC_ARG_ENABLE([cifs-idmap-plugin],
+ [AS_HELP_STRING([--disable-cifs-idmap-plugin],
+ [do not build CIFS idmap plugin])],
+ [build_cifs_idmap_plugin=$enableval],
+ [build_cifs_idmap_plugin=yes])
^^^^^^
This line mean that cifs_idmap_plugin will be build by default,
but sssd.spec.in file is not updated with appropriate devel package.
Something like
%if (0%{?fedora} >= 17)
BuildRequires: cifs-utils-devel
%endif
BTW: cifs-utils-devel is not available on RHEL6, do we want to build
cifs_idmap_plugin by default?
And library cifs_idmap_sss.so should be added to some (or new) subpackage
in spec file. If you are not familiar with rpm packaging, we can help you.
Hi Lukas,
thank you for an additional pair of eyes. Ealier in the thread I said
that I'll take care of enhancing the spec file. I think
cifs_idmap_plugin should be build by default and disabled on platforms
like RHEL6 where cifs-utils do not uspport the plugin interface.
I've planned to add the plugin to the sssd-client package.
+
+AS_IF([test x$build_cifs_idmap_plugin = xyes],
+ [AC_CHECK_HEADER([cifsidmap.h], [],
+ [AC_MSG_ERROR([you must have the cifsidmap header installed to
build the idmap plugin])])
+ ])
+
+AM_CONDITIONAL([BUILD_CIFS_IDMAP_PLUGIN],
+ [test x$build_cifs_idmap_plugin = xyes])
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..bcea6d6d84dbd6184c2f719d0c1b4a2ac3c014ca
--- /dev/null
+++ b/src/lib/cifs_idmap_sss/cifs_idmap_sss.c
... snip ...
+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;
^^^^^^^
Missing brake statement.
Is it aim or mistake?
good catch
+ default:
+ cuxid->type = CIFS_UXID_TYPE_UNKNOWN;
+ return -1;
+ }
+
+ return 0;
+}
And the last note.
You created new dynamic library. Could you add this library to dlopen test?
+++ b/src/tests/dlopen-tests.c
@@ -40,6 +40,7 @@ struct so {
{ "libsss_debug.so", { LIBPFX"libsss_debug.so", NULL } },
{ "libipa_hbac.so", { LIBPFX"libipa_hbac.so", NULL } },
{ "libsss_idmap.so", { LIBPFX"libsss_idmap.so", NULL } },
+ { "cifs_idmap_sss.so", { LIBPFX"cifs_idmap_sss.so", NULL } },
{ "libsss_nss_idmap.so", { LIBPFX"libsss_nss_idmap.so", NULL }
},
{ "libnss_sss.so", { LIBPFX"libnss_sss.so", NULL } },
{ "pam_sss.so", { LIBPFX"pam_sss.so", NULL } },
Thank you very much .
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel