From a6e5e2864f96ebce9ec6656841c8965bcdf1b37f Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 23 Feb 2016 02:15:15 -0500 Subject: credential-cache--daemon: refactor check_socket_directory This function does an early return, and therefore has to repeat its cleanup. We can stick the later bit of the function into an "else" and avoid duplicating the shared part (which will get bigger in a future patch). Let's also rename the function to init_socket_directory. It not only checks the directory but also creates it. Saying "init" is more accurate. Signed-off-by: Jon Griffiths Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- credential-cache--daemon.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'credential-cache--daemon.c') diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index 9365f2ce5c..118d04f137 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -215,7 +215,7 @@ static const char permissions_advice[] = "users may be able to read your cached credentials. Consider running:\n" "\n" " chmod 0700 %s"; -static void check_socket_directory(const char *path) +static void init_socket_directory(const char *path) { struct stat st; char *path_copy = xstrdup(path); @@ -224,20 +224,18 @@ static void check_socket_directory(const char *path) if (!stat(dir, &st)) { if (st.st_mode & 077) die(permissions_advice, dir); - free(path_copy); - return; + } else { + /* + * We must be sure to create the directory with the correct mode, + * not just chmod it after the fact; otherwise, there is a race + * condition in which somebody can chdir to it, sleep, then try to open + * our protected socket. + */ + if (safe_create_leading_directories_const(dir) < 0) + die_errno("unable to create directories for '%s'", dir); + if (mkdir(dir, 0700) < 0) + die_errno("unable to mkdir '%s'", dir); } - - /* - * We must be sure to create the directory with the correct mode, - * not just chmod it after the fact; otherwise, there is a race - * condition in which somebody can chdir to it, sleep, then try to open - * our protected socket. - */ - if (safe_create_leading_directories_const(dir) < 0) - die_errno("unable to create directories for '%s'", dir); - if (mkdir(dir, 0700) < 0) - die_errno("unable to mkdir '%s'", dir); free(path_copy); } @@ -264,7 +262,7 @@ int main(int argc, const char **argv) if (!socket_path) usage_with_options(usage, options); - check_socket_directory(socket_path); + init_socket_directory(socket_path); register_tempfile(&socket_file, socket_path); if (ignore_sighup) -- cgit v1.2.3 From bd93b8d9becb01d21871b63e34c2e824c60b1e8c Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 23 Feb 2016 02:15:41 -0500 Subject: credential-cache--daemon: disallow relative socket path Relative socket paths are dangerous since the user cannot generally control when the daemon starts (initially, after a timeout, kill or crash). Since the daemon creates but does not delete the socket directory, this could lead to spurious directory creation relative to the users cwd. Suggested-by: Jeff King Signed-off-by: Jon Griffiths Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- credential-cache--daemon.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'credential-cache--daemon.c') diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index 118d04f137..fe3779472c 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -262,6 +262,9 @@ int main(int argc, const char **argv) if (!socket_path) usage_with_options(usage, options); + if (!is_absolute_path(socket_path)) + die("socket directory must be an absolute path"); + init_socket_directory(socket_path); register_tempfile(&socket_file, socket_path); -- cgit v1.2.3 From 6e6144905188a76ef75c5418bd48c333adeebdcd Mon Sep 17 00:00:00 2001 From: Jon Griffiths Date: Tue, 23 Feb 2016 02:16:04 -0500 Subject: credential-cache--daemon: change to the socket dir on startup Changing to the socket path stops the daemon holding open the directory the user was in when it was started, preventing umount from working. We're already holding open a socket in that directory, so there's no downside. Thanks-to: Jeff King Signed-off-by: Jon Griffiths Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- credential-cache--daemon.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'credential-cache--daemon.c') diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index fe3779472c..6b00ee0ee6 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -236,6 +236,15 @@ static void init_socket_directory(const char *path) if (mkdir(dir, 0700) < 0) die_errno("unable to mkdir '%s'", dir); } + + if (chdir(dir)) + /* + * We don't actually care what our cwd is; we chdir here just to + * be a friendly daemon and avoid tying up our original cwd. + * If this fails, it's OK to just continue without that benefit. + */ + ; + free(path_copy); } -- cgit v1.2.3