[SSSD] Review of umask() in SSSD

Petr Cech pcech at redhat.com
Tue Sep 15 14:55:09 UTC 2015


On 09/11/2015 01:47 PM, Jakub Hrozek wrote:
> 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 at lists.fedorahosted.org
>> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>
Thanks, Jakub, for comments. There is a patch attached.
Petr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-REFACTOR-Review-of-umask-function.patch
Type: text/x-patch
Size: 8798 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150915/426809b4/attachment.bin>


More information about the sssd-devel mailing list