Could you use SSS_DFL_UMASK instead here?

Could you use fchown here on the fd instead of chown to avoid e.g. Coverity complaining about TOCTOU issues?

Thank you for the patches. I only have two minor issues that I pointed out inline.

Sure, fixed

I had a fchown here but it did not work for me on f27.
It returned 0 but owner was not changed.

Would you mind to try? Maybe I dis some mistake on my side.

BTW feel free to squash following oneliner if it works for you

diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c
index dd524bd0a..9c0163ca6 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -231,7 +231,7 @@ int main(int argc, const char *argv[])
             return 4;
         }
-        ret = chown(SSS_SUDO_SOCKET_NAME, uid, 0);
+        ret = fchown(pipe_fd, uid, 0);
         if (ret != EOK) {
             close(pipe_fd);
             DEBUG(SSSDBG_FATAL_FAILURE,

IMHO TOCTOU is irrelevant here because socket was already created with strict permissions due to umast 0117 and we are just "downgrading" permissions with chown()

Hmm, it doesn't work for me either. And I don't see anything in the manual page that would tell me why. The only strange thing in strace is that the fd number is 0, so I wonder if libc has some optimization for the standard fds in fchown..

But if you agree, I would like to squash this change, because on failure, ret is always -1, but the error code is in errno.

diff --git a/src/responder/sudo/sudosrv.c b/src/responder/sudo/sudosrv.c
index dd524bd0a..6cb5347e7 100644
--- a/src/responder/sudo/sudosrv.c
+++ b/src/responder/sudo/sudosrv.c
@@ -232,8 +232,9 @@ int main(int argc, const char *argv[])
         }
         ret = chown(SSS_SUDO_SOCKET_NAME, uid, 0);
-        if (ret != EOK) {
+        if (ret != 0) {
+            ret = errno;
             close(pipe_fd);
             DEBUG(SSSDBG_FATAL_FAILURE,
                   "create_pipe_fd failed [%d]: %s.\n",
                   ret, sss_strerror(ret));

Hmm, it doesn't work for me either. And I don't see anything in the manual page that would tell me why. The only strange thing in strace is that the fd number is 0, so I wonder if libc has some optimization for the standard fds in fchown..
But if you agree, I would like to squash this change, because on failure, ret is always -1, but the error code is in errno.

Missing errno was a bug. Thank you for catching it. I fixed that.

And if you want to replace EOK with 0 feel free to do that.

-        if (ret != EOK) {
+        if (ret != 0) {

Commit ececbf9c fixes this pull-request

Commit 21ea8204 fixes this pull-request

Commit 4900b8e5 fixes this pull-request

• master:
• 21ea8204a0bd8ea4451f420713e909d3cfee34ef
• 4900b8e59bdbb89fbc1c9718969aabe26f3db34a