New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NSS: Avoid changing the memory cache ownership away from the sssd user (sssd-1-16 backport) #717
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…'t exist Previously, we tried to optimize too much and only set the SELinux user to Linux user mapping in case the SELinux user was different from the system default. But this doesn't work for the case where the Linux user has a non-standard home directory, because then SELinux would not have any idea that this user's home directory should be labeled as a home directory. This patch relaxes the optimization in the sense that on the first login, the SELinux context is saved regardless of whether it is the same as the default or different. Resolves: https://pagure.io/SSSD/sssd/issue/3819 Reviewed-by: Michal Židek <mzidek@redhat.com> (cherry picked from commit 945865a)
To make sure that SSSD has synced with the latest data added to the passwd file sss_cache is called in two places where the current sync scheme was not reliable. This was mainly observed when running the integration tests on Debian. Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 1e23988)
If the domain is not case sensitive and the case of the original user or group name differs from the name in the rule we failed to find the rule. Now we filter the rule only with lower cased values in such domain. Steps to reproduce: 1. Add user/group with upper case, e.g. USER-1 2. Add sudo rule with lower cased name, e.g. sudoUser: user-1 3. Login to system with lower case, e.g. user-1 4. Run sudo -l Without the patch, rule is not found. Resolves: https://pagure.io/SSSD/sssd/issue/3820 Reviewed-by: Michal Židek <mzidek@redhat.com> (cherry picked from commit d7f0b58)
This code: pkcs11_txt.write("library=libsoftokn3.so\nname=soft\n" + "parameters=configdir='sql:" + config.ABS_BUILDDIR + "/../test_CA/p11_nssdb' " + "dbSlotDescription='SSSD Test Slot' " + "dbTokenDescription='SSSD Test Token' " + "secmod='secmod.db' flags=readOnly)\n\n") pkcs11_txt.close() Was producing warnings such as: ./src/tests/intg/test_pam_responder.py:143:22: W504 line break after binary operator Even though it looks OK visually and conforms to pep8's written form. Additionaly, this regular expression compilation: Template = re.compile( ' *<template name="(\S+)">(.*?)</template>\r?\n?', re.MULTILINE | re.DOTALL ) Was producing a warning such as: ./src/sbus/codegen/sbus_Template.py:156:29: W605 invalid escape sequence '\S' Since the \S literal is part of a regular expression, let's suppress this warning as well. Reviewed-by: Michal Židek <mzidek@redhat.com> (cherry picked from commit ec76659)
Otherwise we end up with memory leak since the result is never freed. We need to convert nctx->*ent structures into talloc pointer so we can use enum_ctx as parent. Resolves: https://pagure.io/SSSD/sssd/issue/3870 Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 406b731)
Related: https://pagure.io/SSSD/sssd/issue/3451 A tevent _send() function should only return NULL on ENOMEM, otherwise it should mark the request as failed but return the req pointer. This was not much of an issue, before, but the next patch will add another function call to the auth_send call which would make error handling awkward. Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit 09091b4)
…r information Related: https://pagure.io/SSSD/sssd/issue/3451 Commit add7286 initially addressed SSSD#3451 by using the full sdap_cli_connect() request during LDAP authentication. This was a good idea as it addressed the case where the authentication connection must also look up some user information (typically with id_provider=proxy where you don't know the DN to bind as during authentication), but this approach also broke the use-case of id_provider=ldap and auth_provider=ldap with ldap_sasl_auth=gssapi. This is because (for reason I don't know) AD doesn't like if you use both GSSAPI and startTLS on the same connection. But the code would force TLS during the authentication as a general measure to not transmit passwords in the clear, but then, the connection would also see that ldap_sasl_auth=gssapi is set and also bind with GSSAPI. This patch checks if the user DN is already known and if yes, then doesn't authenticate the connection as the connection will then only be used for the user simple bind. Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit 57fc60c)
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 6f113c7)
The macro CURLE_SSL_CACERT is deprecated in upstream curl since commit 3f3b26d6feb0667714902e836af608094235fca2. commit 3f3b26d6feb0667714902e836af608094235fca2 Author: Han Han <hhan@thousandeyes.com> Date: Wed Aug 22 11:13:32 2018 -0700 ssl: deprecate CURLE_SSL_CACERT in favour of a unified error code Long live CURLE_PEER_FAILED_VERIFICATION sh$ git tag --contains 3f3b26d6feb0667714902e836af608094235fca2 curl-7_62_0 It was not removed. It is just an alias to CURLE_PEER_FAILED_VERIFICATION which causes compile time failures in switch/case. ./src/util/tev_curl.c: In function 'curl_code2errno': ./src/util/tev_curl.c:113:5: error: duplicate case value case CURLE_PEER_FAILED_VERIFICATION: ^~~~ ./src/util/tev_curl.c: 100:5: note: previously used here case CURLE_SSL_CACERT: ^~~~ Merges: https://pagure.io/SSSD/sssd/pull-request/3878 Resolves: https://pagure.io/SSSD/sssd/issue/3875 Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit 1ee12b0)
Merges: https://pagure.io/SSSD/sssd/pull-request/3881 Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit 4f824ec)
It will show reasons why tests were skipped. e.g. ====================== test session starts ======================== platform linux -- Python 3.7.1, pytest-3.9.3, py-1.5.4, pluggy-0.7.1 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /dev/shm/sssd/src/tests/intg, inifile: collected 286 items / 285 deselected test_pac_responder.py::test_multithreaded_pac_client SKIPPED [100%] ==================== short test summary info ====================== SKIP [1] test_pac_responder.py:108: No PAC responder, skipping Merges: https://pagure.io/SSSD/sssd/pull-request/3881 Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit fdbe67a)
Valgrind does not generate full stack trace for errors. It is just limited amount of frames. Therefore we cannot see main function with the new c-ares. The suppression file generated with c-ares-1.14.0 { <insert_a_suppression_name_here> Memcheck:Leak match-leak-kinds: possible fun:malloc fun:strdup fun:ares_init_options fun:recreate_ares_channel fun:resolv_init fun:be_res_init fun:be_res_init fun:be_init_failover fun:test_ipa_server_create_trusts_setup obj:/usr/lib64/libcmocka.so.0.5.1 fun:_cmocka_run_group_tests fun:main } The suppression file generated with c-ares-1.15.0 { <insert_a_suppression_name_here> Memcheck:Leak match-leak-kinds: possible fun:malloc fun:strdup obj:/usr/lib64/libcares.so.2.3.0 obj:/usr/lib64/libcares.so.2.3.0 fun:ares_init_options fun:recreate_ares_channel fun:resolv_init fun:be_res_init fun:be_res_init fun:be_init_failover fun:test_ipa_server_create_trusts_setup obj:/usr/lib64/libcmocka.so.0.5.1 fun:_cmocka_run_group_tests } Merges: https://pagure.io/SSSD/sssd/pull-request/3884 Reviewed-by: Sumit Bose <sbose@redhat.com> (cherry picked from commit f02714d)
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 53e6fdf)
To just test some ccache related functionality without talking to an actual KDC to get the tickets some needed libkrb5 structs were mocked based on tests from the MIT Kerberos source code. One struct member (is_skey) was so far not regarded by libkrb5 for out test case. But a recent fix for http://krbdev.mit.edu/rt/Ticket/Display.html?id=8718 changed this and we have to change the mocking. Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 08bba3a)
With recent version of valgrind some tests failed during a CI run with a timeout. To avoid this the related p11_child_timeout is increased for the affected tests. Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 1617f3e)
While creating the domains and sub-domains each domain gets a global catalog services assigned but only one should be used because the global catalog is by definition responsible for the whole forest so it does not make sense to use a global catalog service for each domain and in the worst case connect to the same GC multiple times. In the AD provider this is simple because the GC service of the configured domain AD_GC_SERVICE_NAME ("AD_GC") can be used. In the IPA case all domains from the trusted forest are on the level of sub-domains so we have to pick one. Since the forest root is linked from all domain of the same forest it will be the most straight forward choice. Related to https://pagure.io/SSSD/sssd/issue/3902 Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 62d671b)
For empty home directory in passwd file sssd returns "/". Sssd should respect system behaviour and return the same as nsswitch "files" module - return empty string. Resolves: https://pagure.io/SSSD/sssd/issue/3901 Reviewed-by: Simo Sorce <simo@redhat.com> Reviewed-by: Jakub Hrozek <jhrozek@redhat.com> (cherry picked from commit 90f3239)
Resolves: https://pagure.io/SSSD/sssd/issue/3890 In case SSSD is compiled --with-sssd-user but run as root (which is the default on RHEL and derivatives), then the memory cache will be owned by the user that sssd_nss runs as, so root. This conflicts with the packaging which specifies sssd.sssd as the owner. And in turn, this means that users can't reliably assess the package integrity using rpm -V. This patch makes sure that the memory cache files are chowned to sssd.sssd even if the nss responder runs as root. Also, this patch changes the sssd_nss responder so that is becomes a member of the supplementary sssd group. Even though in traditional UNIX sense, a process running as root could write to a file owned by sssd:sssd, with SELinux enforcing mode this becomes problematic as SELinux emits an error such as: type=AVC msg=audit(1543524888.125:1495): avc: denied { fsetid } for pid=7706 comm="sssd_nss" capability=4 scontext=system_u:system_r:sssd_t:s0 tcontext=system_u:system_r:sssd_t:s0 tclass=capability To make it possible for the sssd_nss process to write to the files, the files are also made group-writable. The 'others' permission is still set to read only. Reviewed-by: Michal Židek <mzidek@redhat.com> (cherry picked from commit 61e4ba5)
hmm, this is supposed to be merged to sssd-1-16.. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves: https://pagure.io/SSSD/sssd/issue/3890
In case SSSD is compiled --with-sssd-user but run as root (which is the
default on RHEL and derivatives), then the memory cache will be owned by
the user that sssd_nss runs as, so root.
This conflicts with the packaging which specifies sssd.sssd as the owner.
And in turn, this means that users can't reliably assess the package
integrity using rpm -V.
This patch makes sure that the memory cache files are chowned to sssd.sssd
even if the nss responder runs as root.
Also, this patch changes the sssd_nss responder so that is becomes a member
of the supplementary sssd group. Even though in traditional UNIX sense, a
process running as root could write to a file owned by sssd:sssd, with
SELinux enforcing mode this becomes problematic as SELinux emits an error
such as:
type=AVC msg=audit(1543524888.125:1495): avc: denied { fsetid } for
pid=7706 comm="sssd_nss" capability=4 scontext=system_u:system_r:sssd_t:s0
tcontext=system_u:system_r:sssd_t:s0 tclass=capability
To make it possible for the sssd_nss process to write to the files, the
files are also made group-writable. The 'others' permission is still set to
read only.
Reviewed-by: Michal Židek mzidek@redhat.com
(cherry picked from commit 61e4ba5)