[SSSD] [sssd PR#5776][comment] kcm: replace existing credentials to avoid unnecessary ccache growth

2021-10-04 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5776
Title: #5776: kcm: replace existing credentials to avoid unnecessary ccache 
growth

simo5 commented:
"""
Saw no issues, LGTM
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5776#issuecomment-933509682
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure


[SSSD] [sssd PR#5776][comment] kcm: replace existing credentials to avoid unnecessary ccache growth

2021-09-20 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5776
Title: #5776: kcm: replace existing credentials to avoid unnecessary ccache 
growth

simo5 commented:
"""
Testing the f34 packages you have provided me, will report if anything's wrong 
with them.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5776#issuecomment-923177105
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure


[SSSD] [sssd PR#5506][comment] SSSD Log: write_krb5info_file word replacement

2021-02-16 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5506
Title: #5506: SSSD Log: write_krb5info_file word replacement

simo5 commented:
"""
FWIW, the file location depends on ./configure settings, so the whole path 
should be taken from a #define, and not hard coded, as it will be misleading if 
the package is built with a different path for varlib.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5506#issuecomment-779875805
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure


[SSSD] [sssd PR#5375][comment] kcm: improve performance for large ccaches

2020-10-23 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5375
Title: #5375: kcm: improve performance for large ccaches

simo5 commented:
"""
Nice work!
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5375#issuecomment-715530897
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5370][comment] nss: Use posix_fallocate() to alloc memcache file

2020-10-16 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5370
Title: #5370: nss: Use posix_fallocate() to alloc memcache file

simo5 commented:
"""
It would be nicer to retry, operating w/o the cache is a very degraded 
operation mode.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5370#issuecomment-710471490
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5370][comment] nss: Use posix_fallocate() to alloc memcache file

2020-10-15 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5370
Title: #5370: nss: Use posix_fallocate() to alloc memcache file

simo5 commented:
"""
> > > Another thing to consider is that posix_fallocate will fail for some file 
> > > systems, for example NFS.
> > 
> > 
> > My understanding is that `posix_fallocate` will fallback to an emulated 
> > mode if the filesystem doesn't support `fallocate`.
> 
> Right, indeed.
> 
> Btw, `posix_fallocate` can return `EINTR` but perhaps handling this would be 
> too nitpicky :)

I would handle it, it's not hard, and can really happen on busy systems. Not 
handling it can lead to heisenbugs that are hard to figure out later.
If handling is too hard then a log should be emitted that allows to understand 
the nature of the failure.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5370#issuecomment-709482400
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5370][comment] nss: Use posix_fallocate() to alloc memcache file

2020-10-15 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5370
Title: #5370: nss: Use posix_fallocate() to alloc memcache file

simo5 commented:
"""
> > I think you are correct that we cannot rely on the file appearing zero 
> > filled following a call to `posix_fallocate()`. There could then be a small 
> > race where a client does not treat the cache file as 
> > [un-initialised](https://github.com/SSSD/sssd/blob/b913ddbd5aec9ea87ddbf10dd09299edd87854dd/src/util/mmap_cache.h#L81).
> 
> Yes, this is what I was thinking about.
> On the other hand: even taking into account `fallocate` (used under the hood 
> on Linux systems) doesn't initialize allocated FS blocks, what else could 
> `read` return if not zeros?
> I mean FS "knows" blocks aren't initiated and thus will not read garbage left 
> on the disk (would be a security threat).
> Would be good to find a confirmation :) I wouldn't like to complicate things 
> without a reason.

Certainly you won't get random disk sectors out of it, and in case of mmap new 
memory pages are always zeroed before handling them to the user process, so you 
can bet that all this space is intialized to zero.
(If you have doubts write a simple test program to check it :-)

> > On the concern over the note in the glibc docs, the most obvious way I can 
> > see this manifesting is when working with copy-on-write cloned files, which 
> > we aren't doing here (however I guess there are other scenarios). It's also 
> > not clear if this note relates only to the emulated behaviour of 
> > `posix_fallocate()`. I think it's fair to say that the general behaviour of 
> > `posix_fallocate()` is closer to what we need than `ftruncate()`, even with 
> > this caveat.
> 
> Agree.

+1

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5370#issuecomment-709481365
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5356][comment] krb5_child: Harden credentials validation code

2020-10-07 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5356
Title: #5356: krb5_child: Harden credentials validation code

simo5 commented:
"""
Sounds liek some tests failed due to infrastructure issues ...
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5356#issuecomment-705140663
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5356][comment] krb5_child: Harden credentials validation code

2020-10-07 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5356
Title: #5356: krb5_child: Harden credentials validation code

simo5 commented:
"""
> Hi,
> 
> thanks for the patch, it looks good. I just wonder if it might be risky to 
> use the pretty generic macro `TRUE` which other libraries we might include as 
> well may define completely different? With a quick grep in /usr/include I 
> didn't found an example but even in the MIT krb5 source code `1`is used in 
> `krb_auth_su.c`.

TRUE is defined in /usr/include/krb5/krb5.h but I can change the code to just 
use '1' if you think this is too risky.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5356#issuecomment-705053343
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5356][opened] krb5_child: Harden credentials validation code

2020-10-07 Thread simo5
   URL: https://github.com/SSSD/sssd/pull/5356
Author: simo5
 Title: #5356: krb5_child: Harden credentials validation code
Action: opened

PR body:
"""
The krb5_verify_init_creds() call is used to validate the credentials
just obtained by trying to acquire a ticket from the KDC that we can
decrypti. This insures the KDC is indeed legitimate as it proves
possesion of the shared key.

However this function will *enforce* this behavior only if the
KRB5_VERIFY_INIT_CREDS_OPT_AP_REQ_NOFAIL options is set to the value
TRUE.

If this option is unset it defaults to FALSE which means verify will
silently return success if no key is available.

SSSD *does* ensure that a key is always available for validation, so
this is not a security bug with the current code. However we add belt
and suspenders here to futureproof this code in case of future
inadvertent changes that may lead to a code path where a key may be
missing.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5356/head:pr5356
git checkout pr5356
From 4e4a2739f4f4831b1ab3592b0b995c0a86f8d728 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Wed, 7 Oct 2020 09:15:34 -0400
Subject: [PATCH] krb5_child: Harden credentials validation code

The krb5_verify_init_creds() call is used to validate the credentials
just obtained by trying to acquire a ticket from the KDC that we can
decrypti. This insures the KDC is indeed legitimate as it proves
possesion of the shared key.

However this function will *enforce* this behavior only if the
KRB5_VERIFY_INIT_CREDS_OPT_AP_REQ_NOFAIL options is set to the value
TRUE.

If this option is unset it defaults to FALSE which means verify will
silently return success if no key is available.

SSSD *does* ensure that a key is always available for validation, so
this is not a security bug with the current code. However we add belt
and suspenders here to futureproof this code in case of future
inadvertent changes that may lead to a code path where a key may be
missing.

Signed-off-by: Simo Sorce 
---
 src/providers/krb5/krb5_child.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index adbdffaa87..0aea3dcdad 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1406,6 +1406,7 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
 
 
 krb5_verify_init_creds_opt_init();
+krb5_verify_init_creds_opt_set_ap_req_nofail(, TRUE);
 kerr = krb5_verify_init_creds(kr->ctx, kr->creds, validation_princ, keytab,
   _ccache, );
 
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5284][comment] Remove leftover ccache from SSH credentials delegation

2020-08-21 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5284
Title: #5284: Remove leftover ccache from SSH credentials delegation

simo5 commented:
"""
@justin-stephenson first of all I'd like to thank you for this PR as it raised 
very interesting questions and aspects that evidently had not been though 
through enough.
At this poj tI think the only way to move forward is to first write down what 
is the behavior we need to employ exactly, from the pov of KCM, regardless of 
what client process is calling in.
After that, as much as possible we should write tests that expect the agreed on 
behavior.
And finally change code accordingly making sure test do not break.

I feel like proceeding w/o these steps would be stumbling in the dark and 
creating a semantic quagmire that will require endless course correction.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5284#issuecomment-678362457
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5284][comment] Remove leftover ccache from SSH credentials delegation

2020-08-20 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5284
Title: #5284: Remove leftover ccache from SSH credentials delegation

simo5 commented:
"""
> > Maybe I am missing something but how do you distinguish between a ccache 
> > created by SSSD when user1@REALM logs into the console, from the ccache 
> > created by ssh when the same user connects to the same machine via SSH 
> > later on ?
> 
> My understanding is that to distinguish the two, the KCM initialize operation 
> checks for the existence of a "dummy" ccache created by an earlier KCM 
> operation to handle the following sequence of calls performed by sshd:
> 
> cc = krb5_cc_new_unique
> krb5_cc_switch(cc)
> krb5_cc_initialize(cc, principal)
> 
> KCM creates a 'dummy' ccache with krb5_cc_switch() and then the initialize 
> call just fills in the details. Inside the kcm initialize operation we have 
> this logic, the first few lines representing the code block in my previous 
> comment.
> 
> ```
>   if ccache exists already for this input parameter `name`
>   if ccache principal is empty, we have a "dummy" ccache

How do you know this dummy entry was created by SSH and not another similar 
service being accessed concurrently by the user?

>   proceed with filling in and initializing the "dummy" ccache
>   else
>   remove the existing ccache and contents, then initialize the new 
> ccache

How do you know what created this ccache in the else part ? (specifically that 
this is an SSH created ccache)

If I read this correctly, 'name' is the user principal, and it will be the same 
for all ccaches created by this user, whether coming via ssh connection or 
locally running kinit or logging in at console via SSSD, or via GSSAPI KEYTAB 
auto-init, or ...

>   else
>   initialize the ccache
> ```


"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5284#issuecomment-677894098
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5284][comment] Remove leftover ccache from SSH credentials delegation

2020-08-20 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5284
Title: #5284: Remove leftover ccache from SSH credentials delegation

simo5 commented:
"""
A question also occurred to me, is there any concurrency issue with this 
process?
What happens if two ssh connections are initiated simultaneously by the same 
user ?
Could they end up trying to delete each other ccaches once both sshd workers 
end up concurrently calling into sssd-kcm ?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5284#issuecomment-677858069
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5284][comment] Remove leftover ccache from SSH credentials delegation

2020-08-20 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5284
Title: #5284: Remove leftover ccache from SSH credentials delegation

simo5 commented:
"""
Maybe I am missing something but how do you distinguish between a ccache 
created by SSSD when user1@REALM logs into the console, from the ccache created 
by ssh when the same user connects to the same machine via SSH later on ?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5284#issuecomment-677856261
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#5284][comment] Remove leftover ccache from SSH credentials delegation

2020-08-20 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/5284
Title: #5284: Remove leftover ccache from SSH credentials delegation

simo5 commented:
"""
not all tickets are created equally, so removing other ccaches may end up 
removing tickets that are different on purpose.
for example a process may create  new ccache to get a forwardable ticket, or a 
shorter lived ticket.
Those ccaches should not arbitrarily removed.
Expired ccaches should be removed though (IMO, I know upstream/Robbie have a 
different view on keeping expired ccaches).
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5284#issuecomment-677745024
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#837][comment] p11_child: make OCSP digest configurable

2020-08-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/837
Title: #837: p11_child: make OCSP digest configurable

simo5 commented:
"""
> > Although it is true @dpward that SHA-1 is allowed, you need to read the 
> > fine print as well "for applications that do not require collision 
> > resistance".
> > Basically this exclude unique identifier if those are used in any 
> > "security" sense, while you still can use SHA-1 in applications where a 
> > collision is handled appropriately (for example hash-maps).
> 
> @simo5 Are there any concerns about that for the issuer hash in the OCSP 
> request though?

I haven't looked carefully enough to say if this is just a short hand identifer 
upon which no security at all impinge, or not. sounds *ok* for now at any rate.

> > So it really depend more and more on the kinds of usage, which is why it is 
> > preferable, where possible, to simply move off of SHA-1.
> 
> This is an OCSP client implementation, which needs to be compatible with the 
> OCSP server in order for it to be useful. The reason given for this 
> particular change was "FIPS compliance". Not only am I unable to locate such 
> a requirement in FIPS, but this change has the practical effect of actually 
> breaking out-of-the-box compatibility with OCSP servers that have a 
> **mandate** to comply with FIPS.

Yeah I think the message got simplified to "remove SHA-1 everywhere" in transit.
It is an easy mistake, and as I said it is better to move to SHA-2 wherever 
possible anyway.
In this case the "wherever possible" threshold seem to not have been met.

> How was this change tested? As another data point here, please see: 
> https://devicepki.idmanagement.gov/certificateprofiles/#ocsp-response-profile

It's a bug, it will be fixed, doesn't help that OCSP is not yet that common 
that you'd spot it right away ...

> > Field
> > Value and Requirements
> > 
> > 
> > 
> > 
> > CertID
> > hashAlgorithm shall be SHA-1The issuerKeyHash and issuerNameHash pair must 
> > be identical within all Single Responses appearing in an OCSP Response

This will come up over and over again probably, damn if you do, damned if you 
don't :-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/837#issuecomment-675027739
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#837][comment] p11_child: make OCSP digest configurable

2020-08-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/837
Title: #837: p11_child: make OCSP digest configurable

simo5 commented:
"""
> > Although it is true @dpward that SHA-1 is allowed, you need to read the 
> > fine print as well "for applications that do not require collision 
> > resistance".
> > Basically this exclude unique identifier if those are used in any 
> > "security" sense, while you still can use SHA-1 in applications where a 
> > collision is handled appropriately (for example hash-maps).
> 
> @simo5 Are there any concerns about that for the issuer hash in the OCSP 
> request though?

I haven't looked carefully enough to say if this is just a short hand identifer 
upon which no security at all impinge, or not. sounds *ok* for now at any rate.

> > So it really depend more and more on the kinds of usage, which is why it is 
> > preferable, where possible, to simply move off of SHA-1.
> 
> This is an OCSP client implementation, which needs to be compatible with the 
> OCSP server in order for it to be useful. The reason given for this 
> particular change was "FIPS compliance". Not only am I unable to locate such 
> a requirement in FIPS, but this change has the practical effect of actually 
> breaking out-of-the-box compatibility with OCSP servers that have a 
> **mandate** to comply with FIPS.

Yeah I think the message got simplified to "remove SHA-1 everywhere" in transit.
It is an easy mistake, and as I said it is better to move to SHA-2 wherever 
possible anyway.
In this case the "wherever possible" threshold seem to not have been met.

How was this change tested? As another data point here, please see: 
https://devicepki.idmanagement.gov/certificateprofiles/#ocsp-response-profile

It's a bug, it will be fixed, doesn't help that OCSP is not yet that common 
that you'd spot it right away ...

> > Field
> > Value and Requirements
> > 
> > 
> > 
> > 
> > CertID
> > hashAlgorithm shall be SHA-1The issuerKeyHash and issuerNameHash pair must 
> > be identical within all Single Responses appearing in an OCSP Response

This will come up over and over again probably, damn if you do, damned if you 
don't :-)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/837#issuecomment-675027739
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#837][comment] p11_child: make OCSP digest configurable

2020-08-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/837
Title: #837: p11_child: make OCSP digest configurable

simo5 commented:
"""
@alexey-tikhonov if there is a compatibility issue, I would certainly allow 
SHA-1 to be used, but perhaps with an easier way to flip it off in future.

Although it is true @dpward that SHA-1 is allowed, you need to read the fine 
print as well "for applications that do not require collision resistance".

Basically this exclude unique identifier if those are used in any "security" 
sense, while you still can use SHA-1 in applications where a collision is 
handled appropriately (for example hash-maps).

So it really depend more and more on the kinds of usage, which is why it is 
preferable, where possible, to simply move off of SHA-1.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/837#issuecomment-674968818
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#837][comment] p11_child: make OCSP digest configurable

2020-08-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/837
Title: #837: p11_child: make OCSP digest configurable

simo5 commented:
"""
@alexey-tikhonov if there is a compatibility issue, I would certainly allow 
SHA-1 to be used, but perhaps with an easier way to flip it off in future.

Alsthough it is true @dpward that SHA-1 is allowed, you need to read the fine 
print as well "for applications that do not require collision resistance".

Basically this exclude unique identifier if those are used in any "security" 
sense, while you still can use SHA-1 in applications where a collision is 
handled appropriately (for example hash-maps).

So it really depend more and more on the kinds of usage, which is why it is 
preferable, where possible, to simply move off of SHA-1.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/837#issuecomment-674968818
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#837][comment] p11_child: make OCSP digest configurable

2020-08-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/837
Title: #837: p11_child: make OCSP digest configurable

simo5 commented:
"""
@alexey-tikhonov if there is a compatibility issue, I would certainly allow 
sah-1 to be used, but perhaps with an easier way to flip it off in future.

Alsthough it is true @dpward that SHA-1 is allowed, you need to read the fine 
print as well "for applications that do not require collision resistance".

Basically this exclude unique identifier if those are used in any "security" 
sense, while you still can use SHA-1 in applications where a collision is 
handled appropriately (for example hash-maps).

So it really depend more and more on the kinds of usage, which is why it is 
preferable, where possible, to simply move off of SHA-1.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/837#issuecomment-674968818
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#854][comment] LDAP: Do not require START_TLS for loopback connections

2019-07-30 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/854
Title: #854: LDAP: Do not require START_TLS for loopback connections

simo5 commented:
"""
@scabrero @jhrozek I see no discussion ensued, just to be clear I didn't give 
and didn't mean my messages to be a complete NACK on the approach. But I wanted 
a better analysis of the situation and options before giving my blessing to any 
approach.
HTH.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/854#issuecomment-516595230
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#854][comment] LDAP: Do not require START_TLS for loopback connections

2019-07-25 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/854
Title: #854: LDAP: Do not require START_TLS for loopback connections

simo5 commented:
"""
@scabrero To be honest I would prefer to write a tool that simply generates a 
local certificate, adds it to the local machine trust store and gives it to the 
LDAP server. Then there is no need for exceptions on the SSSD side, and a local 
attacker can't impersonate the server no matter what.
Is that hard to do for some reason ?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/854#issuecomment-515029731
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#854][comment] LDAP: Do not require START_TLS for loopback connections

2019-07-24 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/854
Title: #854: LDAP: Do not require START_TLS for loopback connections

simo5 commented:
"""
```
$ host localhost
Host localhost not found: 3(NXDOMAIN)
```
:-)

More seriously I would like to understand why this is needed (after all we 
already have the scary hidden option to avoid TLS, why is that not sufficient 
?), and why it is safe(?), noting that in the code proposed you allow any 
connection to any port on *all* localhost addresses (you are checking for 
IN_LOOPBACKNET not just specifically IPADDR_LOOPBACK).

The ldapi:// case is not just about encryption but also to access control, you 
do not get to interpose a socket that root set up with the right permissions on 
the filesystem.

In this patch though there is no way to make sure we are restricting access 
only to the right peer if the port is above 1024. In a setup where root decides 
to set up the LDAP server on a higher port (say because they were told to run 
the LDAP server as non-root to improve security and they find it easier to just 
run on a higher port as that is allowed to non-root users), an attacker that 
finds a way to make the ldap server crash can then run their own ldap server on 
that port and feed arbitrary information to SSSD (which means privilege 
escalation to root almost certainly at that point).

So I guess what I am saying is that it might be acceptable, perhaps with 
limiting also ports to < 1024 ...
... except that then that is also something somewhat easy to work around in 
setups where admins let containers can be spun on other local addresses and 
have CAP_NET_ADMIN within the container (somewhat common in some scenarios).

I do note that using certs in this scenario is hard (which is why I guess 
@scabrero wants to avoid TLS) because no sane public authority will give you a 
cert signed for 127.x.x.x or the name localhost[.localdomain] as is proper. 
However providing a tool that root can use to create a custom, locally trusted 
cert is probably not too hard and will provide a much better proposition wrt 
security.

Soo ... long story short, at the moment I do not see a convincing argument to 
approve this patch as is on my side, can you at least better describe the use 
case ? The commit and this PR description are quite  ... bare.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/854#issuecomment-514783046
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: 
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#826][comment] util/crypto/libcrypto: changed sss_hmac_sha1()

2019-06-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/826
Title: #826: util/crypto/libcrypto: changed sss_hmac_sha1()

simo5 commented:
"""
LGTM
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/826#issuecomment-501428262
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#813][comment] SDAP: allow GSS-SPNEGO for LDAP SASL bind as well

2019-05-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/813
Title: #813: SDAP: allow GSS-SPNEGO for LDAP SASL bind as well

simo5 commented:
"""
We changed GSS-SPNEGO incompatibly at some point, and 2.1.27 may actually be 
that point.
We did change it because it didn't work as it should have (was not 
interoperable with Windows).
So we definitely should check that we have a recent enough cyrus-sasl.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/813#issuecomment-491892022
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#813][comment] SDAP: allow GSS-SPNEGO for LDAP SASL bind as well

2019-05-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/813
Title: #813: SDAP: allow GSS-SPNEGO for LDAP SASL bind as well

simo5 commented:
"""
LGTM, would it make sense to have a followup where GSS-SPNEGO is made the 
default for the AD backend?
GSS_SPNEGO is more efficient as it requires less roundtrips.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/813#issuecomment-491860329
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-11 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
Thank @jhrozek this clears it!
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-446204376
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-11 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
Wait, does this mean it changes current behavior for AD domains ?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-446198589
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-11 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
Or would it previously return "/" unconditionally ?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-446198697
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-04 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
On Tue, 2018-12-04 at 05:51 -0800, Jakub Hrozek wrote:
> Then why not set a default value for fallback homedir? :-)

Because that would override an empty home dir in all cases, and this is not ok.
An empty home dir is a perfectly valid and intentional value in some entries.

It may even lead to security issues to suddenly replace an empty dir
with something else, for example if someone is counting on the empty
dir to make "su" fail.

Simo.

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc



"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-444121508
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-04 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
On Tue, 2018-12-04 at 04:51 -0800, Jakub Hrozek wrote:
> Thanks, this passes the test.  And of course the patch is correct,
> but after some more testing, I wonder if we should at least for one
> release default to fallback_homedir=$something at least for the AD
> provider. Because now with the completely minimal AD provider
> configuration (no POSIX attrs, ID mapping only) I can't log in with
> an AD user:
> ```
> $ getent passwd administra...@win.trust.test
> administra...@win.trust.test:*:215000500:215000513:Administrator::/bi
> n/bash
> $ su - administra...@win.trust.test
> su: user administra...@win.trust.test does not exist
> ```
> Note that this is minimal config, realmd already adds
> fallback_homedir.

Why this fails? Because of the missing homedir ?

> Or at least we should IMO add some backwards compatible handling when
> this patch makes it to fedora or RHEL otherwise admins might not be
> happy. From purely upstream point of view this change is probably OK
> with me.

I think the AD provider should synthetize an home dir by default,
without any specific option being set, it's what is considered normal
also in winbind land, in fact I would look closely at what winbind does
and do the same for AD users by default.

If fallback_homedir is set, skip the default and use what that setting
specifies.

Simo.

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc



"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-444101678
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-03 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
@thalman those other cases are already handled in the call right above your 
change.
So if those are handled homdir will arelady be "not null".

I think all you need to do is return ""; unconditionally when homedir is NULL.

Later if you need additional handling you can do that, but the point is that 
there is no good reason to do the "right thing" only for files.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-443779774
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#703][comment] nss: sssd returns '/' for emtpy home directories

2018-12-03 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/703
Title: #703: nss: sssd returns '/' for emtpy home directories

simo5 commented:
"""
This should probably not be specific to the files provider.
An empty home directory is a valid value and should be returned as is if the 
source database specifies an empty string.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/703#issuecomment-443758006
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#644][comment] When multiple UIDs exist, use the username provided by the user as the first lookup

2018-10-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/644
Title: #644: When multiple UIDs exist, use the username provided by the user as 
the first lookup

simo5 commented:
"""
So if you want to address case (b) you will have to change the code so that it 
returns multiple uids (probably behind a "relax option") and then at time of 
saving it will chose what uid to use based on 2 factors 1) a login happened so 
a specific name was used, 2) what's already in the cache.

It is probably a complicated and invasive change. 
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/644#issuecomment-430692401
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#644][comment] When multiple UIDs exist, use the username provided by the user as the first lookup

2018-10-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/644
Title: #644: When multiple UIDs exist, use the username provided by the user as 
the first lookup

simo5 commented:
"""
@joefischietti we can fixate a user name at login time in the cache but:
a) that may be overidden by a later login if the user uses the "other" name.
b) that may change once the cache expires and someone performs an anumeration 
(like with ls -l) that returns the other name first.

Now we can address (b) by forcing the code to prefer keeping the existing name 
in the cache on cache refreshes, but we cannot solve (a), nor we can predict 
which username will be used *before* the user ever logs in (or after a cache is 
wiped).

So there will be cases when a username change will happen in the system and 
that can cause issues in components that rely on uid -> name mappings.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/644#issuecomment-430690811
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#644][comment] When multiple UIDs exist, use the username provided by the user as the first lookup

2018-10-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/644
Title: #644: When multiple UIDs exist, use the username provided by the user as 
the first lookup

simo5 commented:
"""
I am not talking about the LDAP attribute.
I am saying in posix only one name is possible, so if you have a passwd file 
with multiple names with the same uid or an ldap entry with multiple uids you 
are out of the standard and you get undefined behavior.
Any call that goes UID -> NAME have no way to decide which is the correct name, 
and caching layers may cause different answers at different times, this is true 
for ldap with multiple UIDs and SSSD or passwd with multiple entries and nscd 
caching.

The only way to properly deal with this is to override, per host, the user name 
with a side map (like it is done with id views). Hosts that want to accept 
multiple names per uidNumber are simply going to keep having unpredictable 
results or will have to use alphabetic ordering and always give "unwanted" 
results.
There is no way to handle multiple names for the same user and return the 
"right" name, because "right" is context based, but UID -> Name resolution is 
globablly scoped.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/644#issuecomment-430688448
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#644][comment] When multiple UIDs exist, use the username provided by the user as the first lookup

2018-10-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/644
Title: #644: When multiple UIDs exist, use the username provided by the user as 
the first lookup

simo5 commented:
"""
per posix uid *must* be unique, sorry to say your LDAP setup is simply 
violating standards and cannot be supported in a consistent way.
I do not think sssd (or any other nss ldap module) can really do anything 
useful for you here.
If you have specific systems where a user *always* must use a specific name and 
other systems where it must use the other I would suggest using id views 
(assuming we can do that for generic LDAP) to exactly determine what user name 
to use on any specific host.
In any case the only way to properly handle this is to have a source of 
information that explicitly marks *which* uid is valid and use only that for 
the system.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/644#issuecomment-430611979
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#644][comment] When multiple UIDs exist, use the username provided by the user as the first lookup

2018-10-17 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/644
Title: #644: When multiple UIDs exist, use the username provided by the user as 
the first lookup

simo5 commented:
"""
per posix uid *must* be unique, sorry to say your LDAP setup is simply 
violating standards and cannot be supported in a consistent way.
I do not think sssd (or any other nss ldap module) can really do anything 
useful for you here.
If you have specific systems where a user *always* must use a specific name and 
other systems where it must use the other I would suggest using id views 
(assuming we can do that for generic LDAP) to exactly determine what user name 
to use on any specific host.
In any case the only way to properly handle this is to have a source of 
information that explicitly marks *which* uid is vcalid and use only that for 
the system.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/644#issuecomment-430611979
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org


[SSSD] [sssd PR#511][comment] Do not shutdown KCM/Secrets responders when activities are happening ...

2018-03-29 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/511
Title: #511: Do not shutdown KCM/Secrets responders when activities are 
happening ...

simo5 commented:
"""
Ok moving to a different PR, definitely.
As for keeping a list, the best thing would be to not have explicit book 
keeping (as that's what you are fixing here), because when you write new code 
you invariable tend to forget about the book keeping and bugs creep in.
I was thinking you may be able to simply look at the event context and figure 
out if there are pending operations there. If that is not easy then having some 
central place that does the book keeping indeed.
"""

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


[SSSD] [sssd PR#511][comment] Do not shutdown KCM/Secrets responders when activities are happening ...

2018-03-29 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/511
Title: #511: Do not shutdown KCM/Secrets responders when activities are 
happening ...

simo5 commented:
"""
Sorry for late comment, but should't you simply have a list of "inflight" calls 
and take that in consideration before deciding to shut down ?
"""

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


[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

2018-03-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
No strong opinion beside bikeshedding on the name: ```domain/_defaults_``` :-)
"""

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


[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

2018-03-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
No strong opinion beside bikeshedding on the name: domain/_defaults_ :-)
"""

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


[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

2018-03-09 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
The default should be a global option, not per domain.
"""

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


[SSSD] [sssd PR#530][comment] GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

2018-03-09 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/530
Title: #530: GPO: Add "thinlinc" to ad_gpo_map_remote_interactive

simo5 commented:
"""
We may think of a mechanism to add a comment or a file somewhere we can read, 
so that software can distribute their GPO mappings in their RPMs/DEBs/etc.. 
instead of us having to patch SSSD all the time or admins having to list 
mapping on their own.
"""

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


[SSSD] [sssd PR#498][comment] DESKPROFILE: Do not require CAP_DAC_OVERRIDE

2018-02-15 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/498
Title: #498: DESKPROFILE: Do not require CAP_DAC_OVERRIDE

simo5 commented:
"""
@fidencio fidencio I do not care for unprivileged mode when it makes things way 
harder then they ought to be.
sssd can always make up a root account and escalate privileges anyway so 
running as a non privileged user is not high on my list of things to make work, 
especially because it simply can't for many tasks, making the software more 
complex (and thus buggier) for no good reason.
"""

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


[SSSD] [sssd PR#498][comment] DESKPROFILE: Do not require CAP_DAC_OVERRIDE

2018-02-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/498
Title: #498: DESKPROFILE: Do not require CAP_DAC_OVERRIDE

simo5 commented:
"""
I cannot set labels, but ACK
"""

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


[SSSD] [sssd PR#457][comment] ipa: Removal of umask(0) in selinux_child

2017-11-27 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/457
Title: #457: ipa: Removal of umask(0) in selinux_child

simo5 commented:
"""
You need to detect we have the correct libsemanage and ifdef those out. In a 
few years (when RHEL6 goes away) we can remove the code entirely.
Also add minimum required libsemanage version in sssd spec file in contrib if 
not there yet.
"""

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


[SSSD] [sssd PR#457][comment] ipa: Removal of umask(0) in selinux_child

2017-11-22 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/457
Title: #457: ipa: Removal of umask(0) in selinux_child

simo5 commented:
"""
Also if it works only with a specific version of a dependency we need a 
depndency check/enforcement.
"""

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


[SSSD] [sssd PR#457][comment] ipa: Removal of umask(0) in selinux_child

2017-11-22 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/457
Title: #457: ipa: Removal of umask(0) in selinux_child

simo5 commented:
"""
I see nothing in the commit message that explains why this is needed.
for a thing that changes a core security control I would really like a long and 
detailed explanation!
"""

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


[SSSD] [sssd PR#390][comment] NSS: Add option to disable memcache

2017-09-29 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/390
Title: #390: NSS: Add option to disable memcache

simo5 commented:
"""
@mzidek-rh yes I think in some cases we may want a much larger group memcache 
than user memcache
"""

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


[SSSD] [sssd PR#390][comment] NSS: Add option to disable memcache

2017-09-29 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/390
Title: #390: NSS: Add option to disable memcache

simo5 commented:
"""
@mzidek-rh yes I think in some cases we may want a much larger group memcache 
than user memcache
"""

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


[SSSD] [sssd PR#269][comment] Add support for ActiveDirectory's logonHours restrictions

2017-09-08 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/269
Title: #269: Add support for ActiveDirectory's logonHours restrictions

simo5 commented:
"""
Nobody in their right mind should implement the legacy thing there is in AD, so 
it wouldn't be a bad thing to keep it in the AD provider (it is *very* specific 
to AD :)
"""

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


[SSSD] [sssd PR#225][comment] SECRETS: Apply separate quotas for cn=secrets and cn=kcm

2017-06-07 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/225
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm

simo5 commented:
"""
Patches look good to me.

I have to say I think we could make the quoate system *really* generic by using 
dynamic configuration handles to pair the quota to a specific hive, but given 
the rest of the infrastructure is currently hardcoded on kcm and secrets hives 
the current solution is good enough.
"""

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


[SSSD] [sssd PR#225][comment] SECRETS: Apply separate quotas for cn=secrets and cn=kcm

2017-06-01 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/225
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm

simo5 commented:
"""
1) I do not think we need to add explicitly a quota subsection, but if I do not 
want to set a quota, what values should I use ? 0 ? Why the default is to have 
a quota ?

2) what is max_secrets? Given you suggest adding max_user_secrets I assume 
these are not the same thing ? I am confused.


"""

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


[SSSD] [sssd PR#225][comment] SECRETS: Apply separate quotas for cn=secrets and cn=kcm

2017-05-30 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/225
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm

simo5 commented:
"""
Yes we need to increase the size per secret, but this is why I am not sure 
having a maximum nu,be of secrets will scale well.
If you set the new max to 1MB, and have 256 secreets max then you give 256M of 
storage per user ... that seem a lot. On the other hand if you give too few 
secrets, then a user can run out o them pretty quickly. Think about a user 
using secrets to store their browser keyring, clearly in that case 256 is 
rather low.

Perhaps we should have different allowed size + # of secrets per hive.
Then the /kcm would have something like 16 secrets allowed but each can be 
several megabytes in size, while the regular user space would have a few 
thousand secrets (4k ?) allowed but each no more than a few kilobytes in size 
(say about 16k so that a user can store large symmetric RSA keys) ?

The other way to use just a size value and allow as many secrets as you want is 
to use a "pool of sectors" that can be used to store secrets. Each time you 
store a secret, the size is calculated and detracted and runded up to the 
"sector size" (say 4k), and that amount is removed from the total allowed size. 
If that number goes to zero we return an error. Then when a secret is deleted 
we have to free up sectors. This means we have to modify 2 records each time we 
write to the secrets storage, which may not be ideal.

There are ways around that though. For example we could write the number of 
"sectors" used (or even the size of the value) in the entry itself, and have an 
index on this attribute. Keep the size of the per/user data in a hash table in 
memory and keep track of quota there. If the daemon is restarted, the user 
quota is recomputed, the first time the user makes a write, by searching the 
database for all user's records and summing all records, then from there on 
keep the quota again in memory until the hash table is full (could be fixed 
size with FIFO) or the daemon is restarted.

I am sure we can think of other ways, the point is that we should be fair, and 
it is hard to privilege number of secrets versus size, because KCM vs passwords 
vs RSA keys have quite different per-secret sizes so it would be very difficult 
to find a balance if we have ust one quota and we count number of secrets.
"""

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


[SSSD] [sssd PR#225][comment] SECRETS: Apply separate quotas for cn=secrets and cn=kcm

2017-05-29 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/225
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm

simo5 commented:
"""
Interesting, why maximum number of secrets instead of maximum size ?

"""

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


[SSSD] [sssd PR#193][comment] UTIL: Use max 15 characters for AD host UPN

2017-04-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/193
Title: #193: UTIL: Use max 15 characters for AD host UPN

simo5 commented:
"""
For AD name$ should be the first you try, if there are more then one try them 
in series one after the other, but it is highly unlikely you have more than one.
Calling just the first one is also ok, as that's the noemral behavior when you 
have multiple keys in a keytab (unless you specify exactly what identity you 
want to kinit).
"""

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


[SSSD] [sssd PR#193][comment] UTIL: Use max 15 characters for AD host UPN

2017-04-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/193
Title: #193: UTIL: Use max 15 characters for AD host UPN

simo5 commented:
"""
Sorry to be Johhny come late, but I do not think you can simply truncate the 
hostname here.

This function is used to search a principal from the keytab that would work for 
Windows, I think at this point it would be best to enumrate the keys in the 
keytab and patternmatch on a single componenet principal that ends in a $ 
rather than trying to guess what key you might find in there, no?
"""

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


[SSSD] [sssd PR#231][comment] changing all talloc_get_type() with talloc_get_type_abort()

2017-04-10 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/231
Title: #231: changing all talloc_get_type() with talloc_get_type_abort()

simo5 commented:
"""
So I can't really comment inline because there are too many lines in the patch 
but I grepped throught the code:
I see:
+ void sss_talloc_abort(const char reason){

Well .. that should be a const char * to start with as you want to pass a 
pointer to string not a single character
Also the style is incorrect as the opening brace should be on the next line.

The content of the function also is not what the issue describes.
the issue describes the option to either abort() or print a debug error based 
on configuration (or perhaps build flags). Instead this code unconditionally 
prints a debug error (this  may be ok actually) and then unconditionally calls 
abort().


Finally this patchset is going to be big as it touches thousands of file in a 
mostly mecahnical way.
To improve reviewability it wuld be nice to split this PR in 2/3 patches:
1. Introduce the new sss_talloc_abort() function in the first patch with a good 
commit message that explains what this will bring us.
2. a separate patch that enables it everywhere
3. a separate patch that changes the invocations ofa talloc_get_type() to 
talloc_get_type_abort()

HTH.
"""

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


[SSSD] [sssd PR#231][comment] changing all talloc_get_type() with talloc_get_type_abort()

2017-04-10 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/231
Title: #231: changing all talloc_get_type() with talloc_get_type_abort()

simo5 commented:
"""
Please also do not commit as root, but use a proper user/email setting, and if 
you can add the signed-off-by header.
"""

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


[SSSD] [sssd PR#225][comment] SECRETS: Apply separate quotas for cn=secrets and cn=kcm

2017-04-04 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/225
Title: #225: SECRETS: Apply separate quotas for cn=secrets and cn=kcm

simo5 commented:
"""
We should probbaly have quotas per UID, or one user can DoS all others 
including root and services
"""

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


[SSSD] [sssd PR#197][comment] KCM responder

2017-03-24 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/197
Title: #197: KCM responder

simo5 commented:
"""
All looks good to go.
"""

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


[SSSD] [sssd PR#144][comment] SSSD does not start if using only the local provider and services line is empty

2017-02-09 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/144
Title: #144: SSSD does not start if using only the local provider and services 
line is empty

simo5 commented:
"""
FWIW, I do not see any risk in pushing this patch as it is, and adjusting the 
code in the files PR if it changes anything.
"""

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


[SSSD] [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

2016-12-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/107
Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler

simo5 commented:
"""
On Tue, 2016-12-13 at 08:02 -0800, Jakub Hrozek wrote:
> On Tue, Dec 13, 2016 at 07:06:58AM -0800, Simo Sorce wrote:
> > On Tue, 2016-12-13 at 05:59 -0800, Jakub Hrozek wrote:
> > > On Tue, Dec 13, 2016 at 02:44:44AM -0800, Simo Sorce wrote:
> > > > On Tue, 2016-12-13 at 02:25 -0800, fidencio wrote:
> > > > > Pavel,
> > > > > 
> > > > > On Tue, Dec 13, 2016 at 11:20 AM, Pavel Březina 
> > > > > <notificati...@github.com>
> > > > > wrote:
> > > > > 
> > > > > > There are two scenarios:
> > > > > >
> > > > > >1. timeshift during system boot -- it is very common to be 
> > > > > > several
> > > > > >hours
> > > > > >2. timeshift due to an ntp update when booted up -- usually only 
> > > > > > few
> > > > > >seconds, not a big deal
> > > > > >
> > > > > > The problem with tevent timer is that if we shift backwards the 
> > > > > > timer
> > > > > > remains too far in the future. This applies to all timers, not only 
> > > > > > for the
> > > > > > watchdog. Forward shift is not a problem, it just executes the 
> > > > > > timers
> > > > > > immediately. Resetting the watchdog helps in a way that sssd is not 
> > > > > > killed,
> > > > > > we don't have any capability to reschedule all timed event and we 
> > > > > > actually
> > > > > > can not tell that sssd will be functioning properly (dyndns, sudo 
> > > > > > refresh,
> > > > > > enumeration, domain refresh, even idle timer on socket 
> > > > > > activation)... all
> > > > > > those operations that depends on time() would become unreliable.
> > > > > >
> > > > > > I think the best thing to do would be restart the process (although 
> > > > > > the
> > > > > > question is how would this affect the boot up) and patch tevent to 
> > > > > > deal
> > > > > > with timeshift either by using monotonic clock or by detecting them 
> > > > > > and
> > > > > > altering timers accordingly.
> > > > > >
> > > > > 
> > > > > In the latest version of patch I've just called _exit(1) when the 
> > > > > timeshift
> > > > > is detected.
> > > > > About patching tevent, I've seen some old discussions happening and it
> > > > > doesn't seem a trivial thing to do. Would the patch, as it is right 
> > > > > now, be
> > > > > acceptable and then a work on tevent could be done later (yes, I'd 
> > > > > add it
> > > > > to my queue and do it as soon as we have an agreement on doing this)?
> > > > 
> > > > This is really a blunt tool (calling exit()), but until tevent can be
> > > > fixed the only other option would be to use some wrapper to keep track
> > > > of all existing timed events and cancel and restart them all if the
> > > > clock changes abruptly.
> > > 
> > > that's why I suggested signaling self to a tevent-driven signal handler
> > > from where we can just set up the timer anew. 
> > > 
> > > If there is any other way to 'break out' of the POSIX signal handler
> > > into somewhere where we can call tevent/talloc (or in general unsafe
> > > calls) I'm all ears.
> > 
> > I guess I need to understand better what exactly you want to do to be
> > able to advice on something. I can think of a coulpe of options, none of
> > them particularly elegant :)
> 
> OK, let me try to explain better.
> 
> A machine drifts time. Then an SSSD process receives SIGRT in
> watchdog_handler() and detects the time has drifted, so it avoids
> increasing the watchdog ticks counter -- this is done in
> watchdog_detect_timeshift() at the moment.
> 
> At that point, in the current master, we call teardown_watchdog() and
> setup_watchdog() to set a new watchdog (the part that is based on tevent
> timers). This is unsafe to do in a signal handler because it involves
> malloc and free among others called from tevent.
> 
> What I'm trying to figure out is how to reset the watchdog when I detect
> in watchdog_detect_timeshift() the time is out of sync and the tevent
> timer that resets the ticks will not arrive until the sssd process
> receives enough SIGRT signals to get itself killed.
> 
> Does the question make sense now?

Yes,
I see a few ways, a file descriptor (like tevent does for handling
signals) used to fire a tevent_fd event that will perform these actions.
Or a global variable, that is checked in the main loop at idle times and
resets the watchdog, finally a mutex on which a dedicated thread waits,
the thread only job is to run the reset if the mutex is released (but
then the mutex needs to be re-acquired, probably on the next tick), but
then again not sure if the code run in the setup_watchdog() is thread
safe.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


"""

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


[SSSD] [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

2016-12-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/107
Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler

simo5 commented:
"""
On Tue, 2016-12-13 at 05:59 -0800, Jakub Hrozek wrote:
> On Tue, Dec 13, 2016 at 02:44:44AM -0800, Simo Sorce wrote:
> > On Tue, 2016-12-13 at 02:25 -0800, fidencio wrote:
> > > Pavel,
> > > 
> > > On Tue, Dec 13, 2016 at 11:20 AM, Pavel Březina <notificati...@github.com>
> > > wrote:
> > > 
> > > > There are two scenarios:
> > > >
> > > >1. timeshift during system boot -- it is very common to be several
> > > >hours
> > > >2. timeshift due to an ntp update when booted up -- usually only few
> > > >seconds, not a big deal
> > > >
> > > > The problem with tevent timer is that if we shift backwards the timer
> > > > remains too far in the future. This applies to all timers, not only for 
> > > > the
> > > > watchdog. Forward shift is not a problem, it just executes the timers
> > > > immediately. Resetting the watchdog helps in a way that sssd is not 
> > > > killed,
> > > > we don't have any capability to reschedule all timed event and we 
> > > > actually
> > > > can not tell that sssd will be functioning properly (dyndns, sudo 
> > > > refresh,
> > > > enumeration, domain refresh, even idle timer on socket activation)... 
> > > > all
> > > > those operations that depends on time() would become unreliable.
> > > >
> > > > I think the best thing to do would be restart the process (although the
> > > > question is how would this affect the boot up) and patch tevent to deal
> > > > with timeshift either by using monotonic clock or by detecting them and
> > > > altering timers accordingly.
> > > >
> > > 
> > > In the latest version of patch I've just called _exit(1) when the 
> > > timeshift
> > > is detected.
> > > About patching tevent, I've seen some old discussions happening and it
> > > doesn't seem a trivial thing to do. Would the patch, as it is right now, 
> > > be
> > > acceptable and then a work on tevent could be done later (yes, I'd add it
> > > to my queue and do it as soon as we have an agreement on doing this)?
> > 
> > This is really a blunt tool (calling exit()), but until tevent can be
> > fixed the only other option would be to use some wrapper to keep track
> > of all existing timed events and cancel and restart them all if the
> > clock changes abruptly.
> 
> that's why I suggested signaling self to a tevent-driven signal handler
> from where we can just set up the timer anew. 
> 
> If there is any other way to 'break out' of the POSIX signal handler
> into somewhere where we can call tevent/talloc (or in general unsafe
> calls) I'm all ears.

I guess I need to understand better what exactly you want to do to be
able to advice on something. I can think of a coulpe of options, none of
them particularly elegant :)

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


"""

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


[SSSD] [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

2016-12-13 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/107
Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler

simo5 commented:
"""
On Tue, 2016-12-13 at 02:25 -0800, fidencio wrote:
> Pavel,
> 
> On Tue, Dec 13, 2016 at 11:20 AM, Pavel Březina <notificati...@github.com>
> wrote:
> 
> > There are two scenarios:
> >
> >1. timeshift during system boot -- it is very common to be several
> >hours
> >2. timeshift due to an ntp update when booted up -- usually only few
> >seconds, not a big deal
> >
> > The problem with tevent timer is that if we shift backwards the timer
> > remains too far in the future. This applies to all timers, not only for the
> > watchdog. Forward shift is not a problem, it just executes the timers
> > immediately. Resetting the watchdog helps in a way that sssd is not killed,
> > we don't have any capability to reschedule all timed event and we actually
> > can not tell that sssd will be functioning properly (dyndns, sudo refresh,
> > enumeration, domain refresh, even idle timer on socket activation)... all
> > those operations that depends on time() would become unreliable.
> >
> > I think the best thing to do would be restart the process (although the
> > question is how would this affect the boot up) and patch tevent to deal
> > with timeshift either by using monotonic clock or by detecting them and
> > altering timers accordingly.
> >
> 
> In the latest version of patch I've just called _exit(1) when the timeshift
> is detected.
> About patching tevent, I've seen some old discussions happening and it
> doesn't seem a trivial thing to do. Would the patch, as it is right now, be
> acceptable and then a work on tevent could be done later (yes, I'd add it
> to my queue and do it as soon as we have an agreement on doing this)?

This is really a blunt tool (calling exit()), but until tevent can be
fixed the only other option would be to use some wrapper to keep track
of all existing timed events and cancel and restart them all if the
clock changes abruptly.
The easiest option is to teach tevent about the existence of monotonic
clocks and allow to set a default clock type to use as well as overrides
for specific timers that want to fire on the actual time not just X
seconds in the future.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York


"""

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


[SSSD] [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

2016-12-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/107
Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler

simo5 commented:
"""
well you could have a globalk variable for the watchdog and change it from a 
custom signal handler, but the point of the watchdog is to go thorugh the 
tevent handler instead so that we are sure the machinery is working and not 
stuck somwhere.
Resetting directly from the singal handler would bypass all processing and 
therefore render the watchdog useless I guess.
"""

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


[SSSD] [sssd PR#107][comment] WATCHDOG: Avoid non async-signal-safe from the signal_handler

2016-12-12 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/107
Title: #107: WATCHDOG: Avoid non async-signal-safe from the signal_handler

simo5 commented:
"""
Yes we should ask, I think we really need to try to use monotonic clocks for 
most tasks.
"""

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


[SSSD] [sssd PR#32][comment] Requesting a pull to SSSD:master from fidencio:wip/#3138

2016-09-28 Thread simo5
  URL: https://github.com/SSSD/sssd/pull/32
Title: #32: Requesting a pull to SSSD:master from fidencio:wip/#3138

simo5 commented:
"""
The problem of using genconf is race conditions and the fact it won't be 
possible to call it again after the service is started.
I am for adding socket activation to the monitor and have the moinitor only 
regen the config (and exit ?) when socket activated.
"""

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