[SSSD] [sssd PR#47][opened] BUILD: Fix build without /sbin/service installed on the build host
URL: https://github.com/SSSD/sssd/pull/47
Author: jhrozek
Title: #47: BUILD: Fix build without /sbin/service installed on the build host
Action: opened
PR body:
"""
There were some issues in the sssctl-related patches that we pushed
recently. First, the build failed if no service binary was around, which
is wrong, we should just proceed and build without the sssctl functionality.
Second, we should make sure that on RHEL-6, /sbin/service is around during
both build and runtime.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/47/head:pr47
git checkout pr47
From ece168576cad2dded3bb7a506b2ade05677120ec Mon Sep 17 00:00:00 2001
From: Jakub Hrozek
Date: Tue, 11 Oct 2016 20:41:02 +0200
Subject: [PATCH 1/2] BUILD: Not having /sbin/service is not fatal
If the target platform does not have the service executable, we must not
fail the build, but proceed, just disabling the sssctl functionality.
---
src/external/service.m4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/external/service.m4 b/src/external/service.m4
index f475f26..709c204 100644
--- a/src/external/service.m4
+++ b/src/external/service.m4
@@ -7,7 +7,7 @@ AC_DEFUN([CHECK_SERVICE_EXECUTABLE],
AC_MSG_RESULT(yes)
else
AC_MSG_RESULT([no])
-AC_MSG_ERROR([the service executable is not available])
+AC_MSG_WARN([the service executable is not available])
fi
]
)
From c6a278b1b3ad032e078664a09a42cfcacc5d5a7c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek
Date: Tue, 11 Oct 2016 20:48:44 +0200
Subject: [PATCH 2/2] RPM: Require initscripts on non-systemd platforms
In order for sssctl to work on platforms that do not use systemd, we
need to BuildRequire initscripts so that they are pulled in during build
and also Require them for sssd-tools so that the binary can be invoked.
---
contrib/sssd.spec.in | 5 +
1 file changed, 5 insertions(+)
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index 40e4454..a795431 100644
--- a/contrib/sssd.spec.in
+++ b/contrib/sssd.spec.in
@@ -197,6 +197,8 @@ BuildRequires: nss_wrapper
BuildRequires: libnl3-devel
%if (0%{?use_systemd} == 1)
BuildRequires: systemd-devel
+%else
+BuildRequires: initscripts
%endif
%if (0%{?with_cifs_utils_plugin} == 1)
BuildRequires: cifs-utils-devel
@@ -296,6 +298,9 @@ Requires: python3-sssdconfig = %{version}-%{release}
Requires: python2-sss = %{version}-%{release}
Requires: python2-sssdconfig = %{version}-%{release}
%endif
+%if (0%{?use_systemd} == 0)
+Requires: initscripts
+%endif
%description tools
Provides userspace tools for manipulating users, groups, and nested groups in
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: Monotonic clock for timed events
On Mon, 2016-10-10 at 11:19 +0200, Jakub Hrozek wrote: > On Mon, Oct 10, 2016 at 11:09:35AM +0200, Victor Tapia wrote: > > El 10/10/16 a las 10:56, Ian Kent escribió: > > > On Mon, 2016-10-10 at 10:42 +0200, Jakub Hrozek wrote: > > >> On Mon, Oct 10, 2016 at 10:04:30AM +0200, Victor Tapia wrote: > > >>> Hi list, > > >>> > > >>> I've faced a race condition when SSSD boots in a machine with a big > > >>> clock drift. This is what I see: > > >>> > > >>> 1. SSSD starts before the network is up, queries the LDAP server without > > >>> success and sets a retry timer (~60 secs) > > >>> 2. NTP starts and corrects the clock, 1 hour back for example. > > >>> 3. SSSD takes ~60 secs + the drift correction (1 hour) to retry the > > >>> connection. > > >>> > > >>> In this particular scenario the credentials cache is disabled, so the > > >>> wait time to login is noticeable. How feasible would it be to use a > > >>> monotonic clock for this kind of timed events? > > >> > > >> I really have not tried this and I guess I don't know tevent internals > > >> well enough if this works, but I wonder if just using: > > >> clock_getime() > > > > > > With a CLOCK_MONOTONIC I presume? > > > I think that's what has been suggested. > > > > > > > I was thinking about using CLOCK_MONOTONIC_RAW: > > > > CLOCK_MONOTONIC_RAW (since Linux 2.6.28; Linux-specific) > > Similar to CLOCK_MONOTONIC, but provides access to a raw hardware-based > > time that is not subject to NTP adjustments or the incremental > > adjustments performed by adjtime(3). > > > > >> and constructing struct timeval > > >> in place of: > > >> tevent_timeval_current_ofs() > > >> could solve this particular issue. > > >> > > >> On the other hand, this is a pattern we use in SSSD all through the code > > >> for timed events and we're just not well equipped to handle time drifts. > > >> Did you investigate why doesn't sssd detect the networking change from > > >> libnl messages or from resolv.conf being touched? > > > > I didn't dig much into it yet (I just checked tevent to confirm it uses > > gettimeofday()), so I'll take this as my next step. > > btw the samba-technical mailing list is the best source of info about > libtevent and the best place to ask questions about libtevent.. We should suggest that libtevent starts using timerfd with epoll to do time tracking instead of the internal computations, and then a monotonic clock can be used with some calculation at set time to convert the time of day to the current monotonic clock. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: Monotonic clock for timed events
On Mon, 2016-10-10 at 14:04 +0200, Pavel Březina wrote: > On 10/10/2016 10:09 AM, Fabiano Fidêncio wrote: > > Victor, > > > > On Mon, Oct 10, 2016 at 10:04 AM, Victor Tapia > > wrote: > >> Hi list, > >> > >> I've faced a race condition when SSSD boots in a machine with a big > >> clock drift. This is what I see: > >> > >> 1. SSSD starts before the network is up, queries the LDAP server without > >> success and sets a retry timer (~60 secs) > >> 2. NTP starts and corrects the clock, 1 hour back for example. > >> 3. SSSD takes ~60 secs + the drift correction (1 hour) to retry the > >> connection. > >> > >> In this particular scenario the credentials cache is disabled, so the > >> wait time to login is noticeable. How feasible would it be to use a > >> monotonic clock for this kind of timed events? > > > > Are you running git master? This issue is supposed to be already > > solved by > > https://github.com/SSSD/sssd/commit/b8ceaeb80cffb00c26390913ea959b77f7e848b9 > > This patch fix the issue only in watchdog which would result in > terminating sssd otherwise. Fixing it across whole sssd would be > difficult. The fix should go to tevent. It also seem to fix the issue only if the time jumps backwards, not if it jumps forward, in that case if I read the code right, we'd still end up killing sssd. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: Monotonic clock for timed events
On Mon, 2016-10-10 at 10:04 +0200, Victor Tapia wrote: > Hi list, > > I've faced a race condition when SSSD boots in a machine with a big > clock drift. This is what I see: > > 1. SSSD starts before the network is up, queries the LDAP server without > success and sets a retry timer (~60 secs) > 2. NTP starts and corrects the clock, 1 hour back for example. > 3. SSSD takes ~60 secs + the drift correction (1 hour) to retry the > connection. > > In this particular scenario the credentials cache is disabled, so the > wait time to login is noticeable. How feasible would it be to use a > monotonic clock for this kind of timed events? We should use a monotonic clock for most internal events. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][+Pushed] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API Label: +Pushed ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][comment] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API jhrozek commented: """ * master: 03713a6..761515e """ See the full comment at https://github.com/SSSD/sssd/pull/30#issuecomment-252905006 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][closed] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Author: pbrezina Title: #30: sssctl: use systemd D-Bus API Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/30/head:pr30 git checkout pr30 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][-Accepted] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API Label: -Accepted ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#45][closed] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Author: jhrozek Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME Action: closed To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/45/head:pr45 git checkout pr45 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#45][+Pushed] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME Label: +Pushed ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#45][comment] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME jhrozek commented: """ * master: * eb9bc1c590b8c3b3b58574c70d9fe5357ef3e901 * 03713a6444fdecd01465b9d5fbfead9601adce6e """ See the full comment at https://github.com/SSSD/sssd/pull/45#issuecomment-252903922 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#45][comment] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME sumit-bose commented: """ The patches look good and make sense. ACK """ See the full comment at https://github.com/SSSD/sssd/pull/45#issuecomment-252898723 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#45][+Accepted] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME Label: +Accepted ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates
On Thu, Oct 06, 2016 at 12:49:30PM +0200, Sumit Bose wrote:
> Hi,
>
> I've started to write a SSSD design page about enhancing the current
> mapping of certificates to users and how to select/match a suitable
> certificate if multiple certificates are on a Smartcard.
>
> My currently thoughts and idea and be found at
> https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates
> and for your convenience below as well.
>
> Comments and suggestions are welcome. Please let me know about concerns,
> alternatives and missing use-cases/user-stories.
>
> bye,
> Sumit
>
Hi,
Rob, Fraser, Alexander, thank you for your comments. I think both the
issuer specific matching and the OID in the SUBJECT matching are good
ideas. I updated the design page accordingly. The changes can be shown
with
https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates?action=diff&version=9&old_version=6
The updated version can be found below as well. Of course more comments and
suggestions are still very welcome.
bye,
Sumit
= Matching and Mapping Certificates =
Related ticket(s):
* http://www.freeipa.org/page/V4/User_Certificates#Certificate_Identity_Mapping
=== Problem statement ===
Mapping
Currently it is required that a certificate used for authentication is either
stored in the LDAP user entry or in a matching override. This might not always
be applicable and other ways are needed to relate a user with a certificate.
Matching
Even if SSSD will support multiple certificates on a Smartcard in the context
of https://fedorahosted.org/sssd/ticket/3050 it might be necessary to restrict
(or relax) the current certificate selection in certain environments.
=== Use cases ===
Mapping
In some environments it might not be possible or would cause unwanted effort to
add certificates to the LDAP entry of the users to allow Smartcard based
authentication. Reasons might be:
* Certificates/Smartcards are issued externally
* LDAP schema extension is not possible or not allowed
Matching
A user might have multiple certificate on a Smartcard which are suitable for
authentication. But on some host in the environment only certificates from a
specific CA (while all other CAs are trusted as well) or with some special
extension should be valid for login.
=== Overview of the solution ===
To match a certificate a language/syntax has to be defined which allows to
reference items from the certificate and compare the values with the expected
data. To map the certificates to a user the language/syntax should allow to
relate certificate items with LDAP attributes so that the value(s) from the
certificate item can be used in a LDAP search filter.
=== Implementation details ===
Matching
The pkinit plugin of MIT Kerberos must find a suitable certificate from a
Smartcard as well and has defined the following syntax (see the
pkinit_cert_match section of the krb5.conf man page or
http://web.mit.edu/Kerberos/krb5-1.14/doc/admin/conf_files/krb5_conf.html for
details). The main components are
* regular-expression
* regular-expression
* regular-expression
* extended-key-usage-list
* key-usage-list
and can be grouped together with a prefixed '&&' (and) or '`||`' (or) operator
('&&' is the default). If multiple rules are given they are iterated with the
order in the config file as long as a rule matches exactly one certificate.
'''Question: MIT Kerberos use case-sensitive matching and POSIX Extended
Regular Expression syntax, shall we do the same?'''
While and are (imo) already quite flexible I can see some
potential extensions for the other components.
and in MIT Kerberos only accept certain string values related to
some allowed values in those field as defined in
https://www.ietf.org/rfc/rfc3280.txt . The selection is basically determined by
what is supported on server side of the pkinit plugin of MIT Kerberos. Since we
plan to extend pkinit and support local authentication without pkinit as well I
would suggest to allow OID strings for those components as well (the comparison
is done on the OID level nonetheless).
The component in MIT Kerberos only checks the otherName SAN component for
the id-pkinit-san OID as defined in https://www.ietf.org/rfc/rfc4556.txt or the
szOID_NT_PRINCIPAL_NAME OID as mentioned in
https://support.microsoft.com/en-us/kb/287547. While this is sufficient for the
default pkinit user case of MIT Kerberos I would suggest to extend this
component by allowing to specific an OID with
= Issuer specific matching =
Although the MIT Kerberos rules allow to select the issuer of a certificate
there are use cases where a more specific selection is needed. E.g. if there
are some default matching rules for all issuers and some other issuer specific
rules where the default rules should not apply. To make this possible with the
above scheme the default rules must have an clause which matches all
but the iss
[SSSD] [sssd PR#45][comment] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45 Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME fidencio commented: """ @jhrozek: code-wise the bew patches look good. However, I really would like to have a review from @sumit-bose here as well before considering this series ACK'ed. """ See the full comment at https://github.com/SSSD/sssd/pull/45#issuecomment-252893042 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][comment] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API pbrezina commented: """ No problem. Feel free to push it. """ See the full comment at https://github.com/SSSD/sssd/pull/30#issuecomment-252873755 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][+Accepted] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API Label: +Accepted ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][-Changes requested] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API Label: -Changes requested ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#30][comment] sssctl: use systemd D-Bus API
URL: https://github.com/SSSD/sssd/pull/30 Title: #30: sssctl: use systemd D-Bus API jhrozek commented: """ On Mon, Oct 10, 2016 at 03:04:35AM -0700, Pavel Březina wrote: > Hi, your patch looks good to me. There was no specific reason, see > https://github.com/SSSD/sssd/pull/30#issuecomment-251485499 So if you agree, I will push the patchset with my RB for your patches and your RB for my patch. (And yes, sorry, I knew we talked about this in the past, I just disliked the non-absolute patch when I saw it in the code..which is also why I wanted to write the patch myself and not to ask you to do X after asking you to do Y before..) """ See the full comment at https://github.com/SSSD/sssd/pull/30#issuecomment-252871228 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#34][comment] cache_req: move from switch to plugins
URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins
jhrozek commented:
"""
Well, it's still a lot of code :) but at least I started the review now. For
starters, Coverity found some warnings:
```
Error: FORWARD_NULL (CWE-476):
sssd-1.14.2/src/responder/common/cache_req/cache_req_search.c:104:
var_compare_op: Comparing "*_result" to null implies that "*_result" might be
null.
sssd-1.14.2/src/responder/common/cache_req/cache_req_search.c:114:
var_deref_op: Dereferencing null pointer "*_result".
# 112| cr->debugobj, ret, sss_strerror(ret));
# 113| return ret;
# 114|-> } else if (cr->plugin->only_one_result && (*_result)->count > 1) {
# 115| CACHE_REQ_DEBUG(SSSDBG_CRIT_FAILURE, cr,
# 116| "Multiple objects were found when "
Error: UNINIT (CWE-457):
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_group_by_filter.c:34:
var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_group_by_filter.c:68:
uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling
"_talloc_free".
# 66|
# 67| done:
# 68|-> talloc_free(tmp_ctx);
# 69| return ret;
# 70| }
Error: UNINIT (CWE-457):
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_group_by_name.c:34:
var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_group_by_name.c:74:
uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling
"_talloc_free".
# 72|
# 73| done:
# 74|-> talloc_free(tmp_ctx);
# 75| return ret;
# 76| }
Error: UNINIT (CWE-457):
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_initgroups_by_name.c:34:
var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_initgroups_by_name.c:74:
uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling
"_talloc_free".
# 72|
# 73| done:
# 74|-> talloc_free(tmp_ctx);
# 75| return ret;
# 76| }
Error: UNINIT (CWE-457):
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_user_by_filter.c:34:
var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_user_by_filter.c:68:
uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling
"_talloc_free".
# 66|
# 67| done:
# 68|-> talloc_free(tmp_ctx);
# 69| return ret;
# 70| }
Error: UNINIT (CWE-457):
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_user_by_name.c:34:
var_decl: Declaring variable "tmp_ctx" without initializer.
sssd-1.14.2/src/responder/common/cache_req/plugins/cache_req_user_by_name.c:74:
uninit_use_in_call: Using uninitialized value "tmp_ctx" when calling
"_talloc_free".
# 72|
# 73| done:
# 74|-> talloc_free(tmp_ctx);
# 75| return ret;
# 76| }
```
"""
See the full comment at
https://github.com/SSSD/sssd/pull/34#issuecomment-252864641
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#46][comment] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam operations jhrozek commented: """ ok to test """ See the full comment at https://github.com/SSSD/sssd/pull/46#issuecomment-252854747 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#46][comment] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam operations centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/46#issuecomment-252853017 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#46][comment] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46 Title: #46: sss_client: Defer thread cancellation until completion of nss/pam operations centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/46#issuecomment-252853023 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#46][opened] sss_client: Defer thread cancellation until completion of nss/pam operations
URL: https://github.com/SSSD/sssd/pull/46
Author: HouzuoGuo
Title: #46: sss_client: Defer thread cancellation until completion of nss/pam
operations
Action: opened
PR body:
"""
https://fedorahosted.org/sssd/ticket/3156
The client code is not cancellation-safe, an application which has cancelled an
NSS operation
will experience subtle bugs, hence thread cancellation is deferred until
completion of client
operations.
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/46/head:pr46
git checkout pr46
From 176a84c1add79398f342157a12945838bbb1ef0e Mon Sep 17 00:00:00 2001
From: Howard Guo
Date: Tue, 11 Oct 2016 10:35:13 +0200
Subject: [PATCH] sss_client: Defer thread cancellation until completion of
nss/pam operations
https://fedorahosted.org/sssd/ticket/3156
The client code is not cancellation-safe, an application which has cancelled an NSS operation
will experience subtle bugs, hence thread cancellation is deferred until completion of client
operations.
---
Makefile.am | 4 ---
src/sss_client/common.c | 74 +
2 files changed, 7 insertions(+), 71 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 1ea8f50..ca561de 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -771,10 +771,6 @@ endif
CLIENT_LIBS = $(LTLIBINTL)
-if HAVE_PTHREAD
-CLIENT_LIBS += -lpthread
-endif
-
if WITH_JOURNALD
SYSLOG_LIBS = $(JOURNALD_LIBS)
endif
diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 20106b1..61ec55e 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -1071,85 +1071,33 @@ struct sss_mutex {
pthread_mutex_t mtx;
pthread_once_t once;
-sss_mutex_init init;
+int old_cancel_state;
};
-static void sss_nss_mt_init(void);
-static void sss_pam_mt_init(void);
static void sss_nss_mc_mt_init(void);
static struct sss_mutex sss_nss_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_nss_mt_init };
+.once = PTHREAD_ONCE_INIT };
static struct sss_mutex sss_pam_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
-.once = PTHREAD_ONCE_INIT,
-.init = sss_pam_mt_init };
+.once = PTHREAD_ONCE_INIT };
static struct sss_mutex sss_nss_mc_mtx = { .mtx = PTHREAD_MUTEX_INITIALIZER,
- .once = PTHREAD_ONCE_INIT,
- .init = sss_nss_mc_mt_init };
-
-/* Wrappers for robust mutex support */
-static int sss_mutexattr_setrobust (pthread_mutexattr_t *attr)
-{
-#ifdef HAVE_PTHREAD_MUTEXATTR_SETROBUST
-return pthread_mutexattr_setrobust(attr, PTHREAD_MUTEX_ROBUST);
-#elif defined(HAVE_PTHREAD_MUTEXATTR_SETROBUST_NP)
-return pthread_mutexattr_setrobust_np(attr, PTHREAD_MUTEX_ROBUST_NP);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-static int sss_mutex_consistent(pthread_mutex_t *mtx)
-{
-#ifdef HAVE_PTHREAD_MUTEX_CONSISTENT
-return pthread_mutex_consistent(mtx);
-#elif defined(HAVE_PTHREAD_MUTEX_CONSISTENT_NP)
-return pthread_mutex_consistent_np(mtx);
-#else
-#warning Robust mutexes are not supported on this platform.
-return 0;
-#endif
-}
-
-/* Generic mutex init, lock, unlock functions */
-static void sss_mt_init(struct sss_mutex *m)
-{
-pthread_mutexattr_t attr;
-
-if (pthread_mutexattr_init(&attr) != 0) {
-return;
-}
-if (sss_mutexattr_setrobust(&attr) != 0) {
-return;
-}
-
-pthread_mutex_init(&m->mtx, &attr);
-pthread_mutexattr_destroy(&attr);
-}
+ .once = PTHREAD_ONCE_INIT };
static void sss_mt_lock(struct sss_mutex *m)
{
-pthread_once(&m->once, m->init);
-if (pthread_mutex_lock(&m->mtx) == EOWNERDEAD) {
-sss_cli_close_socket();
-sss_mutex_consistent(&m->mtx);
-}
+pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &m->old_cancel_state);
+pthread_mutex_lock(&m->mtx);
}
static void sss_mt_unlock(struct sss_mutex *m)
{
pthread_mutex_unlock(&m->mtx);
+pthread_setcancelstate(m->old_cancel_state, NULL);
}
/* NSS mutex wrappers */
-static void sss_nss_mt_init(void)
-{
-sss_mt_init(&sss_nss_mtx);
-}
void sss_nss_lock(void)
{
sss_mt_lock(&sss_nss_mtx);
@@ -1160,10 +1108,6 @@ void sss_nss_unlock(void)
}
/* NSS mutex wrappers */
-static void sss_pam_mt_init(void)
-{
-sss_mt_init(&sss_pam_mtx);
-}
void sss_pam_lock(void)
{
sss_mt_lock(&sss_pam_mtx);
@@ -1174,10 +1118,6 @@ void sss_pam_unlock(void)
}
/* NSS mutex wrappers */
-static void sss_nss_mc_mt_init(void)
-{
-sss_mt_init(&sss_nss_mc_mtx);
-}
void sss_nss_mc_lock(void)
{
sss_mt_lock(&sss_nss_mc_mtx);
_
[SSSD] [sssd PR#45][synchronized] tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and SSS_NSS_GETORIGBYNAME
URL: https://github.com/SSSD/sssd/pull/45
Author: jhrozek
Title: #45: tests: Add unit tests for UPNs in SSS_NSS_GETSIDBYNAME and
SSS_NSS_GETORIGBYNAME
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/45/head:pr45
git checkout pr45
From b3f4b9b94d089ee9f05fe5045a29893aa86e644f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek
Date: Mon, 26 Sep 2016 10:44:16 +0200
Subject: [PATCH 1/2] tests: Add tests for sidbyname NSS operation
---
src/tests/cmocka/test_nss_srv.c | 129
1 file changed, 129 insertions(+)
diff --git a/src/tests/cmocka/test_nss_srv.c b/src/tests/cmocka/test_nss_srv.c
index 41425e7..8e9f5fe 100644
--- a/src/tests/cmocka/test_nss_srv.c
+++ b/src/tests/cmocka/test_nss_srv.c
@@ -3446,6 +3446,129 @@ void test_nss_getnamebycert_neg(void **state)
assert_int_equal(nss_test_ctx->ncache_hits, 1);
}
+struct passwd sid_user = {
+.pw_name = discard_const("testusersid"),
+.pw_uid = 1234,
+.pw_gid = 5678,
+.pw_dir = discard_const("/home/testusersid"),
+.pw_gecos = discard_const("test user"),
+.pw_shell = discard_const("/bin/sh"),
+.pw_passwd = discard_const("*"),
+};
+
+static int test_nss_getsidbyname_check(uint32_t status,
+ uint8_t *body,
+ size_t blen)
+{
+const char *name;
+enum sss_id_type type;
+size_t rp = 2 * sizeof(uint32_t);
+char *expected_result = sss_mock_ptr_type(char *);
+
+if (expected_result == NULL) {
+assert_int_equal(status, EINVAL);
+assert_int_equal(blen, 0);
+} else {
+assert_int_equal(status, EOK);
+
+SAFEALIGN_COPY_UINT32(&type, body+rp, &rp);
+
+name = (char *) body+rp;
+
+assert_int_equal(type, SSS_ID_TYPE_UID);
+assert_string_equal(name, expected_result);
+}
+
+return EOK;
+}
+
+void test_nss_getsidbyname(void **state)
+{
+errno_t ret;
+struct sysdb_attrs *attrs;
+const char *testuser_sid = "S-1-2-3-4";
+
+attrs = sysdb_new_attrs(nss_test_ctx);
+assert_non_null(attrs);
+
+ret = sysdb_attrs_add_string(attrs, SYSDB_SID_STR, testuser_sid);
+assert_int_equal(ret, EOK);
+
+ret = store_user(nss_test_ctx, nss_test_ctx->tctx->dom,
+ &sid_user, attrs, 0);
+assert_int_equal(ret, EOK);
+
+mock_input_user_or_group("testusersid");
+will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
+will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+will_return(test_nss_getsidbyname_check, testuser_sid);
+
+/* Query for that user, call a callback when command finishes */
+set_cmd_cb(test_nss_getsidbyname_check);
+ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+ nss_test_ctx->nss_cmds);
+assert_int_equal(ret, EOK);
+
+/* Wait until the test finishes with EOK */
+ret = test_ev_loop(nss_test_ctx->tctx);
+assert_int_equal(ret, EOK);
+}
+
+void test_nss_getsidbyupn(void **state)
+{
+errno_t ret;
+struct sysdb_attrs *attrs;
+const char *testuser_sid = "S-1-2-3-4";
+const char *testuser_upn = "testuser...@upndomain.test";
+
+attrs = sysdb_new_attrs(nss_test_ctx);
+assert_non_null(attrs);
+
+ret = sysdb_attrs_add_string(attrs, SYSDB_SID_STR, testuser_sid);
+assert_int_equal(ret, EOK);
+
+ret = sysdb_attrs_add_string(attrs, SYSDB_UPN, testuser_upn);
+assert_int_equal(ret, EOK);
+
+ret = store_user(nss_test_ctx, nss_test_ctx->tctx->dom,
+ &sid_user, attrs, 0);
+assert_int_equal(ret, EOK);
+
+mock_input_user_or_group(testuser_upn);
+will_return(__wrap_sss_packet_get_cmd, SSS_NSS_GETSIDBYNAME);
+will_return(__wrap_sss_packet_get_body, WRAP_CALL_REAL);
+
+will_return(test_nss_getsidbyname_check, testuser_sid);
+
+/* Query for that user, call a callback when command finishes */
+set_cmd_cb(test_nss_getsidbyname_check);
+ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+ nss_test_ctx->nss_cmds);
+assert_int_equal(ret, EOK);
+
+/* Wait until the test finishes with EOK */
+ret = test_ev_loop(nss_test_ctx->tctx);
+assert_int_equal(ret, EOK);
+}
+
+void test_nss_getsidbyname_neg(void **state)
+{
+errno_t ret;
+
+mock_input_user_or_group("testnosuchsid");
+mock_account_recv_simple();
+
+/* Query for that user, call a callback when command finishes */
+ret = sss_cmd_execute(nss_test_ctx->cctx, SSS_NSS_GETSIDBYNAME,
+ nss_test_ctx->nss_cmds);
+assert_int_equal(ret, EOK);
+
+/* Wait until the test finishes with ENOENT (because there is no such SID */
+ret = test_ev_loop(nss_test_ctx->tctx);
+assert_int_equal(ret, ENOENT);
+}
+
int main(int argc, const char *argv[])
{
int rv;
@@ -3563,6 +3686,12 @@ int main(int argc, const char *argv[])
