URL: https://github.com/SSSD/sssd/pull/5407
Title: #5407: kcm: check socket path loaded from configuration

ikerexxe commented:
"""
> I have several comments to the code. However, I'm sorry, but I don't think 
> this is correct way to solve it. This is not a problem of KCM only, but it 
> affects all responders because all sockets are activated in 
> `activate_unix_sockets()` so it should be also solved here. Obviously, if the 
> socket path is invalid then `bind()` should fail, so the question is why it 
> succeeds? I don't know, my guess is that systemd socket activation is somehow 
> affecting it - maybe because it already created a socket for us and the 
> socket name is defined in kcm.socket unit file instead of sssd.conf so the 
> one in sssd.conf is ignored? Probably, because if socket is already created 
> by systemd, SSSD just skips its creation: (from `set_unix_socket()`:

I think I'm missing something from your point. IIUC, sssd-kcm loads the socket 
path from sssd.conf, whilst kcm.socket unit file loads it from its own unit 
file configuration. If sssd.conf and unit file path are the same, then both 
processes should fail because the path isn't valid. Otherwise, only sssd-kcm 
should fail because sssd.conf socket path is invalid. As said, I must be 
missing something because either sssd-kcm or kcm.socket unit file should fail.

> 
> ```c
>         if (rctx->lfd == -1) {
>             ret = create_pipe_fd(rctx->sock_name, &rctx->lfd, SCKT_RSP_UMASK);
>             if (ret != EOK) {
>                 return ret;
>             }
>         }
> ```
> 
> So KCM always binds to whatever is set in kcm.socket. So the question is - 
> should we take kcm.socket unit file for granted (and print error if systemd 
> socket != sssd.conf socket) or should we open a new socket in this case? 
> Probably the first thing, otherwise socket activation will stop working.

I don't think opening a new socket is a solution, so I'd prefer to print an 
error if paths are different.

And now, going back to the first part of your comment.

> I have several comments to the code. However, I'm sorry, but I don't think 
> this is correct way to solve it. This is not a problem of KCM only, but it 
> affects all responders because all sockets are activated in 
> `activate_unix_sockets()` so it should be also solved here.

If we choose to print an error, should we implement some algorithm that checks 
if the responder can be activated by systemd and check the path provided in the 
unit file against the one provided in sssd.conf?


"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5407#issuecomment-743277752
_______________________________________________
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

Reply via email to