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/pu
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
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
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
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://
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.
>
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 coul
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/pu
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 li
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
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
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 t
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 simultaneou
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
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
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 r
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 r
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.
Alth
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.
Alsth
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.
Alsth
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
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
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
a
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
_
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
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
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/S
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/S
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://gi
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 overr
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 so
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 n
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
sou
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
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 i
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 pa
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
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
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
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 consideratio
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://gi
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/S
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/S
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
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 esca
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
__
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 m
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:/
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
detai
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/S
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/S
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 provid
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
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 def
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
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/p
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 on
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
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_tal
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.
"""
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
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-de
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 anythin
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, 2
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, 2
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
> wrote:
>
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
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
htt
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 addi
69 matches
Mail list logo
