sssd-devel November 2015

sssd-devel@lists.fedorahosted.org

12 participants
80 discussions

[RFC] Use of DP_ERR_* codes
by Pavel Březina 06 Nov '15

06 Nov '15

Hi! Since we have pretty nice custom error codes, I would like to get rid of DP_ERR_* codes. They are practically useless and are adding non-trivial complexity to the code. I did only a quick look so it is possible that I missed something, but let's talk about it. We have the following codes: > #define DP_ERR_OK 0 DP_ERR_OK is usually used in combination with EOK and in most cases it is treated as error (or worse not handled at all). There are some situations when DP_ERR_OK and !EOK is used, it is mostly in combination with PAM error code and access control, but we can handle this with custom error codes. > #define DP_ERR_OFFLINE 1 DP_ERR_OFFLINE means EAGAIN, always. This should be changed to ERR_OFFLINE or similar custom error code. > #define DP_ERR_TIMEOUT 2 This is not used at all. > #define DP_ERR_FATAL 3 This is other than EOK code and should be avoided. Having only one error code from sdap operations would simplify code flow and make the code less error prone. In my opinion this should be part of or even prerequisite to ldap provider and sdap_id_op refactoring.

2 2

release 1.13.2
by Jakub Hrozek 06 Nov '15

06 Nov '15

Hi, I would like to release 1.13.2 soon. I'd like to only leave: https://fedorahosted.org/sssd/ticket/2779 and: https://fedorahosted.org/sssd/ticket/2861 in 1.13.2 so we can release it early next week. in the milestone and move the others to 1.13.3. In order to not make 1.13.3 too big, I would also like to move these tickets into CI milestone: * https://fedorahosted.org/sssd/ticket/2820 * https://fedorahosted.org/sssd/ticket/2823 And this one to 1.14 as we are phasing 1.13 into more of a maintenance mode (we can always push patches to sssd-1-13): * https://fedorahosted.org/sssd/ticket/2791 * https://fedorahosted.org/sssd/ticket/1372 If there are more tickets to demote, please bring them up or move them with an explanatory message.

2 1

Review of umask() in SSSD
by Petr Cech 05 Nov '15

05 Nov '15

Hi, I am reviewing umask() in our code according to https://fedorahosted.org/sssd/ticket/2424 There are many use like umask(DFL_RSP_UMASK): src/responder/autofs/autofssrv.c:223 src/responder/ifp/ifpsrv.c:401 src/responder/nss/nsssrv.c:589 src/responder/pac/pacsrv.c:232 src/responder/pam/pamsrv.c:369 src/responder/ssh/sshsrv.c:209 src/responder/sudo/sudosrv.c:215 where DFL_RSP_UMASK is defined as 0177. There are another three use of umask 0177: src/confdb/confdb.c:662 src/util/debug.c:365 src/util/server.c:495 And then I see many use of umask 077: src/p11_child/p11_child_nss.c:485 src/providers/krb5/krb5_child.c:723 src/tests/check_and_open-tests.c:51 src/tests/debug-tests.c:136 src/tests/debug-tests.c:276 src/tests/util-tests.c:596 src/util/domain_info_utils.c:312 src/util/domain_info_utils.c:562 src/tools/tools_util.c:503 I would like to ask you if we would like to use 0077 or 0177 as our very restrictive mask. I see that our code is not consistent on this question. I know the difference is small, but it is. Then we have some unsecure use: src/providers/ipa/selinux_child.c:154: umask = 0 src/providers/krb5/krb5_ccache.c:188: umask = 0000 src/responder/nss/nsssrv_mmap_cache.c:1121: umask = 0022 but I think there is reason for it. And the last one is at src/responder/common/responder_common.c:561: int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval) We use it secure (0177) at: src/responder/common/responder_common.c:693 src/responder/pam/pamsrv.c:399 And not so secure: src/responder/common/responder_common.c:670 umask = 0111 src/responder/pam/pamsrv.c:391 umask = 0111 src/tests/cwrap/test_responder_common.c:173 umask = 0111 src/tests/cwrap/test_responder_common.c:179 umask = 0000 So, what could I do? Maybe we could have only one very secure umask and maybe we could have CONSTANT for every use of umask. Any another ideas? Regards Petr

3 17

[PATCH] BUILD: Remove sudo doxygen file
by Lukas Slebodnik 05 Nov '15

05 Nov '15

ehlo, There aren't any documented files in directory src/sss_client/sudo/ LS

2 2

[PATCH] LDAP: Fix leak of file descriptors
by Lukas Slebodnik 05 Nov '15

05 Nov '15

ehlo, Details are in commit message. BTW it would be good to have at least two reviews. Reproducer which I used for https://fedorahosted.org/sssd/ticket/2792: Setup: * two active directories with sites; so sometimes sssd connect to server A and sometims to server B. * block connection to one server [root@host sssd]# iptables -n -L Chain INPUT (policy ACCEPT) target prot opt source destination Chain FORWARD (policy ACCEPT) target prot opt source destination Chain OUTPUT (policy ACCEPT) target prot opt source destination DROP tcp -- 0.0.0.0/0 10.12.0.158 tcp dpt:389 * force sssd to go offline (-USR1) and online (-USR2) You might be able to reproduce ti even with plan LDAP. It might help if set value of options that dns_resolver_timeout < ldap_network_timeout Do you have an idea how to test such fd leak? (either using cmocka or integration tests) LS

2 7

[PATCH] BUILD: Fix doc directory for sss_simpleifp
by Lukas Slebodnik 04 Nov '15

04 Nov '15

ehlo, make all docs && make install DESTDIR=`pwd`/_instdir will not install doxygen generated files for sss_simpleifp because directory was wrong. Simple patch is attached. LS

2 2

[PATCH] NSS: fix a use-after-free issue
by Sumit Bose 04 Nov '15

04 Nov '15

Hi, I found this accidentally because I was still running SSSD with MALLOC_PERTURB_ set which I used some time ago to hunt a glibc issue. This issue wasn't caught before by the unit-tests because sss_cmd_done() which frees the context is overwritten in the tests and so far didn't free the context. Additionally, since the use after free was in a debug message, the unit tests must be run with debugging enable to run over it. I think this is also the reason why Coverity or Clang didn't found this issue before. Does anyone know if it is possible to tell Converity to assume debug_level is set to 10? bye, Sumit

3 9

[PATCH] PAM: successful authentication sets explicitly PAM_SUCCESSS
by Pavel Reichl 03 Nov '15

03 Nov '15

Hello, while I worked on tests for PAM responder Sumit proposed to use attached patch. QA have already kindly run their tests (successfully). I'll do the review myself (But all opinions are welcomed for sure). Thanks

3 6

[PATCH] sudo: remove unused param name in sdap_sudo_get_usn()
by Pavel Reichl 02 Nov '15

02 Nov '15

Hello, please see simple patch attached. Thanks!

3 3

[PATCH] SDAP: Remove unused sdap_id_ctx from sdap_id_conn_cache_create
by Lukas Slebodnik 02 Nov '15

02 Nov '15

ehlo, simple patch is attached. LS

2 3

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

sssd-devel November 2015