----- Original Message -----
On Fri, 2014-04-04 at 10:03 +0200, Jakub Hrozek wrote:
> On Fri, Apr 04, 2014 at 09:25:50AM +0200, Lukas Slebodnik wrote:
> > On (03/04/14 14:47), Yassir Elley wrote:
> > >Hi all,
> > >
> > >I have been using a samba function (string_to_sid) in my gpo code and
> > >have
> > >had no trouble so far compiling and linking my libsss_ad.so gpo code.
> > >
> > >I simply needed to apply the following patches, which includes having
> > >to declare the string_to_sid function before it is used in the file.
> > The function string_to_sid is from private samba library
> > libsamba-security.so
> > It is not declared in any public header file.
> >
> > sh-4.2$ grep -RniI string_to_sid /usr/include/
> > sh-4.2$ echo $?
> > 1
> > sh-4.2$ nm --dynamic --defined-only /usr/lib64/samba/libsamba-security.so
> > |
> > grep string_to
> > 0000000000008a80 T string_to_sid
> >
> > >--- /dev/null
> > >+++ b/src/providers/ad/ad_gpo.c
> > >@@ @@
> > >+
> > >+ bool string_to_sid(struct dom_sid *sidout, const char *sidstr);
> > >+
> > >--- a/Makefile.am
> > >+++ b/Makefile.am
> > >@@ @@ libsss_ad_la_SOURCES = \
> > > src/providers/ad/ad_access.h \
> > >+ src/providers/ad/ad_gpo.c \
> > >+ src/providers/ad/ad_gpo.h \
> > > src/providers/ad/ad_opts.h \
> > >
> > >
> > >However, now that I'm creating a gpo unit test (using the same
> > >string_to_sid
> > > function), the linker fails with the following message:
> > >/usr/bin/ld: src/tests/cmocka/ad_gpo_tests-test_ad_gpo.o: undefined
> > >reference to symbol 'string_to_sid@(a)SAMBA_4.1.5'
> > >/usr/bin/ld: note: 'string_to_sid@(a)SAMBA_4.1.5' is defined in DSO
> > >/usr/lib64/samba/libsamba-security.so so try adding it to the linker
> > >command line
> > >/usr/lib64/samba/libsamba-security.so: could not read symbols: Invalid
> > >operation
> > >
> > >I applied the following patches to get ad_gpo_test.c, which allowed me
> > > to compile successfully, but the link failed with the above message.
> > >--- a/Makefile.am
> > >+++ b/Makefile.am
> > >@@ @@ if HAVE_CMOCKA
> > > ad_access_filter_tests \
> > >+ ad_gpo_tests \
> > > ad_common_tests \
> > >@@ @@ ad_access_filter_tests_LDADD = \
> > >+ad_gpo_tests_SOURCES = \
> > >+ $(sssd_be_SOURCES) \
> > >+ src/util/sss_ldap.c \
> > >+ src/util/sss_krb5.c \
> > >+ src/util/find_uid.c \
> > >+ src/util/user_info_msg.c \
> > >+ src/providers/ad/ad_common.c \
> > >+ src/tests/cmocka/test_ad_gpo.c
> > >+ad_gpo_tests_CFLAGS = \
> > >+ $(AM_CFLAGS) \
> > >+ $(SYSTEMD_LOGIN_CFLAGS) \
> > >+ $(NDR_NBT_CFLAGS) \
> > >+ -DUNIT_TESTING
> > >+ad_gpo_tests_LDADD = \
> > >+ $(PAM_LIBS) \
> > >+ $(CMOCKA_LIBS) \
> > >+ $(SSSD_LIBS) \
> > >+ $(CARES_LIBS) \
> > >+ $(KRB5_LIBS) \
> > >+ $(SSSD_INTERNAL_LTLIBS) \
> > >+ $(SYSTEMD_LOGIN_LIBS) \
> > >+ $(NDR_NBT_LIBS) \
> > >+ libsss_ldap_common.la \
> > >+ libsss_idmap.la \
> > >+ libsss_krb5_common.la \
> > >+ libsss_ad.la \
> > >+ libsss_test_common.la
> > >+
> > > ad_common_tests_SOURCES = \
> > >
> > >If I apply the following patch, I am able to compile and link
> > >successfully!!:
> > >--- a/Makefile.am
> > >+++ b/Makefile.am
> > >@@ -1625,7 +1625,10 @@ ad_gpo_tests_CFLAGS = \
> > > $(SYSTEMD_LOGIN_CFLAGS) \
> > > $(NDR_NBT_CFLAGS) \
> > > -DUNIT_TESTING
> > >+ad_gpo_tests_LDFLAGS = \
> > >+ -rpath $(libdir)/samba
> > > ad_gpo_tests_LDADD = \
> > >+ $(libdir)/samba/libsamba-security.so \
> > > $(PAM_LIBS) \
> > > $(CMOCKA_LIBS) \
> > >
> > >I am confused by a few things:
> > >* Why do we need to explicitly add libsamba-security.so (and -rpath) to
> > >the
> > > linker line for the gpo tests, but not for libsss_ad.so
> > Because linker does not search libraries in directory
"$(libdir)/samba"
> > by
> > default.
> >
> > >* The patch that allows the gpo_tests to link/compile might not be
> > >portable
> > > b/c it assumes that libsamba-security.so is in $(libdir)/samba for all
> > > distributions. Is that a valid assumption?
> > There is no problem with location of library libsamba-security.so in
> > directory
> > "$(libdir)/samba". This library is private and API can be changed.
This
> > is only
> > problem I can see with poratability.
> >
> > >* One of the issues seems to be that the linker only looks in $(libdir)
> > >and
> > >$(libdir)/sssd, which is why adding the rpath works. Is this really the
> > >first
> > >time that we are using a library not in $(libdir) or $(libdir)/sssd. In
> > >other
> > >words, I would have expected this to have happened before (i.e. linking
> > >against a library not in the default locations).
> > In sssd, we can search libraries in "private" directory
"$(libdir)/sssd"
> > because we control API of these functions.
> >
> > >* Presumably, the libsss_ad.so gpo code links successfully because it is
> > >pulling in a library (which is internally pulling in libsamba-security),
> > > but I'm not sure which library that is. Any ideas?
> > >* Is there a better solution for getting the gpo tests to link?
> > Better solution will be to move necessary functions in upstream
> > to public samba libraries. Your solution can be temporary.
> >
> > bool string_to_sid(struct dom_sid *sidout, const char *sidstr);
> > int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen);
> > bool dom_sid_equal(const struct dom_sid *sid1, const struct dom_sid
> > *sid2);
>
> That's a good question for Samba developers. Linking with a private library
> is always a risk. Was there any conversation already about exporting these
> functions with Guenther maybe?
No, there was no such conversation.
>
> The alternative would be to add a configure time option
> (--with-samba-libraries?) that distributions could use to specify where
> the private Samba libraries are.
Do not link to private libraries, please!
Samba's private libraries cannot be used, they tend to change w/o
warning.
> A last-resort alternative for us would be to re-implement the functions
> ourselves.
We've done it a couple of times, perhaps even in sssd yet (Sumit's
mapping stuff) and in FreeIPA (MSPAC stuff), they are simple just take
them from there, or maybe we can come up with a library and change all
our callers.
Simo.
The consensus seems to be:
* Don't use functions that require linking to private samba libraries
* Use equivalent sssd functions; else, re-implement (i.e. essentially copy) the samba
functions
There are three functions that the GPO code is using from libsamba-security:
* string_to_sid() ... as sumit suspected, sss_idmap_sid_to_smb_sid() is indeed equivalent
for my purposes
* dom_sid_equal() ... reimplemented in 35 lines
* ndr_pull_security_descriptor() ... reimplemented in 445 lines (placed in separate
ad_gpo_ndr.c file b/c of large number of lines)
After making these changes, I no longer need to link against libsamba-security.so in order
to build and run my unit tests.
Thanks for all the feedback!
Regards,
Yassir.