<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">On 15.09.2014 16:21, Pavel Emelyanov
wrote:<br>
</div>
<blockquote cite="mid:5416E7DA.5050205@parallels.com" type="cite">
<pre wrap="">On 09/15/2014 05:01 PM, Ruslan Kuprieiev wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 15.09.2014 15:11, Pavel Emelyanov wrote:
</pre>
<blockquote type="cite">
<pre wrap="">On 09/13/2014 02:12 PM, Ruslan Kuprieiev wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Signed-off-by: Ruslan Kuprieiev <a class="moz-txt-link-rfc2396E" href="mailto:kupruser@gmail.com"><kupruser@gmail.com></a>
---
image.c | 12 ++++++++++++
include/crtools.h | 1 +
security.c | 43 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 56 insertions(+)
diff --git a/image.c b/image.c
index 566073b..9f49887 100644
--- a/image.c
+++ b/image.c
@@ -218,6 +218,18 @@ int open_image_at(int dfd, int type, unsigned long flags, ...)
goto err;
}
+ if (flags == O_RDONLY) {
+ if (!check_file_ids(ret)) {
+ pr_err("User has no rights to open image %s\n", path);
+ goto err;
+ }
+ } else {
+ if (fchown(ret, 0, 0)) {
</pre>
</blockquote>
<pre wrap="">This looks strange. Chown is root-only operation. If we're not root,
this will fail, if we are -- this is pointless.
</pre>
</blockquote>
<pre wrap="">
We are always root. If we set suid bit, criu process will have root uid,
but group will be equal to
user group(keep in mind CR_FD_PERM=rw-rw-r--). So, either we need to not
only set suid bit, but
also sgid, or just chown images.
</pre>
</blockquote>
<pre wrap="">
Or call setfsuid()?</pre>
</blockquote>
<br>
Oh! Good point! So, lets just call setfs<b>g</b>id(0)(because our
uid is 0 anyway) inside restrict_uid(), right?<br>
<br>
<blockquote cite="mid:5416E7DA.5050205@parallels.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">I'm actually not sure, that chowing is better than setting sgid bit.
</pre>
</blockquote>
<pre wrap="">
On images? We must make sure user doesn't modify them, suid bit won't help
with it :)</pre>
</blockquote>
<br>
No, setting sgid on criu binary.<br>
<br>
<blockquote cite="mid:5416E7DA.5050205@parallels.com" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">But, it allows logs, stats, pidfiles
to be modified by user group, which, i guess, may be (somehow?) useful.
</pre>
<blockquote type="cite">
<blockquote type="cite">
<pre wrap="">+ pr_perror("Can't chown image %s to uid 0, gid 0", path);
+ goto err;
+ }
+ }
+
if (fdset_template[type].magic == RAW_IMAGE_MAGIC)
goto skip_magic;
diff --git a/include/crtools.h b/include/crtools.h
index 75047fc..0f73f27 100644
--- a/include/crtools.h
+++ b/include/crtools.h
@@ -29,5 +29,6 @@ struct proc_status_creds;
extern bool may_dump(struct proc_status_creds *);
struct _CredsEntry;
extern bool may_restore(struct _CredsEntry *);
+extern bool check_file_ids(int fd);
#endif /* __CR_CRTOOLS_H__ */
diff --git a/security.c b/security.c
index a801005..75086dc 100644
--- a/security.c
+++ b/security.c
@@ -4,6 +4,7 @@
#include <limits.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/stat.h>
#include "crtools.h"
#include "proc_parse.h"
@@ -164,3 +165,45 @@ bool may_restore(CredsEntry *creds)
check_groups(creds->groups, creds->n_groups) &&
check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
}
+
+static char *mode_str(char *buf, mode_t mode)
+{
+ buf[0] = (mode & S_IRUSR) ? 'r' : '-';
+ buf[1] = (mode & S_IWUSR) ? 'w' : '-';
+ buf[2] = (mode & S_IXUSR) ? 'x' : '-';
+ buf[3] = (mode & S_IRGRP) ? 'r' : '-';
+ buf[4] = (mode & S_IWGRP) ? 'w' : '-';
+ buf[5] = (mode & S_IXGRP) ? 'x' : '-';
+ buf[6] = (mode & S_IROTH) ? 'r' : '-';
+ buf[7] = (mode & S_IWOTH) ? 'w' : '-';
+ buf[8] = (mode & S_IXOTH) ? 'x' : '-';
+ buf[9] = '\0';
+
+ return buf;
+}
+
+bool check_file_ids(int fd)
+{
+ struct stat st;
+ char buf[10];
+
+ if (cr_uid == 0 && cr_gid == 0)
+ return true;
+
+ if (fstat(fd, &st)) {
+ pr_perror("Can't stat file");
+ return false;
+ }
+
+ if (!(st.st_mode & CR_FD_PERM)) {
+ pr_err("File mode %s != %s\n", mode_str(buf, st.st_mode), mode_str(buf, CR_FD_PERM));
+ return false;
+ }
+
+ if (st.st_uid != 0 || st.st_gid != 0) {
+ pr_err("File uid/gid (%d,%d) != (0,0)\n", st.st_uid, st.st_gid);
+ return false;
+ }
+
+ return true;
+}
</pre>
</blockquote>
</blockquote>
<pre wrap="">
_______________________________________________
CRIU mailing list
<a class="moz-txt-link-abbreviated" href="mailto:CRIU@openvz.org">CRIU@openvz.org</a>
<a class="moz-txt-link-freetext" href="https://lists.openvz.org/mailman/listinfo/criu">https://lists.openvz.org/mailman/listinfo/criu</a>
.
</pre>
</blockquote>
<pre wrap="">
</pre>
</blockquote>
<br>
</body>
</html>