[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 09/15] qga: set umask 0077 when daemonizing (CVE-201
From: |
Michael Roth |
Subject: |
[Qemu-devel] [PATCH 09/15] qga: set umask 0077 when daemonizing (CVE-2013-2007) |
Date: |
Tue, 14 May 2013 16:53:06 -0500 |
From: Laszlo Ersek <address@hidden>
The qemu guest agent creates a bunch of files with insecure permissions
when started in daemon mode. For example:
-rw-rw-rw- 1 root root /var/log/qemu-ga.log
-rw-rw-rw- 1 root root /var/run/qga.state
-rw-rw-rw- 1 root root /var/log/qga-fsfreeze-hook.log
In addition, at least all files created with the "guest-file-open" QMP
command, and all files created with shell output redirection (or
otherwise) by utilities invoked by the fsfreeze hook script are affected.
For now mask all file mode bits for "group" and "others" in
become_daemon().
Temporarily, for compatibility reasons, stick with the 0666 file-mode in
case of files newly created by the "guest-file-open" QMP call. Do so
without changing the umask temporarily.
Signed-off-by: Laszlo Ersek <address@hidden>
Signed-off-by: Anthony Liguori <address@hidden>
(cherry picked from commit c689b4f1bac352dcfd6ecb9a1d45337de0f1de67)
Conflicts:
qga/commands-posix.c
*update includes to match stable
Signed-off-by: Michael Roth <address@hidden>
---
qga/commands-posix.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++++--
qga/main.c | 2 +-
2 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 1c2aff3..08f3473 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -162,9 +162,122 @@ static GuestFileHandle *guest_file_handle_find(int64_t
id, Error **err)
return NULL;
}
+typedef const char * const ccpc;
+
+/* http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html */
+static const struct {
+ ccpc *forms;
+ int oflag_base;
+} guest_file_open_modes[] = {
+ { (ccpc[]){ "r", "rb", NULL }, O_RDONLY },
+ { (ccpc[]){ "w", "wb", NULL }, O_WRONLY | O_CREAT | O_TRUNC },
+ { (ccpc[]){ "a", "ab", NULL }, O_WRONLY | O_CREAT | O_APPEND },
+ { (ccpc[]){ "r+", "rb+", "r+b", NULL }, O_RDWR },
+ { (ccpc[]){ "w+", "wb+", "w+b", NULL }, O_RDWR | O_CREAT | O_TRUNC },
+ { (ccpc[]){ "a+", "ab+", "a+b", NULL }, O_RDWR | O_CREAT | O_APPEND }
+};
+
+static int
+find_open_flag(const char *mode_str, Error **err)
+{
+ unsigned mode;
+
+ for (mode = 0; mode < ARRAY_SIZE(guest_file_open_modes); ++mode) {
+ ccpc *form;
+
+ form = guest_file_open_modes[mode].forms;
+ while (*form != NULL && strcmp(*form, mode_str) != 0) {
+ ++form;
+ }
+ if (*form != NULL) {
+ break;
+ }
+ }
+
+ if (mode == ARRAY_SIZE(guest_file_open_modes)) {
+ error_setg(err, "invalid file open mode '%s'", mode_str);
+ return -1;
+ }
+ return guest_file_open_modes[mode].oflag_base | O_NOCTTY | O_NONBLOCK;
+}
+
+#define DEFAULT_NEW_FILE_MODE (S_IRUSR | S_IWUSR | \
+ S_IRGRP | S_IWGRP | \
+ S_IROTH | S_IWOTH)
+
+static FILE *
+safe_open_or_create(const char *path, const char *mode, Error **err)
+{
+ Error *local_err = NULL;
+ int oflag;
+
+ oflag = find_open_flag(mode, &local_err);
+ if (local_err == NULL) {
+ int fd;
+
+ /* If the caller wants / allows creation of a new file, we implement it
+ * with a two step process: open() + (open() / fchmod()).
+ *
+ * First we insist on creating the file exclusively as a new file. If
+ * that succeeds, we're free to set any file-mode bits on it. (The
+ * motivation is that we want to set those file-mode bits independently
+ * of the current umask.)
+ *
+ * If the exclusive creation fails because the file already exists
+ * (EEXIST is not possible for any other reason), we just attempt to
+ * open the file, but in this case we won't be allowed to change the
+ * file-mode bits on the preexistent file.
+ *
+ * The pathname should never disappear between the two open()s in
+ * practice. If it happens, then someone very likely tried to race us.
+ * In this case just go ahead and report the ENOENT from the second
+ * open() to the caller.
+ *
+ * If the caller wants to open a preexistent file, then the first
+ * open() is decisive and its third argument is ignored, and the second
+ * open() and the fchmod() are never called.
+ */
+ fd = open(path, oflag | ((oflag & O_CREAT) ? O_EXCL : 0), 0);
+ if (fd == -1 && errno == EEXIST) {
+ oflag &= ~(unsigned)O_CREAT;
+ fd = open(path, oflag);
+ }
+
+ if (fd == -1) {
+ error_setg_errno(&local_err, errno, "failed to open file '%s' "
+ "(mode: '%s')", path, mode);
+ } else {
+ qemu_set_cloexec(fd);
+
+ if ((oflag & O_CREAT) && fchmod(fd, DEFAULT_NEW_FILE_MODE) == -1) {
+ error_setg_errno(&local_err, errno, "failed to set permission "
+ "0%03o on new file '%s' (mode: '%s')",
+ (unsigned)DEFAULT_NEW_FILE_MODE, path, mode);
+ } else {
+ FILE *f;
+
+ f = fdopen(fd, mode);
+ if (f == NULL) {
+ error_setg_errno(&local_err, errno, "failed to associate "
+ "stdio stream with file descriptor %d, "
+ "file '%s' (mode: '%s')", fd, path, mode);
+ } else {
+ return f;
+ }
+ }
+
+ close(fd);
+ }
+ }
+
+ error_propagate(err, local_err);
+ return NULL;
+}
+
int64_t qmp_guest_file_open(const char *path, bool has_mode, const char *mode,
Error **err)
{
FILE *fh;
+ Error *local_err = NULL;
int fd;
int64_t ret = -1, handle;
@@ -172,10 +285,9 @@ int64_t qmp_guest_file_open(const char *path, bool
has_mode, const char *mode, E
mode = "r";
}
slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
- fh = fopen(path, mode);
- if (!fh) {
- error_setg_errno(err, errno, "failed to open file '%s' (mode: '%s')",
- path, mode);
+ fh = safe_open_or_create(path, mode, &local_err);
+ if (local_err != NULL) {
+ error_propagate(err, local_err);
return -1;
}
diff --git a/qga/main.c b/qga/main.c
index 74ef788..028fceb 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -478,7 +478,7 @@ static void become_daemon(const char *pidfile)
}
}
- umask(0);
+ umask(S_IRWXG | S_IRWXO);
sid = setsid();
if (sid < 0) {
goto fail;
--
1.7.9.5
- [Qemu-devel] Patch Round-up for stable 1.4.2, freeze on Monday, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 01/15] nbd: unlock mutex in nbd_co_send_request() error path, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 02/15] qdev: Fix QOM unrealize behavior, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 03/15] rng random backend: check for -EAGAIN errors on read, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 04/15] tap: properly initialize vhostfds, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 05/15] virtio-ccw: Check indicators location., Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 06/15] configure: Pick up libseccomp include path, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 07/15] target-mips: Fix accumulator arguments to gen_helper_dmult(u), Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 08/15] tcg/optimize: fix setcond2 optimization, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 09/15] qga: set umask 0077 when daemonizing (CVE-2013-2007),
Michael Roth <=
- [Qemu-devel] [PATCH 10/15] tcg: Document tcg_qemu_tb_exec() and provide constants for low bit uses, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 11/15] cpu-exec: wrap tcg_qemu_tb_exec() in a fn to restore the PC, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 12/15] Handle CPU interrupts by inline checking of a flag, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 13/15] translate-all.c: Remove cpu_unlink_tb(), Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 14/15] qga: distinguish binary modes in "guest_file_open_modes" map, Michael Roth, 2013/05/14
- [Qemu-devel] [PATCH 15/15] qga: unlink just created guest-file if fchmod() or fdopen() fails on it, Michael Roth, 2013/05/14
- Re: [Qemu-devel] Patch Round-up for stable 1.4.2, freeze on Monday, Michael Tokarev, 2013/05/15
- Re: [Qemu-devel] Patch Round-up for stable 1.4.2, freeze on Monday, Brad Smith, 2013/05/15