* [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS
@ 2010-05-26 23:21 Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model Venkateswararao Jujjuri (JV)
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri
This patch series introduces the security model for VirtFS.
Brief description of this patch series:
It introduces two type of security models for VirtFS.
They are: mapped and passthrough.
The following is common to both security models.
* Client's VFS determines/enforces the access control.
Largely server should never return EACCESS.
* Client sends gid/mode-bit information as part of creation only.
Changes from V3
---------------
o Return NULL instead of exit(1) on failure in virtio_9p_init()
o Capitalized sm_passthrough, sm_mappe
o Added handling for EINTR for read/write.
o Corrected default permissions for mkdir in mapped mode.
o Added additional error handling.
Changes from V2
---------------
o Removed warnings resulting from chmod/chown.
o Added code to fail normally if secuirty_model option is not specified.
Changes from V1
---------------
o Added support for chmod and chown.
o Used chmod/chown to set credentials instead of setuid/setgid.
o Fixed a bug where uid used instated of uid.
Security model: mapped
----------------------
VirtFS server(QEMU) intercepts and maps all the file object create requests.
Files on the fileserver will be created with QEMU's user credentials and the
client-user's credentials are stored in extended attributes.
During getattr() server extracts the client-user's credentials from extended
attributes and sends to the client.
Given that only the user space extended attributes are available to regular
files, special files are created as regular files on the fileserver and the
appropriate mode bits are stored in xattrs and will be extracted during
getattr.
If the extended attributes are missing, server sends back the filesystem
stat() unaltered. This provision will make the files created on the
fileserver usable to client.
Points to be considered
* Filesystem will be VirtFS'ized. Meaning, other filesystems may not
understand the credentials of the files created under this model.
* Regular utilities like 'df' may not report required results in this model.
Need for special reporting utilities which can understand this security model.
Security model : passthrough
----------------------------
In this security model, VirtFS server passes down all requests to the
underlying filesystem. File system objects on the fileserver will be created
with client-user's credentials. This is done by setting setuid()/setgid()
during creation or ch* after file creation. At the end of create protocol
request, files on the fileserver will be owned by cleint-user's uid/gid.
Points to be considered
* Fileserver should always run as 'root'.
* Root squashing may be needed. Will be for future work.
* Potential for user credential clash between guest's user space IDs and
host's user space IDs.
It also adds security model attribute to -fsdev device and to -virtfs shortcut.
Usage examples:
-fsdev local,id=jvrao,path=/tmp/,security_model=mapped
-virtfs local,path=/tmp/,security_model=passthrough,mnt_tag=v_tmp.
--
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model.
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-06-01 17:24 ` Aneesh Kumar K.V
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures Venkateswararao Jujjuri (JV)
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
The new option is:
-fsdev fstype,id=myid,path=/share_path/,security_model=[mapped|passthrough]
-virtfs fstype,path=/share_path/,security_model=[mapped|passthrough],mnt_tag=tag
In the case of mapped security model, files are created with QEMU user
credentials and the client-user's credentials are saved in extended attributes.
Whereas in the case of passthrough security model, files on the
filesystem are directly created with client-user's credentials.
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
fsdev/qemu-fsdev.c | 14 +++++++++++++-
fsdev/qemu-fsdev.h | 1 +
hw/virtio-9p.c | 22 ++++++++++++++++++----
qemu-config.c | 12 +++++++++---
qemu-options.hx | 15 +++++++++++----
vl.c | 18 +++++++++++++++---
6 files changed, 67 insertions(+), 15 deletions(-)
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 813e1f7..7d7a153 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -34,7 +34,7 @@ int qemu_fsdev_add(QemuOpts *opts)
return -1;
}
- for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
+ for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
if (strcmp(FsTypes[i].name, qemu_opt_get(opts, "fstype")) == 0) {
break;
}
@@ -46,10 +46,22 @@ int qemu_fsdev_add(QemuOpts *opts)
return -1;
}
+ if (qemu_opt_get(opts, "path") == NULL) {
+ fprintf(stderr, "fsdev: No path specified.\n");
+ return -1;
+ }
+
+ if (qemu_opt_get(opts, "security_model") == NULL) {
+ fprintf(stderr, "fsdev: No security_model specified.\n");
+ return -1;
+ }
+
fsle = qemu_malloc(sizeof(*fsle));
fsle->fse.fsdev_id = qemu_strdup(qemu_opts_id(opts));
fsle->fse.path = qemu_strdup(qemu_opt_get(opts, "path"));
+ fsle->fse.security_model = qemu_strdup(qemu_opt_get(opts,
+ "security_model"));
fsle->fse.ops = FsTypes[i].ops;
QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index b50fbe0..6c27881 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -40,6 +40,7 @@ typedef struct FsTypeTable {
typedef struct FsTypeEntry {
char *fsdev_id;
char *path;
+ char *security_model;
FileOperations *ops;
} FsTypeEntry;
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 687abc0..a57562a 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -2402,7 +2402,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
/* We don't have a fsdev identified by fsdev_id */
fprintf(stderr, "Virtio-9p device couldn't find fsdev "
"with the id %s\n", conf->fsdev_id);
- exit(1);
+ return NULL;
}
if (!fse->path || !conf->tag) {
@@ -2410,15 +2410,29 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
fprintf(stderr, "fsdev with id %s needs path "
"and Virtio-9p device needs mount_tag arguments\n",
conf->fsdev_id);
- exit(1);
+ return NULL;
+ }
+
+ if (!strcmp(fse->security_model, "passthrough")) {
+ /* Files on the Fileserver set to client user credentials */
+ } else if (!strcmp(fse->security_model, "mapped")) {
+ /* Files on the fileserver are set to QEMU credentials.
+ * Client user credentials are saved in extended attributes.
+ */
+ } else {
+ /* user haven't specified a correct security option */
+ fprintf(stderr, "one of the following must be specified as the"
+ "security option:\n\t security_model=passthrough \n\t "
+ "security_model=mapped\n");
+ return NULL;
}
if (lstat(fse->path, &stat)) {
fprintf(stderr, "share path %s does not exist\n", fse->path);
- exit(1);
+ return NULL;
} else if (!S_ISDIR(stat.st_mode)) {
fprintf(stderr, "share path %s is not a directory \n", fse->path);
- exit(1);
+ return NULL;
}
s->ctx.fs_root = qemu_strdup(fse->path);
diff --git a/qemu-config.c b/qemu-config.c
index d500885..e1e3aa1 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -160,9 +160,12 @@ QemuOptsList qemu_fsdev_opts = {
{
.name = "fstype",
.type = QEMU_OPT_STRING,
- }, {
+ },{
.name = "path",
.type = QEMU_OPT_STRING,
+ },{
+ .name = "security_model",
+ .type = QEMU_OPT_STRING,
},
{ /*End of list */ }
},
@@ -178,12 +181,15 @@ QemuOptsList qemu_virtfs_opts = {
{
.name = "fstype",
.type = QEMU_OPT_STRING,
- }, {
+ },{
.name = "path",
.type = QEMU_OPT_STRING,
- }, {
+ },{
.name = "mount_tag",
.type = QEMU_OPT_STRING,
+ },{
+ .name = "security_model",
+ .type = QEMU_OPT_STRING,
},
{ /*End of list */ }
diff --git a/qemu-options.hx b/qemu-options.hx
index 12f6b51..d557c92 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -482,7 +482,7 @@ ETEXI
DEFHEADING(File system options:)
DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
- "-fsdev local,id=id,path=path\n",
+ "-fsdev local,id=id,path=path,security_model=[mapped|passthrough]\n",
QEMU_ARCH_ALL)
STEXI
@@ -498,7 +498,7 @@ The specific Fstype will determine the applicable options.
Options to each backend are described below.
-@item -fsdev local ,id=@var{id} ,path=@var{path}
+@item -fsdev local ,id=@var{id} ,path=@var{path} ,security_model=@var{security_model}
Create a file-system-"device" for local-filesystem.
@@ -506,6 +506,9 @@ Create a file-system-"device" for local-filesystem.
@option{path} specifies the path to be exported. @option{path} is required.
+@option{security_model} specifies the security model to be followed.
+@option{security_model} is required.
+
@end table
ETEXI
#endif
@@ -514,7 +517,7 @@ ETEXI
DEFHEADING(Virtual File system pass-through options:)
DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
- "-virtfs local,path=path,mount_tag=tag\n",
+ "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough]\n",
QEMU_ARCH_ALL)
STEXI
@@ -530,7 +533,7 @@ The specific Fstype will determine the applicable options.
Options to each backend are described below.
-@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag}
+@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag} ,security_model=@var{security_model}
Create a Virtual file-system-pass through for local-filesystem.
@@ -538,6 +541,10 @@ Create a Virtual file-system-pass through for local-filesystem.
@option{path} specifies the path to be exported. @option{path} is required.
+@option{security_model} specifies the security model to be followed.
+@option{security_model} is required.
+
+
@option{mount_tag} specifies the tag with which the exported file is mounted.
@option{mount_tag} is required.
diff --git a/vl.c b/vl.c
index 85bcc84..a341781 100644
--- a/vl.c
+++ b/vl.c
@@ -3109,10 +3109,21 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
- len = strlen(",id=,path=");
+ if (qemu_opt_get(opts, "fstype") == NULL ||
+ qemu_opt_get(opts, "mount_tag") == NULL ||
+ qemu_opt_get(opts, "path") == NULL ||
+ qemu_opt_get(opts, "security_model") == NULL) {
+ fprintf(stderr, "Usage: -virtfs fstype,path=/share_path/,"
+ "security_model=[mapped|passthrough],"
+ "mnt_tag=tag.\n");
+ exit(1);
+ }
+
+ len = strlen(",id=,path=,security_model=");
len += strlen(qemu_opt_get(opts, "fstype"));
len += strlen(qemu_opt_get(opts, "mount_tag"));
len += strlen(qemu_opt_get(opts, "path"));
+ len += strlen(qemu_opt_get(opts, "security_model"));
arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
if (!arg_fsdev) {
@@ -3121,10 +3132,11 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
- sprintf(arg_fsdev, "%s,id=%s,path=%s",
+ sprintf(arg_fsdev, "%s,id=%s,path=%s,security_model=%s",
qemu_opt_get(opts, "fstype"),
qemu_opt_get(opts, "mount_tag"),
- qemu_opt_get(opts, "path"));
+ qemu_opt_get(opts, "path"),
+ qemu_opt_get(opts, "security_model"));
len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-06-01 17:22 ` Aneesh Kumar K.V
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model Venkateswararao Jujjuri (JV)
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/virtio-9p.c | 185 ++++++++++++++------------------------------------------
hw/virtio-9p.h | 92 ++++++++++++++++++++++++++++
2 files changed, 138 insertions(+), 139 deletions(-)
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index a57562a..6feaa53 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -21,6 +21,52 @@
int dotu = 1;
int debug_9p_pdu;
+enum {
+ Oread = 0x00,
+ Owrite = 0x01,
+ Ordwr = 0x02,
+ Oexec = 0x03,
+ Oexcl = 0x04,
+ Otrunc = 0x10,
+ Orexec = 0x20,
+ Orclose = 0x40,
+ Oappend = 0x80,
+};
+
+static int omode_to_uflags(int8_t mode)
+{
+ int ret = 0;
+
+ switch (mode & 3) {
+ case Oread:
+ ret = O_RDONLY;
+ break;
+ case Ordwr:
+ ret = O_RDWR;
+ break;
+ case Owrite:
+ ret = O_WRONLY;
+ break;
+ case Oexec:
+ ret = O_RDONLY;
+ break;
+ }
+
+ if (mode & Otrunc) {
+ ret |= O_TRUNC;
+ }
+
+ if (mode & Oappend) {
+ ret |= O_APPEND;
+ }
+
+ if (mode & Oexcl) {
+ ret |= O_EXCL;
+ }
+
+ return ret;
+}
+
static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
{
return s->ops->lstat(&s->ctx, path->data, stbuf);
@@ -999,14 +1045,6 @@ out:
v9fs_string_free(&aname);
}
-typedef struct V9fsStatState {
- V9fsPDU *pdu;
- size_t offset;
- V9fsStat v9stat;
- V9fsFidState *fidp;
- struct stat stbuf;
-} V9fsStatState;
-
static void v9fs_stat_post_lstat(V9fsState *s, V9fsStatState *vs, int err)
{
if (err == -1) {
@@ -1057,19 +1095,6 @@ out:
qemu_free(vs);
}
-typedef struct V9fsWalkState {
- V9fsPDU *pdu;
- size_t offset;
- int16_t nwnames;
- int name_idx;
- V9fsQID *qids;
- V9fsFidState *fidp;
- V9fsFidState *newfidp;
- V9fsString path;
- V9fsString *wnames;
- struct stat stbuf;
-} V9fsWalkState;
-
static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
{
complete_pdu(s, vs->pdu, err);
@@ -1233,62 +1258,6 @@ out:
v9fs_walk_complete(s, vs, err);
}
-typedef struct V9fsOpenState {
- V9fsPDU *pdu;
- size_t offset;
- int8_t mode;
- V9fsFidState *fidp;
- V9fsQID qid;
- struct stat stbuf;
-
-} V9fsOpenState;
-
-enum {
- Oread = 0x00,
- Owrite = 0x01,
- Ordwr = 0x02,
- Oexec = 0x03,
- Oexcl = 0x04,
- Otrunc = 0x10,
- Orexec = 0x20,
- Orclose = 0x40,
- Oappend = 0x80,
-};
-
-static int omode_to_uflags(int8_t mode)
-{
- int ret = 0;
-
- switch (mode & 3) {
- case Oread:
- ret = O_RDONLY;
- break;
- case Ordwr:
- ret = O_RDWR;
- break;
- case Owrite:
- ret = O_WRONLY;
- break;
- case Oexec:
- ret = O_RDONLY;
- break;
- }
-
- if (mode & Otrunc) {
- ret |= O_TRUNC;
- }
-
- if (mode & Oappend) {
- ret |= O_APPEND;
- }
-
- if (mode & Oexcl) {
- ret |= O_EXCL;
- }
-
- return ret;
-}
-
static void v9fs_open_post_opendir(V9fsState *s, V9fsOpenState *vs, int err)
{
if (vs->fidp->dir == NULL) {
@@ -1391,25 +1360,6 @@ out:
complete_pdu(s, pdu, err);
}
-typedef struct V9fsReadState {
- V9fsPDU *pdu;
- size_t offset;
- int32_t count;
- int32_t total;
- int64_t off;
- V9fsFidState *fidp;
- struct iovec iov[128]; /* FIXME: bad, bad, bad */
- struct iovec *sg;
- off_t dir_pos;
- struct dirent *dent;
- struct stat stbuf;
- V9fsString name;
- V9fsStat v9stat;
- int32_t len;
- int32_t cnt;
- int32_t max_count;
-} V9fsReadState;
-
static void v9fs_read_post_readdir(V9fsState *, V9fsReadState *, ssize_t);
static void v9fs_read_post_seekdir(V9fsState *s, V9fsReadState *vs, ssize_t err)
@@ -1597,19 +1547,6 @@ out:
qemu_free(vs);
}
-typedef struct V9fsWriteState {
- V9fsPDU *pdu;
- size_t offset;
- int32_t len;
- int32_t count;
- int32_t total;
- int64_t off;
- V9fsFidState *fidp;
- struct iovec iov[128]; /* FIXME: bad, bad, bad */
- struct iovec *sg;
- int cnt;
-} V9fsWriteState;
-
static void v9fs_write_post_writev(V9fsState *s, V9fsWriteState *vs,
ssize_t err)
{
@@ -1706,19 +1643,6 @@ out:
qemu_free(vs);
}
-typedef struct V9fsCreateState {
- V9fsPDU *pdu;
- size_t offset;
- V9fsFidState *fidp;
- V9fsQID qid;
- int32_t perm;
- int8_t mode;
- struct stat stbuf;
- V9fsString name;
- V9fsString extension;
- V9fsString fullname;
-} V9fsCreateState;
-
static void v9fs_post_create(V9fsState *s, V9fsCreateState *vs, int err)
{
if (err == 0) {
@@ -1938,12 +1862,6 @@ static void v9fs_flush(V9fsState *s, V9fsPDU *pdu)
complete_pdu(s, pdu, 7);
}
-typedef struct V9fsRemoveState {
- V9fsPDU *pdu;
- size_t offset;
- V9fsFidState *fidp;
-} V9fsRemoveState;
-
static void v9fs_remove_post_remove(V9fsState *s, V9fsRemoveState *vs,
int err)
{
@@ -1986,17 +1904,6 @@ out:
qemu_free(vs);
}
-typedef struct V9fsWstatState
-{
- V9fsPDU *pdu;
- size_t offset;
- int16_t unused;
- V9fsStat v9stat;
- V9fsFidState *fidp;
- struct stat stbuf;
- V9fsString nname;
-} V9fsWstatState;
-
static void v9fs_wstat_post_truncate(V9fsState *s, V9fsWstatState *vs, int err)
{
if (err < 0) {
diff --git a/hw/virtio-9p.h b/hw/virtio-9p.h
index 9b6cbde..992c765 100644
--- a/hw/virtio-9p.h
+++ b/hw/virtio-9p.h
@@ -154,6 +154,98 @@ typedef struct V9fsState
enum p9_proto_version proto_version;
} V9fsState;
+typedef struct V9fsCreateState {
+ V9fsPDU *pdu;
+ size_t offset;
+ V9fsFidState *fidp;
+ V9fsQID qid;
+ int32_t perm;
+ int8_t mode;
+ struct stat stbuf;
+ V9fsString name;
+ V9fsString extension;
+ V9fsString fullname;
+} V9fsCreateState;
+
+typedef struct V9fsStatState {
+ V9fsPDU *pdu;
+ size_t offset;
+ V9fsStat v9stat;
+ V9fsFidState *fidp;
+ struct stat stbuf;
+} V9fsStatState;
+
+typedef struct V9fsWalkState {
+ V9fsPDU *pdu;
+ size_t offset;
+ int16_t nwnames;
+ int name_idx;
+ V9fsQID *qids;
+ V9fsFidState *fidp;
+ V9fsFidState *newfidp;
+ V9fsString path;
+ V9fsString *wnames;
+ struct stat stbuf;
+} V9fsWalkState;
+
+typedef struct V9fsOpenState {
+ V9fsPDU *pdu;
+ size_t offset;
+ int8_t mode;
+ V9fsFidState *fidp;
+ V9fsQID qid;
+ struct stat stbuf;
+} V9fsOpenState;
+
+typedef struct V9fsReadState {
+ V9fsPDU *pdu;
+ size_t offset;
+ int32_t count;
+ int32_t total;
+ int64_t off;
+ V9fsFidState *fidp;
+ struct iovec iov[128]; /* FIXME: bad, bad, bad */
+ struct iovec *sg;
+ off_t dir_pos;
+ struct dirent *dent;
+ struct stat stbuf;
+ V9fsString name;
+ V9fsStat v9stat;
+ int32_t len;
+ int32_t cnt;
+ int32_t max_count;
+} V9fsReadState;
+
+typedef struct V9fsWriteState {
+ V9fsPDU *pdu;
+ size_t offset;
+ int32_t len;
+ int32_t count;
+ int32_t total;
+ int64_t off;
+ V9fsFidState *fidp;
+ struct iovec iov[128]; /* FIXME: bad, bad, bad */
+ struct iovec *sg;
+ int cnt;
+} V9fsWriteState;
+
+typedef struct V9fsRemoveState {
+ V9fsPDU *pdu;
+ size_t offset;
+ V9fsFidState *fidp;
+} V9fsRemoveState;
+
+typedef struct V9fsWstatState
+{
+ V9fsPDU *pdu;
+ size_t offset;
+ int16_t unused;
+ V9fsStat v9stat;
+ V9fsFidState *fidp;
+ struct stat stbuf;
+ V9fsString nname;
+} V9fsWstatState;
+
struct virtio_9p_config
{
/* number of characters in tag */
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model.
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-06-01 17:30 ` Aneesh Kumar K.V
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 4/7] virtio-9p: Implement Security model for mknod related files Venkateswararao Jujjuri (JV)
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
Add required infrastructure and modify create/open2 and mkdir per the new
security model.
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/file-op-9p.h | 23 +++++++-
hw/virtio-9p-local.c | 149 ++++++++++++++++++++++++++++++++++++--------------
hw/virtio-9p.c | 42 ++++++++++----
3 files changed, 158 insertions(+), 56 deletions(-)
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 2934ff1..73d59b2 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -19,13 +19,32 @@
#include <sys/stat.h>
#include <sys/uio.h>
#include <sys/vfs.h>
+#define SM_LOCAL_MODE_BITS 0600
+#define SM_LOCAL_DIR_MODE_BITS 0700
+
+typedef enum
+{
+ SM_PASSTHROUGH = 1, /* uid/gid set on fileserver files */
+ SM_MAPPED, /* uid/gid part of xattr */
+} SecModel;
+
+typedef struct FsCred
+{
+ uid_t fc_uid;
+ gid_t fc_gid;
+ mode_t fc_mode;
+ dev_t fc_rdev;
+} FsCred;
typedef struct FsContext
{
char *fs_root;
+ SecModel fs_sm;
uid_t uid;
} FsContext;
+extern void cred_init(FsCred *);
+
typedef struct FileOperations
{
int (*lstat)(FsContext *, const char *, struct stat *);
@@ -43,7 +62,7 @@ typedef struct FileOperations
int (*closedir)(FsContext *, DIR *);
DIR *(*opendir)(FsContext *, const char *);
int (*open)(FsContext *, const char *, int);
- int (*open2)(FsContext *, const char *, int, mode_t);
+ int (*open2)(FsContext *, const char *, int, FsCred *);
void (*rewinddir)(FsContext *, DIR *);
off_t (*telldir)(FsContext *, DIR *);
struct dirent *(*readdir)(FsContext *, DIR *);
@@ -51,7 +70,7 @@ typedef struct FileOperations
ssize_t (*readv)(FsContext *, int, const struct iovec *, int);
ssize_t (*writev)(FsContext *, int, const struct iovec *, int);
off_t (*lseek)(FsContext *, int, off_t, int);
- int (*mkdir)(FsContext *, const char *, mode_t);
+ int (*mkdir)(FsContext *, const char *, FsCred *);
int (*fstat)(FsContext *, int, struct stat *);
int (*rename)(FsContext *, const char *, const char *);
int (*truncate)(FsContext *, const char *, off_t);
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 78960ac..f6c2fe2 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -17,6 +17,7 @@
#include <grp.h>
#include <sys/socket.h>
#include <sys/un.h>
+#include <attr/xattr.h>
static const char *rpath(FsContext *ctx, const char *path)
{
@@ -31,47 +32,39 @@ static int local_lstat(FsContext *ctx, const char *path, struct stat *stbuf)
return lstat(rpath(ctx, path), stbuf);
}
-static int local_setuid(FsContext *ctx, uid_t uid)
+static int local_set_xattr(const char *path, FsCred *credp)
{
- struct passwd *pw;
- gid_t groups[33];
- int ngroups;
- static uid_t cur_uid = -1;
-
- if (cur_uid == uid) {
- return 0;
- }
-
- if (setreuid(0, 0)) {
- return -1;
- }
-
- pw = getpwuid(uid);
- if (pw == NULL) {
- return -1;
+ int err;
+ if (credp->fc_uid != -1) {
+ err = setxattr(path, "user.virtfs.uid", &credp->fc_uid, sizeof(uid_t),
+ 0);
+ if (err) {
+ return err;
+ }
}
-
- ngroups = 33;
- if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) {
- return -1;
+ if (credp->fc_gid != -1) {
+ err = setxattr(path, "user.virtfs.gid", &credp->fc_gid, sizeof(gid_t),
+ 0);
+ if (err) {
+ return err;
+ }
}
-
- if (setgroups(ngroups, groups)) {
- return -1;
- }
-
- if (setregid(-1, pw->pw_gid)) {
- return -1;
+ if (credp->fc_mode != -1) {
+ err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
+ sizeof(mode_t), 0);
+ if (err) {
+ return err;
+ }
}
-
- if (setreuid(-1, uid)) {
- return -1;
+ if (credp->fc_rdev != -1) {
+ err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
+ sizeof(dev_t), 0);
+ if (err) {
+ return err;
+ }
}
-
- cur_uid = uid;
-
- return 0;
-}
+ return 0;
+ }
static ssize_t local_readlink(FsContext *ctx, const char *path,
char *buf, size_t bufsz)
@@ -168,9 +161,44 @@ static int local_mksock(FsContext *ctx2, const char *path)
return 0;
}
-static int local_mkdir(FsContext *ctx, const char *path, mode_t mode)
+static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
{
- return mkdir(rpath(ctx, path), mode);
+ int err = -1;
+ int serrno = 0;
+ /* Determine the security model */
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ err = mkdir(rpath(fs_ctx, path), SM_LOCAL_DIR_MODE_BITS);
+ if (err == -1) {
+ return err;
+ }
+ credp->fc_mode = credp->fc_mode|S_IFDIR;
+ err = local_set_xattr(rpath(fs_ctx, path), credp);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
+ if (err == -1) {
+ return err;
+ }
+ err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ }
+ return err;
+
+err_end:
+ remove(rpath(fs_ctx, path));
+ errno = serrno;
+ return err;
}
static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
@@ -178,9 +206,49 @@ static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
return fstat(fd, stbuf);
}
-static int local_open2(FsContext *ctx, const char *path, int flags, mode_t mode)
+static int local_open2(FsContext *fs_ctx, const char *path, int flags,
+ FsCred *credp)
{
- return open(rpath(ctx, path), flags, mode);
+ int fd = -1;
+ int err = -1;
+ int serrno = 0;
+ /* Determine the security model */
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ int err;
+ fd = open(rpath(fs_ctx, path), flags, SM_LOCAL_MODE_BITS);
+ if (fd == -1) {
+ return fd;
+ }
+ credp->fc_mode = credp->fc_mode|S_IFREG;
+ /* Set cleint credentials in xattr */
+ err = local_set_xattr(rpath(fs_ctx, path), credp);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
+ if (fd == -1) {
+ return fd;
+ }
+ err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ }
+ return fd;
+
+err_end:
+ close(fd);
+ remove(rpath(fs_ctx, path));
+ errno = serrno;
+ return err;
}
static int local_symlink(FsContext *ctx, const char *oldpath,
@@ -269,7 +337,6 @@ static int local_statfs(FsContext *s, const char *path, struct statfs *stbuf)
FileOperations local_ops = {
.lstat = local_lstat,
- .setuid = local_setuid,
.readlink = local_readlink,
.close = local_close,
.closedir = local_closedir,
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 6feaa53..ef870a3 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -67,14 +67,17 @@ static int omode_to_uflags(int8_t mode)
return ret;
}
-static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
+void cred_init(FsCred *credp)
{
- return s->ops->lstat(&s->ctx, path->data, stbuf);
+ credp->fc_uid = -1;
+ credp->fc_gid = -1;
+ credp->fc_mode = -1;
+ credp->fc_rdev = -1;
}
-static int v9fs_do_setuid(V9fsState *s, uid_t uid)
+static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
{
- return s->ops->setuid(&s->ctx, uid);
+ return s->ops->lstat(&s->ctx, path->data, stbuf);
}
static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
@@ -164,9 +167,15 @@ static int v9fs_do_mksock(V9fsState *s, V9fsString *path)
return s->ops->mksock(&s->ctx, path->data);
}
-static int v9fs_do_mkdir(V9fsState *s, V9fsString *path, mode_t mode)
+static int v9fs_do_mkdir(V9fsState *s, V9fsCreateState *vs)
{
- return s->ops->mkdir(&s->ctx, path->data, mode);
+ FsCred cred;
+
+ cred_init(&cred);
+ cred.fc_uid = vs->fidp->uid;
+ cred.fc_mode = vs->perm & 0777;
+
+ return s->ops->mkdir(&s->ctx, vs->fullname.data, &cred);
}
static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
@@ -174,9 +183,17 @@ static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
return s->ops->fstat(&s->ctx, fd, stbuf);
}
-static int v9fs_do_open2(V9fsState *s, V9fsString *path, int flags, mode_t mode)
+static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
{
- return s->ops->open2(&s->ctx, path->data, flags, mode);
+ FsCred cred;
+ int flags;
+
+ cred_init(&cred);
+ cred.fc_uid = vs->fidp->uid;
+ cred.fc_mode = vs->perm & 0777;
+ flags = omode_to_uflags(vs->mode) | O_CREAT;
+
+ return s->ops->open2(&s->ctx, vs->fullname.data, flags, &cred);
}
static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
@@ -348,7 +365,6 @@ static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid)
for (f = s->fid_list; f; f = f->next) {
if (f->fid == fid) {
- v9fs_do_setuid(s, f->uid);
return f;
}
}
@@ -1762,7 +1778,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
}
if (vs->perm & P9_STAT_MODE_DIR) {
- err = v9fs_do_mkdir(s, &vs->fullname, vs->perm & 0777);
+ err = v9fs_do_mkdir(s, vs);
v9fs_create_post_mkdir(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_SYMLINK) {
err = v9fs_do_symlink(s, &vs->extension, &vs->fullname);
@@ -1809,9 +1825,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
err = v9fs_do_mksock(s, &vs->fullname);
v9fs_create_post_mksock(s, vs, err);
} else {
- vs->fidp->fd = v9fs_do_open2(s, &vs->fullname,
- omode_to_uflags(vs->mode) | O_CREAT,
- vs->perm & 0777);
+ vs->fidp->fd = v9fs_do_open2(s, vs);
v9fs_create_post_open2(s, vs, err);
}
@@ -2322,10 +2336,12 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
if (!strcmp(fse->security_model, "passthrough")) {
/* Files on the Fileserver set to client user credentials */
+ s->ctx.fs_sm = SM_PASSTHROUGH;
} else if (!strcmp(fse->security_model, "mapped")) {
/* Files on the fileserver are set to QEMU credentials.
* Client user credentials are saved in extended attributes.
*/
+ s->ctx.fs_sm = SM_MAPPED;
} else {
/* user haven't specified a correct security option */
fprintf(stderr, "one of the following must be specified as the"
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 4/7] virtio-9p: Implement Security model for mknod related files
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
` (2 preceding siblings ...)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 5/7] virtio-9p: Implemented security model for symlink and link Venkateswararao Jujjuri (JV)
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
In the mapped security model all the special files are created as regular files
on the fileserver and appropriate mode bits are added to the extended
attributes. These extended attributes are used to present this file
as special file to the client.
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/file-op-9p.h | 3 +-
hw/virtio-9p-local.c | 58 +++++++++++++++++++++++++++++--------------------
hw/virtio-9p.c | 39 ++++++++++-----------------------
3 files changed, 47 insertions(+), 53 deletions(-)
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 73d59b2..144dd77 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -51,8 +51,7 @@ typedef struct FileOperations
ssize_t (*readlink)(FsContext *, const char *, char *, size_t);
int (*chmod)(FsContext *, const char *, mode_t);
int (*chown)(FsContext *, const char *, uid_t, gid_t);
- int (*mknod)(FsContext *, const char *, mode_t, dev_t);
- int (*mksock)(FsContext *, const char *);
+ int (*mknod)(FsContext *, const char *, FsCred *);
int (*utime)(FsContext *, const char *, const struct utimbuf *);
int (*remove)(FsContext *, const char *);
int (*symlink)(FsContext *, const char *, const char *);
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index f6c2fe2..bd8c1c7 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -134,33 +134,44 @@ static int local_chmod(FsContext *ctx, const char *path, mode_t mode)
return chmod(rpath(ctx, path), mode);
}
-static int local_mknod(FsContext *ctx, const char *path, mode_t mode, dev_t dev)
+static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
{
- return mknod(rpath(ctx, path), mode, dev);
-}
-
-static int local_mksock(FsContext *ctx2, const char *path)
-{
- struct sockaddr_un addr;
- int s;
-
- addr.sun_family = AF_UNIX;
- snprintf(addr.sun_path, 108, "%s", rpath(ctx2, path));
-
- s = socket(PF_UNIX, SOCK_STREAM, 0);
- if (s == -1) {
- return -1;
- }
-
- if (bind(s, (struct sockaddr *)&addr, sizeof(addr))) {
- close(s);
- return -1;
+ int err = -1;
+ int serrno = 0;
+ /* Determine the security model */
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ err = mknod(rpath(fs_ctx, path), SM_LOCAL_MODE_BITS|S_IFREG, 0);
+ if (err == -1) {
+ return err;
+ }
+ local_set_xattr(rpath(fs_ctx, path), credp);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ err = mknod(rpath(fs_ctx, path), credp->fc_mode, credp->fc_rdev);
+ if (err == -1) {
+ return err;
+ }
+ err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
}
+ return err;
- close(s);
- return 0;
+err_end:
+ remove(rpath(fs_ctx, path));
+ errno = serrno;
+ return err;
}
-
static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
{
int err = -1;
@@ -351,7 +362,6 @@ FileOperations local_ops = {
.writev = local_writev,
.chmod = local_chmod,
.mknod = local_mknod,
- .mksock = local_mksock,
.mkdir = local_mkdir,
.fstat = local_fstat,
.open2 = local_open2,
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index ef870a3..38c1d68 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -157,14 +157,15 @@ static int v9fs_do_chmod(V9fsState *s, V9fsString *path, mode_t mode)
return s->ops->chmod(&s->ctx, path->data, mode);
}
-static int v9fs_do_mknod(V9fsState *s, V9fsString *path, mode_t mode, dev_t dev)
+static int v9fs_do_mknod(V9fsState *s, V9fsCreateState *vs, mode_t mode,
+ dev_t dev)
{
- return s->ops->mknod(&s->ctx, path->data, mode, dev);
-}
-
-static int v9fs_do_mksock(V9fsState *s, V9fsString *path)
-{
- return s->ops->mksock(&s->ctx, path->data);
+ FsCred cred;
+ cred_init(&cred);
+ cred.fc_uid = vs->fidp->uid;
+ cred.fc_mode = mode;
+ cred.fc_rdev = dev;
+ return s->ops->mknod(&s->ctx, vs->fullname.data, &cred);
}
static int v9fs_do_mkdir(V9fsState *s, V9fsCreateState *vs)
@@ -1725,22 +1726,6 @@ out:
v9fs_post_create(s, vs, err);
}
-static void v9fs_create_post_mksock(V9fsState *s, V9fsCreateState *vs,
- int err)
-{
- if (err) {
- err = -errno;
- goto out;
- }
-
- err = v9fs_do_chmod(s, &vs->fullname, vs->perm & 0777);
- v9fs_create_post_perms(s, vs, err);
- return;
-
-out:
- v9fs_post_create(s, vs, err);
-}
-
static void v9fs_create_post_fstat(V9fsState *s, V9fsCreateState *vs, int err)
{
if (err) {
@@ -1816,14 +1801,14 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
}
nmode |= vs->perm & 0777;
- err = v9fs_do_mknod(s, &vs->fullname, nmode, makedev(major, minor));
+ err = v9fs_do_mknod(s, vs, nmode, makedev(major, minor));
v9fs_create_post_perms(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_NAMED_PIPE) {
- err = v9fs_do_mknod(s, &vs->fullname, S_IFIFO | (vs->mode & 0777), 0);
+ err = v9fs_do_mknod(s, vs, S_IFIFO | (vs->perm & 0777), 0);
v9fs_post_create(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_SOCKET) {
- err = v9fs_do_mksock(s, &vs->fullname);
- v9fs_create_post_mksock(s, vs, err);
+ err = v9fs_do_mknod(s, vs, S_IFSOCK | (vs->perm & 0777), 0);
+ v9fs_post_create(s, vs, err);
} else {
vs->fidp->fd = v9fs_do_open2(s, vs);
v9fs_create_post_open2(s, vs, err);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 5/7] virtio-9p: Implemented security model for symlink and link.
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
` (3 preceding siblings ...)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 4/7] virtio-9p: Implement Security model for mknod related files Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 6/7] virtio-9p: Implemented Security model for lstat and fstat Venkateswararao Jujjuri (JV)
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/file-op-9p.h | 4 +-
hw/virtio-9p-local.c | 98 ++++++++++++++++++++++++++++++++++++++++---------
hw/virtio-9p.c | 24 +++++++++----
3 files changed, 99 insertions(+), 27 deletions(-)
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 144dd77..c1c08b4 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -54,8 +54,8 @@ typedef struct FileOperations
int (*mknod)(FsContext *, const char *, FsCred *);
int (*utime)(FsContext *, const char *, const struct utimbuf *);
int (*remove)(FsContext *, const char *);
- int (*symlink)(FsContext *, const char *, const char *);
- int (*link)(FsContext *, const char *, const char *);
+ int (*symlink)(FsContext *, const char *, const char *, FsCred *);
+ int (*link)(FsContext *, const char *, const char *, FsCred *);
int (*setuid)(FsContext *, uid_t);
int (*close)(FsContext *, int);
int (*closedir)(FsContext *, DIR *);
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index bd8c1c7..395a33f 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -64,12 +64,27 @@ static int local_set_xattr(const char *path, FsCred *credp)
}
}
return 0;
- }
+}
-static ssize_t local_readlink(FsContext *ctx, const char *path,
- char *buf, size_t bufsz)
+static ssize_t local_readlink(FsContext *fs_ctx, const char *path,
+ char *buf, size_t bufsz)
{
- return readlink(rpath(ctx, path), buf, bufsz);
+ ssize_t tsize = -1;
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ int fd;
+ fd = open(rpath(fs_ctx, path), O_RDONLY);
+ if (fd == -1) {
+ return -1;
+ }
+ do {
+ tsize = read(fd, (void *)buf, bufsz);
+ } while (tsize == -1 && errno == EINTR);
+ close(fd);
+ return tsize;
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+ }
+ return tsize;
}
static int local_close(FsContext *ctx, int fd)
@@ -262,32 +277,79 @@ err_end:
return err;
}
-static int local_symlink(FsContext *ctx, const char *oldpath,
- const char *newpath)
+
+static int local_symlink(FsContext *fs_ctx, const char *oldpath,
+ const char *newpath, FsCred *credp)
{
- return symlink(oldpath, rpath(ctx, newpath));
+ int err = -1;
+ int serrno = 0;
+ /* Determine the security model */
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ int fd;
+ ssize_t oldpath_size, write_size;
+ fd = open(rpath(fs_ctx, newpath), O_CREAT|O_EXCL|O_RDWR,
+ SM_LOCAL_MODE_BITS);
+ if (fd == -1) {
+ return fd;
+ }
+ /* Write the oldpath (target) to the file. */
+ oldpath_size = strlen(oldpath) + 1;
+ do {
+ write_size = write(fd, (void *)oldpath, oldpath_size);
+ } while (write_size == -1 && errno == EINTR);
+
+ if (write_size != oldpath_size) {
+ serrno = errno;
+ close(fd);
+ err = -1;
+ goto err_end;
+ }
+ close(fd);
+ /* Set cleint credentials in symlink's xattr */
+ credp->fc_mode = credp->fc_mode|S_IFLNK;
+ err = local_set_xattr(rpath(fs_ctx, newpath), credp);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ err = symlink(oldpath, rpath(fs_ctx, newpath));
+ if (err) {
+ return err;
+ }
+ err = chmod(rpath(fs_ctx, newpath), credp->fc_mode & 07777);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ err = chown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
+ if (err == -1) {
+ serrno = errno;
+ goto err_end;
+ }
+ }
+ return err;
+
+err_end:
+ remove(rpath(fs_ctx, newpath));
+ errno = serrno;
+ return err;
}
-static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
+static int local_link(FsContext *fs_ctx, const char *oldpath,
+ const char *newpath, FsCred *credp)
{
- char *tmp = qemu_strdup(rpath(ctx, oldpath));
- int err, serrno = 0;
+ char *tmp = qemu_strdup(rpath(fs_ctx, oldpath));
+ int err;
if (tmp == NULL) {
return -ENOMEM;
}
- err = link(tmp, rpath(ctx, newpath));
- if (err == -1) {
- serrno = errno;
- }
+ err = link(tmp, rpath(fs_ctx, newpath));
qemu_free(tmp);
- if (err == -1) {
- errno = serrno;
- }
-
return err;
}
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 38c1d68..90620aa 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -197,15 +197,25 @@ static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
return s->ops->open2(&s->ctx, vs->fullname.data, flags, &cred);
}
-static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
- V9fsString *newpath)
+static int v9fs_do_symlink(V9fsState *s, V9fsCreateState *vs)
{
- return s->ops->symlink(&s->ctx, oldpath->data, newpath->data);
+ FsCred cred;
+ cred_init(&cred);
+ cred.fc_uid = vs->fidp->uid;
+ cred.fc_mode = vs->perm | 0777;
+
+ return s->ops->symlink(&s->ctx, vs->extension.data, vs->fullname.data,
+ &cred);
}
-static int v9fs_do_link(V9fsState *s, V9fsString *oldpath, V9fsString *newpath)
+static int v9fs_do_link(V9fsState *s, V9fsFidState *nfidp, V9fsCreateState *vs)
{
- return s->ops->link(&s->ctx, oldpath->data, newpath->data);
+ FsCred cred;
+ cred_init(&cred);
+ cred.fc_uid = nfidp->uid;
+ cred.fc_mode = vs->perm & 0777;
+
+ return s->ops->link(&s->ctx, nfidp->path.data, vs->fullname.data, &cred);
}
static int v9fs_do_truncate(V9fsState *s, V9fsString *path, off_t size)
@@ -1766,7 +1776,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
err = v9fs_do_mkdir(s, vs);
v9fs_create_post_mkdir(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_SYMLINK) {
- err = v9fs_do_symlink(s, &vs->extension, &vs->fullname);
+ err = v9fs_do_symlink(s, vs);
v9fs_create_post_perms(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_LINK) {
int32_t nfid = atoi(vs->extension.data);
@@ -1775,7 +1785,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
err = -errno;
v9fs_post_create(s, vs, err);
}
- err = v9fs_do_link(s, &nfidp->path, &vs->fullname);
+ err = v9fs_do_link(s, nfidp, vs);
v9fs_create_post_perms(s, vs, err);
} else if (vs->perm & P9_STAT_MODE_DEVICE) {
char ctype;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 6/7] virtio-9p: Implemented Security model for lstat and fstat
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
` (4 preceding siblings ...)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 5/7] virtio-9p: Implemented security model for symlink and link Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp Venkateswararao Jujjuri (JV)
2010-05-27 3:52 ` [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Andy Lutomirski
7 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/virtio-9p-local.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 395a33f..11f3650 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -27,9 +27,38 @@ static const char *rpath(FsContext *ctx, const char *path)
return buffer;
}
-static int local_lstat(FsContext *ctx, const char *path, struct stat *stbuf)
+
+static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
- return lstat(rpath(ctx, path), stbuf);
+ int err;
+ err = lstat(rpath(fs_ctx, path), stbuf);
+ if (err) {
+ return err;
+ }
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ /* Actual credentials are part of extended attrs */
+ uid_t tmp_uid;
+ gid_t tmp_gid;
+ mode_t tmp_mode;
+ dev_t tmp_dev;
+ if (getxattr(rpath(fs_ctx, path), "user.virtfs.uid", &tmp_uid,
+ sizeof(uid_t)) > 0) {
+ stbuf->st_uid = tmp_uid;
+ }
+ if (getxattr(rpath(fs_ctx, path), "user.virtfs.gid", &tmp_gid,
+ sizeof(gid_t)) > 0) {
+ stbuf->st_gid = tmp_gid;
+ }
+ if (getxattr(rpath(fs_ctx, path), "user.virtfs.mode", &tmp_mode,
+ sizeof(mode_t)) > 0) {
+ stbuf->st_mode = tmp_mode;
+ }
+ if (getxattr(rpath(fs_ctx, path), "user.virtfs.rdev", &tmp_dev,
+ sizeof(dev_t)) > 0) {
+ stbuf->st_rdev = tmp_dev;
+ }
+ }
+ return err;
}
static int local_set_xattr(const char *path, FsCred *credp)
@@ -227,9 +256,34 @@ err_end:
return err;
}
-static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
+static int local_fstat(FsContext *fs_ctx, int fd, struct stat *stbuf)
{
- return fstat(fd, stbuf);
+ int err;
+ err = fstat(fd, stbuf);
+ if (err) {
+ return err;
+ }
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ /* Actual credentials are part of extended attrs */
+ uid_t tmp_uid;
+ gid_t tmp_gid;
+ mode_t tmp_mode;
+ dev_t tmp_dev;
+
+ if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+ stbuf->st_uid = tmp_uid;
+ }
+ if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+ stbuf->st_gid = tmp_gid;
+ }
+ if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
+ stbuf->st_mode = tmp_mode;
+ }
+ if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
+ stbuf->st_rdev = tmp_dev;
+ }
+ }
+ return err;
}
static int local_open2(FsContext *fs_ctx, const char *path, int flags,
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp.
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
` (5 preceding siblings ...)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 6/7] virtio-9p: Implemented Security model for lstat and fstat Venkateswararao Jujjuri (JV)
@ 2010-05-26 23:21 ` Venkateswararao Jujjuri (JV)
2010-06-01 17:33 ` Aneesh Kumar K.V
2010-05-27 3:52 ` [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Andy Lutomirski
7 siblings, 1 reply; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-26 23:21 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, Venkateswararao Jujjuri (JV)
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
hw/file-op-9p.h | 4 ++--
hw/virtio-9p-local.c | 18 ++++++++++++++----
hw/virtio-9p.c | 15 ++++++++++++---
3 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index c1c08b4..877faf2 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -49,8 +49,8 @@ typedef struct FileOperations
{
int (*lstat)(FsContext *, const char *, struct stat *);
ssize_t (*readlink)(FsContext *, const char *, char *, size_t);
- int (*chmod)(FsContext *, const char *, mode_t);
- int (*chown)(FsContext *, const char *, uid_t, gid_t);
+ int (*chmod)(FsContext *, const char *, FsCred *);
+ int (*chown)(FsContext *, const char *, FsCred *);
int (*mknod)(FsContext *, const char *, FsCred *);
int (*utime)(FsContext *, const char *, const struct utimbuf *);
int (*remove)(FsContext *, const char *);
diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
index 11f3650..f46acac 100644
--- a/hw/virtio-9p-local.c
+++ b/hw/virtio-9p-local.c
@@ -173,9 +173,14 @@ static ssize_t local_writev(FsContext *ctx, int fd, const struct iovec *iov,
return writev(fd, iov, iovcnt);
}
-static int local_chmod(FsContext *ctx, const char *path, mode_t mode)
+static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp)
{
- return chmod(rpath(ctx, path), mode);
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ return local_set_xattr(rpath(fs_ctx, path), credp);
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ return chmod(rpath(fs_ctx, path), credp->fc_mode);
+ }
+ return -1;
}
static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
@@ -436,9 +441,14 @@ static int local_rename(FsContext *ctx, const char *oldpath,
}
-static int local_chown(FsContext *ctx, const char *path, uid_t uid, gid_t gid)
+static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
{
- return chown(rpath(ctx, path), uid, gid);
+ if (fs_ctx->fs_sm == SM_MAPPED) {
+ return local_set_xattr(rpath(fs_ctx, path), credp);
+ } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ return chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
+ }
+ return -1;
}
static int local_utime(FsContext *ctx, const char *path,
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 90620aa..dceb5fc 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -154,7 +154,11 @@ static int v9fs_do_writev(V9fsState *s, int fd, const struct iovec *iov,
static int v9fs_do_chmod(V9fsState *s, V9fsString *path, mode_t mode)
{
- return s->ops->chmod(&s->ctx, path->data, mode);
+ FsCred cred;
+ cred_init(&cred);
+ cred.fc_mode = mode;
+
+ return s->ops->chmod(&s->ctx, path->data, &cred);
}
static int v9fs_do_mknod(V9fsState *s, V9fsCreateState *vs, mode_t mode,
@@ -231,7 +235,12 @@ static int v9fs_do_rename(V9fsState *s, V9fsString *oldpath,
static int v9fs_do_chown(V9fsState *s, V9fsString *path, uid_t uid, gid_t gid)
{
- return s->ops->chown(&s->ctx, path->data, uid, gid);
+ FsCred cred;
+ cred_init(&cred);
+ cred.fc_uid = uid;
+ cred.fc_gid = gid;
+
+ return s->ops->chown(&s->ctx, path->data, &cred);
}
static int v9fs_do_utime(V9fsState *s, V9fsString *path,
@@ -2022,7 +2031,7 @@ static void v9fs_wstat_post_utime(V9fsState *s, V9fsWstatState *vs, int err)
goto out;
}
- if (vs->v9stat.n_gid != -1) {
+ if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) {
if (v9fs_do_chown(s, &vs->fidp->path, vs->v9stat.n_uid,
vs->v9stat.n_gid)) {
err = -errno;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
` (6 preceding siblings ...)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp Venkateswararao Jujjuri (JV)
@ 2010-05-27 3:52 ` Andy Lutomirski
2010-05-27 17:52 ` Venkateswararao Jujjuri (JV)
7 siblings, 1 reply; 16+ messages in thread
From: Andy Lutomirski @ 2010-05-27 3:52 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: aliguori, qemu-devel
Venkateswararao Jujjuri (JV) wrote:
> This patch series introduces the security model for VirtFS.
>
> Brief description of this patch series:
>
> It introduces two type of security models for VirtFS.
> They are: mapped and passthrough.
>
> The following is common to both security models.
>
> * Client's VFS determines/enforces the access control.
> Largely server should never return EACCESS.
>
> * Client sends gid/mode-bit information as part of creation only.
>
> Changes from V3
> ---------------
> o Return NULL instead of exit(1) on failure in virtio_9p_init()
> o Capitalized sm_passthrough, sm_mappe
> o Added handling for EINTR for read/write.
> o Corrected default permissions for mkdir in mapped mode.
> o Added additional error handling.
>
> Changes from V2
> ---------------
> o Removed warnings resulting from chmod/chown.
> o Added code to fail normally if secuirty_model option is not specified.
>
> Changes from V1
> ---------------
> o Added support for chmod and chown.
> o Used chmod/chown to set credentials instead of setuid/setgid.
> o Fixed a bug where uid used instated of uid.
>
>
> Security model: mapped
> ----------------------
>
> VirtFS server(QEMU) intercepts and maps all the file object create requests.
> Files on the fileserver will be created with QEMU's user credentials and the
> client-user's credentials are stored in extended attributes.
> During getattr() server extracts the client-user's credentials from extended
> attributes and sends to the client.
>
> Given that only the user space extended attributes are available to regular
> files, special files are created as regular files on the fileserver and the
> appropriate mode bits are stored in xattrs and will be extracted during
> getattr.
>
> If the extended attributes are missing, server sends back the filesystem
> stat() unaltered. This provision will make the files created on the
> fileserver usable to client.
>
> Points to be considered
>
> * Filesystem will be VirtFS'ized. Meaning, other filesystems may not
> understand the credentials of the files created under this model.
How hard would it be to make this compatible with rsync's --fake-super?
(--fake-super already does almost what you're doing, and if you make
the formats compatible, then rsync could be used to translate. OTOH,
rsyncing a VirtFS-ified filesystem to a remote --fake-super system might
have odd side-effects.)
--Andy
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS
2010-05-27 3:52 ` [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Andy Lutomirski
@ 2010-05-27 17:52 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-05-27 17:52 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: aliguori, qemu-devel
Andy Lutomirski wrote:
> Venkateswararao Jujjuri (JV) wrote:
>> This patch series introduces the security model for VirtFS.
>>
>> Brief description of this patch series:
>>
>> It introduces two type of security models for VirtFS.
>> They are: mapped and passthrough.
>>
>> The following is common to both security models.
>>
>> * Client's VFS determines/enforces the access control.
>> Largely server should never return EACCESS.
>>
>> * Client sends gid/mode-bit information as part of creation only.
>>
>> Changes from V3
>> ---------------
>> o Return NULL instead of exit(1) on failure in virtio_9p_init()
>> o Capitalized sm_passthrough, sm_mappe
>> o Added handling for EINTR for read/write.
>> o Corrected default permissions for mkdir in mapped mode.
>> o Added additional error handling.
>>
>> Changes from V2
>> ---------------
>> o Removed warnings resulting from chmod/chown.
>> o Added code to fail normally if secuirty_model option is not specified.
>>
>> Changes from V1
>> ---------------
>> o Added support for chmod and chown.
>> o Used chmod/chown to set credentials instead of setuid/setgid.
>> o Fixed a bug where uid used instated of uid.
>>
>>
>> Security model: mapped
>> ----------------------
>>
>> VirtFS server(QEMU) intercepts and maps all the file object create
>> requests.
>> Files on the fileserver will be created with QEMU's user credentials
>> and the
>> client-user's credentials are stored in extended attributes.
>> During getattr() server extracts the client-user's credentials from
>> extended
>> attributes and sends to the client.
>>
>> Given that only the user space extended attributes are available to
>> regular
>> files, special files are created as regular files on the fileserver
>> and the
>> appropriate mode bits are stored in xattrs and will be extracted during
>> getattr.
>>
>> If the extended attributes are missing, server sends back the filesystem
>> stat() unaltered. This provision will make the files created on the
>> fileserver usable to client.
>>
>> Points to be considered
>>
>> * Filesystem will be VirtFS'ized. Meaning, other filesystems may not
>> understand the credentials of the files created under this model.
>
> How hard would it be to make this compatible with rsync's --fake-super?
> (--fake-super already does almost what you're doing, and if you make
> the formats compatible, then rsync could be used to translate. OTOH,
For conversion, I think one can write a very simple converter/translator tool to
convert VrtFS-ified file system. My main idea behind calling this out here is , generic
tools (say tools which track quotas) on the host may not interpret the meta-data
properly. IMO, Just for the sake of conversion, it may not be worth tying VirtFS
to rsync..as it may not give us free hand and things will be more complicated as we
enter into ACLs .
> rsyncing a VirtFS-ified filesystem to a remote --fake-super system might
> have odd side-effects.)
If it is done from the guest, things will be normal.
Even if it is done from host, as long as all the xattrs are restored back properly..
and there is no 'clash' between the name spaces we should be fine I guess.
We re using user.virtfs as the name space...and I don't think it clashes with that of rsync's.
- JV
>
> --Andy
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures Venkateswararao Jujjuri (JV)
@ 2010-06-01 17:22 ` Aneesh Kumar K.V
0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2010-06-01 17:22 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: aliguori, qemu-devel
On Wed, May 26, 2010 at 04:21:41PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> hw/virtio-9p.c | 185 ++++++++++++++------------------------------------------
> hw/virtio-9p.h | 92 ++++++++++++++++++++++++++++
> 2 files changed, 138 insertions(+), 139 deletions(-)
This is a cleanup patch. I guess this should not be part of this series.
-aneesh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model.
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model Venkateswararao Jujjuri (JV)
@ 2010-06-01 17:24 ` Aneesh Kumar K.V
2010-06-03 23:07 ` Venkateswararao Jujjuri (JV)
0 siblings, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2010-06-01 17:24 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: aliguori, qemu-devel
On Wed, May 26, 2010 at 04:21:40PM -0700, Venkateswararao Jujjuri (JV) wrote:
> The new option is:
>
> -fsdev fstype,id=myid,path=/share_path/,security_model=[mapped|passthrough]
> -virtfs fstype,path=/share_path/,security_model=[mapped|passthrough],mnt_tag=tag
>
> In the case of mapped security model, files are created with QEMU user
> credentials and the client-user's credentials are saved in extended attributes.
> Whereas in the case of passthrough security model, files on the
> filesystem are directly created with client-user's credentials.
>
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> fsdev/qemu-fsdev.c | 14 +++++++++++++-
> fsdev/qemu-fsdev.h | 1 +
> hw/virtio-9p.c | 22 ++++++++++++++++++----
> qemu-config.c | 12 +++++++++---
> qemu-options.hx | 15 +++++++++++----
> vl.c | 18 +++++++++++++++---
> 6 files changed, 67 insertions(+), 15 deletions(-)
>
> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
> index 813e1f7..7d7a153 100644
> --- a/fsdev/qemu-fsdev.c
> +++ b/fsdev/qemu-fsdev.c
> @@ -34,7 +34,7 @@ int qemu_fsdev_add(QemuOpts *opts)
> return -1;
> }
>
> - for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
> + for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
> if (strcmp(FsTypes[i].name, qemu_opt_get(opts, "fstype")) == 0) {
> break;
> }
Don't do whitespace fixup in the same patch.
> @@ -46,10 +46,22 @@ int qemu_fsdev_add(QemuOpts *opts)
> return -1;
> }
>
> + if (qemu_opt_get(opts, "path") == NULL) {
> + fprintf(stderr, "fsdev: No path specified.\n");
> + return -1;
> + }
> +
How is this related to new option ?
> + if (qemu_opt_get(opts, "security_model") == NULL) {
> + fprintf(stderr, "fsdev: No security_model specified.\n");
> + return -1;
> + }
> +
> fsle = qemu_malloc(sizeof(*fsle));
>
> fsle->fse.fsdev_id = qemu_strdup(qemu_opts_id(opts));
> fsle->fse.path = qemu_strdup(qemu_opt_get(opts, "path"));
> + fsle->fse.security_model = qemu_strdup(qemu_opt_get(opts,
> + "security_model"));
> fsle->fse.ops = FsTypes[i].ops;
>
> QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);
> diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
> index b50fbe0..6c27881 100644
> --- a/fsdev/qemu-fsdev.h
> +++ b/fsdev/qemu-fsdev.h
> @@ -40,6 +40,7 @@ typedef struct FsTypeTable {
> typedef struct FsTypeEntry {
> char *fsdev_id;
> char *path;
> + char *security_model;
> FileOperations *ops;
> } FsTypeEntry;
>
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 687abc0..a57562a 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -2402,7 +2402,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
> /* We don't have a fsdev identified by fsdev_id */
> fprintf(stderr, "Virtio-9p device couldn't find fsdev "
> "with the id %s\n", conf->fsdev_id);
> - exit(1);
> + return NULL;
> }
>
> if (!fse->path || !conf->tag) {
> @@ -2410,15 +2410,29 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
> fprintf(stderr, "fsdev with id %s needs path "
> "and Virtio-9p device needs mount_tag arguments\n",
> conf->fsdev_id);
> - exit(1);
> + return NULL;
> + }
> +
> + if (!strcmp(fse->security_model, "passthrough")) {
> + /* Files on the Fileserver set to client user credentials */
> + } else if (!strcmp(fse->security_model, "mapped")) {
> + /* Files on the fileserver are set to QEMU credentials.
> + * Client user credentials are saved in extended attributes.
> + */
The above two if should be dropped add them when you have something to do in the if () { }
section.
> + } else {
> + /* user haven't specified a correct security option */
> + fprintf(stderr, "one of the following must be specified as the"
> + "security option:\n\t security_model=passthrough \n\t "
> + "security_model=mapped\n");
> + return NULL;
> }
We should only have this
>
> if (lstat(fse->path, &stat)) {
> fprintf(stderr, "share path %s does not exist\n", fse->path);
> - exit(1);
> + return NULL;
> } else if (!S_ISDIR(stat.st_mode)) {
> fprintf(stderr, "share path %s is not a directory \n", fse->path);
> - exit(1);
> + return NULL;
> }
>
> s->ctx.fs_root = qemu_strdup(fse->path);
> diff --git a/qemu-config.c b/qemu-config.c
> index d500885..e1e3aa1 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -160,9 +160,12 @@ QemuOptsList qemu_fsdev_opts = {
> {
> .name = "fstype",
> .type = QEMU_OPT_STRING,
> - }, {
> + },{
> .name = "path",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "security_model",
> + .type = QEMU_OPT_STRING,
> },
> { /*End of list */ }
> },
> @@ -178,12 +181,15 @@ QemuOptsList qemu_virtfs_opts = {
> {
> .name = "fstype",
> .type = QEMU_OPT_STRING,
> - }, {
> + },{
> .name = "path",
> .type = QEMU_OPT_STRING,
> - }, {
> + },{
> .name = "mount_tag",
> .type = QEMU_OPT_STRING,
> + },{
> + .name = "security_model",
> + .type = QEMU_OPT_STRING,
> },
>
> { /*End of list */ }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 12f6b51..d557c92 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -482,7 +482,7 @@ ETEXI
> DEFHEADING(File system options:)
>
> DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
> - "-fsdev local,id=id,path=path\n",
> + "-fsdev local,id=id,path=path,security_model=[mapped|passthrough]\n",
> QEMU_ARCH_ALL)
>
> STEXI
> @@ -498,7 +498,7 @@ The specific Fstype will determine the applicable options.
>
> Options to each backend are described below.
>
> -@item -fsdev local ,id=@var{id} ,path=@var{path}
> +@item -fsdev local ,id=@var{id} ,path=@var{path} ,security_model=@var{security_model}
>
> Create a file-system-"device" for local-filesystem.
>
> @@ -506,6 +506,9 @@ Create a file-system-"device" for local-filesystem.
>
> @option{path} specifies the path to be exported. @option{path} is required.
>
> +@option{security_model} specifies the security model to be followed.
> +@option{security_model} is required.
> +
> @end table
> ETEXI
> #endif
> @@ -514,7 +517,7 @@ ETEXI
> DEFHEADING(Virtual File system pass-through options:)
>
> DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
> - "-virtfs local,path=path,mount_tag=tag\n",
> + "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough]\n",
> QEMU_ARCH_ALL)
>
> STEXI
> @@ -530,7 +533,7 @@ The specific Fstype will determine the applicable options.
>
> Options to each backend are described below.
>
> -@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag}
> +@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag} ,security_model=@var{security_model}
>
> Create a Virtual file-system-pass through for local-filesystem.
>
> @@ -538,6 +541,10 @@ Create a Virtual file-system-pass through for local-filesystem.
>
> @option{path} specifies the path to be exported. @option{path} is required.
>
> +@option{security_model} specifies the security model to be followed.
> +@option{security_model} is required.
> +
> +
> @option{mount_tag} specifies the tag with which the exported file is mounted.
> @option{mount_tag} is required.
>
> diff --git a/vl.c b/vl.c
> index 85bcc84..a341781 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3109,10 +3109,21 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
>
> - len = strlen(",id=,path=");
> + if (qemu_opt_get(opts, "fstype") == NULL ||
> + qemu_opt_get(opts, "mount_tag") == NULL ||
> + qemu_opt_get(opts, "path") == NULL ||
> + qemu_opt_get(opts, "security_model") == NULL) {
> + fprintf(stderr, "Usage: -virtfs fstype,path=/share_path/,"
> + "security_model=[mapped|passthrough],"
> + "mnt_tag=tag.\n");
> + exit(1);
> + }
> +
> + len = strlen(",id=,path=,security_model=");
> len += strlen(qemu_opt_get(opts, "fstype"));
> len += strlen(qemu_opt_get(opts, "mount_tag"));
> len += strlen(qemu_opt_get(opts, "path"));
> + len += strlen(qemu_opt_get(opts, "security_model"));
> arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
>
> if (!arg_fsdev) {
> @@ -3121,10 +3132,11 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
>
> - sprintf(arg_fsdev, "%s,id=%s,path=%s",
> + sprintf(arg_fsdev, "%s,id=%s,path=%s,security_model=%s",
> qemu_opt_get(opts, "fstype"),
> qemu_opt_get(opts, "mount_tag"),
> - qemu_opt_get(opts, "path"));
> + qemu_opt_get(opts, "path"),
> + qemu_opt_get(opts, "security_model"));
>
> len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
> len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
> --
> 1.6.5.2
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model.
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model Venkateswararao Jujjuri (JV)
@ 2010-06-01 17:30 ` Aneesh Kumar K.V
2010-06-04 0:40 ` Venkateswararao Jujjuri (JV)
0 siblings, 1 reply; 16+ messages in thread
From: Aneesh Kumar K.V @ 2010-06-01 17:30 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: aliguori, qemu-devel
On Wed, May 26, 2010 at 04:21:42PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Add required infrastructure and modify create/open2 and mkdir per the new
> security model.
>
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> hw/file-op-9p.h | 23 +++++++-
> hw/virtio-9p-local.c | 149 ++++++++++++++++++++++++++++++++++++--------------
> hw/virtio-9p.c | 42 ++++++++++----
> 3 files changed, 158 insertions(+), 56 deletions(-)
>
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index 2934ff1..73d59b2 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -19,13 +19,32 @@
> #include <sys/stat.h>
> #include <sys/uio.h>
> #include <sys/vfs.h>
> +#define SM_LOCAL_MODE_BITS 0600
> +#define SM_LOCAL_DIR_MODE_BITS 0700
> +
> +typedef enum
> +{
> + SM_PASSTHROUGH = 1, /* uid/gid set on fileserver files */
> + SM_MAPPED, /* uid/gid part of xattr */
> +} SecModel;
> +
> +typedef struct FsCred
> +{
> + uid_t fc_uid;
> + gid_t fc_gid;
> + mode_t fc_mode;
> + dev_t fc_rdev;
> +} FsCred;
>
> typedef struct FsContext
> {
> char *fs_root;
> + SecModel fs_sm;
> uid_t uid;
> } FsContext;
>
> +extern void cred_init(FsCred *);
> +
> typedef struct FileOperations
> {
> int (*lstat)(FsContext *, const char *, struct stat *);
> @@ -43,7 +62,7 @@ typedef struct FileOperations
> int (*closedir)(FsContext *, DIR *);
> DIR *(*opendir)(FsContext *, const char *);
> int (*open)(FsContext *, const char *, int);
> - int (*open2)(FsContext *, const char *, int, mode_t);
> + int (*open2)(FsContext *, const char *, int, FsCred *);
> void (*rewinddir)(FsContext *, DIR *);
> off_t (*telldir)(FsContext *, DIR *);
> struct dirent *(*readdir)(FsContext *, DIR *);
> @@ -51,7 +70,7 @@ typedef struct FileOperations
> ssize_t (*readv)(FsContext *, int, const struct iovec *, int);
> ssize_t (*writev)(FsContext *, int, const struct iovec *, int);
> off_t (*lseek)(FsContext *, int, off_t, int);
> - int (*mkdir)(FsContext *, const char *, mode_t);
> + int (*mkdir)(FsContext *, const char *, FsCred *);
> int (*fstat)(FsContext *, int, struct stat *);
> int (*rename)(FsContext *, const char *, const char *);
> int (*truncate)(FsContext *, const char *, off_t);
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index 78960ac..f6c2fe2 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -17,6 +17,7 @@
> #include <grp.h>
> #include <sys/socket.h>
> #include <sys/un.h>
> +#include <attr/xattr.h>
>
> static const char *rpath(FsContext *ctx, const char *path)
> {
> @@ -31,47 +32,39 @@ static int local_lstat(FsContext *ctx, const char *path, struct stat *stbuf)
> return lstat(rpath(ctx, path), stbuf);
> }
>
> -static int local_setuid(FsContext *ctx, uid_t uid)
> +static int local_set_xattr(const char *path, FsCred *credp)
> {
> - struct passwd *pw;
> - gid_t groups[33];
> - int ngroups;
> - static uid_t cur_uid = -1;
> -
> - if (cur_uid == uid) {
> - return 0;
> - }
> -
> - if (setreuid(0, 0)) {
> - return -1;
> - }
> -
> - pw = getpwuid(uid);
> - if (pw == NULL) {
> - return -1;
> + int err;
> + if (credp->fc_uid != -1) {
> + err = setxattr(path, "user.virtfs.uid", &credp->fc_uid, sizeof(uid_t),
> + 0);
> + if (err) {
> + return err;
> + }
> }
> -
> - ngroups = 33;
> - if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) {
> - return -1;
> + if (credp->fc_gid != -1) {
> + err = setxattr(path, "user.virtfs.gid", &credp->fc_gid, sizeof(gid_t),
> + 0);
> + if (err) {
> + return err;
> + }
> }
> -
> - if (setgroups(ngroups, groups)) {
> - return -1;
> - }
> -
> - if (setregid(-1, pw->pw_gid)) {
> - return -1;
> + if (credp->fc_mode != -1) {
> + err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
> + sizeof(mode_t), 0);
> + if (err) {
> + return err;
> + }
> }
> -
> - if (setreuid(-1, uid)) {
> - return -1;
> + if (credp->fc_rdev != -1) {
> + err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
> + sizeof(dev_t), 0);
> + if (err) {
> + return err;
> + }
> }
> -
> - cur_uid = uid;
> -
> - return 0;
> -}
> + return 0;
> + }
>
> static ssize_t local_readlink(FsContext *ctx, const char *path,
> char *buf, size_t bufsz)
> @@ -168,9 +161,44 @@ static int local_mksock(FsContext *ctx2, const char *path)
> return 0;
> }
>
> -static int local_mkdir(FsContext *ctx, const char *path, mode_t mode)
> +static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
> {
> - return mkdir(rpath(ctx, path), mode);
> + int err = -1;
> + int serrno = 0;
> + /* Determine the security model */
> + if (fs_ctx->fs_sm == SM_MAPPED) {
> + err = mkdir(rpath(fs_ctx, path), SM_LOCAL_DIR_MODE_BITS);
> + if (err == -1) {
> + return err;
> + }
> + credp->fc_mode = credp->fc_mode|S_IFDIR;
> + err = local_set_xattr(rpath(fs_ctx, path), credp);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> + err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
> + if (err == -1) {
> + return err;
> + }
> + err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + }
> + return err;
> +
> +err_end:
> + remove(rpath(fs_ctx, path));
> + errno = serrno;
> + return err;
> }
>
> static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
> @@ -178,9 +206,49 @@ static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
> return fstat(fd, stbuf);
> }
>
> -static int local_open2(FsContext *ctx, const char *path, int flags, mode_t mode)
> +static int local_open2(FsContext *fs_ctx, const char *path, int flags,
> + FsCred *credp)
> {
> - return open(rpath(ctx, path), flags, mode);
> + int fd = -1;
> + int err = -1;
> + int serrno = 0;
> + /* Determine the security model */
> + if (fs_ctx->fs_sm == SM_MAPPED) {
> + int err;
> + fd = open(rpath(fs_ctx, path), flags, SM_LOCAL_MODE_BITS);
> + if (fd == -1) {
> + return fd;
> + }
> + credp->fc_mode = credp->fc_mode|S_IFREG;
> + /* Set cleint credentials in xattr */
> + err = local_set_xattr(rpath(fs_ctx, path), credp);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> + fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
> + if (fd == -1) {
> + return fd;
> + }
> + err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
> + if (err == -1) {
> + serrno = errno;
> + goto err_end;
> + }
> + }
> + return fd;
> +
> +err_end:
> + close(fd);
> + remove(rpath(fs_ctx, path));
> + errno = serrno;
> + return err;
> }
>
> static int local_symlink(FsContext *ctx, const char *oldpath,
> @@ -269,7 +337,6 @@ static int local_statfs(FsContext *s, const char *path, struct statfs *stbuf)
>
> FileOperations local_ops = {
> .lstat = local_lstat,
> - .setuid = local_setuid,
> .readlink = local_readlink,
> .close = local_close,
> .closedir = local_closedir,
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 6feaa53..ef870a3 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -67,14 +67,17 @@ static int omode_to_uflags(int8_t mode)
> return ret;
> }
>
> -static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
> +void cred_init(FsCred *credp)
> {
> - return s->ops->lstat(&s->ctx, path->data, stbuf);
> + credp->fc_uid = -1;
> + credp->fc_gid = -1;
> + credp->fc_mode = -1;
> + credp->fc_rdev = -1;
> }
>
> -static int v9fs_do_setuid(V9fsState *s, uid_t uid)
> +static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
> {
> - return s->ops->setuid(&s->ctx, uid);
> + return s->ops->lstat(&s->ctx, path->data, stbuf);
> }
>
> static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
> @@ -164,9 +167,15 @@ static int v9fs_do_mksock(V9fsState *s, V9fsString *path)
> return s->ops->mksock(&s->ctx, path->data);
> }
>
> -static int v9fs_do_mkdir(V9fsState *s, V9fsString *path, mode_t mode)
> +static int v9fs_do_mkdir(V9fsState *s, V9fsCreateState *vs)
> {
> - return s->ops->mkdir(&s->ctx, path->data, mode);
> + FsCred cred;
> +
> + cred_init(&cred);
> + cred.fc_uid = vs->fidp->uid;
> + cred.fc_mode = vs->perm & 0777;
> +
> + return s->ops->mkdir(&s->ctx, vs->fullname.data, &cred);
> }
>
> static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
> @@ -174,9 +183,17 @@ static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
> return s->ops->fstat(&s->ctx, fd, stbuf);
> }
>
> -static int v9fs_do_open2(V9fsState *s, V9fsString *path, int flags, mode_t mode)
> +static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
> {
> - return s->ops->open2(&s->ctx, path->data, flags, mode);
> + FsCred cred;
> + int flags;
> +
> + cred_init(&cred);
> + cred.fc_uid = vs->fidp->uid;
> + cred.fc_mode = vs->perm & 0777;
> + flags = omode_to_uflags(vs->mode) | O_CREAT;
> +
> + return s->ops->open2(&s->ctx, vs->fullname.data, flags, &cred);
> }
>
> static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
> @@ -348,7 +365,6 @@ static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid)
>
> for (f = s->fid_list; f; f = f->next) {
> if (f->fid == fid) {
> - v9fs_do_setuid(s, f->uid);
> return f;
> }
> }
> @@ -1762,7 +1778,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
> }
>
> if (vs->perm & P9_STAT_MODE_DIR) {
> - err = v9fs_do_mkdir(s, &vs->fullname, vs->perm & 0777);
> + err = v9fs_do_mkdir(s, vs);
> v9fs_create_post_mkdir(s, vs, err);
> } else if (vs->perm & P9_STAT_MODE_SYMLINK) {
> err = v9fs_do_symlink(s, &vs->extension, &vs->fullname);
> @@ -1809,9 +1825,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
> err = v9fs_do_mksock(s, &vs->fullname);
> v9fs_create_post_mksock(s, vs, err);
> } else {
> - vs->fidp->fd = v9fs_do_open2(s, &vs->fullname,
> - omode_to_uflags(vs->mode) | O_CREAT,
> - vs->perm & 0777);
> + vs->fidp->fd = v9fs_do_open2(s, vs);
> v9fs_create_post_open2(s, vs, err);
> }
>
> @@ -2322,10 +2336,12 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>
> if (!strcmp(fse->security_model, "passthrough")) {
> /* Files on the Fileserver set to client user credentials */
> + s->ctx.fs_sm = SM_PASSTHROUGH;
> } else if (!strcmp(fse->security_model, "mapped")) {
> /* Files on the fileserver are set to QEMU credentials.
> * Client user credentials are saved in extended attributes.
> */
> + s->ctx.fs_sm = SM_MAPPED;
> } else {
> /* user haven't specified a correct security option */
> fprintf(stderr, "one of the following must be specified as the"
> --
> 1.6.5.2
>
>
Can you split this to two patch.
1) Introduce just the security model and related xattr
2) Introduce the open2 section
Can we get something like the below
security_chmod(..)
{
if (fs_ctx->fs_sm == SM_MAPPED) {
} else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
} else
return -1;
}
static int local_open2(FsContext *fs_ctx, const char *path, int flags,
FsCred *credp)
{
fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
if (fd == -1) {
return fd;
}
err = security_chmod(rpath(fs_ctx, path), credp);
if (err == -1) {
serrno = errno;
goto err_end;
}
err = security_chown(rpath(fs_ctx, path), credp);
if (err == -1) {
serrno = errno;
goto err_end;
}
return fd;
err_end:
close(fd);
remove(rpath(fs_ctx, path));
errno = serrno;
return err;
}
So you basically introduce security_chmod ( I guess we need a better name) in patch 1
That also tells us better how is chmod different between two security models.
-aneesh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp.
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp Venkateswararao Jujjuri (JV)
@ 2010-06-01 17:33 ` Aneesh Kumar K.V
0 siblings, 0 replies; 16+ messages in thread
From: Aneesh Kumar K.V @ 2010-06-01 17:33 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: aliguori, qemu-devel
On Wed, May 26, 2010 at 04:21:46PM -0700, Venkateswararao Jujjuri (JV) wrote:
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> hw/file-op-9p.h | 4 ++--
> hw/virtio-9p-local.c | 18 ++++++++++++++----
> hw/virtio-9p.c | 15 ++++++++++++---
> 3 files changed, 28 insertions(+), 9 deletions(-)
>
> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
> index c1c08b4..877faf2 100644
> --- a/hw/file-op-9p.h
> +++ b/hw/file-op-9p.h
> @@ -49,8 +49,8 @@ typedef struct FileOperations
> {
> int (*lstat)(FsContext *, const char *, struct stat *);
> ssize_t (*readlink)(FsContext *, const char *, char *, size_t);
> - int (*chmod)(FsContext *, const char *, mode_t);
> - int (*chown)(FsContext *, const char *, uid_t, gid_t);
> + int (*chmod)(FsContext *, const char *, FsCred *);
> + int (*chown)(FsContext *, const char *, FsCred *);
> int (*mknod)(FsContext *, const char *, FsCred *);
> int (*utime)(FsContext *, const char *, const struct utimbuf *);
> int (*remove)(FsContext *, const char *);
> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
> index 11f3650..f46acac 100644
> --- a/hw/virtio-9p-local.c
> +++ b/hw/virtio-9p-local.c
> @@ -173,9 +173,14 @@ static ssize_t local_writev(FsContext *ctx, int fd, const struct iovec *iov,
> return writev(fd, iov, iovcnt);
> }
>
> -static int local_chmod(FsContext *ctx, const char *path, mode_t mode)
> +static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp)
> {
> - return chmod(rpath(ctx, path), mode);
> + if (fs_ctx->fs_sm == SM_MAPPED) {
> + return local_set_xattr(rpath(fs_ctx, path), credp);
> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> + return chmod(rpath(fs_ctx, path), credp->fc_mode);
> + }
> + return -1;
> }
You should use this in open2.
>
> static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
> @@ -436,9 +441,14 @@ static int local_rename(FsContext *ctx, const char *oldpath,
>
> }
>
> -static int local_chown(FsContext *ctx, const char *path, uid_t uid, gid_t gid)
> +static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
> {
> - return chown(rpath(ctx, path), uid, gid);
> + if (fs_ctx->fs_sm == SM_MAPPED) {
> + return local_set_xattr(rpath(fs_ctx, path), credp);
> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
> + return chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
> + }
> + return -1;
> }
>
Same here. So that we don't have
if (fs_ctx->fs_sm == SM_MAPPED) spread in the open2 code but is logically
grouped at the right place.
> static int local_utime(FsContext *ctx, const char *path,
> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
> index 90620aa..dceb5fc 100644
> --- a/hw/virtio-9p.c
> +++ b/hw/virtio-9p.c
> @@ -154,7 +154,11 @@ static int v9fs_do_writev(V9fsState *s, int fd, const struct iovec *iov,
>
> static int v9fs_do_chmod(V9fsState *s, V9fsString *path, mode_t mode)
> {
> - return s->ops->chmod(&s->ctx, path->data, mode);
> + FsCred cred;
> + cred_init(&cred);
> + cred.fc_mode = mode;
> +
> + return s->ops->chmod(&s->ctx, path->data, &cred);
> }
>
> static int v9fs_do_mknod(V9fsState *s, V9fsCreateState *vs, mode_t mode,
> @@ -231,7 +235,12 @@ static int v9fs_do_rename(V9fsState *s, V9fsString *oldpath,
>
> static int v9fs_do_chown(V9fsState *s, V9fsString *path, uid_t uid, gid_t gid)
> {
> - return s->ops->chown(&s->ctx, path->data, uid, gid);
> + FsCred cred;
> + cred_init(&cred);
> + cred.fc_uid = uid;
> + cred.fc_gid = gid;
> +
> + return s->ops->chown(&s->ctx, path->data, &cred);
> }
>
> static int v9fs_do_utime(V9fsState *s, V9fsString *path,
> @@ -2022,7 +2031,7 @@ static void v9fs_wstat_post_utime(V9fsState *s, V9fsWstatState *vs, int err)
> goto out;
> }
>
> - if (vs->v9stat.n_gid != -1) {
> + if (vs->v9stat.n_gid != -1 || vs->v9stat.n_uid != -1) {
> if (v9fs_do_chown(s, &vs->fidp->path, vs->v9stat.n_uid,
> vs->v9stat.n_gid)) {
> err = -errno;
> --
> 1.6.5.2
>
>
-aneesh
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model.
2010-06-01 17:24 ` Aneesh Kumar K.V
@ 2010-06-03 23:07 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-06-03 23:07 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: aliguori, qemu-devel
Aneesh Kumar K.V wrote:
> On Wed, May 26, 2010 at 04:21:40PM -0700, Venkateswararao Jujjuri (JV) wrote:
>> The new option is:
>>
>> -fsdev fstype,id=myid,path=/share_path/,security_model=[mapped|passthrough]
>> -virtfs fstype,path=/share_path/,security_model=[mapped|passthrough],mnt_tag=tag
>>
>> In the case of mapped security model, files are created with QEMU user
>> credentials and the client-user's credentials are saved in extended attributes.
>> Whereas in the case of passthrough security model, files on the
>> filesystem are directly created with client-user's credentials.
>>
>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>> ---
>> fsdev/qemu-fsdev.c | 14 +++++++++++++-
>> fsdev/qemu-fsdev.h | 1 +
>> hw/virtio-9p.c | 22 ++++++++++++++++++----
>> qemu-config.c | 12 +++++++++---
>> qemu-options.hx | 15 +++++++++++----
>> vl.c | 18 +++++++++++++++---
>> 6 files changed, 67 insertions(+), 15 deletions(-)
>>
>> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
>> index 813e1f7..7d7a153 100644
>> --- a/fsdev/qemu-fsdev.c
>> +++ b/fsdev/qemu-fsdev.c
>> @@ -34,7 +34,7 @@ int qemu_fsdev_add(QemuOpts *opts)
>> return -1;
>> }
>>
>> - for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
>> + for (i = 0; i < ARRAY_SIZE(FsTypes); i++) {
>> if (strcmp(FsTypes[i].name, qemu_opt_get(opts, "fstype")) == 0) {
>> break;
>> }
>
>
> Don't do whitespace fixup in the same patch.
Ok.
>
>
>> @@ -46,10 +46,22 @@ int qemu_fsdev_add(QemuOpts *opts)
>> return -1;
>> }
>>
>> + if (qemu_opt_get(opts, "path") == NULL) {
>> + fprintf(stderr, "fsdev: No path specified.\n");
>> + return -1;
>> + }
>> +
>
>
> How is this related to new option ?
Not related. Will send out another patch.
>
>
>> + if (qemu_opt_get(opts, "security_model") == NULL) {
>> + fprintf(stderr, "fsdev: No security_model specified.\n");
>> + return -1;
>> + }
>> +
>> fsle = qemu_malloc(sizeof(*fsle));
>>
>> fsle->fse.fsdev_id = qemu_strdup(qemu_opts_id(opts));
>> fsle->fse.path = qemu_strdup(qemu_opt_get(opts, "path"));
>> + fsle->fse.security_model = qemu_strdup(qemu_opt_get(opts,
>> + "security_model"));
>> fsle->fse.ops = FsTypes[i].ops;
>>
>> QTAILQ_INSERT_TAIL(&fstype_entries, fsle, next);
>> diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
>> index b50fbe0..6c27881 100644
>> --- a/fsdev/qemu-fsdev.h
>> +++ b/fsdev/qemu-fsdev.h
>> @@ -40,6 +40,7 @@ typedef struct FsTypeTable {
>> typedef struct FsTypeEntry {
>> char *fsdev_id;
>> char *path;
>> + char *security_model;
>> FileOperations *ops;
>> } FsTypeEntry;
>>
>> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
>> index 687abc0..a57562a 100644
>> --- a/hw/virtio-9p.c
>> +++ b/hw/virtio-9p.c
>> @@ -2402,7 +2402,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>> /* We don't have a fsdev identified by fsdev_id */
>> fprintf(stderr, "Virtio-9p device couldn't find fsdev "
>> "with the id %s\n", conf->fsdev_id);
>> - exit(1);
>> + return NULL;
>> }
>>
>> if (!fse->path || !conf->tag) {
>> @@ -2410,15 +2410,29 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>> fprintf(stderr, "fsdev with id %s needs path "
>> "and Virtio-9p device needs mount_tag arguments\n",
>> conf->fsdev_id);
>> - exit(1);
>> + return NULL;
>> + }
>> +
>> + if (!strcmp(fse->security_model, "passthrough")) {
>> + /* Files on the Fileserver set to client user credentials */
>> + } else if (!strcmp(fse->security_model, "mapped")) {
>> + /* Files on the fileserver are set to QEMU credentials.
>> + * Client user credentials are saved in extended attributes.
>> + */
>
>
> The above two if should be dropped add them when you have something to do in the if () { }
> section.
>
>
>> + } else {
>> + /* user haven't specified a correct security option */
>> + fprintf(stderr, "one of the following must be specified as the"
>> + "security option:\n\t security_model=passthrough \n\t "
>> + "security_model=mapped\n");
>> + return NULL;
>> }
>
> We should only have this
Well, it is tricky. Given that we should not fix/change the code in the previous patches
in the same series, code becomes ugly without the above place holders.
Given that I can't change this part of the code in my next patch in the series.. I need to go like this:
if ( !strcmp(fse->security_model, "passthrough") && !strcmp(fse->security_model, "mapped"))
{
Error;
Return NULL;
}
Then in the next patch,
Below this i need to add in the next patch.
if (!strcmp(fse->security_model, "passthrough"))
blah
if (!strcmp(fse->security_model, "mapped"))
blah
Which makes the code to check do two srrncmps for each model.
Hence If there is no major objection.. I would like to keep it this way.
Thanks,
JV
>
>
>
>>
>> if (lstat(fse->path, &stat)) {
>> fprintf(stderr, "share path %s does not exist\n", fse->path);
>> - exit(1);
>> + return NULL;
>> } else if (!S_ISDIR(stat.st_mode)) {
>> fprintf(stderr, "share path %s is not a directory \n", fse->path);
>> - exit(1);
>> + return NULL;
>> }
>>
>> s->ctx.fs_root = qemu_strdup(fse->path);
>> diff --git a/qemu-config.c b/qemu-config.c
>> index d500885..e1e3aa1 100644
>> --- a/qemu-config.c
>> +++ b/qemu-config.c
>> @@ -160,9 +160,12 @@ QemuOptsList qemu_fsdev_opts = {
>> {
>> .name = "fstype",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "path",
>> .type = QEMU_OPT_STRING,
>> + },{
>> + .name = "security_model",
>> + .type = QEMU_OPT_STRING,
>> },
>> { /*End of list */ }
>> },
>> @@ -178,12 +181,15 @@ QemuOptsList qemu_virtfs_opts = {
>> {
>> .name = "fstype",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "path",
>> .type = QEMU_OPT_STRING,
>> - }, {
>> + },{
>> .name = "mount_tag",
>> .type = QEMU_OPT_STRING,
>> + },{
>> + .name = "security_model",
>> + .type = QEMU_OPT_STRING,
>> },
>>
>> { /*End of list */ }
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 12f6b51..d557c92 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -482,7 +482,7 @@ ETEXI
>> DEFHEADING(File system options:)
>>
>> DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
>> - "-fsdev local,id=id,path=path\n",
>> + "-fsdev local,id=id,path=path,security_model=[mapped|passthrough]\n",
>> QEMU_ARCH_ALL)
>>
>> STEXI
>> @@ -498,7 +498,7 @@ The specific Fstype will determine the applicable options.
>>
>> Options to each backend are described below.
>>
>> -@item -fsdev local ,id=@var{id} ,path=@var{path}
>> +@item -fsdev local ,id=@var{id} ,path=@var{path} ,security_model=@var{security_model}
>>
>> Create a file-system-"device" for local-filesystem.
>>
>> @@ -506,6 +506,9 @@ Create a file-system-"device" for local-filesystem.
>>
>> @option{path} specifies the path to be exported. @option{path} is required.
>>
>> +@option{security_model} specifies the security model to be followed.
>> +@option{security_model} is required.
>> +
>> @end table
>> ETEXI
>> #endif
>> @@ -514,7 +517,7 @@ ETEXI
>> DEFHEADING(Virtual File system pass-through options:)
>>
>> DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
>> - "-virtfs local,path=path,mount_tag=tag\n",
>> + "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough]\n",
>> QEMU_ARCH_ALL)
>>
>> STEXI
>> @@ -530,7 +533,7 @@ The specific Fstype will determine the applicable options.
>>
>> Options to each backend are described below.
>>
>> -@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag}
>> +@item -virtfs local ,path=@var{path} ,mount_tag=@var{mount_tag} ,security_model=@var{security_model}
>>
>> Create a Virtual file-system-pass through for local-filesystem.
>>
>> @@ -538,6 +541,10 @@ Create a Virtual file-system-pass through for local-filesystem.
>>
>> @option{path} specifies the path to be exported. @option{path} is required.
>>
>> +@option{security_model} specifies the security model to be followed.
>> +@option{security_model} is required.
>> +
>> +
>> @option{mount_tag} specifies the tag with which the exported file is mounted.
>> @option{mount_tag} is required.
>>
>> diff --git a/vl.c b/vl.c
>> index 85bcc84..a341781 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3109,10 +3109,21 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> - len = strlen(",id=,path=");
>> + if (qemu_opt_get(opts, "fstype") == NULL ||
>> + qemu_opt_get(opts, "mount_tag") == NULL ||
>> + qemu_opt_get(opts, "path") == NULL ||
>> + qemu_opt_get(opts, "security_model") == NULL) {
>> + fprintf(stderr, "Usage: -virtfs fstype,path=/share_path/,"
>> + "security_model=[mapped|passthrough],"
>> + "mnt_tag=tag.\n");
>> + exit(1);
>> + }
>> +
>> + len = strlen(",id=,path=,security_model=");
>> len += strlen(qemu_opt_get(opts, "fstype"));
>> len += strlen(qemu_opt_get(opts, "mount_tag"));
>> len += strlen(qemu_opt_get(opts, "path"));
>> + len += strlen(qemu_opt_get(opts, "security_model"));
>> arg_fsdev = qemu_malloc((len + 1) * sizeof(*arg_fsdev));
>>
>> if (!arg_fsdev) {
>> @@ -3121,10 +3132,11 @@ int main(int argc, char **argv, char **envp)
>> exit(1);
>> }
>>
>> - sprintf(arg_fsdev, "%s,id=%s,path=%s",
>> + sprintf(arg_fsdev, "%s,id=%s,path=%s,security_model=%s",
>> qemu_opt_get(opts, "fstype"),
>> qemu_opt_get(opts, "mount_tag"),
>> - qemu_opt_get(opts, "path"));
>> + qemu_opt_get(opts, "path"),
>> + qemu_opt_get(opts, "security_model"));
>>
>> len = strlen("virtio-9p-pci,fsdev=,mount_tag=");
>> len += 2*strlen(qemu_opt_get(opts, "mount_tag"));
>> --
>> 1.6.5.2
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model.
2010-06-01 17:30 ` Aneesh Kumar K.V
@ 2010-06-04 0:40 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 16+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2010-06-04 0:40 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: aliguori, qemu-devel
Aneesh Kumar K.V wrote:
> On Wed, May 26, 2010 at 04:21:42PM -0700, Venkateswararao Jujjuri (JV) wrote:
>> Add required infrastructure and modify create/open2 and mkdir per the new
>> security model.
>>
>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>> ---
>> hw/file-op-9p.h | 23 +++++++-
>> hw/virtio-9p-local.c | 149 ++++++++++++++++++++++++++++++++++++--------------
>> hw/virtio-9p.c | 42 ++++++++++----
>> 3 files changed, 158 insertions(+), 56 deletions(-)
>>
>> diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
>> index 2934ff1..73d59b2 100644
>> --- a/hw/file-op-9p.h
>> +++ b/hw/file-op-9p.h
>> @@ -19,13 +19,32 @@
>> #include <sys/stat.h>
>> #include <sys/uio.h>
>> #include <sys/vfs.h>
>> +#define SM_LOCAL_MODE_BITS 0600
>> +#define SM_LOCAL_DIR_MODE_BITS 0700
>> +
>> +typedef enum
>> +{
>> + SM_PASSTHROUGH = 1, /* uid/gid set on fileserver files */
>> + SM_MAPPED, /* uid/gid part of xattr */
>> +} SecModel;
>> +
>> +typedef struct FsCred
>> +{
>> + uid_t fc_uid;
>> + gid_t fc_gid;
>> + mode_t fc_mode;
>> + dev_t fc_rdev;
>> +} FsCred;
>>
>> typedef struct FsContext
>> {
>> char *fs_root;
>> + SecModel fs_sm;
>> uid_t uid;
>> } FsContext;
>>
>> +extern void cred_init(FsCred *);
>> +
>> typedef struct FileOperations
>> {
>> int (*lstat)(FsContext *, const char *, struct stat *);
>> @@ -43,7 +62,7 @@ typedef struct FileOperations
>> int (*closedir)(FsContext *, DIR *);
>> DIR *(*opendir)(FsContext *, const char *);
>> int (*open)(FsContext *, const char *, int);
>> - int (*open2)(FsContext *, const char *, int, mode_t);
>> + int (*open2)(FsContext *, const char *, int, FsCred *);
>> void (*rewinddir)(FsContext *, DIR *);
>> off_t (*telldir)(FsContext *, DIR *);
>> struct dirent *(*readdir)(FsContext *, DIR *);
>> @@ -51,7 +70,7 @@ typedef struct FileOperations
>> ssize_t (*readv)(FsContext *, int, const struct iovec *, int);
>> ssize_t (*writev)(FsContext *, int, const struct iovec *, int);
>> off_t (*lseek)(FsContext *, int, off_t, int);
>> - int (*mkdir)(FsContext *, const char *, mode_t);
>> + int (*mkdir)(FsContext *, const char *, FsCred *);
>> int (*fstat)(FsContext *, int, struct stat *);
>> int (*rename)(FsContext *, const char *, const char *);
>> int (*truncate)(FsContext *, const char *, off_t);
>> diff --git a/hw/virtio-9p-local.c b/hw/virtio-9p-local.c
>> index 78960ac..f6c2fe2 100644
>> --- a/hw/virtio-9p-local.c
>> +++ b/hw/virtio-9p-local.c
>> @@ -17,6 +17,7 @@
>> #include <grp.h>
>> #include <sys/socket.h>
>> #include <sys/un.h>
>> +#include <attr/xattr.h>
>>
>> static const char *rpath(FsContext *ctx, const char *path)
>> {
>> @@ -31,47 +32,39 @@ static int local_lstat(FsContext *ctx, const char *path, struct stat *stbuf)
>> return lstat(rpath(ctx, path), stbuf);
>> }
>>
>> -static int local_setuid(FsContext *ctx, uid_t uid)
>> +static int local_set_xattr(const char *path, FsCred *credp)
>> {
>> - struct passwd *pw;
>> - gid_t groups[33];
>> - int ngroups;
>> - static uid_t cur_uid = -1;
>> -
>> - if (cur_uid == uid) {
>> - return 0;
>> - }
>> -
>> - if (setreuid(0, 0)) {
>> - return -1;
>> - }
>> -
>> - pw = getpwuid(uid);
>> - if (pw == NULL) {
>> - return -1;
>> + int err;
>> + if (credp->fc_uid != -1) {
>> + err = setxattr(path, "user.virtfs.uid", &credp->fc_uid, sizeof(uid_t),
>> + 0);
>> + if (err) {
>> + return err;
>> + }
>> }
>> -
>> - ngroups = 33;
>> - if (getgrouplist(pw->pw_name, pw->pw_gid, groups, &ngroups) == -1) {
>> - return -1;
>> + if (credp->fc_gid != -1) {
>> + err = setxattr(path, "user.virtfs.gid", &credp->fc_gid, sizeof(gid_t),
>> + 0);
>> + if (err) {
>> + return err;
>> + }
>> }
>> -
>> - if (setgroups(ngroups, groups)) {
>> - return -1;
>> - }
>> -
>> - if (setregid(-1, pw->pw_gid)) {
>> - return -1;
>> + if (credp->fc_mode != -1) {
>> + err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
>> + sizeof(mode_t), 0);
>> + if (err) {
>> + return err;
>> + }
>> }
>> -
>> - if (setreuid(-1, uid)) {
>> - return -1;
>> + if (credp->fc_rdev != -1) {
>> + err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
>> + sizeof(dev_t), 0);
>> + if (err) {
>> + return err;
>> + }
>> }
>> -
>> - cur_uid = uid;
>> -
>> - return 0;
>> -}
>> + return 0;
>> + }
>>
>> static ssize_t local_readlink(FsContext *ctx, const char *path,
>> char *buf, size_t bufsz)
>> @@ -168,9 +161,44 @@ static int local_mksock(FsContext *ctx2, const char *path)
>> return 0;
>> }
>>
>> -static int local_mkdir(FsContext *ctx, const char *path, mode_t mode)
>> +static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
>> {
>> - return mkdir(rpath(ctx, path), mode);
>> + int err = -1;
>> + int serrno = 0;
>> + /* Determine the security model */
>> + if (fs_ctx->fs_sm == SM_MAPPED) {
>> + err = mkdir(rpath(fs_ctx, path), SM_LOCAL_DIR_MODE_BITS);
>> + if (err == -1) {
>> + return err;
>> + }
>> + credp->fc_mode = credp->fc_mode|S_IFDIR;
>> + err = local_set_xattr(rpath(fs_ctx, path), credp);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
>> + err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
>> + if (err == -1) {
>> + return err;
>> + }
>> + err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + }
>> + return err;
>> +
>> +err_end:
>> + remove(rpath(fs_ctx, path));
>> + errno = serrno;
>> + return err;
>> }
>>
>> static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
>> @@ -178,9 +206,49 @@ static int local_fstat(FsContext *ctx, int fd, struct stat *stbuf)
>> return fstat(fd, stbuf);
>> }
>>
>> -static int local_open2(FsContext *ctx, const char *path, int flags, mode_t mode)
>> +static int local_open2(FsContext *fs_ctx, const char *path, int flags,
>> + FsCred *credp)
>> {
>> - return open(rpath(ctx, path), flags, mode);
>> + int fd = -1;
>> + int err = -1;
>> + int serrno = 0;
>> + /* Determine the security model */
>> + if (fs_ctx->fs_sm == SM_MAPPED) {
>> + int err;
>> + fd = open(rpath(fs_ctx, path), flags, SM_LOCAL_MODE_BITS);
>> + if (fd == -1) {
>> + return fd;
>> + }
>> + credp->fc_mode = credp->fc_mode|S_IFREG;
>> + /* Set cleint credentials in xattr */
>> + err = local_set_xattr(rpath(fs_ctx, path), credp);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
>> + fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
>> + if (fd == -1) {
>> + return fd;
>> + }
>> + err = chmod(rpath(fs_ctx, path), credp->fc_mode & 07777);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + err = chown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
>> + if (err == -1) {
>> + serrno = errno;
>> + goto err_end;
>> + }
>> + }
>> + return fd;
>> +
>> +err_end:
>> + close(fd);
>> + remove(rpath(fs_ctx, path));
>> + errno = serrno;
>> + return err;
>> }
>>
>> static int local_symlink(FsContext *ctx, const char *oldpath,
>> @@ -269,7 +337,6 @@ static int local_statfs(FsContext *s, const char *path, struct statfs *stbuf)
>>
>> FileOperations local_ops = {
>> .lstat = local_lstat,
>> - .setuid = local_setuid,
>> .readlink = local_readlink,
>> .close = local_close,
>> .closedir = local_closedir,
>> diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
>> index 6feaa53..ef870a3 100644
>> --- a/hw/virtio-9p.c
>> +++ b/hw/virtio-9p.c
>> @@ -67,14 +67,17 @@ static int omode_to_uflags(int8_t mode)
>> return ret;
>> }
>>
>> -static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
>> +void cred_init(FsCred *credp)
>> {
>> - return s->ops->lstat(&s->ctx, path->data, stbuf);
>> + credp->fc_uid = -1;
>> + credp->fc_gid = -1;
>> + credp->fc_mode = -1;
>> + credp->fc_rdev = -1;
>> }
>>
>> -static int v9fs_do_setuid(V9fsState *s, uid_t uid)
>> +static int v9fs_do_lstat(V9fsState *s, V9fsString *path, struct stat *stbuf)
>> {
>> - return s->ops->setuid(&s->ctx, uid);
>> + return s->ops->lstat(&s->ctx, path->data, stbuf);
>> }
>>
>> static ssize_t v9fs_do_readlink(V9fsState *s, V9fsString *path, V9fsString *buf)
>> @@ -164,9 +167,15 @@ static int v9fs_do_mksock(V9fsState *s, V9fsString *path)
>> return s->ops->mksock(&s->ctx, path->data);
>> }
>>
>> -static int v9fs_do_mkdir(V9fsState *s, V9fsString *path, mode_t mode)
>> +static int v9fs_do_mkdir(V9fsState *s, V9fsCreateState *vs)
>> {
>> - return s->ops->mkdir(&s->ctx, path->data, mode);
>> + FsCred cred;
>> +
>> + cred_init(&cred);
>> + cred.fc_uid = vs->fidp->uid;
>> + cred.fc_mode = vs->perm & 0777;
>> +
>> + return s->ops->mkdir(&s->ctx, vs->fullname.data, &cred);
>> }
>>
>> static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
>> @@ -174,9 +183,17 @@ static int v9fs_do_fstat(V9fsState *s, int fd, struct stat *stbuf)
>> return s->ops->fstat(&s->ctx, fd, stbuf);
>> }
>>
>> -static int v9fs_do_open2(V9fsState *s, V9fsString *path, int flags, mode_t mode)
>> +static int v9fs_do_open2(V9fsState *s, V9fsCreateState *vs)
>> {
>> - return s->ops->open2(&s->ctx, path->data, flags, mode);
>> + FsCred cred;
>> + int flags;
>> +
>> + cred_init(&cred);
>> + cred.fc_uid = vs->fidp->uid;
>> + cred.fc_mode = vs->perm & 0777;
>> + flags = omode_to_uflags(vs->mode) | O_CREAT;
>> +
>> + return s->ops->open2(&s->ctx, vs->fullname.data, flags, &cred);
>> }
>>
>> static int v9fs_do_symlink(V9fsState *s, V9fsString *oldpath,
>> @@ -348,7 +365,6 @@ static V9fsFidState *lookup_fid(V9fsState *s, int32_t fid)
>>
>> for (f = s->fid_list; f; f = f->next) {
>> if (f->fid == fid) {
>> - v9fs_do_setuid(s, f->uid);
>> return f;
>> }
>> }
>> @@ -1762,7 +1778,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
>> }
>>
>> if (vs->perm & P9_STAT_MODE_DIR) {
>> - err = v9fs_do_mkdir(s, &vs->fullname, vs->perm & 0777);
>> + err = v9fs_do_mkdir(s, vs);
>> v9fs_create_post_mkdir(s, vs, err);
>> } else if (vs->perm & P9_STAT_MODE_SYMLINK) {
>> err = v9fs_do_symlink(s, &vs->extension, &vs->fullname);
>> @@ -1809,9 +1825,7 @@ static void v9fs_create_post_lstat(V9fsState *s, V9fsCreateState *vs, int err)
>> err = v9fs_do_mksock(s, &vs->fullname);
>> v9fs_create_post_mksock(s, vs, err);
>> } else {
>> - vs->fidp->fd = v9fs_do_open2(s, &vs->fullname,
>> - omode_to_uflags(vs->mode) | O_CREAT,
>> - vs->perm & 0777);
>> + vs->fidp->fd = v9fs_do_open2(s, vs);
>> v9fs_create_post_open2(s, vs, err);
>> }
>>
>> @@ -2322,10 +2336,12 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>>
>> if (!strcmp(fse->security_model, "passthrough")) {
>> /* Files on the Fileserver set to client user credentials */
>> + s->ctx.fs_sm = SM_PASSTHROUGH;
>> } else if (!strcmp(fse->security_model, "mapped")) {
>> /* Files on the fileserver are set to QEMU credentials.
>> * Client user credentials are saved in extended attributes.
>> */
>> + s->ctx.fs_sm = SM_MAPPED;
>> } else {
>> /* user haven't specified a correct security option */
>> fprintf(stderr, "one of the following must be specified as the"
>> --
>> 1.6.5.2
>>
>>
>
>
> Can you split this to two patch.
>
> 1) Introduce just the security model and related xattr
> 2) Introduce the open2 section
Yeah.. will be doing it.
>
>
> Can we get something like the below
>
> security_chmod(..)
> {
> if (fs_ctx->fs_sm == SM_MAPPED) {
>
> } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
>
> } else
> return -1;
> }
>
> static int local_open2(FsContext *fs_ctx, const char *path, int flags,
> FsCred *credp)
> {
> fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
I don't think we can do this..
We create a regular file on the fileserver for the special file on the client.
Even for open2 the mode bits are going to be different depending on the security model.
So, practically we need to have security_open(), security_mknod, security_symlink()
ane in each of that we need to compare the security model and act accordingly.
> if (fd == -1) {
> return fd;
> }
> err = security_chmod(rpath(fs_ctx, path), credp);
> if (err == -1) {
> serrno = errno;
> goto err_end;
> }
> err = security_chown(rpath(fs_ctx, path), credp);
> if (err == -1) {
> serrno = errno;
> goto err_end;
> }
> return fd;
What is this going to buy us?
In the current code, one function does everything.
With this, we will be calling 3 functions and all 3 functions are going to compare the security model...so the security model will be spread all over the place anyway.
Also, please note that the whole security model checks are limited to this file only.
Rest of the VirtFS server is mostly agnostic to the security model...with this hope you are
ok with the current model. :)
Thanks,
JV
>
> err_end:
> close(fd);
> remove(rpath(fs_ctx, path));
> errno = serrno;
> return err;
>
>
> }
>
>
>
> So you basically introduce security_chmod ( I guess we need a better name) in patch 1
> That also tells us better how is chmod different between two security models.
>
> -aneesh
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2010-06-04 0:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-26 23:21 [Qemu-devel] [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 1/7] virtio-9p: Introduces an option to specify the security model Venkateswararao Jujjuri (JV)
2010-06-01 17:24 ` Aneesh Kumar K.V
2010-06-03 23:07 ` Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 2/7] virtio-9p: Rearrange fileop structures Venkateswararao Jujjuri (JV)
2010-06-01 17:22 ` Aneesh Kumar K.V
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 3/7] virtio-9p: modify create/open2 and mkdir for new security model Venkateswararao Jujjuri (JV)
2010-06-01 17:30 ` Aneesh Kumar K.V
2010-06-04 0:40 ` Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 4/7] virtio-9p: Implement Security model for mknod related files Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 5/7] virtio-9p: Implemented security model for symlink and link Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 6/7] virtio-9p: Implemented Security model for lstat and fstat Venkateswararao Jujjuri (JV)
2010-05-26 23:21 ` [Qemu-devel] [PATCH -V4 7/7] virtio-9p: Implemented security model for chown and chgrp Venkateswararao Jujjuri (JV)
2010-06-01 17:33 ` Aneesh Kumar K.V
2010-05-27 3:52 ` [Qemu-devel] Re: [PATCH-V4 0/7] virtio-9p:Introducing security model for VirtFS Andy Lutomirski
2010-05-27 17:52 ` Venkateswararao Jujjuri (JV)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).