>From 1da3d15cf5cfcd72742cb05be9a144ab40db7d29 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 3a8a1c01b92e62302ac4f787ccd085be9d8f05c3..d71a50724d292bbea7d49e650062e11066c6ff77 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_VERY_RESTRICTIVE_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..d999e7485f5f67792502400084dbd603f1558a8d 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_VERY_RESTRICTIVE_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..6f9bad4e4caad3ca6f6c277397669ca800a6e64e 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_VERY_RESTRICTIVE_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 36e7f15948632e9c637886dee259b494e46ceecb..1dc9d8ce757d80865e7b21d75fa79d0621b924ec 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..c66579214f9cd8c77efb294fcfb086ca19fe98c2 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_VERY_RESTRICTIVE_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..1e221dfd23dcb49e07b91fa0969b3302b5c21477 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_VERY_RESTRICTIVE_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_VERY_RESTRICTIVE_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..523b282f39ccc8645e8eab18ef238202d59395d4 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_VERY_RESTRICTIVE_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..479aed0cab96c708c10920d553de362085ebe2a6 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_VERY_RESTRICTIVE_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 4eabcff7a0e0af342ec3833d24da26ede0cb5148..f031950e254ce2b9bf25fc901ede85c0ba41d211 100644
--- a/src/util/domain_info_utils.c
+++ b/src/util/domain_info_utils.c
@@ -309,7 +309,7 @@ sss_write_domain_mappings(struct sss_domain_info *domain)
         goto done;
     }
 
-    old_mode = umask(077);
+    old_mode = umask(SSS_VERY_RESTRICTIVE_UMASK);
     fd = mkstemp(tmp_file);
     umask(old_mode);
     if (fd < 0) {
@@ -559,7 +559,7 @@ static errno_t sss_write_krb5_localauth_snippet(const char *path)
         goto done;
     }
 
-    old_mode = umask(077);
+    old_mode = umask(SSS_VERY_RESTRICTIVE_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..f0826d99458a01328547a7dd9c1dab6fe840356a 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_VERY_RESTRICTIVE_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 3e29e748768fc2cac547d16d61b449f1b555a56f..25bcda06eea9b2f6ee6704dec6500bd1eb7e0fcc 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -64,6 +64,8 @@
 #define SSS_ATTRIBUTE_PRINTF(a1, a2)
 #endif
 
+#define SSS_VERY_RESTRICTIVE_UMASK 0177
+
 extern const char *debug_prg_name;
 extern int debug_level;
 extern int debug_timestamps;
-- 
2.4.3

