[SSSD] Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates

2016-10-17 Thread Jan Cholasta 

On 17.10.2016 16:50, Rob Crittenden wrote:

Jan Cholasta wrote:

Hi,

On 13.10.2016 18:52, Sumit Bose wrote:

= Issuer specific matching =
Although the MIT Kerberos rules allow to select the issuer of a
certificate there are use cases where a more specific selection is
needed. E.g. if there are some default matching rules for all issuers
and some other issuer specific rules where the default rules should
not apply. To make this possible with the above scheme the default
rules must have an  clause which matches all but the issuer
with the specific rules. Writing regular-expressions to not match a
specific string or a list of strings is at least error-prone if not
impossible.

To make it easier to define issuer specific rules and default rules at
the same time and optional issuer string can be added to the rule to
indicate that for the given issuer only those rules should be
considered. Given the use-case I think it is acceptable to require
that the full issuer must be specified here in LDAP order (see below)
and case-sensitive matching is used.


This could also be solved by adding priority to rules - if two rules
match, the one with higher priority (the issuer specific rule) is
preferred over the one with lower priority (the default rule). IMO this
is better than an optional issuer string as it offers greater
flexibility.


The use cases I've seen haven't had to do with priority, though that
would be a nice enhancement, but with only allowing certificates issued
by a specific CA to be allowed (this is pretty common in web servers).
Being able to say "only do the matching on certificates issued by foo"
is valuable.


Sure, I'm not suggesting that matching by issuer should be removed, only 
that rule precedence should not be determined by the issuer field setting.


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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

fidencio commented:
"""
On Mon, Oct 17, 2016 at 11:51 PM, lslebodn  wrote:

> On (17/10/16 14:43), fidencio wrote:
> >On Mon, Oct 17, 2016 at 11:37 PM, lslebodn 
> wrote:
> >
> >> On (17/10/16 14:26), fidencio wrote:
> >> >On Mon, Oct 17, 2016 at 10:52 PM, lslebodn 
> >> wrote:
> >> >
> >> >> On (17/10/16 12:34), fidencio wrote:
> >> >> >Please, refer to e1a58f3d in the commit message.
> >> >> >
> >> >> Why? The text is more important. Rest is useless.
> >> >>
> >> >
> >> >Well, you're basically reverting that commit.
> >> >But feel free to ignore in any case.
> >> >
> >> >
> >> >>
> >> >> >This is a genuine question (even in case it's a dumb one), but do we
> >> >> really need to call dlclose() in our tests? Can't we relax this in
> >> order to
> >> >> have a meaningful backtrace?
> >> >> >
> >> >> Let assume:
> >> >> * dlclose was not called
> >> >> * libraryA is linked with libtalloc and libtevent
> >> >> * libraryB is not linked with libtalloc (even though it should be
> >> >> * dlopen test test libraries in following order: 1. libraryA; 2.
> >> libraryB
> >> >>
> >> >> Result: missing dependency in libraryB would not be found because
> >> >> libraryA and its dependencies are still loaded in memory.
> >> >>
> >> >
> >> >Wouldn't make sense to have two tests then? One as it is nowadays. In
> case
> >> >the first passes we run the second one, not calling dlclose() and just
> >> >checking for leaks?
> >> >
> >> What would be a purpose of second test?
> >> Leaks are not checked in tests itself but by valgrind.
> >>
> >
> >By not calling dlclose() during the second run couldn't you have a "not
> >meaningless" (part of the) backtrace?
> >
> If you can reproduce any valgrind error reported by dlopen-test
> then we can consider to do it. Otherwise NACK to the idea.
> I do not want to implement something which I cannot verify whether it work
> or
> no.
>

Then I'm truly lost here, sorry.
If you think we cannot reproduce any valgrind error reported by dlopen-test
why should we remove dlopen-test from valgrind blacklist in the first place?

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

lslebodn commented:
"""
On (17/10/16 14:43), fidencio wrote:
>On Mon, Oct 17, 2016 at 11:37 PM, lslebodn  wrote:
>
>> On (17/10/16 14:26), fidencio wrote:
>> >On Mon, Oct 17, 2016 at 10:52 PM, lslebodn 
>> wrote:
>> >
>> >> On (17/10/16 12:34), fidencio wrote:
>> >> >Please, refer to e1a58f3d in the commit message.
>> >> >
>> >> Why? The text is more important. Rest is useless.
>> >>
>> >
>> >Well, you're basically reverting that commit.
>> >But feel free to ignore in any case.
>> >
>> >
>> >>
>> >> >This is a genuine question (even in case it's a dumb one), but do we
>> >> really need to call dlclose() in our tests? Can't we relax this in
>> order to
>> >> have a meaningful backtrace?
>> >> >
>> >> Let assume:
>> >> * dlclose was not called
>> >> * libraryA is linked with libtalloc and libtevent
>> >> * libraryB is not linked with libtalloc (even though it should be
>> >> * dlopen test test libraries in following order: 1. libraryA; 2.
>> libraryB
>> >>
>> >> Result: missing dependency in libraryB would not be found because
>> >> libraryA and its dependencies are still loaded in memory.
>> >>
>> >
>> >Wouldn't make sense to have two tests then? One as it is nowadays. In case
>> >the first passes we run the second one, not calling dlclose() and just
>> >checking for leaks?
>> >
>> What would be a purpose of second test?
>> Leaks are not checked in tests itself but by valgrind.
>>
>
>By not calling dlclose() during the second run couldn't you have a "not
>meaningless" (part of the) backtrace?
>
If you can reproduce any valgrind error reported by dlopen-test
then we can consider to do it. Otherwise NACK to the idea.
I do not want to implement something which I cannot verify whether it work or
no.

LS

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

fidencio commented:
"""
On Mon, Oct 17, 2016 at 11:37 PM, lslebodn  wrote:

> On (17/10/16 14:26), fidencio wrote:
> >On Mon, Oct 17, 2016 at 10:52 PM, lslebodn 
> wrote:
> >
> >> On (17/10/16 12:34), fidencio wrote:
> >> >Please, refer to e1a58f3d in the commit message.
> >> >
> >> Why? The text is more important. Rest is useless.
> >>
> >
> >Well, you're basically reverting that commit.
> >But feel free to ignore in any case.
> >
> >
> >>
> >> >This is a genuine question (even in case it's a dumb one), but do we
> >> really need to call dlclose() in our tests? Can't we relax this in
> order to
> >> have a meaningful backtrace?
> >> >
> >> Let assume:
> >> * dlclose was not called
> >> * libraryA is linked with libtalloc and libtevent
> >> * libraryB is not linked with libtalloc (even though it should be
> >> * dlopen test test libraries in following order: 1. libraryA; 2.
> libraryB
> >>
> >> Result: missing dependency in libraryB would not be found because
> >> libraryA and its dependencies are still loaded in memory.
> >>
> >
> >Wouldn't make sense to have two tests then? One as it is nowadays. In case
> >the first passes we run the second one, not calling dlclose() and just
> >checking for leaks?
> >
> What would be a purpose of second test?
> Leaks are not checked in tests itself but by valgrind.
>

By not calling dlclose() during the second run couldn't you have a "not
meaningless" (part of the) backtrace?

Best Regards,
--
Fabiano Fidêncio

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

lslebodn commented:
"""
On (17/10/16 14:26), fidencio wrote:
>On Mon, Oct 17, 2016 at 10:52 PM, lslebodn  wrote:
>
>> On (17/10/16 12:34), fidencio wrote:
>> >Please, refer to e1a58f3d in the commit message.
>> >
>> Why? The text is more important. Rest is useless.
>>
>
>Well, you're basically reverting that commit.
>But feel free to ignore in any case.
>
>
>>
>> >This is a genuine question (even in case it's a dumb one), but do we
>> really need to call dlclose() in our tests? Can't we relax this in order to
>> have a meaningful backtrace?
>> >
>> Let assume:
>> * dlclose was not called
>> * libraryA is linked with libtalloc and libtevent
>> * libraryB is not linked with libtalloc (even though it should be
>> * dlopen test test libraries in following order: 1. libraryA; 2. libraryB
>>
>> Result: missing dependency in libraryB would not be found because
>> libraryA and its dependencies are still loaded in memory.
>>
>
>Wouldn't make sense to have two tests then? One as it is nowadays. In case
>the first passes we run the second one, not calling dlclose() and just
>checking for leaks?
>
What would be a purpose of second test?
Leaks are not checked in tests itself but by valgrind.

LS

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

fidencio commented:
"""
On Mon, Oct 17, 2016 at 10:52 PM, lslebodn  wrote:

> On (17/10/16 12:34), fidencio wrote:
> >Please, refer to e1a58f3d in the commit message.
> >
> Why? The text is more important. Rest is useless.
>

Well, you're basically reverting that commit.
But feel free to ignore in any case.


>
> >This is a genuine question (even in case it's a dumb one), but do we
> really need to call dlclose() in our tests? Can't we relax this in order to
> have a meaningful backtrace?
> >
> Let assume:
> * dlclose was not called
> * libraryA is linked with libtalloc and libtevent
> * libraryB is not linked with libtalloc (even though it should be
> * dlopen test test libraries in following order: 1. libraryA; 2. libraryB
>
> Result: missing dependency in libraryB would not be found because
> libraryA and its dependencies are still loaded in memory.
>

Wouldn't make sense to have two tests then? One as it is nowadays. In case
the first passes we run the second one, not calling dlclose() and just
checking for leaks?

Best Regards,
--
Fabiano Fidêncio

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

lslebodn commented:
"""
On (17/10/16 12:34), fidencio wrote:
>Please, refer to e1a58f3d in the commit message.
>
Why? The text is more important. Rest is useless.

>This is a genuine question (even in case it's a dumb one), but do we really 
>need to call dlclose() in our tests? Can't we relax this in order to have a 
>meaningful backtrace?
>
Let assume:
* dlclose was not called
* libraryA is linked with libtalloc and libtevent
* libraryB is not linked with libtalloc (even though it should be
* dlopen test test libraries in following order: 1. libraryA; 2. libraryB

Result: missing dependency in libraryB would not be found because
libraryA and its dependencies are still loaded in memory.

LS

"""

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

[SSSD] [sssd PR#52][comment] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/52
Title: #52: CI: Remove dlopen-test from valgrind blacklist

fidencio commented:
"""
Please, refer to e1a58f3d in the commit message.

This is a genuine question (even in case it's a dumb one), but do we really 
need to call dlclose() in our tests? Can't we relax this in order to have a 
meaningful backtrace?

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/52#issuecomment-254309609
___
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-10-17 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/53
Title: #53: Fixes in the config API related to secrets responder

fidencio commented:
"""
And CI has passed: http://sssd-ci.duckdns.org/logs/job/55/24/summary.html
"""

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

[SSSD] [sssd PR#55][comment] TESTS: Fix check for py bindings in dlopen tests

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/55
Title: #55: TESTS: Fix check for py bindings in dlopen tests

lslebodn commented:
"""
master:
* 8a681cc41672afd1532b4a0c7e9da3a4eb2014a7

sssd-1-13:
* 977e53a67cab7937f16f393b85e4d1afd478b9f4

LS

"""

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

[SSSD] [sssd PR#55][comment] TESTS: Fix check for py bindings in dlopen tests

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/55
Title: #55: TESTS: Fix check for py bindings in dlopen tests

lslebodn commented:
"""
ACK

http://sssd-ci.duckdns.org/logs/job/55/22/summary.html

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/55#issuecomment-254298050
___
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][synchronized] Fixes in the config API related to secrets responder

2016-10-17 Thread fidencio 
   URL: https://github.com/SSSD/sssd/pull/53
Author: fidencio
 Title: #53: Fixes in the config API related to secrets responder
Action: synchronized

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
From 1f1b205991b51667d02c589627d880d92063dedb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 17 Oct 2016 17:07:56 +0200
Subject: [PATCH 1/2] SECRETS: Fix secrets rule in the allowed sections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have been matching an invalid subsection of the secrets' section,
like:
[secrets/users/]

Let's ensure that we only match the following cases:
[secrets]
[secrets/users/[0-9]+]

Signed-off-by: Fabiano Fidêncio 
---
 src/config/cfg_rules.ini | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..23dda12 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\)\?$
 section_re = ^domain/.*$
 
 [rule/allowed_sssd_options]
@@ -212,7 +212,7 @@ option = user_attributes
 
 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\)\?$
 
 option = timeout
 option = debug

From 9b289639cd318b166eaf89fb02737480638f9b74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 17 Oct 2016 18:58:50 +0200
Subject: [PATCH 2/2] SECRETS: Add allowed_sec_users_options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There are options (the proxying related ones) that only apply to the
secrets' subsections. In order to make config API able to catch those,
let's create a new section called allowed_sec_users_options) and move
there these proxying options.

Signed-off-by: Fabiano Fidêncio 
---
 src/config/cfg_rules.ini | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 23dda12..0aecf7e 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -210,9 +210,10 @@ option = description
 option = allowed_uids
 option = user_attributes
 
+# Secrets service
 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/[0-9]\+\)\?$
+section_re = ^secrets$
 
 option = timeout
 option = debug
@@ -225,11 +226,14 @@ option = reconnection_retries
 option = fd_limit
 option = client_idle_timeout
 option = description
-
-# Secrets service
 option = provider
 option = containers_nest_level
 option = max_secrets
+
+[rule/allowed_sec_users_options]
+validator = ini_allowed_options
+section_re = ^secrets/users/[0-9]\+$
+
 # Secrets service - proxy
 option = proxy_url
 option = auth_type
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#55][opened] TESTS: Fix check for py bindings in dlopen tests

2016-10-17 Thread fidencio 
   URL: https://github.com/SSSD/sssd/pull/55
Author: fidencio
 Title: #55: TESTS: Fix check for py bindings in dlopen tests
Action: opened

PR body:
"""
The current code checks only for "HAVE_PYTHON_BINDINGS", which is not
even a valid check. Let's do the proper check according to the python
version (HAVE_PYTHON2_BINDINGS or HAVE_PYTHON3_BINDINGS).

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/55/head:pr55
git checkout pr55
From 1d778d457167ca42a74cd6af4d92243f717d0190 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 17 Oct 2016 19:29:03 +0200
Subject: [PATCH] TESTS: Fix check for py bindings in dlopen tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The current code checks only for "HAVE_PYTHON_BINDINGS", which is not
even a valid check. Let's do the proper check according to the python
version (HAVE_PYTHON2_BINDINGS or HAVE_PYTHON3_BINDINGS).

Signed-off-by: Fabiano Fidêncio 
---
 src/tests/dlopen-tests.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tests/dlopen-tests.c b/src/tests/dlopen-tests.c
index 332b268..96cc4db 100644
--- a/src/tests/dlopen-tests.c
+++ b/src/tests/dlopen-tests.c
@@ -98,13 +98,13 @@ struct so {
LIBPFX"libsss_proxy.so", NULL } },
 { "libdlopen_test_providers.so", { LIBPFX"libdlopen_test_providers.so",
NULL } },
-#ifdef HAVE_PYTHON_BINDINGS
+#ifdef HAVE_PYTHON2_BINDINGS
 { "_py2hbac.so", { LIBPFX"_py2hbac.so", NULL } },
 { "_py2sss.so", { LIBPFX"_py2sss.so", NULL } },
 { "_py2sss_murmur.so", { LIBPFX"_py2sss_murmur.so", NULL } },
 { "_py2sss_nss_idmap.so", { LIBPFX"_py2sss_nss_idmap.so", NULL } },
 #endif
-#ifdef HAVE_PYTHON_BINDINGS
+#ifdef HAVE_PYTHON3_BINDINGS
 { "_py3hbac.so", { LIBPFX"_py3hbac.so", NULL } },
 { "_py3sss.so", { LIBPFX"_py3sss.so", NULL } },
 { "_py3sss_murmur.so", { LIBPFX"_py3sss_murmur.so", NULL } },
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#54][opened] crypto: Port libcrypto code to openssl-1.1

2016-10-17 Thread lslebodn 
   URL: https://github.com/SSSD/sssd/pull/54
Author: lslebodn
 Title: #54: crypto: Port libcrypto code to openssl-1.1
Action: opened

PR body:
"""
I am not sure whether using EVP_.*_init() is required after calling 
"constructor" 
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/54/head:pr54
git checkout pr54
From 33c56eb8e3169592f82c7df368016c0b773044e2 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Mon, 17 Oct 2016 15:44:20 +0200
Subject: [PATCH] crypto: Port libcrypto code to openssl-1.1

---
 src/util/cert/libcrypto/cert.c | 24 ++--
 src/util/crypto/libcrypto/crypto_hmac_sha1.c   | 32 ++-
 src/util/crypto/libcrypto/crypto_nite.c| 78 +-
 src/util/crypto/libcrypto/crypto_obfuscate.c   | 34 +++
 src/util/crypto/libcrypto/crypto_sha512crypt.c | 76 ++---
 5 files changed, 153 insertions(+), 91 deletions(-)

diff --git a/src/util/cert/libcrypto/cert.c b/src/util/cert/libcrypto/cert.c
index a7752d7..eb84b3b 100644
--- a/src/util/cert/libcrypto/cert.c
+++ b/src/util/cert/libcrypto/cert.c
@@ -182,6 +182,9 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
 size_t c;
 X509 *cert = NULL;
 EVP_PKEY *cert_pub_key = NULL;
+RSA *rsa_pub_key = NULL;
+const BIGNUM *n;
+const BIGNUM *e;
 int modulus_len;
 unsigned char modulus[OPENSSL_RSA_MAX_MODULUS_BITS/8];
 int exponent_len;
@@ -208,16 +211,29 @@ errno_t cert_to_ssh_key(TALLOC_CTX *mem_ctx, const char *ca_db,
 goto done;
 }
 
-if (cert_pub_key->type != EVP_PKEY_RSA) {
+if (EVP_PKEY_base_id(cert_pub_key) != EVP_PKEY_RSA) {
 DEBUG(SSSDBG_CRIT_FAILURE,
   "Expected RSA public key, found unsupported [%d].\n",
-  cert_pub_key->type);
+  EVP_PKEY_base_id(cert_pub_key));
 ret = EINVAL;
 goto done;
 }
 
-modulus_len = BN_bn2bin(cert_pub_key->pkey.rsa->n, modulus);
-exponent_len = BN_bn2bin(cert_pub_key->pkey.rsa->e, exponent);
+#if OPENSSL_VERSION_NUMBER >= 0x1010L
+rsa_pub_key = EVP_PKEY_get1_RSA(cert_pub_key);
+if (rsa_pub_key == NULL) {
+ret = ENOMEM;
+goto done;
+}
+
+RSA_get0_key(rsa_pub_key, , , NULL);
+RSA_free(rsa_pub_key);
+#else
+n = cert_pub_key->pkey.rsa->n;
+e = cert_pub_key->pkey.rsa->e;
+#endif
+modulus_len = BN_bn2bin(n, modulus);
+exponent_len = BN_bn2bin(e, exponent);
 
 size = SSH_RSA_HEADER_LEN + 3 * sizeof(uint32_t)
 + modulus_len
diff --git a/src/util/crypto/libcrypto/crypto_hmac_sha1.c b/src/util/crypto/libcrypto/crypto_hmac_sha1.c
index 37d2579..e1964df 100644
--- a/src/util/crypto/libcrypto/crypto_hmac_sha1.c
+++ b/src/util/crypto/libcrypto/crypto_hmac_sha1.c
@@ -33,23 +33,27 @@ int sss_hmac_sha1(const unsigned char *key,
   unsigned char *out)
 {
 int ret;
-EVP_MD_CTX ctx;
+EVP_MD_CTX *ctx;
 unsigned char ikey[HMAC_SHA1_BLOCKSIZE], okey[HMAC_SHA1_BLOCKSIZE];
 size_t i;
 unsigned char hash[SSS_SHA1_LENGTH];
 unsigned int res_len;
 
-EVP_MD_CTX_init();
+ctx = EVP_MD_CTX_create();
+if (ctx == NULL) {
+return ENOMEM;
+}
+EVP_MD_CTX_init(ctx);
 
 if (key_len > HMAC_SHA1_BLOCKSIZE) {
 /* keys longer than blocksize are shortened */
-if (!EVP_DigestInit_ex(, EVP_sha1(), NULL)) {
+if (!EVP_DigestInit_ex(ctx, EVP_sha1(), NULL)) {
 ret = EIO;
 goto done;
 }
 
-EVP_DigestUpdate(, (const unsigned char *)key, key_len);
-EVP_DigestFinal_ex(, ikey, _len);
+EVP_DigestUpdate(ctx, (const unsigned char *)key, key_len);
+EVP_DigestFinal_ex(ctx, ikey, _len);
 memset(ikey + SSS_SHA1_LENGTH, 0, HMAC_SHA1_BLOCKSIZE - SSS_SHA1_LENGTH);
 } else {
 /* keys shorter than blocksize are zero-padded */
@@ -63,25 +67,25 @@ int sss_hmac_sha1(const unsigned char *key,
 ikey[i] ^= 0x36;
 }
 
-if (!EVP_DigestInit_ex(, EVP_sha1(), NULL)) {
+if (!EVP_DigestInit_ex(ctx, EVP_sha1(), NULL)) {
 ret = EIO;
 goto done;
 }
 
-EVP_DigestUpdate(, (const unsigned char *)ikey, HMAC_SHA1_BLOCKSIZE);
-EVP_DigestUpdate(, (const unsigned char *)in, in_len);
-EVP_DigestFinal_ex(, hash, _len);
+EVP_DigestUpdate(ctx, (const unsigned char *)ikey, HMAC_SHA1_BLOCKSIZE);
+EVP_DigestUpdate(ctx, (const unsigned char *)in, in_len);
+EVP_DigestFinal_ex(ctx, hash, _len);
 
-if (!EVP_DigestInit_ex(, EVP_sha1(), NULL)) {
+if (!EVP_DigestInit_ex(ctx, EVP_sha1(), NULL)) {
 ret = EIO;
 goto done;
 }
 
-EVP_DigestUpdate(, (const unsigned char *)okey, HMAC_SHA1_BLOCKSIZE);
-EVP_DigestUpdate(, (const unsigned char *)hash, SSS_SHA1_LENGTH);
-EVP_DigestFinal_ex(, out, _len);
+EVP_DigestUpdate(ctx, (const unsigned char *)okey, HMAC_SHA1_BLOCKSIZE);
+

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

2016-10-17 Thread fidencio 
   URL: https://github.com/SSSD/sssd/pull/53
Author: fidencio
 Title: #53: Fixes in the config API related to secrets responder
Action: opened

PR body:
"""
Those fixes were suggested by Lukaš in the following thread:
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/ZMJSE5ZSICLPNN5M6OOWNRWMG7LBQQIH/

Changes:

28fa419 (Fabiano Fidêncio, 11 minutes ago)
   SECRETS: Add allowed_sec_users_options

   There are options (the proxying related ones) that only apply to the
   secrets' subsections. In order to make config API able to catch those,
   let's create a new section called allowed_sec_users_options) and move there
   these proxying options.

   Signed-off-by: Fabiano Fidêncio 

2aed214 (Fabiano Fidêncio, 2 hours ago)
   SECRETS: Fix secrets rule in the allowed sections

   We have been matching an invalid subsection of the secrets' section, like:
   [secrets/users]

   Let's ensure that we only match the following cases:
   [secrets]
   [secrets/users/[0-9]+?]

   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/53/head:pr53
git checkout pr53
From 2aed214ba17ab7cf3a1393383d1c0b30489eb67e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 17 Oct 2016 17:07:56 +0200
Subject: [PATCH 1/2] SECRETS: Fix secrets rule in the allowed sections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We have been matching an invalid subsection of the secrets' section,
like:
[secrets/users]

Let's ensure that we only match the following cases:
[secrets]
[secrets/users/[0-9]+?]

Signed-off-by: Fabiano Fidêncio 
---
 src/config/cfg_rules.ini | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..5a4394d 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$
 section_re = ^domain/.*$
 
 [rule/allowed_sssd_options]
@@ -212,7 +212,7 @@ option = user_attributes
 
 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$
 
 option = timeout
 option = debug

From 28fa41957d1382216f319620f4a5615a0082c0c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Mon, 17 Oct 2016 18:58:50 +0200
Subject: [PATCH 2/2] SECRETS: Add allowed_sec_users_options
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There are options (the proxying related ones) that only apply to the
secrets' subsections. In order to make config API able to catch those,
let's create a new section called allowed_sec_users_options) and move
there these proxying options.

Signed-off-by: Fabiano Fidêncio 
---
 src/config/cfg_rules.ini | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 5a4394d..49beae6 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -210,9 +210,10 @@ option = description
 option = allowed_uids
 option = user_attributes
 
+# Secrets service
 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/[0-9]\+\?\)\?$
+section_re = ^secrets$
 
 option = timeout
 option = debug
@@ -225,11 +226,14 @@ option = reconnection_retries
 option = fd_limit
 option = client_idle_timeout
 option = description
-
-# Secrets service
 option = provider
 option = containers_nest_level
 option = max_secrets
+
+[rule/allowed_sec_users_options]
+validator = ini_allowed_options
+section_re = ^secrets/users/[0-9]\+\?$
+
 # Secrets service - proxy
 option = proxy_url
 option = auth_type
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#52][opened] CI: Remove dlopen-test from valgrind blacklist

2016-10-17 Thread lslebodn 
   URL: https://github.com/SSSD/sssd/pull/52
Author: lslebodn
 Title: #52: CI: Remove dlopen-test from valgrind blacklist
Action: opened

PR body:
"""
Dlopen test was added to blacklist due to following reason:
> Disable running dlopen-tests under Valgrind as their use of dlclose
> makes Valgrind drop symbols and produce meaningless backtraces, which
> cannot be matched with specific suppressions.

It's true that dlclose makes meaningless backtraces but it can
catch real leaks.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/52/head:pr52
git checkout pr52
From 61ca8f93032119cf975b1898fe9d38fdff70c1d1 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Thu, 29 Sep 2016 13:45:04 +0200
Subject: [PATCH] CI: Remove dlopen-test from valgrind blacklist

Dlopen test was added to blacklist due to following reason:
> Disable running dlopen-tests under Valgrind as their use of dlclose
> makes Valgrind drop symbols and produce meaningless backtraces, which
> cannot be matched with specific suppressions.

It's true that dlclose makes meaningless backtraces but it can
catch real leaks.
---
 contrib/ci/run | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/contrib/ci/run b/contrib/ci/run
index f96476f..6b5c6b8 100755
--- a/contrib/ci/run
+++ b/contrib/ci/run
@@ -187,8 +187,7 @@ function build_debug()
 {
 # Extended glob pattern matching tests to run under Valgrind.
 # NOTE: The particular pattern below is inverted
-declare -r valgrind_test_pattern="\
-!(*.py|*/dlopen-tests|*/whitespace_test|*/double_semicolon_test)"
+declare -r valgrind_test_pattern="!(*.py|*/whitespace_test|*/double_semicolon_test)"
 export CFLAGS="$DEBUG_CFLAGS"
 declare test_dir
 declare test_dir_distcheck
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#48][comment] sssctl: Flags for commadn initialization

2016-10-17 Thread jhrozek 
  URL: https://github.com/SSSD/sssd/pull/48
Title: #48: sssctl: Flags for commadn initialization

jhrozek commented:
"""
I don't know how to unstuck this PR except providing some ideas
 * SSS_TOOL_FLAG_NOCONF
 * SSS_TOOL_FLAG_STATIC
 * SSS_TOOL_FLAG_CONFPARSE_FALSE
"""

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

[SSSD] Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates

2016-10-17 Thread Simo Sorce 
On Thu, 2016-10-13 at 18:52 +0200, Sumit Bose wrote:
>  Compatibility with Active Directory 
> Active Directory uses a per-user LDAP attribute
> [https://msdn.microsoft.com/en-us/library/cc220106.aspx 
> altSecurityIdentities] to allow arbitrary user-certificate mappings is there 
> is no suitable user-principal-name entry in the SAN of the certificate.
> 
> Unfortunately it is more or less undocumented how AD use the values of
> this attribute. The best overview I found is in
> https://blogs.msdn.microsoft.com/spatdsg/2010/06/18/howto-map-a-user-to-a-certificate-via-all-the-methods-available-in-the-altsecurityidentities-attribute/.

A few more pointers Sumit:
- This describes what is allowed for users:
https://msdn.microsoft.com/en-us/library/ms677943%28v=vs.85%29.aspx

- This describes a use for devices:
https://msdn.microsoft.com/en-us/library/dn408946.aspx

- additional description specific for PKINIT:
https://msdn.microsoft.com/en-us/library/hh536384.aspx

- This is a good detailed overview of the Smart Card logon workflow in
windows, it describes Vista but I do not think it changed in fundamental
ways in following releases:
https://msdn.microsoft.com/en-us/library/bb905527.aspx

NOTE: Please look at the small paragraph named "Smart card logon across
forests", we definitely want to think about this problem as well from
the get-go and not try to retrofit something later on.


HTH,
Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#51][comment] TOOL: Fixing of sign-compare warning

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

lslebodn commented:
"""
On (17/10/16 08:20), celestian wrote:
>Hi @lslebodn,
>I know about another three issues which we should backport to 1.13.
>I could prepare patches with ```git cherry-pick -x ``` and 
>create PRs.
>Is this the right way?
>
It is correct for patches which has PR for master.
For older patches it's better to bump the thread with
patches for master.

e.g.
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/message/CVEMZJKDR4DQCFJG567NPQBI52EI7RX4/

"""

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

[SSSD] [sssd PR#51][comment] TOOL: Fixing of sign-compare warning

2016-10-17 Thread celestian 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

celestian commented:
"""
Hi @lslebodn,
I know about another three issues which we should backport to 1.13.
I could prepare patches with ```git cherry-pick -x ``` and 
create PRs.
Is this the right way?
"""

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

[SSSD] [sssd PR#51][comment] TOOL: Fixing of sign-compare warning

2016-10-17 Thread celestian 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

celestian commented:
"""
Hi @lslebodn,
I know about another three issues which we should backport to 1.13.
I could prepare patches with ```git cherry-pick -x ``` and 
create PRs.
Is this the right way?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/51#issuecomment-254239001
___
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#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

2016-10-17 Thread Lukas Slebodnik 
On (17/10/16 14:35), Fabiano Fidêncio wrote:
>On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik  wrote:
>> On (30/09/16 16:55), fidencio wrote:
>>>   URL: https://github.com/SSSD/sssd/pull/33
>>>Author: fidencio
>>> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>>>Action: synchronized
>>>
>>>To pull the PR as Git branch:
>>>git remote add ghsssd https://github.com/SSSD/sssd
>>>git fetch ghsssd pull/33/head:pr33
>>>git checkout pr33
>>
>> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
>>>Date: Sun, 25 Sep 2016 20:49:16 +0200
>>>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>>>MIME-Version: 1.0
>>>Content-Type: text/plain; charset=UTF-8
>>>Content-Transfer-Encoding: 8bit
>>>
>>>The regular expression used is quite specific for the two cases we
>>>support:
>>>- [secrets]
>>>- [secrets/users/$uid]
>>>
>>>It could be done a bit more generic, but the way it's right now it can
>>>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>>>
>>>Related:
>>>https://fedorahosted.org/sssd/ticket/3207
>>>
>>>Signed-off-by: Fabiano Fidêncio 
>>>---
>>> src/config/cfg_rules.ini | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>>>index 01be0c6..023ceac 100644
>>>--- a/src/config/cfg_rules.ini
>>>+++ b/src/config/cfg_rules.ini
>>>@@ -8,6 +8,7 @@ section = autofs
>>> section = ssh
>>> section = pac
>>> section = ifp
>>>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
>>> section_re = ^domain/.*$
>>
>> Is it expected that section the name "secrets/users/"
>> is allowed.
>
>I don't think so.
>I'll answer your questions based on the my understanding of the
>conversation I had with Jakub on the #sssd channel.
>Jakub, Simo, please, feel free to jump in and correct me if I'm
>mistaken in any point.
>
>>
>> Which of following section should be allowed?
>>
>> sh# cat /etc/sssd/conf.d/10_secrets.conf
>> [secrets
>> description = temp
>
>Not allowed, but [secrets] is allowed.
>
This one was a typo on my side.

>>
>> [secrets/users]
>> description = temp
>>
>
>Shouldn't be allowed.
>
>> [secrets/users/]
>> description = temp
>>
>
>Shouldn't be allowed.
>
But it is allowed.

Following change should fix it.
I didn't tested it.

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..0a654ff 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\)\?$
 section_re = ^domain/.*$
 
 [rule/allowed_sssd_options]



>> [secrets/users/$uid]
>> description = temp
>>
>
>Shouldn't be allowed.
>
yes, it's denied.

>> [secrets/users/0]
>> description = temp
>>
>
>Should be allowed.
>
OK, I was not sure about root UID.

>> [secrets/users/1]
>> description = temp
>
>Should be allowed.
>
>>
>> [secrets/users/1000]
>> description = temp
>
>Should be allowed.
>
>>
>> LS
>
>Is some of these cases breaking to you?
>If yes, please, let me know and I'll provide a follow up patch fixing the 
>issue.
>

I think we shoudl also split following rule
"[rule/allowed_sec_options]"

I do not think that following options are read from
sections "secrets/users/.*"

option = timeout
option = debug
option = debug_level
option = debug_timestamps
option = debug_microseconds
option = debug_to_files
option = command
option = reconnection_retries
option = fd_limit
option = client_idle_timeout
option = description

Fabiano,
Could you preapre a patch?

LS
___
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#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

2016-10-17 Thread Fabiano Fidêncio 
Lukaš,

On Mon, Oct 17, 2016 at 4:59 PM, Lukas Slebodnik  wrote:
> On (17/10/16 14:35), Fabiano Fidêncio wrote:
>>On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik  wrote:
>>> On (30/09/16 16:55), fidencio wrote:
   URL: https://github.com/SSSD/sssd/pull/33
Author: fidencio
 Title: #33: SECRETS: Some small misc fixes + fixing #3168
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/33/head:pr33
git checkout pr33
>>>
>>> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Sun, 25 Sep 2016 20:49:16 +0200
Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The regular expression used is quite specific for the two cases we
support:
- [secrets]
- [secrets/users/$uid]

It could be done a bit more generic, but the way it's right now it can
easily catch errors like: [secrets/usrs/$uid] or [secrets/].

Related:
https://fedorahosted.org/sssd/ticket/3207

Signed-off-by: Fabiano Fidêncio 
---
 src/config/cfg_rules.ini | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index 01be0c6..023ceac 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,6 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
 section_re = ^domain/.*$
>>>
>>> Is it expected that section the name "secrets/users/"
>>> is allowed.
>>
>>I don't think so.
>>I'll answer your questions based on the my understanding of the
>>conversation I had with Jakub on the #sssd channel.
>>Jakub, Simo, please, feel free to jump in and correct me if I'm
>>mistaken in any point.
>>
>>>
>>> Which of following section should be allowed?
>>>
>>> sh# cat /etc/sssd/conf.d/10_secrets.conf
>>> [secrets
>>> description = temp
>>
>>Not allowed, but [secrets] is allowed.
>>
> This one was a typo on my side.
>
>>>
>>> [secrets/users]
>>> description = temp
>>>
>>
>>Shouldn't be allowed.

This is wrong right now :-\

>>
>>> [secrets/users/]
>>> description = temp
>>>
>>
>>Shouldn't be allowed.
>>
> But it is allowed.

Anything terminating with a / will cause the following error and it's
not related to secrets only.
(Mon Oct 17 16:59:22:585705 2016) [sssd] [confdb_init_db] (0x0010):
Could not create LDIF for confdb
(Mon Oct 17 16:59:22:585898 2016) [sssd] [confdb_setup] (0x0010):
ConfDB initialization has failed [22]: Invalid argument
(Mon Oct 17 16:59:22:586078 2016) [sssd] [sss_tool_confdb_init]
(0x0010): Unable to setup ConfDB [22]: Invalid argument


>
> Following change should fix it.
> I didn't tested it.
>
> diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
> index b6316be..0a654ff 100644
> --- a/src/config/cfg_rules.ini
> +++ b/src/config/cfg_rules.ini
> @@ -8,7 +8,7 @@ section = autofs
>  section = ssh
>  section = pac
>  section = ifp
> -section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
> +section_re = ^secrets\(/users/[0-9]\+\)\?$
>  section_re = ^domain/.*$
>
>  [rule/allowed_sssd_options]
>

Yeah, the full patch should be:

diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index b6316be..5a4394d 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -8,7 +8,7 @@ section = autofs
 section = ssh
 section = pac
 section = ifp
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$
 section_re = ^domain/.*$

 [rule/allowed_sssd_options]
@@ -212,7 +212,7 @@ option = user_attributes

 [rule/allowed_sec_options]
 validator = ini_allowed_options
-section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
+section_re = ^secrets\(/users/[0-9]\+\?\)\?$

Just tested it on my side here.

>
>
>>> [secrets/users/$uid]
>>> description = temp
>>>
>>
>>Shouldn't be allowed.
>>
> yes, it's denied.
>
>>> [secrets/users/0]
>>> description = temp
>>>
>>
>>Should be allowed.
>>
> OK, I was not sure about root UID.
>
>>> [secrets/users/1]
>>> description = temp
>>
>>Should be allowed.
>>
>>>
>>> [secrets/users/1000]
>>> description = temp
>>
>>Should be allowed.
>>
>>>
>>> LS
>>
>>Is some of these cases breaking to you?
>>If yes, please, let me know and I'll provide a follow up patch fixing the 
>>issue.
>>
>
> I think we shoudl also split following rule
> "[rule/allowed_sec_options]"
>
> I do not think that following options are read from
> sections "secrets/users/.*"
>
> option = timeout
> option = debug
> option = debug_level
> option = debug_timestamps
> option = debug_microseconds
> option = debug_to_files
> option = command
> option = reconnection_retries
> option =

[SSSD] Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates

2016-10-17 Thread Rob Crittenden 

Jan Cholasta wrote:

Hi,

On 13.10.2016 18:52, Sumit Bose wrote:

= Issuer specific matching =
Although the MIT Kerberos rules allow to select the issuer of a
certificate there are use cases where a more specific selection is
needed. E.g. if there are some default matching rules for all issuers
and some other issuer specific rules where the default rules should
not apply. To make this possible with the above scheme the default
rules must have an  clause which matches all but the issuer
with the specific rules. Writing regular-expressions to not match a
specific string or a list of strings is at least error-prone if not
impossible.

To make it easier to define issuer specific rules and default rules at
the same time and optional issuer string can be added to the rule to
indicate that for the given issuer only those rules should be
considered. Given the use-case I think it is acceptable to require
that the full issuer must be specified here in LDAP order (see below)
and case-sensitive matching is used.


This could also be solved by adding priority to rules - if two rules
match, the one with higher priority (the issuer specific rule) is
preferred over the one with lower priority (the default rule). IMO this
is better than an optional issuer string as it offers greater flexibility.


The use cases I've seen haven't had to do with priority, though that 
would be a nice enhancement, but with only allowing certificates issued 
by a specific CA to be allowed (this is pretty common in web servers). 
Being able to say "only do the matching on certificates issued by foo" 
is valuable.


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

[SSSD] [sssd PR#51][comment] TOOL: Fixing of sign-compare warning

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

lslebodn commented:
"""
I hit enter too fast. We backport patches in different way.
```
git checkout sssd-1-13
git cherry-pick -x d3f14ed93ef61268d0a68898ed9c44b4f773081c

```
The important the argument `-x`. It adds a very useful comment to the commit 
message.
`(cherry picked from commit d3f14ed93ef61268d0a68898ed9c44b4f773081c)`
Otherwise it's very difficult to track whether a patch was backported to stable 
branch or no.
"""

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

[SSSD] Re: [PATCHES] Fix warnings Wsign-compare

2016-10-17 Thread Lukas Slebodnik 
On (29/01/16 09:33), Lukas Slebodnik wrote:
>On (28/01/16 21:54), Jakub Hrozek wrote:
>>On Tue, Jan 12, 2016 at 03:21:30PM +0100, Lukas Slebodnik wrote:
>>> python-3.5 is also in debian-sid and migh be in debian-testing
>>> within 10 days. First 3 patches can be reviwed and meanwhile
>>> I will need to figure out solution for 4th patch (but it's only problem for
>>> 32 bit platforms)
>>> 
>>> The first 3 patches are attached.
>>
>>Garbage-collecting patches untouched for a week or more..
>>
>>ACK
>>
>>I tested with a local i386 build and put the patches to CI:
>>http://sssd-ci.duckdns.org/logs/job/36/85/summary.html
>>
>>The code also looks good to me, the pyhbac change is not public API, so
>>it's OK to change the type.
>
>master:
>* f47a339d7794cd5a24d368b3b3640452686e45a5
>* 2ff8131cf02decaf0dd0754e843732fe7774fc59
>* d3f14ed93ef61268d0a68898ed9c44b4f773081c
>

Backported to sssd-1-13 due to PR#51
* 085746864de710adfa506c342434722ad69c3d85
* 9341809d4428cd187aefdb2871afffb327447002
* 0257239c8504435961dc428f719d4d655ba4688d

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

[SSSD] [sssd PR#51][+Rejected] TOOL: Fixing of sign-compare warning

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

Label: +Rejected
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#51][closed] TOOL: Fixing of sign-compare warning

2016-10-17 Thread lslebodn 
   URL: https://github.com/SSSD/sssd/pull/51
Author: celestian
 Title: #51: TOOL: Fixing of sign-compare warning
Action: closed

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

[SSSD] [sssd PR#51][comment] TOOL: Fixing of sign-compare warning

2016-10-17 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/51
Title: #51: TOOL: Fixing of sign-compare warning

lslebodn commented:
"""
I can see that's already fixed in master
https://git.fedorahosted.org/cgit/sssd.git/commit/?id=d3f14ed93ef61268d0a68898ed9c44b4f773081c

LS

"""

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

[SSSD] [sssd PR#51][opened] TOOL: Fixing of sign-compare warning

2016-10-17 Thread celestian 
   URL: https://github.com/SSSD/sssd/pull/51
Author: celestian
 Title: #51: TOOL: Fixing of sign-compare warning
Action: opened

PR body:
"""
Warning was on src/tools/tools_util.c:116.
(We have fixed this issue at master.)
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/51/head:pr51
git checkout pr51
From 566ac847f7edb527a999297cef01c261e7d83cc9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C4=8Cech?= 
Date: Mon, 17 Oct 2016 16:24:26 +0200
Subject: [PATCH] TOOL: Fixing of sign-compare warning

Warning was on src/tools/tools_util.c:116.
---
 src/tools/tools_util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/tools_util.c b/src/tools/tools_util.c
index 82462f3..ea63f62 100644
--- a/src/tools/tools_util.c
+++ b/src/tools/tools_util.c
@@ -94,7 +94,7 @@ int parse_groups(TALLOC_CTX *mem_ctx, const char *optstr, char ***_out)
 char *orig, *n, *o;
 char delim = ',';
 unsigned int tokens = 1;
-int i;
+unsigned int i;
 
 orig = talloc_strdup(mem_ctx, optstr);
 if (!orig) return ENOMEM;
___
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#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

2016-10-17 Thread Fabiano Fidêncio 
On Mon, Oct 17, 2016 at 11:46 AM, Lukas Slebodnik  wrote:
> On (30/09/16 16:55), fidencio wrote:
>>   URL: https://github.com/SSSD/sssd/pull/33
>>Author: fidencio
>> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>>Action: synchronized
>>
>>To pull the PR as Git branch:
>>git remote add ghsssd https://github.com/SSSD/sssd
>>git fetch ghsssd pull/33/head:pr33
>>git checkout pr33
>
> >From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
>>Date: Sun, 25 Sep 2016 20:49:16 +0200
>>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>>MIME-Version: 1.0
>>Content-Type: text/plain; charset=UTF-8
>>Content-Transfer-Encoding: 8bit
>>
>>The regular expression used is quite specific for the two cases we
>>support:
>>- [secrets]
>>- [secrets/users/$uid]
>>
>>It could be done a bit more generic, but the way it's right now it can
>>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>>
>>Related:
>>https://fedorahosted.org/sssd/ticket/3207
>>
>>Signed-off-by: Fabiano Fidêncio 
>>---
>> src/config/cfg_rules.ini | 1 +
>> 1 file changed, 1 insertion(+)
>>
>>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>>index 01be0c6..023ceac 100644
>>--- a/src/config/cfg_rules.ini
>>+++ b/src/config/cfg_rules.ini
>>@@ -8,6 +8,7 @@ section = autofs
>> section = ssh
>> section = pac
>> section = ifp
>>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
>> section_re = ^domain/.*$
>
> Is it expected that section the name "secrets/users/"
> is allowed.

I don't think so.
I'll answer your questions based on the my understanding of the
conversation I had with Jakub on the #sssd channel.
Jakub, Simo, please, feel free to jump in and correct me if I'm
mistaken in any point.

>
> Which of following section should be allowed?
>
> sh# cat /etc/sssd/conf.d/10_secrets.conf
> [secrets
> description = temp

Not allowed, but [secrets] is allowed.

>
> [secrets/users]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/$uid]
> description = temp
>

Shouldn't be allowed.

> [secrets/users/0]
> description = temp
>

Should be allowed.

> [secrets/users/1]
> description = temp

Should be allowed.

>
> [secrets/users/1000]
> description = temp

Should be allowed.

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

Is some of these cases breaking to you?
If yes, please, let me know and I'll provide a follow up patch fixing the issue.

Best Regards,
--
Fabiano Fidêncio
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#43][-Changes requested] RESPONDER: Enable sudoRule in case insen. domains​ (1.14)

2016-10-17 Thread celestian 
  URL: https://github.com/SSSD/sssd/pull/43
Title: #43: RESPONDER: Enable sudoRule in case insen. domains​ (1.14)

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] Re: [sssd PR#33][synchronized] SECRETS: Some small misc fixes + fixing #3168

2016-10-17 Thread Lukas Slebodnik 
On (30/09/16 16:55), fidencio wrote:
>   URL: https://github.com/SSSD/sssd/pull/33
>Author: fidencio
> Title: #33: SECRETS: Some small misc fixes + fixing #3168
>Action: synchronized
>
>To pull the PR as Git branch:
>git remote add ghsssd https://github.com/SSSD/sssd
>git fetch ghsssd pull/33/head:pr33
>git checkout pr33

>From 06a0a81193d6bbe3a0932c8b584433f3cc13fa51 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
>Date: Sun, 25 Sep 2016 20:49:16 +0200
>Subject: [PATCH 1/6] CONFIG: Add secrets responder to the allowed sections
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>The regular expression used is quite specific for the two cases we
>support:
>- [secrets]
>- [secrets/users/$uid]
>
>It could be done a bit more generic, but the way it's right now it can
>easily catch errors like: [secrets/usrs/$uid] or [secrets/].
>
>Related:
>https://fedorahosted.org/sssd/ticket/3207
>
>Signed-off-by: Fabiano Fidêncio 
>---
> src/config/cfg_rules.ini | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
>index 01be0c6..023ceac 100644
>--- a/src/config/cfg_rules.ini
>+++ b/src/config/cfg_rules.ini
>@@ -8,6 +8,7 @@ section = autofs
> section = ssh
> section = pac
> section = ifp
>+section_re = ^secrets\(/users/\([0-9]\+\)\?\)\?$
> section_re = ^domain/.*$

Is it expected that section the name "secrets/users/"
is allowed.

Which of following section should be allowed?

sh# cat /etc/sssd/conf.d/10_secrets.conf
[secrets
description = temp

[secrets/users]
description = temp

[secrets/users/]
description = temp

[secrets/users/$uid]
description = temp

[secrets/users/0]
description = temp

[secrets/users/1]
description = temp

[secrets/users/1000]
description = temp

LS
___
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][opened] [RFC] Use GNULIB's compiler warning code

2016-10-17 Thread fidencio 
   URL: https://github.com/SSSD/sssd/pull/50
Author: fidencio
 Title: #50: [RFC] Use GNULIB's compiler warning code
Action: opened

PR body:
"""
This patch series was sent to the sssd-devel and some discussions
already happened there[0]. I've decided to open the PR because there
are some few patches that can be pushed even if we decide to not use
the "many warnings" patches.

Let's keep track of those patches (and discussions related to them) in
the github, in this way we can avoid them to get lost. :-)

[0]: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org/thread/CTGR5CYV2PT3PLFLIRBCRTJP5ZW2XBMO/#CTGR5CYV2PT3PLFLIRBCRTJP5ZW2XBMO

Best Regards,

Changes:

683f72d (Fabiano Fidêncio, 10 weeks ago)
   BUILD: Make use of GNULIB's compiler warning code

   As GNULIB has the 'manywarnings' module, which basically turns on every GCC
   warning, let's make use of it. We can easily blacklist the warnings we
   cannot cope with, but the main goal should be to have enabled every
   possible GCC warning.

   When new GCC warnings are created the 'manywarnings' file can be refreshed
   from upstream GNULIB.

   Signed-off-by: Fabiano Fidêncio 

f59828a (Fabiano Fidêncio, 5 days ago)
   NSS: Fix "old-style-definition" warning caught by GCC

   Signed-off-by: Fabiano Fidêncio 

f22aff7 (Fabiano Fidêncio, 5 days ago)
   SIFP: Fix a "jump-misses-init" warning caught by GCC

   Signed-off-by: Fabiano Fidêncio 

58609d3 (Fabiano Fidêncio, 5 days ago)
   RESOLV: Fix a "-Werror=null-dereference" caught by GCC

   Signed-off-by: Fabiano Fidêncio 

bd1d7fd (Fabiano Fidêncio, 5 days ago)
   RESOLV: Simplify reply_weight_rearrange() a little bit

   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/50/head:pr50
git checkout pr50
From bd1d7fd946edb021467e1c9c65ea31f5384fd40a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Wed, 12 Oct 2016 14:57:50 +0200
Subject: [PATCH 1/5] RESOLV: Simplify reply_weight_rearrange() a little bit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fabiano Fidêncio 
---
 src/resolv/async_resolv.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index e859556..daa6496 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -2190,14 +2190,13 @@ static int reply_weight_rearrange(int len,
 
 /* add to the head of the new list */
 tmp = r;
-r = r->next;
 
 tmp->next = *start;
 *start = tmp;
 } else {
 prev = r;
-r = r->next;
 }
+r = r->next;
 }
 *end = prev ? prev : *start;
 
@@ -2241,11 +2240,10 @@ static int reply_weight_rearrange(int len,
 /* add r to the end of the new list */
 if (!new_start) {
 new_start = r;
-new_end = r;
 } else {
 new_end->next = r;
-new_end = r;
 }
+new_end = r;
 }
 new_end->next = NULL;
 

From 58609d376f81881269c5527a508eb00bc03d33a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Wed, 12 Oct 2016 15:00:04 +0200
Subject: [PATCH 2/5] RESOLV: Fix a "-Werror=null-dereference" caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fabiano Fidêncio 
---
 src/resolv/async_resolv.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index daa6496..2b46d02 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -2245,7 +2245,9 @@ static int reply_weight_rearrange(int len,
 }
 new_end = r;
 }
-new_end->next = NULL;
+if (new_end) {
+new_end->next = NULL;
+}
 
 /* return the rearranged list */
 *start = new_start;

From f22aff71ab527d6e2c672485f83c9ee8540b1f94 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Wed, 12 Oct 2016 16:02:55 +0200
Subject: [PATCH 3/5] SIFP: Fix a "jump-misses-init" warning caught by GCC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fabiano Fidêncio 
---
 src/lib/sifp/sss_sifp_parser.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/lib/sifp/sss_sifp_parser.c b/src/lib/sifp/sss_sifp_parser.c
index 65babb5..43eab4d 100644
--- a/src/lib/sifp/sss_sifp_parser.c
+++ b/src/lib/sifp/sss_sifp_parser.c
@@ -283,7 +283,8 @@ sss_sifp_parse_basic(sss_sifp_ctx *ctx,
 uint64_t, uint64_t, uint64,

[SSSD] [sssd PR#34][comment] cache_req: move from switch to plugins

2016-10-17 Thread pbrezina 
  URL: https://github.com/SSSD/sssd/pull/34
Title: #34: cache_req: move from switch to plugins

pbrezina commented:
"""
Updated patch pushed. Diff:

```c
diff --git a/src/responder/common/cache_req/cache_req_search.c 
b/src/responder/common/cache_req/cache_req_search.c
index 3c11efd..a36a900 100644
--- a/src/responder/common/cache_req/cache_req_search.c
+++ b/src/responder/common/cache_req/cache_req_search.c
@@ -102,7 +102,7 @@ static errno_t cache_req_search_cache(TALLOC_CTX *mem_ctx,
 cr->debugobj);

 ret = cr->plugin->lookup_fn(mem_ctx, cr, cr->data, cr->domain, );
-if (ret == EOK && result != NULL && result->count == 0) {
+if (ret == EOK && (result == NULL || result->count == 0)) {
 ret = ENOENT;
 }
```
"""

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

[SSSD] Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates

2016-10-17 Thread Jan Cholasta 

Hi,

On 13.10.2016 18:52, Sumit Bose wrote:

On Tue, Oct 11, 2016 at 01:37:09PM +0200, Sumit Bose wrote:

On Thu, Oct 06, 2016 at 12:49:30PM +0200, Sumit Bose wrote:

Hi,

I've started to write a SSSD design page about enhancing the current
mapping of certificates to users and how to select/match a suitable
certificate if multiple certificates are on a Smartcard.

My currently thoughts and idea and be found at
https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates
and for your convenience below as well.

Comments and suggestions are welcome. Please let me know about concerns,
alternatives and missing use-cases/user-stories.

bye,
Sumit



Hi,

Rob, Fraser, Alexander, thank you for your comments. I think both the
issuer specific matching and the OID in the SUBJECT matching are good
ideas. I updated the design page accordingly. The changes can be shown
with
https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates?action=diff=9_version=6

The updated version can be found below as well. Of course more comments and
suggestions are still very welcome.



I did another update. A "Compatibility with Active Director" section is
added which made me realize that there are use-cases for using the
issuer in the mapping as well and the sub-strings in LDAP search filters
might be useful as well.

The changes can be seen with
https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates?action=diff=10_version=9

Please let me know your comments and suggestions.

bye,
Sumit

= Matching and Mapping Certificates =

Related ticket(s):
 * http://www.freeipa.org/page/V4/User_Certificates#Certificate_Identity_Mapping

=== Problem statement ===
 Mapping 
Currently it is required that a certificate used for authentication is either 
stored in the LDAP user entry or in a matching override. This might not always 
be applicable and other ways are needed to relate a user with a certificate.

 Matching 
Even if SSSD will support multiple certificates on a Smartcard in the context 
of https://fedorahosted.org/sssd/ticket/3050 it might be necessary to restrict 
(or relax) the current certificate selection in certain environments.

=== Use cases ===
 Mapping 
In some environments it might not be possible or would cause unwanted effort to 
add certificates to the LDAP entry of the users to allow Smartcard based 
authentication. Reasons might be:
* Certificates/Smartcards are issued externally
* LDAP schema extension is not possible or not allowed

 Matching 
A user might have multiple certificate on a Smartcard which are suitable for 
authentication. But on some host in the environment only certificates from a 
specific CA (while all other CAs are trusted as well) or with some special 
extension should be valid for login.

=== Overview of the solution ===
To match a certificate a language/syntax has to be defined which allows to 
reference items from the certificate and compare the values with the expected 
data. To map the certificates to a user the language/syntax should allow to 
relate certificate items with LDAP attributes so that the value(s) from the 
certificate item can be used in a LDAP search filter.


Note that in some cases it might be possible to map a certificate to a 
user without having to do an extra LDAP search, for example when the 
certificate contains the principal name of the user. Does the design 
allow this? Or is there no extra LDAP search?





=== Implementation details ===
 Matching 
The pkinit plugin of MIT Kerberos must find a suitable certificate from a 
Smartcard as well and has defined the following syntax (see the 
pkinit_cert_match section of the krb5.conf man page or 
http://web.mit.edu/Kerberos/krb5-1.14/doc/admin/conf_files/krb5_conf.html for 
details). The main components are

* regular-expression
* regular-expression
* regular-expression
* extended-key-usage-list
* key-usage-list

and can be grouped together with a prefixed '&&' (and) or '`||`' (or) operator 
('&&' is the default). If multiple rules are given they are iterated with the order in 
the config file as long as a rule matches exactly one certificate.

'''Question: MIT Kerberos use case-sensitive matching and POSIX Extended 
Regular Expression syntax, shall we do the same?'''

While  and  are (imo) already quite flexible I can see some 
potential extensions for the other components.


I don't think regular expressions are a particularly good choice for DN 
matching. It is difficult to express assertions which are quite natural 
for DNs (matching multi-attribute RDNs, matching the same attribute type 
by different identifiers, respecting the defined matching rules of 
attribute types) and at the same time it is easy to express assertions 
which do not make much sense for DNs (matching substrings in attribute 
names, matching accross multiple syntactical elements, etc.).


That said, does the design have to be based on the MIT pkinit matching?