[SSSD] [sssd PR#446][closed] RESP: Add some missing NULL checks

2017-11-09 Thread lslebodn 
   URL: https://github.com/SSSD/sssd/pull/446
Author: jhrozek
 Title: #446: RESP: Add some missing NULL checks
Action: closed

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

[SSSD] [sssd PR#446][comment] RESP: Add some missing NULL checks

2017-11-09 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/446
Title: #446: RESP: Add some missing NULL checks

lslebodn commented:
"""
master:
* 2c6c3cff23a0750a5f68b52cb1a52e241aa20615
"""

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

[SSSD] [sssd PR#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

fidencio commented:
"""
@lslebodn, so, if I revert that patch the warning is back. I, personally, don't 
think that the safety check would hurt us in any way, so I'd prefer keeping it 
and avoiding the covscan warning.
"""

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

[SSSD] [sssd PR#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

fidencio commented:
"""
Let me re-run the covscan just to give you a proper answer, but the patch 
you're proposing to drop was proposed exactly to fix that issue.
"""

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

[SSSD] [sssd PR#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

lslebodn commented:
"""
@fidencio Are you still able to see warning with latest version after dropping 
change in `sss_authtok_get_type` ?
"""

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

[SSSD] [sssd PR#433][-Accepted] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

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#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread lslebodn 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

lslebodn commented:
"""
Just a note. I would like to avoid following change:

```
diff --git a/src/util/authtok.c b/src/util/authtok.c
index c2f78be32..2c5a26ce3 100644
--- a/src/util/authtok.c
+++ b/src/util/authtok.c
@@ -27,6 +27,10 @@ struct sss_auth_token {

 enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok)
 {
+if (tok == NULL) {
+return SSS_AUTHTOK_TYPE_EMPTY;
+}
+
 return tok->type;
 }
```

We need to properly initialise authtok in all cases. It must not be `NULL`
BTW following report from static analysers part is outdated:

```
Error: FORWARD_NULL (CWE-476): [#def2]

sssd-1.16.1/src/responder/pam/pamsrv_p11.c:433: var_compare_op: Comparing 
"pd->authtok" to null implies that "pd->authtok" might be null.
sssd-1.16.1/src/responder/pam/pamsrv_p11.c:461: var_deref_model: Passing null 
pointer "pd->authtok" to "sss_authtok_get_type", which dereferences it.
sssd-1.16.1/src/util/authtok.c:30:5: deref_parm: Directly dereferencing 
parameter "tok".
#   28|   enum sss_authtok_type sss_authtok_get_type(struct sss_auth_token *tok)
#   29|   {
#   30|-> return tok->type;
#   31|   }
#   32|   

"""

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

[SSSD] [sssd PR#441][comment] CHILD: Pass information about logger to children

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/441
Title: #441: CHILD: Pass information about logger to children

fidencio commented:
"""
@lslebodn, although the "SYSTEMD: Clean pid file in corner cases" is not 
related to this series, I'm fine on having it pushed together.

Code-wise the patch looks good and I'm just waiting the results of downstream 
tests to ack the series.

"""

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

[SSSD] [sssd PR#442][comment] LDAP: Improve error treatment from sdap_cli_connect() in ldap_auth

2017-11-09 Thread sumit-bose 
  URL: https://github.com/SSSD/sssd/pull/442
Title: #442: LDAP: Improve error treatment from sdap_cli_connect() in ldap_auth

sumit-bose commented:
"""
The patch makes sense and passes the CI (with the current exception on F27 and 
rawhide caused by the newer libldb). ACK

@fidencio, please let me know when you have the results of the regression test.
"""

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

[SSSD] Re: [SSSD-users] Re: Re: [Freeipa-interest] Announcing SSSD 1.16.0

2017-11-09 Thread Lukas Slebodnik 
On (23/10/17 21:33), Michael Ströder wrote:
>Jakub Hrozek wrote:
>> On Mon, Oct 23, 2017 at 09:19:21PM +0200, Michael Ströder wrote:
>>> Jakub Hrozek wrote:
 On Mon, Oct 23, 2017 at 08:46:08PM +0200, Michael Ströder wrote:
> Has anything changed with building the man pages?
>
> I'm asking because I now get formatting markup in the output of man (see
> below).

 No, not that I'm aware of. You render the man pages locally, right,
 because the tarball only contains the XML sources?
>>>
>>> I'm looking at the man pages generated by the RPM build on openSUSE
>>> build server which IIRC used to work up to sssd release 1.15.2:
>>>
>>> https://build.opensuse.org/package/view_file/home:stroeder:branches:network:ldap/sssd/sssd.spec?expand=1
>> 
>> Nothing changed there for a long time.. If you build the last-known-good
>> (so, 1.15.2) tarball in exactly the same build root, the man pages are
>> rendered fine?
>
>Ummpf! It seems that the man pages were broken recently even for the
>1.15.2 builds.
>
>Any hints which man build tools to check?
>Related build dependencies seem to be:
>
>BuildRequires:  docbook-xsl-stylesheets
>BuildRequires:  libxml2-tools
>BuildRequires:  libxslt-tools
>
Hello,

have you found the bug?
Because I've just tried with latest openSUSE Tumbleweed
and it is still broken.

Does it work on openSUSE Leap?

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#446][comment] RESP: Add some missing NULL checks

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/446
Title: #446: RESP: Add some missing NULL checks

fidencio commented:
"""
ACK!
"""

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

[SSSD] [sssd PR#446][+Accepted] RESP: Add some missing NULL checks

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/446
Title: #446: RESP: Add some missing NULL checks

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#446][opened] RESP: Add some missing NULL checks

2017-11-09 Thread jhrozek 
   URL: https://github.com/SSSD/sssd/pull/446
Author: jhrozek
 Title: #446: RESP: Add some missing NULL checks
Action: opened

PR body:
"""
This trivial bug was copied all around the responder code. Only when I
initially copied it for another time I noticed we should check the info
structure pointer after allocating it.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/446/head:pr446
git checkout pr446
From 1a0cf82c7e48ca6e8cfd66265f47b8dcec2703fd Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Thu, 9 Nov 2017 13:24:47 +0100
Subject: [PATCH] RESP: Add some missing NULL checks

---
 src/responder/autofs/autofssrv_dp.c | 4 
 src/responder/common/responder_dp.c | 4 
 src/responder/common/responder_dp_ssh.c | 4 
 src/responder/sudo/sudosrv_dp.c | 4 
 4 files changed, 16 insertions(+)

diff --git a/src/responder/autofs/autofssrv_dp.c b/src/responder/autofs/autofssrv_dp.c
index a323d83d9..bb8c2a428 100644
--- a/src/responder/autofs/autofssrv_dp.c
+++ b/src/responder/autofs/autofssrv_dp.c
@@ -65,6 +65,10 @@ sss_dp_get_autofs_send(TALLOC_CTX *mem_ctx,
 }
 
 info = talloc_zero(state, struct sss_dp_get_autofs_info);
+if (info == NULL) {
+ret = ENOMEM;
+goto error;
+}
 info->fast_reply = fast_reply;
 info->type = type;
 info->name = name;
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index a75a61196..935a36d28 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -536,6 +536,10 @@ sss_dp_get_account_send(TALLOC_CTX *mem_ctx,
 }
 
 info = talloc_zero(state, struct sss_dp_account_info);
+if (info == NULL) {
+ret = ENOMEM;
+goto error;
+}
 info->fast_reply = fast_reply;
 info->type = type;
 info->opt_name = opt_name;
diff --git a/src/responder/common/responder_dp_ssh.c b/src/responder/common/responder_dp_ssh.c
index 303ba1568..f78052296 100644
--- a/src/responder/common/responder_dp_ssh.c
+++ b/src/responder/common/responder_dp_ssh.c
@@ -64,6 +64,10 @@ sss_dp_get_ssh_host_send(TALLOC_CTX *mem_ctx,
 }
 
 info = talloc_zero(state, struct sss_dp_get_ssh_host_info);
+if (info == NULL) {
+ret = ENOMEM;
+goto error;
+}
 info->fast_reply = fast_reply;
 info->name = name;
 info->alias = alias;
diff --git a/src/responder/sudo/sudosrv_dp.c b/src/responder/sudo/sudosrv_dp.c
index 3a4a79473..f8ec8abc2 100644
--- a/src/responder/sudo/sudosrv_dp.c
+++ b/src/responder/sudo/sudosrv_dp.c
@@ -72,6 +72,10 @@ sss_dp_get_sudoers_send(TALLOC_CTX *mem_ctx,
 }
 
 info = talloc_zero(state, struct sss_dp_get_sudoers_info);
+if (info == NULL) {
+ret = ENOMEM;
+goto error;
+}
 info->fast_reply = fast_reply;
 info->type = type;
 info->name = name;
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org

[SSSD] [sssd PR#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

fidencio commented:
"""
Please, whoever pushes this patch set, add Scott Poore as reviewer as well.
"""

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

[SSSD] [sssd PR#433][+Accepted] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

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#433][comment] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

fidencio commented:
"""
Okay, all the downstream tests passed (one of the them only in the second run, 
though).

ACK!
"""

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

[SSSD] [sssd PR#433][-Changes requested] PAM: Multiple certificates on a Smartcard

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/433
Title: #433: PAM: Multiple certificates on a Smartcard

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: Question about memory mapped cache

2017-11-09 Thread Michal Židek 

On 11/08/2017 11:28 AM, Sumit Bose wrote:

Hi,

while trying to prepare some diagrams to illustrate how the memory mapped
cache works I realized that we might not use the payload factor as
expected.

In sss_mmap_cache_init() we set a payload depending of the type of
cached data, e.g. for passwd data 4 * MC_SLOT_SIZE or for group data 3 *
MC_SLOT_SIZE. This value is used to calculate the data table size:

#define MC_DT_SIZE(elems, payload) ( (elems) * (payload) )

 mc_ctx->dt_size = MC_DT_SIZE(n_elem, payload);

since this does not change the slot size there is now room for e.g
(n_elem * 4) slots for passwd data, which makes sense since we assume
that a typical passwd entry will occupy 4 slot and we want the cache to
be able to store n_elem passwd entries.

But the memory for the tree table is calculated only with n_elem

#define MC_FT_SIZE(elems) ( (elems) / 8 )

 mc_ctx->ft_size = MC_FT_SIZE(n_elem);

This means that although we allocated memory for e.g. (4 * n_elem) slots
we can only handle n_elem slots because there are only n_elem bits in
the free table. As a result 3/4 of the allocate memory for the data is
unused and entries in the cache are overwritten earlier than needed.

Do you agree with this or did I miss something in the remaining part of
the code which makes sure the memory allocated for the data table can be
used completely?

If not I would suggest to make the payload only a factor which then can
be used to allocate the memory for the data table and the free table.

bye,
Sumit


Hi Sumit,

I looked into it and I think you are correct. Will you file
a ticket?

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#424][-Changes requested] TOOLS: Add a new sssctl command access-report

2017-11-09 Thread fidencio 
  URL: https://github.com/SSSD/sssd/pull/424
Title: #424: TOOLS: Add a new sssctl command access-report

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