URL:
https://github.com/SSSD/sssd/pull/94
Title: #94: Enable {socket,dbus}-activation for responders
fidencio commented:
"""
On Mon, Dec 12, 2016 at 12:40 PM, Jakub Hrozek <notifications(a)github.com>
wrote:
Hi,
I read the patches, so far I didn't do a whole lot of testing, so maybe
some questions here are invalid, but nonetheless, here is my take on the
patchset:
1.
currently the patches don't work well if sssd is running with user=root
because then the confdb is owned by root and the responders, which always
start as the sssd user cannot read it. I think we could solve this
particular problem by owning the confdb as sssd/sssd from the start -- I
don't think there would be any information leak because the confdb is then
only readable to sssd and root and the sssd user's shell is hopefully
/sbin/nologin. However, just the presence of the sssd user is
currently conditional (--with-sssd-user still defaults to root). I
wonder if it was the easiest to conditionalize the unit and socket files
and expand the sssd user instead of hardcoding sssd there?
Hmmm. Yes, I guess the best thing to do here is to expand --with-sssd-user
parameter in the unit files.
1.
There seems to be some issue with PAM socket ownership because I can't
authenticate with socket-activated PAM responder:
Dec 12 11:36:11 client.ipa.test su[29517]: pam_sss(su-l:auth): Request
to sssd failed. Public socket has wrong ownership or permissions.
Most likely caused by the 1.
1.
Would it make any sense to own the sockets by the sssd user as well?
Currently it seems the sockets are owned by the sssd user for services
started by monitor if they are running un-privileged but as root for
systemd-activated services. Because the
Hmm. Probably it does, but I'd like to hear other developers' opinion here
as well.
And seems that we missed some part of your explanation.
1.
There were some SELinux denials on my test VM, but granted, I run F-24
there. We need to make sure that no SELinux AVC denials are present in
Fedora later.
Lukáš replied this one, thanks!
1.
In the upstream reference specfile, should we use the systemd macros
as we do for services so that all sockets are enabled and started?
I'll check, but most-likely, yes!
1.
The documentation should be improved a bit. At least we should say
that the services line is only optional on systemd platforms. I think we
should also add an example that lists how to disable a socket if the admin
wants to totally disable some service.
Well, my patch in the manual explicitly mentions that the services line is
optional and the sentence is added only on systemd platforms.
For sure I can expand it.
1.
I'm wondering if the restart on-failure is correct. I think it is, but
I would like to discuss it a bit. Currently the restart is done on any
return code other than 0. The only issue I see is that the restart is
retried several times if the service fails to start up, but in general I
think it's much safer than trying to be too smart and only restart with
on-abnormal. Do you agree?
Had the same thought when adding it. I'd prefer to keep it as it is instead
of trying to be smart (and failing on that).
That's all for now, I will continue reading the patches and testing them
later.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<
https://github.com/SSSD/sssd/pull/94#issuecomment-266407528>, or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAG4esxEc1sdyvB65jm3C1y...
.
I'll go through the other comments and prepare a new version of the series.
Thanks for the review!
"""
See the full comment at
https://github.com/SSSD/sssd/pull/94#issuecomment-266428998