On Thu, Sep 10, 2015 at 12:27:17PM +0200, Petr Cech wrote:
> Hi,
>
> I am reviewing umask() in our code according to
>
https://fedorahosted.org/sssd/ticket/2424
>
> There are many use like umask(DFL_RSP_UMASK):
> src/responder/autofs/autofssrv.c:223
> src/responder/ifp/ifpsrv.c:401
> src/responder/nss/nsssrv.c:589
> src/responder/pac/pacsrv.c:232
> src/responder/pam/pamsrv.c:369
> src/responder/ssh/sshsrv.c:209
> src/responder/sudo/sudosrv.c:215
> where DFL_RSP_UMASK is defined as 0177.
>
> There are another three use of umask 0177:
> src/confdb/confdb.c:662
> src/util/debug.c:365
> src/util/server.c:495
>
> And then I see many use of umask 077:
> src/p11_child/p11_child_nss.c:485
> src/providers/krb5/krb5_child.c:723
> src/tests/check_and_open-tests.c:51
> src/tests/debug-tests.c:136
> src/tests/debug-tests.c:276
> src/tests/util-tests.c:596
> src/util/domain_info_utils.c:312
> src/util/domain_info_utils.c:562
> src/tools/tools_util.c:503
>
> I would like to ask you if we would like to use 0077 or 0177 as our very
> restrictive mask. I see that our code is not consistent on this question. I
> know the difference is small, but it is.
I guess 0177 should be used.
>
> Then we have some unsecure use:
> src/providers/ipa/selinux_child.c:154: umask = 0
> src/providers/krb5/krb5_ccache.c:188: umask = 0000
> src/responder/nss/nsssrv_mmap_cache.c:1121: umask = 0022
> but I think there is reason for it.
Yes, it would be nice if there was always a comment explaining the
umask.
>
> And the last one is at src/responder/common/responder_common.c:561:
> int create_pipe_fd(const char *sock_name, int *_fd, mode_t umaskval)
> We use it secure (0177) at:
> src/responder/common/responder_common.c:693
> src/responder/pam/pamsrv.c:399
If this is in responder, would it make sense to just use DFL_RSP_UMASK ?
>
> And not so secure:
> src/responder/common/responder_common.c:670 umask = 0111
This one has a comment explaining why the umask it is the way it is, but
would it make sense to add a note about public/private sockets as well
(maybe not to the code but to the InternalsDocs) and #define a constant
for the public pipes?
> src/responder/pam/pamsrv.c:391 umask = 0111
> src/tests/cwrap/test_responder_common.c:173 umask = 0111
> src/tests/cwrap/test_responder_common.c:179 umask = 0000
>
> So, what could I do? Maybe we could have only one very secure umask and
> maybe we could have CONSTANT for every use of umask. Any another ideas?
I like this idea, the constant could describe why we need this
particular umask better than the number also.
>
> Regards
>
> Petr
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Thanks, Jakub, for comments. There is a patch attached.
Petr