[PATCH] krb5: Check return value of sss_krb5_princ_realm
by Lukas Slebodnik
ehlo,
yet another warning from clang static analyser.
sss_krb5_princ_realm set output parameter realm to NULL and len to 0
in case of failure. Clang static analysers repoted warning
"Null pointer passed as an argument to a 'nonnull' parameter"
in function match_principal. It was possible, that realm_name with value NULL
could be used in strncmp.
Function sss_krb5_princ_realm is used on other places for printing(formatting)
realm_name and NULL can be safely used as a argument for printf-like
functions.
Patch is attached.
LS
9 years
[PATCH] LDAP: Try all attributes when saving an entry
by Jakub Hrozek
Hi,
this bug was reported on #sssd by a user. He was running some flavor of
IBM Tivoli where the entries only had an "ID", not separate "UID" and
"GID". But due to a bug in sssd he couldn't use the same value for both,
this configuration:
ldap_user_uid_number = idAttribute
ldap_user_gid_number = idAttribute
only saved the ID into UID and left GID empty. It appears we have a long
standing bug in sdap_parse where we only consider first match. If this
patch is accepted, I would also like to refactor sdap_parse in master
because currently it is a 250-lines long function with multiple
branches..
9 years, 5 months
Build Fixes
by Benjamin Franzke
Hi,
These two patches add missing CFLAGS/LIBS to Makefile.am:
[PATCH 1/2] BUILD: Link libsss_ad.so to sasl libs
[PATCH 2/2] BUILD: Use OPENLDAP_CFLAGS instead of LDAP_CFLAGS
This underlinking was noticed in make check (dlopen-test).
Note:
It failed for me since my openldap build had no sasl support,
which would otherwise have pulled in libsasl2.so.
Of course, that support should be in place, but the linking should still be
fixed.
BTW: It would propably be nice to have a configure check whether
openldap has sasl support, but it seems that would need a check if
ldap_sasl_interactive_bind returns LDAP_NOT_SUPPORTED.
Regards, Ben
9 years, 6 months
[PATCH] BUILD: Link libsss_krb5_common.so to libkeyutils.so
by Benjamin Franzke
Hi List,
The symbol add_key is used by
src/providers/krb5/krb5_delayed_online_authentication.c
which is part of libsss_krb5_common.so
Fixes following error:
[sssd[be[default]]] [load_backend_module]
(0x0010): Unable to load ad module with path
(/usr/lib64/sssd/libsss_ad.so), error:
/usr/lib64/sssd/libsss_krb5_common.so: undefined symbol: add_key
-lkeyutils was passed to the libraries libsss_{krb5,ipa,ad}.so,
but when compiling with -Wl,--as-needed this flag will be ignored,
since it is not used directly. So it was unavailable to
libsss_krb5_common.so which actually needs it.
This patch removes $(KEYUTILS_LIBS) from those libraries and adds it to
libsss_krb5_common.so
Maybe libsss_krb5_common.so should be added to dlopen-tests?
But then other libraries and functions are needed as well,
which it currently inherits from libsss_{krb5,ipa,ad}.so.
BTW: are these common libraries (i mean ldap too) convenience build
libraries, or to save disk space?
If they're just for convencience maybe they should not be installed?
Regards, Ben
9 years, 7 months
[PATCH] nested groups unit test
by Pavel Březina
Hi,
this is the first unit test for nested groups. It covers only the most
basic situation when we are trying to resolve one group with no members.
Even though it is only one test, the patch set is quite big. This is
because it creates the possibility to mock providers related modules.
Most of the patches are just a preparation for unit testing providers.
Patches 1-5
Moves the code around to reduce number of dependencies. (E.g. you do not
want to load fail over when you are testing nested groups.)
Patch 6
Mocks basic SDAP interface.
Patch 7
Mocks sysdb objects - currently user and rfc2307bis group. You can
decide what set of attributes the object should posses. For example,
creating a user requires only basedn and name parameter, to construct
originalDN and name attributes. The rest is provided by (attrname,
value) pairs via variadic function.
E.g.:
mock_sysdb_user(mem_ctx, basedn, name, SYSDB_UIDNUM, uid, ...)
get_attr_type() translates the sysdb attribute name to proper data type.
This should be extended as needed.
Patch 8
Adds provider tests related common object files and cflags in makefile
Patch 9
New macro sss_will_return_always(fn, value). This can be used to mock
function data in such way that any call of mock() will return the value.
It was just pushed also to cmocka upstream as will_return_always().
Patch 10-11
Unit test.
Patch 12
Removes a noisy debug message.
I would like to get this reviewed before I continue with more test
cases, so the framework is tuned enough.
Off topic:
I also created new macro called fail_msg, which will make the test fail
printing a message. I didn't use this macro in the end, but it made its
way to cmocka upstream.
9 years, 9 months
[INI][PATCHES] All of them (C-Style Comments, Unicode conversion, Space processing, UNIT tests)
by Dmitri Pal
Hello,
This mail contains all the patches that I have so far in my git.
--------------------------
The following set of patches is related to comments and unchanged
from the previous submission.
I replied to the comments about them in another mail.
I do not think there is anything to address.
If there is please let me know.
0001-INI-Do-not-check-validity-of-comments.patch
0002-INI-Extend-error-set-and-add-parsing-error.patch
0003-INI-Process-c-style-comments.patch
0004-INI-Test-files-for-unit-test.patch
0005-INI-Unit-test-for-c-style-comments.patch
--------------------------
The following set of patches addresses comments and fixes
bugs in Unicode/UTF8 conversion
0006-INI-Expose-buffer-context-as-void.patch
0007-INI-Extend-internal-file-handle.patch
0008-INI-Convert-files-to-UTF.patch
0009-INI-Updated-unit-test-for-UTF8-conversion.patch
Thanks for review Lukas. I reworked the code significantly.
I think I addressed everything except the initialization comment.
I agree with you that it would have been better to
initialize things outside the loop but I need to read the first buffer
to be able to determine the encoding. This is what causes this logic.
I do not see a way to avoid it so left that part as is.
----------------------------
The following one liner fixes a trace text
0010-INI-Fix-typo-in-comment.patch
----------------------------
The following patches are related to the problem I noticed earlier
and recorded in the ticket https://fedorahosted.org/sssd/ticket/2119
But in reality the cause was bug in the Unicode conversion
and a bug with space processing.
0011-INI-Fix-processing-of-the-white-space-at-the-end-of-.patch
0012-INI-Unit-test-for-space-trimming-in-multiline-values.patch
----------------------------
The following patch adds more unit test for both Unicode conversion
and folding by combining the tests together. The unit tests written
for each of them individually are not sufficient to catch corner cases.
This test does. And I played with different constants before checking
in to make sure that all sorts of combinations are caught.
I decided to settle on these numbers because they provide
sufficient amount of testing but do not extend the time too much.
Right now all make check tests take about 7-8 seconds on my laptop.
0013-INI-Adding-more-unit-tests.patch
--
Thank you,
Dmitri Pal
Sr. Engineering Manager for IdM portfolio
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
9 years, 11 months
[PATCHES] Heimdal Support
by Benjamin Franzke
Hi list,
I've tried to use sssd with heimdal, there were some fixes to be done.
Are you intrested in reviewing and integrating them?
They are available at: https://git.bnfr.net/sssd/log/?h=heimdal-1
Note: They are on top of other build fixes i've send to the list (but thats
visible in the log).
This compiles without warnings and passes all make tests.
Actually i've added alternatives for deprecated (in terms of heimdal)
kerberos functions to avoid warnings there.
I've tested this in a samba 4 environment (with the sssd-ad module).
Regards, Ben
9 years, 11 months
DEBUG macro refactoring
by Nikolai Kondrashov
Hi everyone,
I've been looking at the sssd sources for some test failure investigation and
noticed that the DEBUG macro expands to quite a lot of code. I decided to see
if it can be reduced and what the impact will be. The result is the patches
that follow.
I've noticed that the total size of installed binaries produced by the
standard Fedora build with "reconfig && chmake" drops by 20% after applying
these patches.
A build on Debian, configured similarly to the one used in the Debian package
dropped by 44%.
I've verified that all the patches build and pass the tests run by "chmake",
but otherwise haven't tested them.
Sincerely,
Nick
9 years, 11 months