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 <notificati...@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/AAG4esxEc1sdyvB65jm3C1y3pdo8ojI7ks5rHTKbgaJpZM4K8AJs> > . > 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
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
