----- 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@@SAMBA_4.1.5' > > > >/usr/bin/ld: note: 'string_to_sid@@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. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel