date:20161110

[SSSD] [sssd PR#39][comment] RESPONDER: Enable sudoRule in case insen. domains (1.13)

2016-11-10 Thread celestian

  URL: https://github.com/SSSD/sssd/pull/39
Title: #39: RESPONDER: Enable sudoRule in case insen. domains (1.13)

celestian commented:
"""
I am sure this is enough. Maybe it is not the most direct solution. I try to 
explain it:

We have user ```Administrator```, sysdb record looks like (minor items missed):
```
dn: name=Administrator,cn=users,cn=scorpion.domain,cn=sysdb
fullName: Administrator
gecos: Administrator
gidNumber: 342400513
name: Administrator
objectClass: user
uidNumber: 342400500
objectSIDString: S-1-5-21-2022941956-2492201804-3493196904-500
uniqueID: c153af46-809a-41a0-baa6-de76b587e061
originalDN: CN=Administrator,CN=Users,DC=scorpion,DC=domain
entryUSN: 69662
nameAlias: administrator
```
And we have ```lessrule```:
```
dn: name=lessrule,cn=sudorules,cn=custom,cn=scorpion.domain,cn=sysdb
cn: lessrule
dataExpireTimestamp: 1478853348
entryUSN: 45204
name: lessrule
objectClass: sudoRule
originalDN: CN=lessrule,OU=sudoers,DC=scorpion,DC=domain
sudoCommand: /usr/bin/less
sudoHost: ALL
sudoUser: Administrator
sudoUser: administrator
distinguishedName: name=lessrule,cn=sudorules,cn=custom,cn=scorpion.domain,cn=
 sysdb
```

If we look at ```/var/log/secure```:
```
Nov 11 08:02:59 client sudo: pam_sss(sudo:auth): authentication success; 
logname=administrator uid=342400500 euid=0 tty=/dev/pts/2 ruser=administrator 
rhost= user=administrator
Nov 11 08:02:59 client sudo: administrator : TTY=pts/2 ; 
PWD=/home/administrator@scorpion.domain ; USER=root ; COMMAND=/bin/less 
/etc/resolv.conf
Nov 11 08:02:59 client sudo: pam_systemd(sudo:session): Cannot create session: 
Already running in a session
Nov 11 08:02:59 client sudo: pam_unix(sudo:session): session opened for user 
root by administrator(uid=0)
```

I understand that it is searched by the correct name, but sudo finally accepts 
a name with lowercase letters. If I remove lowercase name from ```sudoRule``` 
it doesn't work anymore. I wonder if it is a way to make sudo to work with 
original login name.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/39#issuecomment-259897355
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#75][synchronized] Add configuirable max payload size limit of a secret

2016-11-10 Thread fidencio

   URL: https://github.com/SSSD/sssd/pull/75
Author: fidencio
 Title: #75: Add configuirable max payload size limit of a secret
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/75/head:pr75
git checkout pr75
From d890d26f7755b3851578688e9b481c3b52212292 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 8 Nov 2016 16:39:48 +0100
Subject: [PATCH 1/2] SECRETS: Delete all secret stored during "max_secrets"
 test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Otherwise we will have an 507 error in case any secret is added by any
of the tests that may be implemented in the future.

Signed-off-by: Fabiano Fidêncio 
---
 src/tests/intg/test_secrets.py | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/tests/intg/test_secrets.py b/src/tests/intg/test_secrets.py
index 57b8f3f..09a91e0 100644
--- a/src/tests/intg/test_secrets.py
+++ b/src/tests/intg/test_secrets.py
@@ -151,6 +151,10 @@ def test_crd_ops(setup_for_secrets, secrets_cli):
 cli.set_secret(str(MAX_SECRETS), sec_value)
 assert str(err507.value).startswith("507")
 
+# Delete all stored secrets used for max secrets tests
+for x in xrange(MAX_SECRETS):
+cli.del_secret(str(x))
+
 
 def test_containers(setup_for_secrets, secrets_cli):
 """

From 96e23e6da06fcd6ac4469e9354ee7cebac1a4d3f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 8 Nov 2016 16:46:21 +0100
Subject: [PATCH 2/2] SECRETS: Add configurable payload size limit of a secret
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Resolves:
https://fedorahosted.org/sssd/ticket/3169

Signed-off-by: Fabiano Fidêncio 
---
 src/confdb/confdb.h|  1 +
 src/config/SSSDConfig/__init__.py.in   |  1 +
 src/config/cfg_rules.ini   |  1 +
 src/config/etc/sssd.api.conf   |  1 +
 src/man/sssd-secrets.5.xml | 12 
 src/responder/secrets/local.c  | 29 +
 src/responder/secrets/providers.c  |  4 
 src/responder/secrets/secsrv.c | 13 +
 src/responder/secrets/secsrv.h |  1 +
 src/responder/secrets/secsrv_private.h |  1 +
 src/tests/intg/test_secrets.py | 15 +++
 src/util/util_errors.c |  1 +
 src/util/util_errors.h |  1 +
 13 files changed, 81 insertions(+)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 2a1e581..12beaab 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -226,6 +226,7 @@
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
 #define CONFDB_SEC_CONTAINERS_NEST_LEVEL "containers_nest_level"
 #define CONFDB_SEC_MAX_SECRETS "max_secrets"
+#define CONFDB_SEC_MAX_PAYLOAD_SIZE "max_payload_size"
 
 
 struct confdb_ctx;
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index 381ff95..be09e8f 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -123,6 +123,7 @@ option_strings = {
 'provider': _('The provider where the secrets will be stored in'),
 'containers_nest_level': _('The maximum allowed number of nested containers'),
 'max_secrets': _('The maximum number of secrets that can be stored'),
+'max_payload_size': _('The maximum payload size of a secret in kilobytes'),
 # secrets - proxy
 'proxy_url': _('The URL Custodia server is listening on'),
 'auth_type': _('The method to use when authenticating to a Custodia server'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 882a185..ec44bff 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -230,6 +230,7 @@ option = client_idle_timeout
 option = description
 option = containers_nest_level
 option = max_secrets
+option = max_payload_size
 
 [rule/allowed_sec_users_options]
 validator = ini_allowed_options
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index be24bce..d591228 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -100,6 +100,7 @@ user_attributes = str, None, false
 provider = str, None, false
 containers_nest_level = int, None, false
 max_secrets = int, None, false
+max_payload_size = int, None, false
 # Secrets service - proxy
 proxy_url = str, None, false
 auth_type = str, None, false
diff --git a/src/man/sssd-secrets.5.xml b/src/man/sssd-secrets.5.xml
index 7ec54c2..80e9c40 100644
--- a/src/man/sssd-secrets.5.xml
+++ b/src/man/sssd-secrets.5.xml
@@ -168,6 +168,18 @@ systemctl enable sssd-secrets.service
 
 
 
+
+max_payload_size (integer)
+
+
+This option specifies the maximum payload size

[SSSD] [sssd PR#74][closed] IPA/AD: check auth ctx before using it

2016-11-10 Thread jhrozek

   URL: https://github.com/SSSD/sssd/pull/74
Author: sumit-bose
 Title: #74: IPA/AD: check auth ctx before using it
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/74/head:pr74
git checkout pr74
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#53][-Accepted] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

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#53][closed] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

   URL: https://github.com/SSSD/sssd/pull/53
Author: fidencio
 Title: #53: Fixes in the config API related to secrets responder
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/53/head:pr53
git checkout pr53
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

jhrozek commented:
"""
Since there is no ticket, I only pushed the patches to master:
682c9c3467055c2149af28826f7458b857b0f8c4
da8801c363716533f60bc78e10f3a2100cebc3a1

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/53#issuecomment-259814966
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#53][+Pushed] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

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#53][comment] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

jhrozek commented:
"""
Thanks, ACK
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/53#issuecomment-259810451
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#53][+Accepted] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

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#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

jhrozek commented:
"""
master: ea11ed3ea6291488dd762033246edc4ce3951aeb
sssd-1-14: 37e070c8c2ea79d8d84bae3da3a34c81212744ab
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259806918
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][-Accepted] IPA/AD: check auth ctx before using it

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

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#74][+Pushed] IPA/AD: check auth ctx before using it

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

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#79][comment] LIBSSS_CONFIG: Drop libsss_config

2016-11-10 Thread fidencio

  URL: https://github.com/SSSD/sssd/pull/79
Title: #79: LIBSSS_CONFIG: Drop libsss_config

fidencio commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/56/93/summary.html
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/79#issuecomment-259803386
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#69][+Changes requested] krb5: Use command line arguments instead env vars for krb5_child

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/69
Title: #69: krb5: Use command line arguments instead env vars for krb5_child

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#79][opened] LIBSSS_CONFIG: Drop libsss_config

2016-11-10 Thread fidencio

   URL: https://github.com/SSSD/sssd/pull/79
Author: fidencio
 Title: #79: LIBSSS_CONFIG: Drop libsss_config
Action: opened

PR body:
"""
lib_config has been used only by OpenLMI and the project has been
deprecated making, then, no sense to keep the support on SSSD.

Distros that, for some reason, are still packing and distributing
OpenLMI can stick to SSSD 1.14 branch.

Signed-off-by: Fabiano Fidêncio 
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/79/head:pr79
git checkout pr79
From c4a08ee4a5fe1fdf363ce15b305c71ef875392c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Thu, 10 Nov 2016 18:31:02 +0100
Subject: [PATCH] LIBSSS_CONFIG: Drop libsss_config
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

lib_config has been used only by OpenLMI and the project has been
deprecated making, then, no sense to keep the support on SSSD.

Distros that, for some reason, are still packing and distributing
OpenLMI can stick to SSSD 1.14 branch.

Signed-off-by: Fabiano Fidêncio 
---
 Makefile.am|  47 --
 configure.ac   |   5 -
 contrib/sssd.spec.in   |   1 -
 src/external/configlib.m4  |  12 -
 src/external/libaugeas.m4  |  10 -
 src/responder/ifp/ifp_components.c | 228 --
 src/responder/ifp/ifp_components.h |   8 -
 src/responder/ifp/ifp_iface.c  |   3 -
 src/tests/dlopen-tests.c   |   3 -
 src/tests/sss_config-tests.c   | 884 -
 src/util/sss_config.c  | 509 -
 src/util/sss_config.h  |  71 ---
 12 files changed, 1781 deletions(-)
 delete mode 100644 src/external/configlib.m4
 delete mode 100644 src/external/libaugeas.m4
 delete mode 100644 src/tests/sss_config-tests.c
 delete mode 100644 src/util/sss_config.c
 delete mode 100644 src/util/sss_config.h

diff --git a/Makefile.am b/Makefile.am
index e037930..0c7797b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -208,12 +208,6 @@ if BUILD_SSH
 non_interactive_check_based_tests += sysdb_ssh-tests
 endif
 
-if BUILD_IFP
-if BUILD_CONFIG_LIB
-non_interactive_check_based_tests += sss_config-tests
-endif # BUILD_CONFIG_LIB
-endif # BUILD_IFP
-
 if BUILD_DBUS_TESTS
 non_interactive_check_based_tests += \
 	sbus_tests \
@@ -604,7 +598,6 @@ dist_noinst_HEADERS = \
 src/util/sss_ssh.h \
 src/util/sss_ini.h \
 src/util/sss_format.h \
-src/util/sss_config.h \
 src/util/refcount.h \
 src/util/find_uid.h \
 src/util/user_info_msg.h \
@@ -1028,24 +1021,6 @@ SSSD_INTERNAL_LTLIBS = \
 libsss_child.la \
 $(NULL)
 
-if BUILD_IFP
-if BUILD_CONFIG_LIB
-pkglib_LTLIBRARIES += libsss_config.la
-libsss_config_la_SOURCES = \
-src/util/sss_config.c
-libsss_config_la_CFLAGS = \
-$(AM_CFLAGS) \
-$(AUGEAS_CFLAGS) \
-$(TALLOC_CFLAGS)
-libsss_config_la_LIBADD = \
-$(AUGEAS_LIBS) \
-$(TALLOC_LIBS) \
-$(SSSD_INTERNAL_LTLIBS)
-libsss_config_la_LDFLAGS = \
--avoid-version
-endif # BUILD_CONFIG_LIB
-endif # BUILD_IFP
-
 lib_LTLIBRARIES = libipa_hbac.la \
   libsss_idmap.la \
   libsss_nss_idmap.la \
@@ -1387,11 +1362,6 @@ dist_dbuspolicy_DATA = \
 src/responder/ifp/org.freedesktop.sssd.infopipe.conf
 dist_dbusservice_DATA = \
 src/responder/ifp/org.freedesktop.sssd.infopipe.service
-
-if BUILD_CONFIG_LIB
-sssd_ifp_LDADD += libsss_config.la
-endif
-
 endif
 
 if BUILD_SECRETS
@@ -2094,23 +2064,6 @@ sbus_codegen_tests_LDADD = \
 
 endif # BUILD_DBUS_TESTS
 
-if BUILD_IFP
-if BUILD_CONFIG_LIB
-sss_config_tests_SOURCES = \
-src/tests/sss_config-tests.c \
-src/tests/common.c
-sss_config_tests_CFLAGS = \
-$(AM_CFLAGS) \
-$(CHECK_CFLAGS)
-sss_config_tests_LDADD = \
-$(SSSD_LIBS) \
-$(CHECK_LIBS) \
-$(SSSD_INTERNAL_LTLIBS) \
-libsss_config.la \
-libsss_test_common.la
-endif # BUILD_CONFIG_LIB
-endif # BUILD_IFP
-
 if HAVE_CMOCKA
 
 TEST_MOCK_RESP_OBJ = \
diff --git a/configure.ac b/configure.ac
index d3ef1e1..d48f08c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -195,7 +195,6 @@ m4_include([src/external/signal.m4])
 m4_include([src/external/inotify.m4])
 m4_include([src/external/samba.m4])
 m4_include([src/external/sasl.m4])
-m4_include([src/external/configlib.m4])
 m4_include([src/external/libnfsidmap.m4])
 m4_include([src/external/cwrap.m4])
 m4_include([src/external/libresolv.m4])
@@ -208,10 +207,6 @@ if test x$with_secrets = xyes; then
 m4_include([src/external/libjansson.m4])
 fi
 
-if test x$build_config_lib = xyes; then
-m4_include([src/external/libaugeas.m4])
-fi
-
 WITH_UNICODE_LIB
 if test x$unicode_lib = xlibunistring; then
 m4_include([src/external/libunistring.m4])
diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
index 62f3e41..2917629 100644
---

[SSSD] [sssd PR#68][comment] MAN: Document different defaults for IPA and AD providers

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/68
Title: #68: MAN: Document different defaults for IPA and AD providers

jhrozek commented:
"""
ACK. I won't push the patches until tomorrow though in case @lslebodn or 
someone else wanted to take a look as well. I'll also wait for CI results.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/68#issuecomment-259801186
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#68][+Accepted] MAN: Document different defaults for IPA and AD providers

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/68
Title: #68: MAN: Document different defaults for IPA and AD providers

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#53][comment] Fixes in the config API related to secrets responder

2016-11-10 Thread fidencio

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

fidencio commented:
"""
Change done and patchset updated.

Just to make things easier for the reviewer, here is what the fix up looks like:
```

[ffidenci@cat sssd]$ git diff
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 8a5290e..882a185 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -228,7 +228,6 @@ option = reconnection_retries
 option = fd_limit
 option = client_idle_timeout
 option = description
-option = provider
 option = containers_nest_level
 option = max_secrets
 
@@ -237,6 +236,7 @@ validator = ini_allowed_options
 section_re = ^secrets/users/[0-9]\+$
 
 # Secrets service - proxy
+option = provider
 option = proxy_url
 option = auth_type
 option = auth_header_name

```
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/53#issuecomment-259799269
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#53][-Changes requested] Fixes in the config API related to secrets responder

2016-11-10 Thread fidencio

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

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#53][+Changes requested] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

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#53][comment] Fixes in the config API related to secrets responder

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

jhrozek commented:
"""
On Tue, Nov 08, 2016 at 05:40:21AM -0800, lslebodn wrote:
> On (08/11/16 05:15), fidencio wrote:
> >@jhrozek:
> >For the first patch the tests are correct. @lslebodn also complained that 
> >[secrets/users/] could be a valid case in the way the code is in git right 
> >now and it's also fixed by my patch. In any case, seems that we don't allow 
> >any config section to end with "/".
> >
> >For the second test, I guess that good tests are adding configuration 
> >options that are only allowed for [secrets] into the [secrets/users/123] 
> >section and vice-versa.
> >
> >Example of a config that should fail:
> >```
> >[secrets]
> >proxy_url = foo
> >
> >[secrets/users/123]
> >timeout = 10
> >```
> >
> >Example of a config that should not fail:
> >```
> >[secrets]
> >debug_level = 9
> >
> >[secrets/users/123]
> >proxy_url = foo
> >```
> >@lslebodn, does it make sense for you?
> >
> I am fine with the 1st patch. But I am not very familiar with
> the secrets code therefore It would take me much more time to review
> 2nd patch. I prefer if @jhrozek could review it.

(Sorry, I accidentally replied to sssd-devel instead of here).

The provider= option should be moved to the per-uid section, because
currently only the user sections can select a provider, the global
secrets section is always 'local'.

Otherwise LGTM.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/53#issuecomment-259797538
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: [sssd PR#53][comment] Fixes in the config API related to secrets responder

2016-11-10 Thread Jakub Hrozek

On Tue, Nov 08, 2016 at 02:40:23PM +0100, lslebodn wrote:
> I am fine with the 1st patch. But I am not very familiar with
> the secrets code therefore It would take me much more time to review
> 2nd patch. I prefer if @jhrozek could review it.

I see one glitch there. We should move the provider option to the
per-user sections. This should be allowed:

```
[secrets/users/123]
provider = proxy
```

On the other hand, the "global" section cannot select a provider, the
global section is (at the moment) only local.

Otherwise LGTM.
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#68][comment] MAN: Document different defaults for IPA and AD providers

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/68
Title: #68: MAN: Document different defaults for IPA and AD providers

lslebodn commented:
"""
I can see that tokengroups are documented in this PR. So I think we can close 
upstream ticket https://fedorahosted.org/sssd/ticket/3233 as a duplicate of 
https://fedorahosted.org/sssd/ticket/3214
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/68#issuecomment-259777558
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#68][-Changes requested] MAN: Document different defaults for IPA and AD providers

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/68
Title: #68: MAN: Document different defaults for IPA and AD providers

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#69][comment] krb5: Use command line arguments instead env vars for krb5_child

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/69
Title: #69: krb5: Use command line arguments instead env vars for krb5_child

lslebodn commented:
"""
The password changed failed for me with this patches. I had also applied 
patches for PR  #77.
But I doubt they could cause such change. I tested just with ad_provider.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/69#issuecomment-259758498
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#69][-Accepted] krb5: Use command line arguments instead env vars for krb5_child

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/69
Title: #69: krb5: Use command line arguments instead env vars for krb5_child

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#50][comment] [RFC] Use GNULIB's compiler warning code

2016-11-10 Thread fidencio

  URL: https://github.com/SSSD/sssd/pull/50
Title: #50: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
So, here are some stats about the configure time that you complained before:
So, I've ran the contrib/ci/run script on our F25 machine on Jenkins.

This is what we have **without**  GNULIB patch:
```
-bash-4.3$ ./contrib/ci/run
install-deps:   success  00:00:05 ci-install-deps.log
autoreconf: success  00:00:22 ci-autoreconf.log
DEBUG BUILD:  ci-build-debug
configure:  success  00:00:16 ci-build-debug/ci-configure.log
make-tests: success  00:02:55 ci-build-debug/ci-make-tests.log
make-check-valgrind:success  00:01:33 
ci-build-debug/ci-make-check-valgrind.log
SUCCESS

```

And this is what we have **with** GNULIB patch applied:
```
-bash-4.3$ ./contrib/ci/run 
install-deps:   success  00:00:05 ci-install-deps.log
autoreconf: success  00:00:23 ci-autoreconf.log
DEBUG BUILD:  ci-build-debug
configure:  success  00:00:22 ci-build-debug/ci-configure.log
make-tests: success  00:03:56 ci-build-debug/ci-make-tests.log
make-check-valgrind:success  00:01:33 
ci-build-debug/ci-make-check-valgrind.log
SUCCESS

```

It adds 6 seconds more to the configure time. I really think that dropping 
those patches because of 6 seconds on the configure is far, but really really, 
far away from being reasonable.

So, can we have the patches reviewed and merged?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/50#issuecomment-259738984
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#50][comment] [RFC] Use GNULIB's compiler warning code

2016-11-10 Thread fidencio

  URL: https://github.com/SSSD/sssd/pull/50
Title: #50: [RFC] Use GNULIB's compiler warning code

fidencio commented:
"""
So, here are some stats about the configure time that you complained before:
So, I've ran the contrib/ci/run script on our F25 machine on Jenkins.

This is what we have **without**  GNULIB patch:
```
-bash-4.3$ ./contrib/ci/run
install-deps:   success  00:00:05 ci-install-deps.log
autoreconf: success  00:00:22 ci-autoreconf.log
DEBUG BUILD:  ci-build-debug
configure:  success  00:00:16 ci-build-debug/ci-configure.log
make-tests: success  00:02:55 ci-build-debug/ci-make-tests.log
make-check-valgrind:success  00:01:33 
ci-build-debug/ci-make-check-valgrind.log
SUCCESS

```

And this is what we have **with** GNULIB patch applied:
```
-bash-4.3$ ./contrib/ci/run 
install-deps:   success  00:00:05 ci-install-deps.log
autoreconf: success  00:00:23 ci-autoreconf.log
DEBUG BUILD:  ci-build-debug
configure:  success  00:00:22 ci-build-debug/ci-configure.log
make-tests: success  00:03:56 ci-build-debug/ci-make-tests.log
make-check-valgrind:success  00:01:33 
ci-build-debug/ci-make-check-valgrind.log
SUCCESS

```

It adds 6 seconds more in the configure time. I really think that dropping 
those patches because of 6 seconds on the configure is far, but really really, 
far away from being reasonable.

So, can we have the patches reviewed and merged?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/50#issuecomment-259738984
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Nested netgroups with IPA provider

2016-11-10 Thread Michal Židek


On 11/10/2016 03:18 PM, Michal Židek wrote:

On 11/10/2016 02:13 PM, Lukas Slebodnik wrote:

On (10/11/16 13:57), Michal Židek wrote:

On 11/10/2016 12:29 PM, Jakub Hrozek wrote:

On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:

Hi,

this is continuation of discussion about pull
request 51 and associated tickets.

For context, see:
https://github.com/SSSD/sssd/pull/59
https://fedorahosted.org/sssd/ticket/3159
https://fedorahosted.org/sssd/ticket/3116

The FreeIPA UQE guys added upstream test for this issue
because we do not have upstream CI tests in SSSD with
IPA provider yet and this bug is not present in the
plain LDAP.

We use hash tables to store members of netgroups while
processing netgroups (and creating the netgroup triples).
The netgroup names are lowercased before they are stored
in the hash table. The reason for this normalization is
unknown to me.


I also don't know the reason behind this normalization. There is a
comment above the code that lowercases the DN that says:
646 /* Transform the DN to lower case.
647  * this is important, as the member/memberof attributes
648  * have the value also in lower-case
649  */

But I really have no idea what this means. We're using
SYSDB_NETGROUP_MEMBER,
but we're storing names in that attribute and the IPA code lowercases
original DNs, so I'm quite confused about it.


FreeIPA only creates lowercased netgroup
names, so lowercasing only affects the attribute name
(that is stored as prefix to the netgroup name in the hash table,
and maybe it can happen that the attribute name can be
stored in different cases at some point, which would
explain why we lower case it, however I was not able
to confirm if this is the case).


I really think this is about the original DN values. This is what we
enter:
(gdb) p key.str

"ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"




When we read the hash table, we do not lowercase the keys,
so the nested netgroups are not found and this is the
reason why the bug appears. The patch in PR 51 lower cases
the keys before reading the hash table and the bug does not
appear after that. Lukas thinks however that this is not
good approach, because there should be no need for the lower
casing in the first place.

Patch that removes the lower casing before adding the keys
to htable should also fix the issue. I did not send the patch
with this approach, because I was not sure why the lowercasing
happens in the first place and I know that lowercasing is not
harmful for the IPA netgroups, so I find it safer to use
the approach in the PR 51 especially while we do not have good
code coverage for IPA provider, however as I mentioned in the
PR 51, I am looking for opinions on this.


I'm fine with removing the lowercasing if this moves fixing the issue
forward.


I just quickly tried it without the lowercasing and it does work
for me.


awesome

LS


The patch for master is in attachment.

Michal




Lukas wanted a new PR, so here it is:
https://github.com/SSSD/sssd/pull/78

Michal
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#78][opened] ipa: Nested netgroups do not work

2016-11-10 Thread mzidek-rh

   URL: https://github.com/SSSD/sssd/pull/78
Author: mzidek-rh
 Title: #78: ipa: Nested netgroups do not work
Action: opened

PR body:
"""
We lowercase the keys to the hash table used to store netgroups
but do not lowercase it when reading the table. This results
in nested netgroups not being found when they should and
the processing fails.

The lowercasing does not seem to be necessary anymore (not
sure if it ever was) so we can skip it.

Resolves:
https://fedorahosted.org/sssd/ticket/3159
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/78/head:pr78
git checkout pr78
From e736c0f9bdd778bef04341e3aa33ea0e5a426a52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Thu, 10 Nov 2016 15:04:57 +0100
Subject: [PATCH] ipa: Nested netgroups do not work

We lowercase the keys to the hash table used to store netgroups
but do not lowercase it when reading the table. This results
in nested netgroups not being found when they should and
the processing fails.

The lowercasing does not seem to be necessary anymore (not
sure if it ever was) so we can skip it.

Resolves:
https://fedorahosted.org/sssd/ticket/3159
---
 src/providers/ipa/ipa_netgroups.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/providers/ipa/ipa_netgroups.c b/src/providers/ipa/ipa_netgroups.c
index a19e5e0..17b11af 100644
--- a/src/providers/ipa/ipa_netgroups.c
+++ b/src/providers/ipa/ipa_netgroups.c
@@ -563,7 +563,6 @@ static void ipa_netgr_members_process(struct tevent_req *subreq)
 size_t count;
 int ret, i;
 const char *orig_dn;
-char *orig_dn_lower;
 hash_table_t *table;
 hash_key_t key;
 hash_value_t value;
@@ -638,20 +637,12 @@ static void ipa_netgr_members_process(struct tevent_req *subreq)
 goto fail;
 }
 
-orig_dn_lower = talloc_strdup(table, orig_dn);
-if (orig_dn_lower == NULL) {
+key.str = talloc_strdup(table, orig_dn);
+if (key.str == NULL) {
 ret = ENOMEM;
 goto fail;
 }
-/* Transform the DN to lower case.
- * this is important, as the member/memberof attributes
- * have the value also in lower-case
- */
-key.str = orig_dn_lower;
-while (*orig_dn_lower != '\0') {
-*orig_dn_lower = tolower(*orig_dn_lower);
-orig_dn_lower++;
-}
+
 value.ptr = entities[i];
 ret = hash_enter(table, , );
 if (ret !=  HASH_SUCCESS) {
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Nested netgroups with IPA provider

2016-11-10 Thread Michal Židek


On 11/10/2016 02:13 PM, Lukas Slebodnik wrote:

On (10/11/16 13:57), Michal Židek wrote:

On 11/10/2016 12:29 PM, Jakub Hrozek wrote:

On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:

Hi,

this is continuation of discussion about pull
request 51 and associated tickets.

For context, see:
https://github.com/SSSD/sssd/pull/59
https://fedorahosted.org/sssd/ticket/3159
https://fedorahosted.org/sssd/ticket/3116

The FreeIPA UQE guys added upstream test for this issue
because we do not have upstream CI tests in SSSD with
IPA provider yet and this bug is not present in the
plain LDAP.

We use hash tables to store members of netgroups while
processing netgroups (and creating the netgroup triples).
The netgroup names are lowercased before they are stored
in the hash table. The reason for this normalization is
unknown to me.


I also don't know the reason behind this normalization. There is a
comment above the code that lowercases the DN that says:
646 /* Transform the DN to lower case.
647  * this is important, as the member/memberof attributes
648  * have the value also in lower-case
649  */

But I really have no idea what this means. We're using SYSDB_NETGROUP_MEMBER,
but we're storing names in that attribute and the IPA code lowercases
original DNs, so I'm quite confused about it.


FreeIPA only creates lowercased netgroup
names, so lowercasing only affects the attribute name
(that is stored as prefix to the netgroup name in the hash table,
and maybe it can happen that the attribute name can be
stored in different cases at some point, which would
explain why we lower case it, however I was not able
to confirm if this is the case).


I really think this is about the original DN values. This is what we
enter:
(gdb) p key.str

"ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"



When we read the hash table, we do not lowercase the keys,
so the nested netgroups are not found and this is the
reason why the bug appears. The patch in PR 51 lower cases
the keys before reading the hash table and the bug does not
appear after that. Lukas thinks however that this is not
good approach, because there should be no need for the lower
casing in the first place.

Patch that removes the lower casing before adding the keys
to htable should also fix the issue. I did not send the patch
with this approach, because I was not sure why the lowercasing
happens in the first place and I know that lowercasing is not
harmful for the IPA netgroups, so I find it safer to use
the approach in the PR 51 especially while we do not have good
code coverage for IPA provider, however as I mentioned in the
PR 51, I am looking for opinions on this.


I'm fine with removing the lowercasing if this moves fixing the issue
forward.


I just quickly tried it without the lowercasing and it does work
for me.


awesome

LS


The patch for master is in attachment.

Michal
>From e70366ed2d5c4eb4b08237bd78f5fbced95fefec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Thu, 10 Nov 2016 15:04:57 +0100
Subject: [PATCH] ipa: Nested netgroups do not work

We lowercase the keys to the hash table used to store netgroups
but do not lowercase it when reading the table. This results
in nested netgroups not being found when they should and
the processing fails.

The lowercasing does not seem to be necessary anymore (not
sure if it ever was) so we can skip it.

Resolves:
https://fedorahosted.org/sssd/ticket/3159
---
 src/providers/ipa/ipa_netgroups.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/providers/ipa/ipa_netgroups.c b/src/providers/ipa/ipa_netgroups.c
index a19e5e0..17b11af 100644
--- a/src/providers/ipa/ipa_netgroups.c
+++ b/src/providers/ipa/ipa_netgroups.c
@@ -563,7 +563,6 @@ static void ipa_netgr_members_process(struct tevent_req *subreq)
 size_t count;
 int ret, i;
 const char *orig_dn;
-char *orig_dn_lower;
 hash_table_t *table;
 hash_key_t key;
 hash_value_t value;
@@ -638,20 +637,12 @@ static void ipa_netgr_members_process(struct tevent_req *subreq)
 goto fail;
 }
 
-orig_dn_lower = talloc_strdup(table, orig_dn);
-if (orig_dn_lower == NULL) {
+key.str = talloc_strdup(table, orig_dn);
+if (key.str == NULL) {
 ret = ENOMEM;
 goto fail;
 }
-/* Transform the DN to lower case.
- * this is important, as the member/memberof attributes
- * have the value also in lower-case
- */
-key.str = orig_dn_lower;
-while (*orig_dn_lower != '\0') {
-*orig_dn_lower = tolower(*orig_dn_lower);
-orig_dn_lower++;
-}
+
 value.ptr = entities[i];
 ret = hash_enter(table, , );
 if (ret !=  HASH_SUCCESS) {
-- 
2.5.5

___
sssd-devel mailing list --

[SSSD] Re: Nested netgroups with IPA provider

2016-11-10 Thread Lukas Slebodnik

On (10/11/16 13:57), Michal Židek wrote:
>On 11/10/2016 12:29 PM, Jakub Hrozek wrote:
>> On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:
>> > Hi,
>> > 
>> > this is continuation of discussion about pull
>> > request 51 and associated tickets.
>> > 
>> > For context, see:
>> > https://github.com/SSSD/sssd/pull/59
>> > https://fedorahosted.org/sssd/ticket/3159
>> > https://fedorahosted.org/sssd/ticket/3116
>> > 
>> > The FreeIPA UQE guys added upstream test for this issue
>> > because we do not have upstream CI tests in SSSD with
>> > IPA provider yet and this bug is not present in the
>> > plain LDAP.
>> > 
>> > We use hash tables to store members of netgroups while
>> > processing netgroups (and creating the netgroup triples).
>> > The netgroup names are lowercased before they are stored
>> > in the hash table. The reason for this normalization is
>> > unknown to me.
>> 
>> I also don't know the reason behind this normalization. There is a
>> comment above the code that lowercases the DN that says:
>> 646 /* Transform the DN to lower case.
>> 647  * this is important, as the member/memberof attributes
>> 648  * have the value also in lower-case
>> 649  */
>> 
>> But I really have no idea what this means. We're using SYSDB_NETGROUP_MEMBER,
>> but we're storing names in that attribute and the IPA code lowercases
>> original DNs, so I'm quite confused about it.
>> 
>> > FreeIPA only creates lowercased netgroup
>> > names, so lowercasing only affects the attribute name
>> > (that is stored as prefix to the netgroup name in the hash table,
>> > and maybe it can happen that the attribute name can be
>> > stored in different cases at some point, which would
>> > explain why we lower case it, however I was not able
>> > to confirm if this is the case).
>> 
>> I really think this is about the original DN values. This is what we
>> enter:
>> (gdb) p key.str
>> 
>> "ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"
>> 
>> > 
>> > When we read the hash table, we do not lowercase the keys,
>> > so the nested netgroups are not found and this is the
>> > reason why the bug appears. The patch in PR 51 lower cases
>> > the keys before reading the hash table and the bug does not
>> > appear after that. Lukas thinks however that this is not
>> > good approach, because there should be no need for the lower
>> > casing in the first place.
>> > 
>> > Patch that removes the lower casing before adding the keys
>> > to htable should also fix the issue. I did not send the patch
>> > with this approach, because I was not sure why the lowercasing
>> > happens in the first place and I know that lowercasing is not
>> > harmful for the IPA netgroups, so I find it safer to use
>> > the approach in the PR 51 especially while we do not have good
>> > code coverage for IPA provider, however as I mentioned in the
>> > PR 51, I am looking for opinions on this.
>> 
>> I'm fine with removing the lowercasing if this moves fixing the issue
>> forward.
>
>I just quickly tried it without the lowercasing and it does work
>for me.
>
awesome

LS
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Nested netgroups with IPA provider

2016-11-10 Thread Michal Židek




On 11/10/2016 12:29 PM, Jakub Hrozek wrote:

On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:

Hi,

this is continuation of discussion about pull
request 51 and associated tickets.

For context, see:
https://github.com/SSSD/sssd/pull/59
https://fedorahosted.org/sssd/ticket/3159
https://fedorahosted.org/sssd/ticket/3116

The FreeIPA UQE guys added upstream test for this issue
because we do not have upstream CI tests in SSSD with
IPA provider yet and this bug is not present in the
plain LDAP.

We use hash tables to store members of netgroups while
processing netgroups (and creating the netgroup triples).
The netgroup names are lowercased before they are stored
in the hash table. The reason for this normalization is
unknown to me.


I also don't know the reason behind this normalization. There is a
comment above the code that lowercases the DN that says:
646 /* Transform the DN to lower case.
647  * this is important, as the member/memberof attributes
648  * have the value also in lower-case
649  */

But I really have no idea what this means. We're using SYSDB_NETGROUP_MEMBER,
but we're storing names in that attribute and the IPA code lowercases
original DNs, so I'm quite confused about it.


FreeIPA only creates lowercased netgroup
names, so lowercasing only affects the attribute name
(that is stored as prefix to the netgroup name in the hash table,
and maybe it can happen that the attribute name can be
stored in different cases at some point, which would
explain why we lower case it, however I was not able
to confirm if this is the case).


I really think this is about the original DN values. This is what we
enter:
(gdb) p key.str

"ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"



When we read the hash table, we do not lowercase the keys,
so the nested netgroups are not found and this is the
reason why the bug appears. The patch in PR 51 lower cases
the keys before reading the hash table and the bug does not
appear after that. Lukas thinks however that this is not
good approach, because there should be no need for the lower
casing in the first place.

Patch that removes the lower casing before adding the keys
to htable should also fix the issue. I did not send the patch
with this approach, because I was not sure why the lowercasing
happens in the first place and I know that lowercasing is not
harmful for the IPA netgroups, so I find it safer to use
the approach in the PR 51 especially while we do not have good
code coverage for IPA provider, however as I mentioned in the
PR 51, I am looking for opinions on this.


I'm fine with removing the lowercasing if this moves fixing the issue
forward.


I just quickly tried it without the lowercasing and it does work
for me.

Michal
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#39][comment] RESPONDER: Enable sudoRule in case insen. domains (1.13)

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/39
Title: #39: RESPONDER: Enable sudoRule in case insen. domains (1.13)

jhrozek commented:
"""
Are you sure this is enough? Because when the patch is applied, I see that we 
only match the sudoUser value with the original case. Don't we also need to 
match the lowercase version of the username?

This is what sssd_sudo searches for:
```
(Thu Nov 10 13:11:01 2016) [sssd[sudo]] [sudosrv_get_sudorules_query_cache] 
(0x0200): Searching sysdb with 
[(&(objectClass=sudoRule)(|(sudoUser=ALL)(sudoUser=Administrator)(sudoUser=#679800500)(sudoUser=%Group\20Policy\20Creator\20Owners)(sudoUser=%Enterprise\20Admins)(sudoUser=%Domain\20Admins)(sudoUser=%Schema\20Admins)(sudoUser=%Domain\20Users)(sudoUser=%Denied\20RODC\20Password\20Replication\20Group)(sudoUser=%sudogroup)(sudoUser=%Domain\20Users)(sudoUser=+*)))]
```

And this is the rule definition:
```
dn: name=morerule,cn=sudorules,cn=custom,cn=win.trust.test,cn=sysdb
cn: morerule
dataExpireTimestamp: 1478785266
entryUSN: 65695
name: morerule
objectClass: sudoRule
originalDN: CN=morerule,OU=sudoers,DC=win,DC=trust,DC=test
sudoCommand: /bin/more
sudoCommand: /usr/bin/more
sudoHost: ALL
sudoUser: administrator
distinguishedName: name=morerule,cn=sudorules,cn=custom,cn=win.trust.test,cn=s
 ysdb
```

So 
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/39#issuecomment-259675726
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#39][+Changes requested] RESPONDER: Enable sudoRule in case insen. domains (1.13)

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/39
Title: #39: RESPONDER: Enable sudoRule in case insen. domains (1.13)

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#39][comment] RESPONDER: Enable sudoRule in case insen. domains (1.13)

2016-11-10 Thread jhrozek

  URL: https://github.com/SSSD/sssd/pull/39
Title: #39: RESPONDER: Enable sudoRule in case insen. domains (1.13)

jhrozek commented:
"""
oops...clicked send to early. I meant to say "So the filter never matches the 
lowercase sudoUser".
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/39#issuecomment-259675812
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread celestian

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

celestian commented:
"""
I wrote comment to https://fedorahosted.org/sssd/ticket/2818 and I closed 
https://fedorahosted.org/sssd/ticket/3238.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259671320
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread celestian

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

celestian commented:
"""
I wrote comment to t2818 and I closed t3238.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259671320
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread sumit-bose

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

sumit-bose commented:
"""
opps, sorry, I just opened https://fedorahosted.org/sssd/ticket/3238, feel free 
to close it as duplicate.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259669837
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

lslebodn commented:
"""
On (10/11/16 03:12), celestian wrote:
>Code looks good to me. CI tests passed: 
>http://sssd-ci.duckdns.org/logs/job/56/88/summary.html
>And I manually test it.
>
>@sumit-bose, could you please file ticket for witting test on it? I understand 
>it is not possible to write it now.
This one would require ad provider.
And IIRC you (@celestian) should prepare infrastructure for testing it :-)
And we already have a ticket for it
https://fedorahosted.org/sssd/ticket/2818

You can write a comment there for testing this case.
Otherwise we would have buch of tickets for testing AD provider.

LS


"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259668596
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Re: Nested netgroups with IPA provider

2016-11-10 Thread Jakub Hrozek

On Thu, Nov 10, 2016 at 10:49:55AM +0100, Michal Židek wrote:
> Hi,
> 
> this is continuation of discussion about pull
> request 51 and associated tickets.
> 
> For context, see:
> https://github.com/SSSD/sssd/pull/59
> https://fedorahosted.org/sssd/ticket/3159
> https://fedorahosted.org/sssd/ticket/3116
> 
> The FreeIPA UQE guys added upstream test for this issue
> because we do not have upstream CI tests in SSSD with
> IPA provider yet and this bug is not present in the
> plain LDAP.
> 
> We use hash tables to store members of netgroups while
> processing netgroups (and creating the netgroup triples).
> The netgroup names are lowercased before they are stored
> in the hash table. The reason for this normalization is
> unknown to me. 

I also don't know the reason behind this normalization. There is a
comment above the code that lowercases the DN that says:
646 /* Transform the DN to lower case.
647  * this is important, as the member/memberof attributes
648  * have the value also in lower-case
649  */

But I really have no idea what this means. We're using SYSDB_NETGROUP_MEMBER,
but we're storing names in that attribute and the IPA code lowercases
original DNs, so I'm quite confused about it.

> FreeIPA only creates lowercased netgroup
> names, so lowercasing only affects the attribute name
> (that is stored as prefix to the netgroup name in the hash table,
> and maybe it can happen that the attribute name can be
> stored in different cases at some point, which would
> explain why we lower case it, however I was not able
> to confirm if this is the case).

I really think this is about the original DN values. This is what we
enter:
(gdb) p key.str

"ipauniqueid=026e49e8-969d-11e6-a2f4-525400f6cf82,cn=ng,cn=alt,dc=ipa,dc=test"

> 
> When we read the hash table, we do not lowercase the keys,
> so the nested netgroups are not found and this is the
> reason why the bug appears. The patch in PR 51 lower cases
> the keys before reading the hash table and the bug does not
> appear after that. Lukas thinks however that this is not
> good approach, because there should be no need for the lower
> casing in the first place.
> 
> Patch that removes the lower casing before adding the keys
> to htable should also fix the issue. I did not send the patch
> with this approach, because I was not sure why the lowercasing
> happens in the first place and I know that lowercasing is not
> harmful for the IPA netgroups, so I find it safer to use
> the approach in the PR 51 especially while we do not have good
> code coverage for IPA provider, however as I mentioned in the
> PR 51, I am looking for opinions on this.

I'm fine with removing the lowercasing if this moves fixing the issue
forward.
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#74][+Accepted] IPA/AD: check auth ctx before using it

2016-11-10 Thread celestian

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

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#74][comment] IPA/AD: check auth ctx before using it

2016-11-10 Thread celestian

  URL: https://github.com/SSSD/sssd/pull/74
Title: #74: IPA/AD: check auth ctx before using it

celestian commented:
"""
Code looks good to me. CI tests passed: 
http://sssd-ci.duckdns.org/logs/job/56/88/summary.html
And I manually test it.

@sumit-bose, could you please file ticket for witting test on it? I understand 
it is not possible to write it now.

ACK
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/74#issuecomment-259664277
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] Nested netgroups with IPA provider

2016-11-10 Thread Michal Židek


Hi,

this is continuation of discussion about pull
request 51 and associated tickets.

For context, see:
https://github.com/SSSD/sssd/pull/59
https://fedorahosted.org/sssd/ticket/3159
https://fedorahosted.org/sssd/ticket/3116

The FreeIPA UQE guys added upstream test for this issue
because we do not have upstream CI tests in SSSD with
IPA provider yet and this bug is not present in the
plain LDAP.

We use hash tables to store members of netgroups while
processing netgroups (and creating the netgroup triples).
The netgroup names are lowercased before they are stored
in the hash table. The reason for this normalization is
unknown to me. FreeIPA only creates lowercased netgroup
names, so lowercasing only affects the attribute name
(that is stored as prefix to the netgroup name in the hash table,
and maybe it can happen that the attribute name can be
stored in different cases at some point, which would
explain why we lower case it, however I was not able
to confirm if this is the case).

When we read the hash table, we do not lowercase the keys,
so the nested netgroups are not found and this is the
reason why the bug appears. The patch in PR 51 lower cases
the keys before reading the hash table and the bug does not
appear after that. Lukas thinks however that this is not
good approach, because there should be no need for the lower
casing in the first place.

Patch that removes the lower casing before adding the keys
to htable should also fix the issue. I did not send the patch
with this approach, because I was not sure why the lowercasing
happens in the first place and I know that lowercasing is not
harmful for the IPA netgroups, so I find it safer to use
the approach in the PR 51 especially while we do not have good
code coverage for IPA provider, however as I mentioned in the
PR 51, I am looking for opinions on this.

Thanks,

Michal
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#59][comment] ipa_netgroups: Lowercase key to htable

2016-11-10 Thread lslebodn

  URL: https://github.com/SSSD/sssd/pull/59
Title: #59: ipa_netgroups: Lowercase key to htable

lslebodn commented:
"""
FYI: there is already better ticket
https://fedorahosted.org/sssd/ticket/3159
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/59#issuecomment-259633405
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

45 matches

Mail list logo