>From 0f1946aecec78e7faaa3f5815ad06969b1234389 Mon Sep 17 00:00:00 2001
From: Petr Cech <pcech@redhat.com>
Date: Tue, 15 Sep 2015 10:50:37 -0400
Subject: [PATCH] REFACTOR: Review of umask() function

We have many uses of umask() in our code. This patch substitute
values with constants and add comments at some cases.

Resolves:
https://fedorahosted.org/sssd/ticket/2424
---
 src/confdb/confdb.c                     | 2 +-
 src/p11_child/p11_child_nss.c           | 2 +-
 src/providers/krb5/krb5_ccache.c        | 1 +
 src/providers/krb5/krb5_child.c         | 2 +-
 src/responder/common/responder_common.c | 3 ++-
 src/responder/pam/pamsrv.c              | 3 ++-
 src/tests/check_and_open-tests.c        | 2 +-
 src/tests/debug-tests.c                 | 4 ++--
 src/tests/util-tests.c                  | 2 +-
 src/util/debug.c                        | 2 +-
 src/util/domain_info_utils.c            | 4 ++--
 src/util/server.c                       | 2 +-
 src/util/util.h                         | 2 ++
 13 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/confdb/confdb.c b/src/confdb/confdb.c
index d811f7cbf597db5c5ee5fa658c8864233da8f2e0..0f76a3d140ec832467c8382df088ac0e279207c0 100644
--- a/src/confdb/confdb.c
+++ b/src/confdb/confdb.c
@@ -659,7 +659,7 @@ int confdb_init(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
-    old_umask = umask(0177);
+    old_umask = umask(SSS_DFL_UMASK);
 
     ret = ldb_connect(cdb->ldb, confdb_location, 0, NULL);
     umask(old_umask);
diff --git a/src/p11_child/p11_child_nss.c b/src/p11_child/p11_child_nss.c
index 44ba6678893408dbfc0c6c7cfd5edcdaa789f518..87bc376bcd2add74388504ba7e591592d2a818c7 100644
--- a/src/p11_child/p11_child_nss.c
+++ b/src/p11_child/p11_child_nss.c
@@ -482,7 +482,7 @@ int main(int argc, const char *argv[])
     debug_level = SSSDBG_INVALID;
 
     clearenv();
-    umask(077);
+    umask(SSS_DFL_UMASK);
 
     pc = poptGetContext(argv[0], argc, argv, long_options, 0);
     while ((opt = poptGetNextOpt(pc)) != -1) {
diff --git a/src/providers/krb5/krb5_ccache.c b/src/providers/krb5/krb5_ccache.c
index f9bb25efd4ca3257845c3b157667d21d24299f4a..5de596f341a53958f312d114c1f95c4728d9d5df 100644
--- a/src/providers/krb5/krb5_ccache.c
+++ b/src/providers/krb5/krb5_ccache.c
@@ -185,6 +185,7 @@ static errno_t create_ccache_dir(const char *ccdirname, uid_t uid, gid_t gid)
               "Creating directory [%s].\n", li->s);
         new_dir_mode = 0700;
 
+        /* We need umask 0000 because we will create directory. */
         old_umask = umask(0000);
         ret = mkdir(li->s, new_dir_mode);
         umask(old_umask);
diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 1edf10ab81d283c45e9c3343341ceaa524970e11..be8db23df4660adcb59fcd2677b28ee415cd18d8 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -720,7 +720,7 @@ static krb5_error_code create_ccache(char *ccname, krb5_creds *creds)
 #endif
 
     /* Set a restrictive umask, just in case we end up creating any file */
-    umask(077);
+    umask(SSS_DFL_UMASK);
 
     /* we create a new context here as the main process one may have been
      * opened as root and contain possibly references (even open handles ?)
diff --git a/src/responder/common/responder_common.c b/src/responder/common/responder_common.c
index 2097004cb0fc24d8b356f9d924243f948227ef58..baaf0412b4a70537a2523a98ff33d8f34f194b47 100644
--- a/src/responder/common/responder_common.c
+++ b/src/responder/common/responder_common.c
@@ -690,7 +690,8 @@ static int set_unix_socket(struct resp_ctx *rctx)
     if (rctx->priv_sock_name != NULL ) {
         /* create privileged pipe */
         if (rctx->priv_lfd == -1) {
-            ret = create_pipe_fd(rctx->priv_sock_name, &rctx->priv_lfd, 0177);
+            ret = create_pipe_fd(rctx->priv_sock_name, &rctx->priv_lfd,
+                                 DFL_RSP_UMASK);
             if (ret != EOK) {
                 goto failed;
             }
diff --git a/src/responder/pam/pamsrv.c b/src/responder/pam/pamsrv.c
index 3fe467c3cfc4c63b9c261065a17a54c20ea4a546..6ac770b7ac80676824cd572444359b96279902f7 100644
--- a/src/responder/pam/pamsrv.c
+++ b/src/responder/pam/pamsrv.c
@@ -396,7 +396,8 @@ int main(int argc, const char *argv[])
         return 2;
     }
 
-    ret = create_pipe_fd(SSS_PAM_PRIV_SOCKET_NAME, &priv_pipe_fd, 0177);
+    ret = create_pipe_fd(SSS_PAM_PRIV_SOCKET_NAME, &priv_pipe_fd,
+                         DFL_RSP_UMASK);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE,
               "create_pipe_fd failed (priviledged pipe) [%d]: %s.\n",
diff --git a/src/tests/check_and_open-tests.c b/src/tests/check_and_open-tests.c
index e5981c8580f1a03afab252c20990230d9f13469b..525e28aedd9ad02fffdebdb95cff9df153642ddb 100644
--- a/src/tests/check_and_open-tests.c
+++ b/src/tests/check_and_open-tests.c
@@ -48,7 +48,7 @@ void setup_check_and_open(void)
     filename = strdup(FILENAME_TEMPLATE);
     fail_unless(filename != NULL, "strdup failed");
 
-    old_umask = umask(077);
+    old_umask = umask(SSS_DFL_UMASK);
     ret = mkstemp(filename);
     umask(old_umask);
     fail_unless(ret != -1, "mkstemp failed [%d][%s]", errno, strerror(errno));
diff --git a/src/tests/debug-tests.c b/src/tests/debug-tests.c
index 067209b1dda1c445b971bcec7108d0b886d55e53..d904d7eb8b5418608023faca0d62067f3106d23b 100644
--- a/src/tests/debug-tests.c
+++ b/src/tests/debug-tests.c
@@ -133,7 +133,7 @@ int test_helper_debug_check_message(int level)
 
     strncpy(filename, "sssd_debug_tests.XXXXXX", 24);
 
-    old_umask = umask(077);
+    old_umask = umask(SSS_DFL_UMASK);
     fd = mkstemp(filename);
     umask(old_umask);
     if (fd == -1) {
@@ -273,7 +273,7 @@ int test_helper_debug_is_empty_message(int level)
 
     strncpy(filename, "sssd_debug_tests.XXXXXX", 24);
 
-    old_umask = umask(077);
+    old_umask = umask(SSS_DFL_UMASK);
     fd = mkstemp(filename);
     umask(old_umask);
     if (fd == -1) {
diff --git a/src/tests/util-tests.c b/src/tests/util-tests.c
index bfdf078027250b8ff0ce0da2d37fbb20f391d06b..df4cf969fb48c4834b5a84aa1700fc2ea4eb74b1 100644
--- a/src/tests/util-tests.c
+++ b/src/tests/util-tests.c
@@ -593,7 +593,7 @@ void setup_atomicio(void)
     fail_unless(filename != NULL, "strdup failed");
 
     atio_fd = -1;
-    old_umask = umask(077);
+    old_umask = umask(SSS_DFL_UMASK);
     ret = mkstemp(filename);
     umask(old_umask);
     fail_unless(ret != -1, "mkstemp failed [%d][%s]", errno, strerror(errno));
diff --git a/src/util/debug.c b/src/util/debug.c
index 69df54386101973548108c3194a1bfd111f046f0..bd13fdecdbd37da8e13ed492c115570657d2588c 100644
--- a/src/util/debug.c
+++ b/src/util/debug.c
@@ -362,7 +362,7 @@ int open_debug_file_ex(const char *filename, FILE **filep, bool want_cloexec)
 
     if (debug_file && !filep) fclose(debug_file);
 
-    old_umask = umask(0177);
+    old_umask = umask(SSS_DFL_UMASK);
     errno = 0;
     f = fopen(logpath, "a");
     if (f == NULL) {
diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c
index ffbb9475b27a45c07e2e0936464c6e68ed682052..baed132ce72824138be5d1dae4f9d815e927be24 100644
--- a/src/util/domain_info_utils.c
+++ b/src/util/domain_info_utils.c
@@ -312,7 +312,7 @@ sss_write_domain_mappings(struct sss_domain_info *domain)
         goto done;
     }
 
-    old_mode = umask(077);
+    old_mode = umask(SSS_DFL_UMASK);
     fd = mkstemp(tmp_file);
     umask(old_mode);
     if (fd < 0) {
@@ -562,7 +562,7 @@ static errno_t sss_write_krb5_localauth_snippet(const char *path)
         goto done;
     }
 
-    old_mode = umask(077);
+    old_mode = umask(SSS_DFL_UMASK);
     fd = mkstemp(tmp_file);
     umask(old_mode);
     if (fd < 0) {
diff --git a/src/util/server.c b/src/util/server.c
index 7e9b76f74ee5e76d2481eb425eff4811cc2e780e..bb90c8464ce42a8c0414e6eed288339311b6a693 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -492,7 +492,7 @@ int server_setup(const char *name, int flags,
 
     /* we want default permissions on created files to be very strict,
        so set our umask to 0177 */
-    umask(0177);
+    umask(SSS_DFL_UMASK);
 
     if (flags & FLAGS_DAEMON) {
         DEBUG(SSSDBG_IMPORTANT_INFO, "Becoming a daemon.\n");
diff --git a/src/util/util.h b/src/util/util.h
index f9fe1ca7189c6b2cdcb29f143005b20a2d969fee..63bb8dcd82a27cfa6dc6e43be9e4cca91e173fa4 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -64,6 +64,8 @@
 #define SSS_ATTRIBUTE_PRINTF(a1, a2)
 #endif
 
+#define SSS_DFL_UMASK 0177
+
 extern const char *debug_prg_name;
 extern int debug_level;
 extern int debug_timestamps;
-- 
2.4.3

