On 10/01/2015 01:19 PM, Jakub Hrozek wrote:
On Thu, Oct 01, 2015 at 12:38:49PM +0200, Petr Cech wrote:
Bump.
Thanks for reply, Jakub.
Why was 077 changed for 0177?
This change is something, which I think was discussed earlier in this
thread.
# pcech:
# > 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.
#
# jhrozek:
# I guess 0177 should be used.
I think that we work only with files, not with directories, I should
check it again.
So, if it is risky, I will changed it. :-)
About the name -- shouldn't we say just "SSS_DFL_UMASK" ? We are a
security project, therefore restrictive by default :-)
You're right, we are security project by default, so I changed the
constant name.
>From 0f1946aecec78e7faaa3f5815ad06969b1234389 Mon Sep 17 00:00:00 2001
From: Petr Cech <pc...@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
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
