Re: [SSSD] [PATCH] confdb: Remove unused function confdb_get_long

2015-09-30 Thread Pavel Reichl



On 09/30/2015 07:42 AM, Lukas Slebodnik wrote:

On (29/09/15 17:45), Jakub Hrozek wrote:

On Tue, Sep 29, 2015 at 11:38:58AM +0200, Sumit Bose wrote:

On Tue, Sep 29, 2015 at 10:50:06AM +0200, Pavel Reichl wrote:



On 09/29/2015 10:44 AM, Lukas Slebodnik wrote:

On (29/09/15 10:42), Pavel Reichl wrote:



On 09/29/2015 10:35 AM, Lukas Slebodnik wrote:

On (29/09/15 10:16), Pavel Reichl wrote:

On 09/29/2015 10:08 AM, Lukas Slebodnik wrote:

On (29/09/15 08:45), Pavel Reichl wrote:



On 09/29/2015 08:31 AM, Lukas Slebodnik wrote:

On (27/09/15 12:49), Pavel Reichl wrote:

Hello, please see trivial patch attached.


>From b9f938087973444f0ec26fc24ad68dca7ac63034 Mon Sep 17 00:00:00 2001

From: Pavel Reichl 
Date: Sun, 27 Sep 2015 12:34:20 +0200
Subject: [PATCH] confdb: Remove unused function confdb_get_long

---
src/confdb/confdb.c | 51 ---
1 file changed, 51 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index 
c097aad7745eda4fff051c7da027776f95db0f03..eebd478f74041d2050df9edc283df43a65462340
 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -475,57 +475,6 @@ failed:
 return ret;
}

-long confdb_get_long(struct confdb_ctx *cdb,
- const char *section, const char *attribute,
- long defval, long *result)
-{

Would it be better to consider this function as confdb API
and add to src/confdb/confdb.h?


It could be.


It might be useful in the future.


I thought that out policy towards unused functions were to remove them.

Could you point me to the description of such policy?
I'm not aware of it.


I don't think it's written anywhere

Good to know. I thought I missed something


I just saw we did it repeatedly before and thought it's our general practice.


One more time:
If we consider confdb as library than we should never remove functions.

Removing functions from other parts of code is something
else.




We did so in commit 2b94ab415b30861f42b68725d9231905baf8c3bd

I didn't notice that patch. I'm sorry I do not have a time
to follow each patchset.


We can always resurrect removed code if needed.


If we consider confdb as library than
we should never remove functions.


Then we should decide which parts of SSSD are public libraries and which are 
not, then we could avoid this kind of discussions.


I wrote "consider confdb as library" and not as a public library with
stable API. And libraries many times contains function which are not used.
So we should not remove them. Even though it would be still in git history.


Why? If we don't use them and nobody else does why do we have to keep them?


libraries many times contains function which are not used


Well, but if they are not public we can change that, right? We can change them 
as we need and removing unused code is one of those things. I don't see the 
difference between private library and 'other parts of code'.


I think we should be explicit here. If we want to consider confdb as a
library we should make it a public library with a stable API with all
the consequences. In general I think this is not a bad idea e.g. with
respect to all the config related tickets we have planned for 1.14. But
we should define the initial version of a stable API, based on the
result and requirements from the 1.14 tickets.


I'm more with Simo here that we shouldn't consider confdb and sysdb as
stable ABIs, at least not yet. I think we need the flexibility of a private,
unstable library.

At the same time, I think we should try harder to avoid polluting the
sysdb/confdb APIs with new calls and in general strive to make even the
internal APIs easily usable. Here I agree with Lukas that the
current interface is not very nice -- and the sysdb interface would be
something that any new external contributor would be confronted with.

Maybe when we review (hint, nudge, nudge) Michal's sysdb patches, we can
take a look at the API and massage it a bit, move related functions
together in the header files and merge or remove extra functions? (Yes,
we can also file a ticket, but I don't think it would land in a
milestone where we would touch it soon...)



Having private/internal libraries with a more relaxed policy might turn
out to be problematic as e.g. can be seen by some internal samba
libraries which are used by us, OpenChange and maybe other projects
where upstream changes might force changes in the other projects as
well. Ok, the argument here is that they shouldn't have been used in the
first place, but what's the point in re-implementing existing
functionality.

Currently I wouldn't see libsss_util where confdb.c is included as a
library at all, but just as a mean to make the build process easier and
faster.

Coming to the original topic of the discussion, I would agree to remove
confdb_get_long() because it is nowhere used (and it looks like it
never was) and it does not even have a test.


Is that an ack? :-)

No,



Re: [SSSD] [PATCHES] fix minor memory leaks

2015-09-30 Thread Pavel Reichl



On 09/29/2015 05:49 PM, Jakub Hrozek wrote:

On Tue, Sep 29, 2015 at 02:20:39PM +0200, Pavel Reichl wrote:

In my opinion we can drop the change. This is not an imminent bug it's rather 
code style dispute and I don't think that we have to bother Sumit for that. 
Unless of course pbrezina feels different.


I don't mind too much, it just doesn't seem to be helpful change, that's
all, so I'll be a bit happier if we drop it.


Updated patch set attached. I removed the discussed 'talloc_new' line from 1st 
patch.

Bye.
>From 9cae5048afcfe16b053aa8322a275a9682d8bcdc Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:02:42 -0400
Subject: [PATCH 1/4] AD: fix minor memory leak

---
 src/providers/ad/ad_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
index 130cdeb613aae3843f7453a478815daaae6aab77..88f36f8ead64443e07b2393aa22ad4d9708d0a0e 100644
--- a/src/providers/ad/ad_common.c
+++ b/src/providers/ad/ad_common.c
@@ -745,7 +745,7 @@ ad_failover_init(TALLOC_CTX *mem_ctx, struct be_ctx *bectx,
 ret = be_add_online_cb(bectx, bectx, ad_online_cb, service, NULL);
 if (ret != EOK) {
 DEBUG(SSSDBG_CRIT_FAILURE, "Could not set up AD online callback\n");
-return ret;
+goto done;
 }
 
 ret = be_fo_service_add_callback(mem_ctx, bectx, ad_service,
@@ -797,7 +797,8 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
 sdata = fo_get_server_user_data(server);
 if (fo_is_srv_lookup(server) == false && sdata == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "No user data?\n");
-return;
+ret = EINVAL;
+goto done;
 }
 
 service = talloc_get_type(private_data, struct ad_service);
-- 
2.4.3

>From 5a20e0120278dba83381cc41f95024f2850ff745 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:03:45 -0400
Subject: [PATCH 2/4] IPA: fix minor memory leak

---
 src/providers/ipa/ipa_hbac_rules.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/providers/ipa/ipa_hbac_rules.c b/src/providers/ipa/ipa_hbac_rules.c
index ffef6dc4ce4229f2063d1b00308892bd3765f398..6ce1f76bb656352ec61332007a6fb62568374203 100644
--- a/src/providers/ipa/ipa_hbac_rules.c
+++ b/src/providers/ipa/ipa_hbac_rules.c
@@ -86,7 +86,7 @@ ipa_hbac_rule_info_send(TALLOC_CTX *mem_ctx,
 req = tevent_req_create(mem_ctx, , struct ipa_hbac_rule_state);
 if (req == NULL) {
 DEBUG(SSSDBG_CRIT_FAILURE, "tevent_req_create failed.\n");
-return NULL;
+goto error;
 }
 
 state->ev = ev;
-- 
2.4.3

>From d33c71c28261f357965344e6c48a989cf168e060 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:10 -0400
Subject: [PATCH 3/4] SDAP: fix minor memory leak

---
 src/providers/ldap/sdap_async_groups.c | 2 +-
 src/providers/ldap/sdap_idmap.c| 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
index 653187b3add172d37f234850ccab8ddf109e229c..609668339b7d7d37e578bd3cca8f8c7cac1e6671 100644
--- a/src/providers/ldap/sdap_async_groups.c
+++ b/src/providers/ldap/sdap_async_groups.c
@@ -592,7 +592,7 @@ static int sdap_save_group(TALLOC_CTX *memctx,
 if (ret != EOK) {
 DEBUG(SSSDBG_OP_FAILURE,
   "Error: Failed to mark group as non-posix!\n");
-return ret;
+goto done;
 }
 }
 
diff --git a/src/providers/ldap/sdap_idmap.c b/src/providers/ldap/sdap_idmap.c
index dd959b2c133b342f105f76c26c889d678ce40391..36d529836eb7e4becd27721df15cfbccf035ae3b 100644
--- a/src/providers/ldap/sdap_idmap.c
+++ b/src/providers/ldap/sdap_idmap.c
@@ -206,7 +206,8 @@ sdap_idmap_init(TALLOC_CTX *mem_ctx,
 if (err != IDMAP_SUCCESS) {
 /* This should never happen */
 DEBUG(SSSDBG_CRIT_FAILURE, "sss_idmap_ctx corrupted\n");
-return EIO;
+ret = EIO;
+goto done;
 }
 
 
-- 
2.4.3

>From e63211aaa15887c78deee5a76ed60684f9e6aaf7 Mon Sep 17 00:00:00 2001
From: Pavel Reichl 
Date: Fri, 4 Sep 2015 07:04:40 -0400
Subject: [PATCH 4/4] PROXY: fix minor memory leak

---
 src/providers/proxy/proxy_services.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/proxy/proxy_services.c b/src/providers/proxy/proxy_services.c
index 2aa44dbf7f061b99a551b199d0265393e5399a06..ee545cedc9f8dc36981eb3190ff16763e1da390f 100644
--- a/src/providers/proxy/proxy_services.c
+++ b/src/providers/proxy/proxy_services.c
@@ -130,7 +130,7 @@ get_serv_byname(struct proxy_id_ctx *ctx,
 if (status != NSS_STATUS_SUCCESS && status != NSS_STATUS_NOTFOUND) {
 DEBUG(SSSDBG_MINOR_FAILURE,
   "getservbyname_r failed for service [%s].\n", name);
-return ret;
+goto done;
 }
 
 if (status == 

Re: [SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Pavel Reichl



On 09/30/2015 09:38 AM, Jakub Hrozek wrote:

Hi,

to help the OpenSCAP integration, I prepared a wiki page that contains
options which have a security impact -- either positive (drop root) or
negative (ignore certificate validation issues).

I also tried to explain the effect of the options along with the
description. There are some more items that can be included, but I
wasn't sure about them myself, like:
 * should obfuscated passwords be mentioned? I wasn't sure because on
   one hand it really doesn't provide any benefit, on the other hand,
   the option can be used to check a compliance box that requires no
   passwords be stored in files..
 * should the page warn against the
   auth-option-that-shall-not-be-mentioned or politely deny its
   existence? :-)
 * What about fd_limit ? Should resource consumption be considered
   a security property, especially if we already honor system default? I
   think here the default is enough, so I didn't document that option.

Please provide your comments or edit the wiki directly. Thanks!
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



I was thinking about 'cached_auth_timeout' timeout. If misconfigured and set 
for really long time changes of passwords on server would be ignored. I know 
that same effect could be achieved by maintaining SSSD in offline mode...
What do you think?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] confdb: Remove unused function confdb_get_long

2015-09-30 Thread Sumit Bose
On Wed, Sep 30, 2015 at 08:31:57AM +0200, Pavel Reichl wrote:
> 
> 
> On 09/30/2015 07:42 AM, Lukas Slebodnik wrote:
> >On (29/09/15 17:45), Jakub Hrozek wrote:
> >>On Tue, Sep 29, 2015 at 11:38:58AM +0200, Sumit Bose wrote:
> >>>On Tue, Sep 29, 2015 at 10:50:06AM +0200, Pavel Reichl wrote:
> 
> 
> On 09/29/2015 10:44 AM, Lukas Slebodnik wrote:
> >On (29/09/15 10:42), Pavel Reichl wrote:
> >>
> >>
> >>On 09/29/2015 10:35 AM, Lukas Slebodnik wrote:
> >>>On (29/09/15 10:16), Pavel Reichl wrote:
> On 09/29/2015 10:08 AM, Lukas Slebodnik wrote:
> >On (29/09/15 08:45), Pavel Reichl wrote:
> >>
> >>
> >>On 09/29/2015 08:31 AM, Lukas Slebodnik wrote:
> >>>On (27/09/15 12:49), Pavel Reichl wrote:
> Hello, please see trivial patch attached.
> >>>
> From b9f938087973444f0ec26fc24ad68dca7ac63034 Mon Sep 17 00:00:00 
> 2001
> From: Pavel Reichl 
> Date: Sun, 27 Sep 2015 12:34:20 +0200
> Subject: [PATCH] confdb: Remove unused function confdb_get_long
> 
> ---
> src/confdb/confdb.c | 51 
> ---
> 1 file changed, 51 deletions(-)
> 
> diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
> index 
> c097aad7745eda4fff051c7da027776f95db0f03..eebd478f74041d2050df9edc283df43a65462340
>  100644
> --- a/src/confdb/confdb.c
> +++ b/src/confdb/confdb.c
> @@ -475,57 +475,6 @@ failed:
>  return ret;
> }
> 
> -long confdb_get_long(struct confdb_ctx *cdb,
> - const char *section, const char *attribute,
> - long defval, long *result)
> -{
> >>>Would it be better to consider this function as confdb API
> >>>and add to src/confdb/confdb.h?
> >>
> >>It could be.
> >>
> >>>It might be useful in the future.
> >>
> >>I thought that out policy towards unused functions were to remove 
> >>them.
> >Could you point me to the description of such policy?
> >I'm not aware of it.
> 
> I don't think it's written anywhere
> >>>Good to know. I thought I missed something
> >>>
> I just saw we did it repeatedly before and thought it's our general 
> practice.
> 
> >>>One more time:
> >>>If we consider confdb as library than we should never remove functions.
> >>>
> >>>Removing functions from other parts of code is something
> >>>else.
> >>>
> >
> >>We did so in commit 2b94ab415b30861f42b68725d9231905baf8c3bd
> >I didn't notice that patch. I'm sorry I do not have a time
> >to follow each patchset.
> >
> >>We can always resurrect removed code if needed.
> >>
> >If we consider confdb as library than
> >we should never remove functions.
> 
> Then we should decide which parts of SSSD are public libraries and 
> which are not, then we could avoid this kind of discussions.
> 
> >>>I wrote "consider confdb as library" and not as a public library with
> >>>stable API. And libraries many times contains function which are not 
> >>>used.
> >>>So we should not remove them. Even though it would be still in git 
> >>>history.
> >>
> >>Why? If we don't use them and nobody else does why do we have to keep 
> >>them?
> >>
> >libraries many times contains function which are not used
> 
> Well, but if they are not public we can change that, right? We can change 
> them as we need and removing unused code is one of those things. I don't 
> see the difference between private library and 'other parts of code'.
> >>>
> >>>I think we should be explicit here. If we want to consider confdb as a
> >>>library we should make it a public library with a stable API with all
> >>>the consequences. In general I think this is not a bad idea e.g. with
> >>>respect to all the config related tickets we have planned for 1.14. But
> >>>we should define the initial version of a stable API, based on the
> >>>result and requirements from the 1.14 tickets.
> >>
> >>I'm more with Simo here that we shouldn't consider confdb and sysdb as
> >>stable ABIs, at least not yet. I think we need the flexibility of a private,
> >>unstable library.
> >>
> >>At the same time, I think we should try harder to avoid polluting the
> >>sysdb/confdb APIs with new calls and in general strive to make even the
> >>internal APIs easily usable. Here I agree with Lukas that the
> >>current interface is not very nice -- and the sysdb interface would be
> >>something that any new external contributor 

Re: [SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Lukas Slebodnik
On (30/09/15 09:38), Jakub Hrozek wrote:
>Hi,
>
>to help the OpenSCAP integration, I prepared a wiki page that contains
>options which have a security impact -- either positive (drop root) or
>negative (ignore certificate validation issues).
>
>I also tried to explain the effect of the options along with the
>description. There are some more items that can be included, but I
>wasn't sure about them myself, like:
>* should obfuscated passwords be mentioned? I wasn't sure because on
>  one hand it really doesn't provide any benefit, on the other hand,
>  the option can be used to check a compliance box that requires no
>  passwords be stored in files..
>* should the page warn against the
>  auth-option-that-shall-not-be-mentioned or politely deny its
>  existence? :-)
>* What about fd_limit ? Should resource consumption be considered
>  a security property, especially if we already honor system default? I
>  think here the default is enough, so I didn't document that option.
>
>Please provide your comments or edit the wiki directly. Thanks!

What about:

   simple_deny_users (string)
   Comma separated list of users who are explicitly denied access.

   simple_deny_groups (string)
   Comma separated list of groups that are explicitly denied access. 
This applies only to
   groups within this SSSD domain. Local groups are not evaluated.

Whitelisting is much secure than blacklisting.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Run integration tests on debian testing

2015-09-30 Thread Lukas Slebodnik
On (29/09/15 16:47), Nikolai Kondrashov wrote:
>On 09/29/2015 03:41 PM, Lukas Slebodnik wrote:
>>ehlo,
>>
>>Integration tests are enabled on debian with the last patch.
>
>So we've got more cwrap packages into Debian? Awesome :)! Or were they there
>all the time and I simply failed to notice them?
>
uid-wrapper 1.0.2-1 was in testing since 2014-08-14
nss-wrapper 1.0.3-2 was in testing since 2015-06-05

https://packages.qa.debian.org/u/uid-wrapper.html
https://packages.qa.debian.org/n/nss-wrapper.html

>>I just changed DEPS_INTGCHECK_SATISFIED to true for debian
>>because in future we might introduce new dependencies
>>which will not be in debian (su_wrapper).
>
>Ah, alright.
>
>>The 1st patch is prequisity for the last patch because
>>installation of slapd requires user interaction.
>>The ticket #2433 is finally fixed after 13 months.
>
>Thank you, Lukas.
>
You are welcome

>>If we do not want to introduce new dependency /usr/bin/libtool
>>for debian then there is alternative solution of bug fixed
>>in the 2nd patch. We can run libtool from CWD generated by autotools.
>>In both cases it's a oneliner :-)
>>
>>Here is an alternative version:
>>
>>diff --git a/contrib/ci/run b/contrib/ci/run
>>index 5f668ff..1f64e67 100755
>>--- a/contrib/ci/run
>>+++ b/contrib/ci/run
>>@@ -204,7 +204,7 @@ function build_debug()
>>  CK_FORK=no \
>>  stage make-check-valgrind \
>>  make-check-wrap -j $CPU_NUM check -- \
>>-libtool --mode=execute \
>>+./libtool --mode=execute \
>>  valgrind-condense 99 \
>>  '!(*.py|*dlopen-tests)' -- \
>>  --trace-children=yes \
>
>I like this way a little better, but not enough to argue :)
>
hmm,
I tried it but it failed in cwrap test due to different directory.
I could use full path but nicer solution
is to install missing dependency

=
   sssd 1.13.1: src/tests/cwrap/test-suite.log
=

# TOTAL: 4
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  4
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: become_user-tests
===

/home/build/sssd/build/test-driver: line 107: ./libtool: No such file or 
directory
FAIL become_user-tests (exit status: 127)

FAIL: server-tests
==

/home/build/sssd/build/test-driver: line 107: ./libtool: No such file or 
directory
FAIL server-tests (exit status: 127)

FAIL: usertools-tests
=

/home/build/sssd/build/test-driver: line 107: ./libtool: No such file or 
directory
FAIL usertools-tests (exit status: 127)

FAIL: responder_common-tests


/home/build/sssd/build/test-driver: line 107: ./libtool: No such file or 
directory
FAIL responder_common-tests (exit status: 127)

>> From 6175bd3f05cbfd5f7d9b26770d9a98012803745c Mon Sep 17 00:00:00 2001
>>From: Lukas Slebodnik
>>Date: Tue, 29 Sep 2015 11:52:30 +0200
>>Subject: [PATCH 1/3] CI: Don't depend on user input with apt-get
>>
>>Resolves:
>>https://fedorahosted.org/sssd/ticket/2433
>>---
>>  contrib/ci/README.md | 6 ++
>>  contrib/ci/distro.sh | 2 ++
>>  2 files changed, 8 insertions(+)
>>
>>diff --git a/contrib/ci/README.md b/contrib/ci/README.md
>>index 
>>6b5f7f30eac8327d5aa45c3bfefd57e8d3109fe0..075bc3e074cb13916619f46c12c6d1a4de0158a2
>> 100644
>>--- a/contrib/ci/README.md
>>+++ b/contrib/ci/README.md
>>@@ -47,6 +47,12 @@ and Debian-based distros:
>>
>>  Where `` is the user invoking CI.
>>
>>+You might also want to allow to keep environment variable DEBIAN_FRONTEND
>>+on debian. So script can perform an unattended installation of a Debian 
>>package
>>+with apt-get.
>>+Defaults!/usr/bin/apt-get env_keep += "DEBIAN_FRONTEND
>>+
>>+
>
>The terminating double quote is missing here.
cop problem

>Also, may I suggest simplifying
>the text a little? Like this:
>
>You may also want to allow passing DEBIAN_FRONTEND environment variable to
>apt-get on Debian, so CI can request non-interactive package installation:
>
>Defaults!/usr/bin/apt-get env_keep += "DEBIAN_FRONTEND"
>
Done

>>  On Red Hat distros a repository carrying dependencies missing from some
>>  distros needs to be added to yum configuration. See instructions on the
>>  [Copr project 
>> page](http://copr-fe.cloud.fedoraproject.org/coprs/lslebodn/sssd-deps/).
>>diff --git a/contrib/ci/distro.sh b/contrib/ci/distro.sh
>>index 
>>5416bfff325c4e5d0a10ebea67cba26e20e03fd5..095985ccae81e54bcd79607e455a1c9295aad867
>> 100644
>>--- a/contrib/ci/distro.sh
>>+++ b/contrib/ci/distro.sh
>>@@ -30,6 +30,8 @@ if [ -e /etc/redhat-release ]; then
>>  DISTRO_FAMILY=redhat
>>  elif [ -e /etc/debian_version ]; then
>>  DISTRO_FAMILY=debian
>>+# Perform an unattended installation of a Debian package with apt-get
>>+export DEBIAN_FRONTEND=noninteractive

Re: [SSSD] [PATCH] Add a client-side hook to prevent pushes without Reviewed-By

2015-09-30 Thread Jakub Hrozek
On Tue, Sep 29, 2015 at 08:28:30AM +0200, Lukas Slebodnik wrote:
> On (28/09/15 14:19), Jakub Hrozek wrote:
> >Hi,
> >
> >to activate this hook, copy it from contrib to .git/hooks and make sure
> >the executable flag is on. Attempting to push a commit without
> >Reviewed-By will then trigger an error.
> >
> Good idea.
> 
> >If we want to be truly strict about not pushing commits without a RB
> >tag, then we need a server-side hook.
> As you wish.
> 
> 
> >From 391666fda49fef9ac003d192e7ae8a3c2b00e113 Mon Sep 17 00:00:00 2001
> >From: Jakub Hrozek 
> >Date: Mon, 28 Sep 2015 13:46:39 +0200
> >Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
> > Reviewed-By
> >
> >---
> > contrib/git/pre-push | 62 
> > 
> > 1 file changed, 62 insertions(+)
> > create mode 100755 contrib/git/pre-push
> >
> >diff --git a/contrib/git/pre-push b/contrib/git/pre-push
> Initially, I thought it does not work because I tested with
> creating a new branch
>   git push origin master:new_branch
> However it works after trying to push another patch to this branch.
> 
> There are few pep8 warning
> sh$ pep8 contrib/git/pre-push
> contrib/git/pre-push:10:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:11:13: E201 whitespace after '['
> contrib/git/pre-push:11:70: E202 whitespace before ']'
> contrib/git/pre-push:14:13: E201 whitespace after '['
> contrib/git/pre-push:14:74: E202 whitespace before ']'
> contrib/git/pre-push:16:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:17:13: E201 whitespace after '['
> contrib/git/pre-push:17:54: E202 whitespace before ']'
> contrib/git/pre-push:22:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:31:1: E302 expected 2 blank lines, found 1
> contrib/git/pre-push:39:1: E302 expected 2 blank lines, found 1

pep8 should be happy about the attached patch.

> 
> It would be also good to add small howto from mail.
> "copy it from contrib to .git/hooks and make sure
> the executable flag is on" to the commit message or better to
> README into directory contrib/git/

OK, Added.

> 
> and you can also mention hint about .git-commit-template

I tried to add something, hope it makes sense.
>From 66df6b096bcb3044431b1d65cb426a1f1fb1f37c Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Mon, 28 Sep 2015 13:46:39 +0200
Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
 Reviewed-By

---
 contrib/git/pre-push | 73 
 1 file changed, 73 insertions(+)
 create mode 100755 contrib/git/pre-push

diff --git a/contrib/git/pre-push b/contrib/git/pre-push
new file mode 100755
index 
..d05202dfdd2f59a885b843e25e89878d985909e4
--- /dev/null
+++ b/contrib/git/pre-push
@@ -0,0 +1,73 @@
+#!/usr/bin/env python
+
+# A git pre-push hook that declines commits that don't contain a Reviewed-By:
+# tag. The tag must be present on the beginning of the line. To activate, copy
+# to $GIT_DIR/hooks/pre-push and make sure the executable flag is on.
+
+# The commit message should also be based on .git-commit-template, although
+# that is just best practice and not enforced
+
+import sys
+import re
+import subprocess
+
+
+def get_all_commits(ref_from, ref_to):
+args = ['git', 'rev-list', '{:s}..{:s}'.format(ref_from, ref_to)]
+p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+out, err = p.communicate()
+return [commit.strip() for commit in out.split('\n') if commit != '']
+
+
+def commit_message(commit_hash):
+args = ['git', 'cat-file', 'commit', commit_hash]
+p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+out, err = p.communicate()
+return out
+
+
+def commit_has_rb(commit):
+msg = commit_message(commit)
+for l in msg.split('\n'):
+has_rb = re.search('^Reviewed-by:', l)
+if has_rb:
+return True
+
+return False
+
+
+def report_commit(commit_hash):
+print("Commit {:s} does not have Reviewed-By!".format(commit_hash))
+print("Full message:\n==")
+print("{:s}".format(commit_message(commit_hash)))
+print("==")
+
+
+# man 5 githooks says:
+#   Information about what is to be pushed is provided on the hook's
+#   standard input with lines of the form:
+#SP  SP  SP  LF
+def check_push(hook_input):
+ref_to = hook_input.split()[1][:6]
+ref_from = hook_input.split()[3][:6]
+commit_list = get_all_commits(ref_from, ref_to)
+
+no_rb_list = []
+for commit in commit_list:
+if not commit_has_rb(commit):
+no_rb_list.append(commit)
+
+return no_rb_list
+
+# Don't warn when pushing to personal repositories, only origin
+remote = sys.argv[1]
+if remote != 'origin':
+sys.exit(0)
+
+for hook_input in sys.stdin.readlines():
+no_rb_list = check_push(hook_input)
+
+if len(no_rb_list) 

[SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Jakub Hrozek
Hi,

to help the OpenSCAP integration, I prepared a wiki page that contains
options which have a security impact -- either positive (drop root) or
negative (ignore certificate validation issues).

I also tried to explain the effect of the options along with the
description. There are some more items that can be included, but I
wasn't sure about them myself, like:
* should obfuscated passwords be mentioned? I wasn't sure because on
  one hand it really doesn't provide any benefit, on the other hand,
  the option can be used to check a compliance box that requires no
  passwords be stored in files..
* should the page warn against the
  auth-option-that-shall-not-be-mentioned or politely deny its
  existence? :-)
* What about fd_limit ? Should resource consumption be considered
  a security property, especially if we already honor system default? I
  think here the default is enough, so I didn't document that option.

Please provide your comments or edit the wiki directly. Thanks!
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Pavel Reichl

On 09/30/2015 09:38 AM, Jakub Hrozek wrote:

Hi,

to help the OpenSCAP integration, I prepared a wiki page that contains
options which have a security impact -- either positive (drop root) or
negative (ignore certificate validation issues).

I also tried to explain the effect of the options along with the
description. There are some more items that can be included, but I
wasn't sure about them myself, like:
 * should obfuscated passwords be mentioned? I wasn't sure because on
   one hand it really doesn't provide any benefit, on the other hand,
   the option can be used to check a compliance box that requires no
   passwords be stored in files..
 * should the page warn against the
   auth-option-that-shall-not-be-mentioned or politely deny its
   existence? :-)
 * What about fd_limit ? Should resource consumption be considered
   a security property, especially if we already honor system default? I
   think here the default is enough, so I didn't document that option.

Please provide your comments or edit the wiki directly. Thanks!
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel



Should not we also mention dreadful option 
ldap_auth_disable_tls_never_use_in_production ?

I know it's undocumented, but I suppose OpenSCAP should report its presence 
anyway.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] confdb: Remove unused function confdb_get_long

2015-09-30 Thread Lukas Slebodnik
On (30/09/15 10:06), Sumit Bose wrote:
>On Wed, Sep 30, 2015 at 08:31:57AM +0200, Pavel Reichl wrote:
>> 
>> 
>> On 09/30/2015 07:42 AM, Lukas Slebodnik wrote:
>> >On (29/09/15 17:45), Jakub Hrozek wrote:
>> >>On Tue, Sep 29, 2015 at 11:38:58AM +0200, Sumit Bose wrote:
>> >>>On Tue, Sep 29, 2015 at 10:50:06AM +0200, Pavel Reichl wrote:
>> 
>> 
>> On 09/29/2015 10:44 AM, Lukas Slebodnik wrote:
>> >On (29/09/15 10:42), Pavel Reichl wrote:
>> >>
>> >>
>> >>On 09/29/2015 10:35 AM, Lukas Slebodnik wrote:
>> >>>On (29/09/15 10:16), Pavel Reichl wrote:
>> On 09/29/2015 10:08 AM, Lukas Slebodnik wrote:
>> >On (29/09/15 08:45), Pavel Reichl wrote:
>> >>
>> >>
>> >>On 09/29/2015 08:31 AM, Lukas Slebodnik wrote:
>> >>>On (27/09/15 12:49), Pavel Reichl wrote:
>> Hello, please see trivial patch attached.
>> >>>
>> From b9f938087973444f0ec26fc24ad68dca7ac63034 Mon Sep 17 
>> 00:00:00 2001
>> From: Pavel Reichl 
>> Date: Sun, 27 Sep 2015 12:34:20 +0200
>> Subject: [PATCH] confdb: Remove unused function confdb_get_long
>> 
>> ---
>> src/confdb/confdb.c | 51 
>> ---
>> 1 file changed, 51 deletions(-)
>> 
>> diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
>> index 
>> c097aad7745eda4fff051c7da027776f95db0f03..eebd478f74041d2050df9edc283df43a65462340
>>  100644
>> --- a/src/confdb/confdb.c
>> +++ b/src/confdb/confdb.c
>> @@ -475,57 +475,6 @@ failed:
>>  return ret;
>> }
>> 
>> -long confdb_get_long(struct confdb_ctx *cdb,
>> - const char *section, const char *attribute,
>> - long defval, long *result)
>> -{
>> >>>Would it be better to consider this function as confdb API
>> >>>and add to src/confdb/confdb.h?
>> >>
>> >>It could be.
>> >>
>> >>>It might be useful in the future.
>> >>
>> >>I thought that out policy towards unused functions were to remove 
>> >>them.
>> >Could you point me to the description of such policy?
>> >I'm not aware of it.
>> 
>> I don't think it's written anywhere
>> >>>Good to know. I thought I missed something
>> >>>
>> I just saw we did it repeatedly before and thought it's our general 
>> practice.
>> 
>> >>>One more time:
>> >>>If we consider confdb as library than we should never remove 
>> >>>functions.
>> >>>
>> >>>Removing functions from other parts of code is something
>> >>>else.
>> >>>
>> >
>> >>We did so in commit 2b94ab415b30861f42b68725d9231905baf8c3bd
>> >I didn't notice that patch. I'm sorry I do not have a time
>> >to follow each patchset.
>> >
>> >>We can always resurrect removed code if needed.
>> >>
>> >If we consider confdb as library than
>> >we should never remove functions.
>> 
>> Then we should decide which parts of SSSD are public libraries and 
>> which are not, then we could avoid this kind of discussions.
>> 
>> >>>I wrote "consider confdb as library" and not as a public library with
>> >>>stable API. And libraries many times contains function which are not 
>> >>>used.
>> >>>So we should not remove them. Even though it would be still in git 
>> >>>history.
>> >>
>> >>Why? If we don't use them and nobody else does why do we have to keep 
>> >>them?
>> >>
>> >libraries many times contains function which are not used
>> 
>> Well, but if they are not public we can change that, right? We can 
>> change them as we need and removing unused code is one of those things. 
>> I don't see the difference between private library and 'other parts of 
>> code'.
>> >>>
>> >>>I think we should be explicit here. If we want to consider confdb as a
>> >>>library we should make it a public library with a stable API with all
>> >>>the consequences. In general I think this is not a bad idea e.g. with
>> >>>respect to all the config related tickets we have planned for 1.14. But
>> >>>we should define the initial version of a stable API, based on the
>> >>>result and requirements from the 1.14 tickets.
>> >>
>> >>I'm more with Simo here that we shouldn't consider confdb and sysdb as
>> >>stable ABIs, at least not yet. I think we need the flexibility of a 
>> >>private,
>> >>unstable library.
>> >>
>> >>At the same time, I think we should try harder to avoid polluting the
>> >>sysdb/confdb APIs with new calls and in general strive to make even the
>> >>internal 

Re: [SSSD] RFC: Improving the debug messages

2015-09-30 Thread Sumit Bose
On Tue, Sep 29, 2015 at 07:53:41PM +0200, Jakub Hrozek wrote:
> On Tue, Sep 29, 2015 at 06:15:35PM +0200, Sumit Bose wrote:
> > On Tue, Sep 29, 2015 at 05:57:32PM +0200, Jakub Hrozek wrote:
> > > On Mon, Sep 28, 2015 at 10:18:07AM +0200, Sumit Bose wrote:
> > > > On Mon, Jun 29, 2015 at 11:07:30PM -0400, Dan Lavu wrote:
> > > > > I've been watching various logs for the past few minutes, FWIW, I 
> > > > > think a more casual message will help people better understand what 
> > > > > SSSD is doing. Look at debug level 4, the first instance of a user 
> > > > > name look up (getent passwd dlavu) a common command we tell folks to 
> > > > > test to see if SSSD is working 
> > > > > 
> > > > > (Mon Jun 29 22:10:59 2015) [sssd[be[lab.runlevelone.lan]]] 
> > > > > [be_get_account_info] (0x0200): Got request for 
> > > > > [0x1001][1][name=dlavu] 
> > > > > 
> > > > > Just imagine if you knew nothing about SSSD and how it worked, what 
> > > > > can you gather from this line of text? Timestamp, process, domain, 
> > > > > get_account_info, got_request and userid and the rest is sort of 
> > > > > gibberish and what is the difference between be_get_account_info and 
> > > > > got_request_for? I think it's confusing, now looking at some other 
> > > > > logs messages. 
> > > > 
> > > > What is missing in the list of components above is '(0x0200)' which is
> > > > the debug_level.  We recently had a number of questions about messages
> > > > from a high debug level which in general have an information character,
> > > > but since it not easy to identify the level of the message it was
> > > > considered as an error message. Maybe it would be good to have a some
> > > > translations here as well to make it easier to separate errors from
> > > > infos.
> > > 
> > > Do you mean printing the level as string, so that instead of:
> > > [sssd[be[lab.runlevelone.lan]]] [be_get_account_info] (0x0200): Got 
> > > request for [0x1001][1][name=dlavu] 
> > > we would have:
> > > [sssd[be[lab.runlevelone.lan]]] [be_get_account_info] 
> > > (SSSDBG_FUNC_DATA): Got request for [0x1001][1][name=dlavu] 
> > > 
> > > ?
> > > 
> > > If yes, then I agree. But at the same time, I think it's more irritating
> > 
> > yes, that's what I meant, but maybe we should not use the internal
> > macros here, but something more explicit like ERR_CRIT, ERR_FUNC,
> > INF_CONFIG, INF_DATA, TRACE_... to make it more obvious what the message
> > might be about.
> 
> Yes, sure. Care to file a ticket? :-)

It's https://fedorahosted.org/sssd/ticket/2808 . Please add ideas and
suggestions how those tags shall look like.

bye,
Sumit

> 
> > 
> > > that so many debug levels (mainly those that were mass-converted) are
> > > misplaced.
> > 
> > of course, but maybe having the level spelled out will help to identify
> > the ones that are misplaced more easy?
> 
> Yes, it would be easier to read a word than a hexa number. We could
> even colorize the output when running interactively :-)
> 
> What we can also do easily and with relatively low effort is to
> select a couple of the most common usage patterns like:
> * login with the correct password, online and offline
> * login with wrong password
> * run sudo
> * change password
> * 
> 
> And make sure they don't print any messages with debug_level <= 3.
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] AD: inicialize root_domain_attrs field

2015-09-30 Thread Jakub Hrozek
On Tue, Sep 29, 2015 at 09:25:13AM +0200, Jakub Hrozek wrote:
> On Fri, Sep 25, 2015 at 11:00:07AM +0200, Jakub Hrozek wrote:
> > On Fri, Sep 25, 2015 at 10:31:02AM +0200, Lukas Slebodnik wrote:
> > > >The test_ipa_subdom_server test took me some time, that's why I don't
> > > >think we need to block the release over a test.
> > > We are not in hurry.
> > 
> > Define "We". The Prominent North American Enterprise Linux Vendor
> > downstream kinda is in hurry.
> 
> Since I broke the code in ece345a74cec793e6d970a4955beb3d4a05935b3 which
> had no tests for the modified source, I should write the tests now:
> https://fedorahosted.org/sssd/ticket/2806

ACK and I pushed the patch to master: 
master: 101628a48d25ffae3b13c75d0b0b01577188c803 

I just confused this patch and another one and set a wrong R-B :-/ sorry
about that.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Fix configure script arguments for CentOS

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 08:31 AM, Lukas Slebodnik wrote:

On (29/09/15 17:02), Nikolai Kondrashov wrote:

Er, no, I don't mind which way we go, but we'd better be consistent, so please
either change Lukas's patch, or all the other places :)

Or, Lukas, did you mean that "||" outside []/[[]] tests is different and
separate and we should have these changed only?

yes, it's something different.
It's hard to notice || outside of tests []/[[]].

But it's better to be consistent and || before next command
outsde tests is more awkward and more confusing.

Updated patch is attached.


Thank you, Lukas!

I don't have CentOS handy, so I haven't tested it, but I trust you did.

ACK.

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


[SSSD] [PATCH] MAN: Clarify pam_trusted_users option description

2015-09-30 Thread Jakub Hrozek
Hi,

while working on the hardening wiki page, I realized the
pam_trusted_users option can be improved. Please see the attached patch.
>From ac09c8dabb706ad1a870354a2879eb899d17c5fc Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Wed, 30 Sep 2015 09:33:17 +0200
Subject: [PATCH] MAN: Clarify pam_trusted_users option description

---
 src/man/sssd.conf.5.xml | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
0ab4381819fa2f47e9a4bfb4897cb94144b96ff8..21acb63805c3906859130222cf335ad53be5894a
 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -927,14 +927,18 @@ fallback_homedir = /home/%u
 pam_trusted_users (string)
 
 
-Specifies the comma-separated list of UID values or
-user names that are allowed to access the PAM
-responder. User names are resolved to UIDs at
+Specifies the comma-separated list of UID
+values or user names that are allowed to run
+PAM conversations against trusted domains.
+Users not included in this list can only access
+domains marked as public with
+pam_public_domains.
+User names are resolved to UIDs at
 startup.
 
 
-Default: all (All users are allowed to access
-the PAM responder)
+Default: All users are considered trusted
+by default
 
 
 Please note that UID 0 is always allowed to access
-- 
2.4.3

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


[SSSD] [PATCH] MAN: proxy and krb5 are valid access control modules

2015-09-30 Thread Jakub Hrozek
Hi,

while documenting the security options I realized man sssd.conf doesn't
include the krb5 and proxy access control modules. I hope I worded the
sentence about krb5 correctly.
>From 9c8ce997e738c537061d53c5499ad0d0417c012e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Wed, 30 Sep 2015 04:25:54 +0200
Subject: [PATCH] MAN: proxy and krb5 are valid access control modules

---
 src/man/sssd.conf.5.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 
9701f2a15fd67c799e199a99049bdcf259fdc254..0ab4381819fa2f47e9a4bfb4897cb94144b96ff8
 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -1716,6 +1716,16 @@ pam_account_expired_message = Account expired, please 
call help desk.
 information on configuring the simple access 
module.
 
 
+krb5: .k5login based access control.
+See 
+sssd-krb5
+5 for more
+information on configuring Kerberos.
+
+
+proxy for relaying access control 
to another PAM module.
+
+
 Default: permit
 
 
-- 
2.4.3

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 30, 2015 at 10:37:30AM +0200, Lukas Slebodnik wrote:
> On (30/09/15 09:38), Jakub Hrozek wrote:
> >Hi,
> >
> >to help the OpenSCAP integration, I prepared a wiki page that contains
> >options which have a security impact -- either positive (drop root) or
> >negative (ignore certificate validation issues).
> >
> >I also tried to explain the effect of the options along with the
> >description. There are some more items that can be included, but I
> >wasn't sure about them myself, like:
> >* should obfuscated passwords be mentioned? I wasn't sure because on
> >  one hand it really doesn't provide any benefit, on the other hand,
> >  the option can be used to check a compliance box that requires no
> >  passwords be stored in files..
> >* should the page warn against the
> >  auth-option-that-shall-not-be-mentioned or politely deny its
> >  existence? :-)
> >* What about fd_limit ? Should resource consumption be considered
> >  a security property, especially if we already honor system default? I
> >  think here the default is enough, so I didn't document that option.
> >
> >Please provide your comments or edit the wiki directly. Thanks!
> 
> What about:
> 
>simple_deny_users (string)
>Comma separated list of users who are explicitly denied access.
> 
>simple_deny_groups (string)
>Comma separated list of groups that are explicitly denied access. 
> This applies only to
>groups within this SSSD domain. Local groups are not evaluated.
> 
> Whitelisting is much secure than blacklisting.

Good idea, added:

https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions?action=diff=6_version=5
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] RFC: Improving the debug messages

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 30, 2015 at 01:10:20PM +0200, Petr Cech wrote:
> On 09/30/2015 11:15 AM, Jakub Hrozek wrote:
> >On Wed, Sep 30, 2015 at 09:53:24AM +0200, Sumit Bose wrote:
> >>It's https://fedorahosted.org/sssd/ticket/2808 . Please add ideas and
> >>suggestions how those tags shall look like.
> >
> >Thanks, I ressurected
> >https://fedorahosted.org/sssd/ticket/1372 from Deferred as well.
> >___
> 
> This topic resonates with me. Text instead of hexadecimal numbers is better
> and it could make our logs more understandable.
> And usage patterns are very nice guides for orientation in logs.
> 
> I would like to work on this ticket.

Sure, pick it up.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


[SSSD] Announcing SSSD 1.13.1

2015-09-30 Thread Jakub Hrozek
  == SSSD 1.13.1 ===

The SSSD team is proud to announce the release of version 1.13.1 of
the System Security Services Daemon.

As always, the source is available from https://fedorahosted.org/sssd

RPM packages will be made available for Fedora shortly.

== Feedback ==

Please provide comments, bugs and other feedback via the sssd-devel
or sssd-users mailing lists:
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
https://lists.fedorahosted.org/mailman/listinfo/sssd-users

== Highlights ==
  * Initial support for Smart Card authentication was added. The feature
can be activated with the new pam_cert_auth option
  * The PAM prompting was enhanced so that when Two-Factor Authentication
is used, both factors (password and token) can be entered separately
on separate prompts. At the same time, only the long-term password is
cached, so offline access would still work using the long term password
  * A new command line tool sss_override is present in this release. The
tools allows to override attributes on the SSSD side. It's helpful in
environment where e.g. some hosts need to have a different view of POSIX
attributes than others. Please note that the overrides are stored in
the cache as well, so removing the cache will also remove the overrides
  * New methods were added to the SSSD D-Bus interface. Notably support
for looking up a user by certificate and looking up multiple users
using a wildcard was added. Please see the interface introspection or
the design pages for full details
  * Several enhancements to the dynamic DNS update code. Notably, clients
that update multiple interfaces work better with this release
  * This release supports authenticating againt a KDC proxy
  * The fail over code was enhanced so that if a trusted domain is not
reachable, only that domain will be marked as inactive but the backed
would stay in online mode
  * Several fixes to the GPO access control code are present 

== Packaging Changes ==
  * The Smart Card authentication feature requires a helper process
p11_child that needs to be marked as setgid if SSSD needs to be able
to. Please note the p11_child requires the NSS crypto library at the moment
  * The sss_override tool was added along with its own manpage
  * The upstream RPM can now build on RHEL/CentOS 6.7 

== Documentation Changes ==
  * The config_file_version configuration option now defaults to 2. As
an effect, this option doesn't have to be set anymore unless the config
file format is changed again by SSSD upstream
  * It is now possible to specify a comma-separated list of interfaces in
the dyndns_iface option
  * The InfoPipe responder and the LDAP provider gained a new option
wildcard_lookup that specifies an upper limit on the number of entries
that can be returned with a wildcard lookup
  * A new option dyndns_server was added. This option allows to attempt
a fallback DNS update against a specific DNS server. Please note this
option only works as a fallback, the first attempt will always be
performed against autodiscovered servers.
  * The PAM responder gained a new option ca_db that allows the storage
of trusted CA certificates to be specified
  * The time the p11_child is allowed to operate can be specified using
a new option p11_child_timeout

== Tickets Fixed ==

https://fedorahosted.org/sssd/ticket/546
[RFE] Support for smart cards
https://fedorahosted.org/sssd/ticket/1697
sssd: incorrect checks on length values during packet decoding
https://fedorahosted.org/sssd/ticket/1926
[RFE] Start the dynamic DNS update after the SSSD has been setup for
the first time
https://fedorahosted.org/sssd/ticket/1994
Complain loudly if backend doesn't start due to missing or invalid keytab
https://fedorahosted.org/sssd/ticket/2275
nested netgroups do not work in IPA provider
https://fedorahosted.org/sssd/ticket/2283
test dyndns failed.
https://fedorahosted.org/sssd/ticket/2335
Investigate using the krb5 responder for driving the PAM conversation
with OTPs
https://fedorahosted.org/sssd/ticket/2463
Pass error messages via the extdom plugin
https://fedorahosted.org/sssd/ticket/2495
[RFE]Allow sssd to add a new option that would specify which server
to update DNS with
https://fedorahosted.org/sssd/ticket/2549
RFE: Support multiple interfaces with the dyndns_iface option
https://fedorahosted.org/sssd/ticket/2553
RFE: Add support for wildcard-based cache updates
https://fedorahosted.org/sssd/ticket/2558
Add dualstack and multihomed support
https://fedorahosted.org/sssd/ticket/2561
Too much logging
https://fedorahosted.org/sssd/ticket/2579
TRACKER: Support one-way trusts for IPA
https://fedorahosted.org/sssd/ticket/2581
Re-check memcache after acquiring the lock in the client code
https://fedorahosted.org/sssd/ticket/2584
RFE: Support client-side overrides

Re: [SSSD] RFC: Improving the debug messages

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 30, 2015 at 09:53:24AM +0200, Sumit Bose wrote:
> It's https://fedorahosted.org/sssd/ticket/2808 . Please add ideas and
> suggestions how those tags shall look like.

Thanks, I ressurected
https://fedorahosted.org/sssd/ticket/1372 from Deferred as well.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] Please review: https://fedorahosted.org/sssd/wiki/SecuritySensitiveOptions

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 30, 2015 at 10:07:48AM +0200, Pavel Reichl wrote:
> Should not we also mention dreadful option 
> ldap_auth_disable_tls_never_use_in_production ?

That's what I was trying to ask :)
> > * should the page warn against the
> >   auth-option-that-shall-not-be-mentioned or politely deny its
> >   existence? :-)

I think it depends on the presentation a bit, if the tool just tells
user 'remove this option' than it would be fine to include it. If the
tool would present a list of options, like the wiki page does, then no.

I guess we can sort this out with the SCAP developers..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] RFC: Improving the debug messages

2015-09-30 Thread Petr Cech

On 09/30/2015 11:15 AM, Jakub Hrozek wrote:

On Wed, Sep 30, 2015 at 09:53:24AM +0200, Sumit Bose wrote:

It's https://fedorahosted.org/sssd/ticket/2808 . Please add ideas and
suggestions how those tags shall look like.


Thanks, I ressurected
https://fedorahosted.org/sssd/ticket/1372 from Deferred as well.
___


This topic resonates with me. Text instead of hexadecimal numbers is 
better and it could make our logs more understandable.

And usage patterns are very nice guides for orientation in logs.

I would like to work on this ticket.

Petr
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCHES] DYDNDS: update quality of input for nsupdate

2015-09-30 Thread Jakub Hrozek
On Tue, Sep 29, 2015 at 11:40:34AM +0200, Pavel Reichl wrote:
> 
> 
> On 09/21/2015 05:47 PM, Jakub Hrozek wrote:
> >On Mon, Sep 21, 2015 at 01:03:13PM +0200, Pavel Reichl wrote:
> >>
> >>
> >>On 09/21/2015 10:52 AM, Pavel Březina wrote:
> >>>On 08/28/2015 11:05 AM, Pavel Reichl wrote:
> On 08/14/2015 04:12 PM, Jakub Hrozek wrote:
> >>
> Test failed because of missing patch  [PATCH 6/9] DYNDNS: use realm and
> server commands only as fallback
> 
> I must have skipped it while respinning the patch set, sorry.
> 
> I attached this missing patch and test that was failing because of it.
> >>>
> >>>Hi,
> >>>
> DYNDNS: use realm and server commands only as fallback
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/2195
> >>>
> >>>You have a wrong ticket number here, I think. The patches look good 
> >>>otherwise.
> >>
> >>Yes, thanks for noticing! Updated patchset attached.
> >
> >https://fedorahosted.org/sssd/ticket/2495 that the ticket reference is
> >closed, did you mean another one?
> 
> We cleared that off-list - patch was accidentally omitted from the original 
> patch set.

Can you rebase these patches, please?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Run integration tests on debian testing

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 09:59 AM, Lukas Slebodnik wrote:

On (29/09/15 16:47), Nikolai Kondrashov wrote:

>On 09/29/2015 03:41 PM, Lukas Slebodnik wrote:

>>ehlo,
>>
>>Integration tests are enabled on debian with the last patch.

>
>So we've got more cwrap packages into Debian? Awesome:)! Or were they there
>all the time and I simply failed to notice them?
>

uid-wrapper 1.0.2-1 was in testing since 2014-08-14
nss-wrapper 1.0.3-2 was in testing since 2015-06-05

https://packages.qa.debian.org/u/uid-wrapper.html
https://packages.qa.debian.org/n/nss-wrapper.html


Ah, so it was added while I was working on it and I didn't notice that. Ouch.


>>Here is an alternative version:
>>
>>diff --git a/contrib/ci/run b/contrib/ci/run
>>index 5f668ff..1f64e67 100755
>>--- a/contrib/ci/run
>>+++ b/contrib/ci/run
>>@@ -204,7 +204,7 @@ function build_debug()
>>  CK_FORK=no \
>>  stage make-check-valgrind \
>>  make-check-wrap -j $CPU_NUM check -- \
>>-libtool --mode=execute \
>>+./libtool --mode=execute \
>>  valgrind-condense 99 \
>>  '!(*.py|*dlopen-tests)' -- \
>>  --trace-children=yes \

>
>I like this way a little better, but not enough to argue:)
>


hmm,
I tried it but it failed in cwrap test due to different directory.
I could use full path but nicer solution
is to install missing dependency


Right, dependency it is then.


>>All test failed due to missing /usr/bin/libtool
>>
>>e.g.
>>/home/build/sssd/build/test-driver: line 107: libtool: command not found
>>FAIL test-io (exit status: 127)

>
>Whoa! When did that start happening?
>

It happens if you run script on machine with minimal dependencies.
libtool-bin could be a dependency of another package in past.


D'oh, right.

Looks fine now, tested on my Debian laptop - worked fine.

Thank you, Lukas!

ACK.

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] [sssd-1.11] pysss_nss_idmap: Use wrapper for older python

2015-09-30 Thread Lukas Slebodnik
On (19/06/15 14:25), Jakub Hrozek wrote:
>On Tue, Jun 16, 2015 at 11:40:21PM +0200, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> we dropped support for old version of python (<2.6)
>> in recent version of sssd. It should work in 1.11 branch
>> but there was a small issue in pysss_nss_idmap bindings.
>> 
>> Attached is a simple patch wich reuse our internal python wrappers.
>> 
>> LS
>
>ACK, builds fine:
>http://koji.fedoraproject.org/koji/taskinfo?taskID=10164975
sssd-1-11:
* 0ffc58e2407f1756b0c09fe31ecd21a75e88e833

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Fix configure script arguments for CentOS

2015-09-30 Thread Lukas Slebodnik
On (30/09/15 11:02), Nikolai Kondrashov wrote:
>On 09/30/2015 08:31 AM, Lukas Slebodnik wrote:
>>On (29/09/15 17:02), Nikolai Kondrashov wrote:
>>>Er, no, I don't mind which way we go, but we'd better be consistent, so 
>>>please
>>>either change Lukas's patch, or all the other places :)
>>>
>>>Or, Lukas, did you mean that "||" outside []/[[]] tests is different and
>>>separate and we should have these changed only?
>>yes, it's something different.
>>It's hard to notice || outside of tests []/[[]].
>>
>>But it's better to be consistent and || before next command
>>outsde tests is more awkward and more confusing.
>>
>>Updated patch is attached.
>
>Thank you, Lukas!
>
>I don't have CentOS handy, so I haven't tested it, but I trust you did.
>
>ACK.
>
master:
* 09365a02c9ff68f16227b69a348511bb584060bc

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] MAN: proxy and krb5 are valid access control modules

2015-09-30 Thread Alexander Bokovoy

On Wed, 30 Sep 2015, Jakub Hrozek wrote:

Hi,

while documenting the security options I realized man sssd.conf doesn't
include the krb5 and proxy access control modules. I hope I worded the
sentence about krb5 correctly.

ACK

--
/ Alexander Bokovoy
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] MAN: Clarify pam_trusted_users option description

2015-09-30 Thread Alexander Bokovoy

On Wed, 30 Sep 2015, Jakub Hrozek wrote:

Hi,

while working on the hardening wiki page, I realized the
pam_trusted_users option can be improved. Please see the attached patch.

ACK, this is much better explanation than it was before.

--
/ Alexander Bokovoy
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Run integration tests on debian testing

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 04:08 PM, Nikolai Kondrashov wrote:

Looks fine now, tested on my Debian laptop - worked fine.


Additionally, it worked on a fresh Debian Testing Docker image - niiice :)

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] confdb: warn if memcache_timeout > than entry_cache

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 23, 2015 at 08:55:11PM +0200, Pavel Březina wrote:
> Ack. Thank you!

CI: http://sssd-ci.duckdns.org/logs/job/28/14/summary.html

* master: 3fb1ee96f508784d7e06f079111d4d32d401a99b

btw the ticket was in Backlog, not in the currently-in-progress 1.13.2,
so I just moved it.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] CI: Already fixed bug for TODO

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 08:20 AM, Lukas Slebodnik wrote:

On (29/09/15 17:35), Nikolai Kondrashov wrote:

On 09/29/2015 05:13 PM, Nikolai Kondrashov wrote:

On 09/29/2015 03:54 PM, Lukas Slebodnik wrote:

ehlo,

I touched the CI script and I found an interesting todo

distro.sh-52-{
distro.sh-53-declare prompt=$'Need root permissions to install packages.\n'
distro.sh-54-prompt+="Enter sudo password for $USER: "
distro.sh-55-if [[ "$DISTRO_BRANCH" == -redhat-* ]]; then
distro.sh-56-[ $# != 0 ] && sudo -p "$prompt" yum --assumeyes install -- 
"$@" |&
distro.sh-57-# Pass input to output, fail if a missing package is 
reported
distro.sh-58-# TODO Remove and switch to DNF once
distro.sh:59:# https://bugzilla.redhat.com/show_bug.cgi?id=1128139 
is fixed
distro.sh-60-awk 'BEGIN {s=0}
distro.sh-61- /^No package .* available.$/ {s=1}
distro.sh-62- {print}
distro.sh-63- END {exit s}'
distro.sh-64-elif [[ "$DISTRO_BRANCH" == -debian-* ]]; then
distro.sh-65-[ $# != 0 ] && sudo -p "$prompt" apt-get --yes install -- 
"$@"
distro.sh-66-else


The BZ1128139 was closed as duplicate of BZ1107737.
The BZ1107737 was fixed 7 months ago and is available in
dnf >= 0.6.4-2. The Fedora 21 currently have dnf-0.6.4-7.
I think it's the best time for removing this todo.


I would very much like to get rid of it, but unfortunately we're blocked by
this DNF bug here:

 https://bugzilla.redhat.com/show_bug.cgi?id=1215208

Due to this bug we still have to use yum-deprecated on Fedora Rawhide (by
modifying /usr/bin/yum to invoke yum-deprecated instead of dnf) and the bug
mentioned in the TODO wasn't and likely won't be fixed in yum.


So, I think we can only update the bug number for now. The patch is attached.

Nick


Would you mind open upstream tracking ticket rather then changing
number of BZ? We have Continuous integration bucket.


I opened the ticket: https://fedorahosted.org/sssd/ticket/2809

However, I think it's better to also update the source, so the next person
reading it doesn't have the same question and/or tries to switch to DNF only
to find another problem, the one described in the bug I mention.


It's fine there is a plan how to fix BZ1215208 but it would be also good to
know what is an ETA (at least approximately :-)


I asked one of the DNF developers in a comment to the DNF bug. Let's see if we
get any answer.

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] AD: add debug messages for netlogon get info

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 23, 2015 at 07:11:32AM +0200, Petr Cech wrote:
> On 09/22/2015 05:25 PM, Pavel Reichl wrote:
> >Hello, please see trivial patch attached.
> Hello,
> it works.
> CI tests: http://sssd-ci.duckdns.org/logs/job/27/54/summary.html
> 
> ACK
> 
> Petr

* master: 1e87219471c1220c773ea75b211ad0a4d087d869
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] intg: Add more LDAP tests

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:

NOTE: These still break test_memory_cache.py as seen in the attached log file.


Here's a fresher log, with the correct line numbers:

http://sssd-ci.duckdns.org/logs/job/28/19/rhel7/ci-build-debug/ci-make-intgcheck.log

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] CI: Run integration tests on debian testing

2015-09-30 Thread Jakub Hrozek
On Wed, Sep 30, 2015 at 04:08:09PM +0300, Nikolai Kondrashov wrote:
> Thank you, Lukas!
> 
> ACK.

CI: http://sssd-ci.duckdns.org/logs/job/28/16/summary.html
* master:
* cf37196dca93a8785c5a4e0af6e9a5053bff4e3a
* 90063840941efb2015d4375333677e3c26b1f4e6
* cccbf6f5b2b80cd3f6311ccad96780faab93e1f7
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


Re: [SSSD] [PATCH] intg: Add more LDAP tests

2015-09-30 Thread Nikolai Kondrashov

On 09/30/2015 06:15 PM, Nikolai Kondrashov wrote:

-create_conf_fixture(request, conf)
+create_conf_fixture(request, format_basic_conf(ldap_conn, False, True))


Actually, I wanted to do one more thing with this and forgot:
name the "bis" and "enum" parameters explicitly on each invocation to make it
clear what's happening. E.g.:

create_conf_fixture(request,
format_basic_conf(ldap_conn, bis=False, enum=True))

Anyone in favor of/against this?

Nick
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


[SSSD] [PATCH] man: Minor fixes to filter_groups description

2015-09-30 Thread Nikolai Kondrashov

Hi everyone,

I noticed one little thing was wrong with the combined
filter_users/filter_groups description on the sssd.conf(5) manpage and also
wanted to add a note WRT nested groups behavior with filter_groups which was a
bit surprising to me. The trivial patches are attached.

Nick
>From 6cfaeaaf82f9cd8e1400ba4db6875646749c0375 Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov 
Date: Wed, 30 Sep 2015 18:33:04 +0300
Subject: [PATCH 1/2] man: Mention groups in filter_groups description

Mention groups (not only users) in the combined
"filter_users"/"filter_groups" option description on the sssd.conf(5)
manpage.
---
 src/man/sssd.conf.5.xml | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index 9701f2a..a49938b 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -583,11 +583,11 @@ subdomain_inherit = ldap_purge_cache_timeout
 filter_users, filter_groups (string)
 
 
-Exclude certain users from being fetched from the sss
-NSS database. This is particularly useful for system
-accounts. This option can also be set per-domain or
-include fully-qualified names to filter only users from
-the particular domain.
+Exclude certain users or groups from being fetched
+from the sss NSS database. This is particularly
+useful for system accounts. This option can also
+be set per-domain or include fully-qualified names
+to filter only users from the particular domain.
 
 
 Default: root
-- 
2.5.1

>From 761e845dc0fac806e6c1587239a54d393452970c Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov 
Date: Wed, 30 Sep 2015 18:34:44 +0300
Subject: [PATCH 2/2] man: Note filter_groups are not affecting nesting

Note that the "filter_groups" option doesn't affect nested member
inheritance, on the sssd.conf(5) manpage.
---
 src/man/sssd.conf.5.xml | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index a49938b..fa1ea31 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -590,6 +590,14 @@ subdomain_inherit = ldap_purge_cache_timeout
 to filter only users from the particular domain.
 
 
+NOTE: The filter_groups option doesn't affect
+inheritance of nested group members, since
+filtering happens after they are propagated for
+returning via NSS. E.g. a group having a member
+group filtered out will still have the member
+users of the latter listed.
+
+
 Default: root
 
 
-- 
2.5.1

___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


[SSSD] [PATCH] intg: Add more LDAP tests

2015-09-30 Thread Nikolai Kondrashov

Hi everyone,

Here is a patch set fixing some things in integration tests and adding more
LDAP tests:

* Adding/removing a user/group/membership with rfc2307(bis) schema.
* Filtering users/groups with rfc2307(bis) schema.
* The effect of override_homedir option.
* The effect of fallback_homedir option.
* The effect of override_shell option.
* The effect of shell_fallback option.
* The effect of default_shell option.
* The effect of vetoed_shells option.

These are pretty basic, but I think they're good for the start.
Suggestions for more tests are welcome :)

NOTE: These still break test_memory_cache.py as seen in the attached log file.
  We need to figure out why and do something with it. Otherwise, the
  tests work fine.

Nick
>From d09a103901a6f86873f9510152c600a062e255ed Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov 
Date: Tue, 29 Sep 2015 20:00:14 +0300
Subject: [PATCH 1/7] intg: Get base DN from LDAP connection object

Don't use the global LDAP_BASE_DN in integration tests and fixtures, but
instead take it from the LDAP connection object (ldap_conn) passed to
them explicitly. This makes the tests and fixtures a bit more modular.
---
 src/tests/intg/ldap_test.py | 8 
 src/tests/intg/test_memory_cache.py | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index ea114dc..0287a28 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -105,7 +105,7 @@ def create_sssd_fixture(request):
 
 @pytest.fixture
 def sanity_rfc2307(request, ldap_conn):
-ent_list = ldap_ent.List(LDAP_BASE_DN)
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)
 ent_list.add_user("user2", 1002, 2002)
 ent_list.add_user("user3", 1003, 2003)
@@ -150,7 +150,7 @@ def sanity_rfc2307(request, ldap_conn):
 
 @pytest.fixture
 def simple_rfc2307(request, ldap_conn):
-ent_list = ldap_ent.List(LDAP_BASE_DN)
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user('usr001', 181818, 181818)
 ent_list.add_group("group1", 181818)
 
@@ -181,7 +181,7 @@ def simple_rfc2307(request, ldap_conn):
 
 @pytest.fixture
 def sanity_rfc2307_bis(request, ldap_conn):
-ent_list = ldap_ent.List(LDAP_BASE_DN)
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)
 ent_list.add_user("user2", 1002, 2002)
 ent_list.add_user("user3", 1003, 2003)
@@ -309,7 +309,7 @@ def test_sanity_rfc2307_bis(ldap_conn, sanity_rfc2307_bis):
 
 @pytest.fixture
 def refresh_after_cleanup_task(request, ldap_conn):
-ent_list = ldap_ent.List(LDAP_BASE_DN)
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)
 
 ent_list.add_group_bis("group1", 2001, ["user1"])
diff --git a/src/tests/intg/test_memory_cache.py b/src/tests/intg/test_memory_cache.py
index 1f4ccb0..76d85fd 100644
--- a/src/tests/intg/test_memory_cache.py
+++ b/src/tests/intg/test_memory_cache.py
@@ -109,7 +109,7 @@ def create_sssd_fixture(request):
 
 
 def load_data_to_ldap(request, ldap_conn):
-ent_list = ldap_ent.List(LDAP_BASE_DN)
+ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)
 ent_list.add_user("user2", 1002, 2002)
 ent_list.add_user("user3", 1003, 2003)
-- 
2.5.1

>From 8eb90565998f43fdc6b9ea3564527620c862f7fb Mon Sep 17 00:00:00 2001
From: Nikolai Kondrashov 
Date: Tue, 29 Sep 2015 20:13:04 +0300
Subject: [PATCH 2/7] intg: Remove _rfc2307 from function names

Remove "_rfc2307" from integration test function names for brevity.
---
 src/tests/intg/ldap_test.py | 12 +++
 src/tests/intg/test_memory_cache.py | 70 ++---
 2 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/src/tests/intg/ldap_test.py b/src/tests/intg/ldap_test.py
index 0287a28..e359ab4 100644
--- a/src/tests/intg/ldap_test.py
+++ b/src/tests/intg/ldap_test.py
@@ -104,7 +104,7 @@ def create_sssd_fixture(request):
 
 
 @pytest.fixture
-def sanity_rfc2307(request, ldap_conn):
+def sanity(request, ldap_conn):
 ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)
 ent_list.add_user("user2", 1002, 2002)
@@ -149,7 +149,7 @@ def sanity_rfc2307(request, ldap_conn):
 
 
 @pytest.fixture
-def simple_rfc2307(request, ldap_conn):
+def simple(request, ldap_conn):
 ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user('usr001', 181818, 181818)
 ent_list.add_group("group1", 181818)
@@ -180,7 +180,7 @@ def simple_rfc2307(request, ldap_conn):
 
 
 @pytest.fixture
-def sanity_rfc2307_bis(request, ldap_conn):
+def sanity_bis(request, ldap_conn):
 ent_list = ldap_ent.List(ldap_conn.ds_inst.base_dn)
 ent_list.add_user("user1", 1001, 2001)