4:17 p.m.
Resubmitting based on feedback from this list. Notably, *alloc calls
have been removed and syntax-check completes without error for all
files.
Jamie
On Fri, 04 Sep 2009, Jamie Strandboge wrote:
This patch series implements the AppArmor security driver for sVirt.
This implementation was developed for the Ubuntu AppArmorLibvirtProfile
specification[1], but is general enough for any AppArmor deployment
(such as Ubuntu, *SUSE and Mandriva).
This patch has seen quite a bit of real world testing in Ubuntu 9.10
(our development release) in our 0.7.0-1ubuntu3 package. I did make a
few small changes after going through HACKING, but mostly I got the
tests going and added documentation.
DESIGN
------
When a virtual machine is started, determine if a profile is currently
defined for the machine, and use it if available. If not, generate a new
profile for the machine based on a template, which is by default a very
restrictive profile allowing access to disk files, and anything else
needed to run, such as the pid, monitor and log files.
Virtual machines should have a unique profile specific to that machine.
To ensure uniqueness, the profile name will be derived from the UUID of
the virtual machine. These profiles should be configurable, either by
adjusting the profile template for new machines, creating/modifying the
VM profile directly or through the use of AppArmor abstractions. This
will allow for administrators to fine-tune confinement for individual
machines if desired.
If enabled at compile time, the sVirt security model will be activated
if AppArmor is available on the host OS and a profile for the libvirtd
daemon is loaded when libvirtd is started.
libvirtd should not be allowed to create arbitrary profiles or modify
profiles directly, so as to not allow libvirtd to potentially (ie via a
security bug in libvirtd itself) bootstrap out of AppArmor confinement.
Because root privileges are needed to manipulate AppArmor profiles,
qemu:///session will not be supported at this time, but the
implementation must allow for a confined libvirtd with qemu:///session
guests running unconfined. This can be revisited when AppArmor supports
per-user profiles.
Please see the specification[1] for more details.
PATCHES
-------
The patches are all against trunk as of yesterday. Testing was done on
trunk and there seem to be no regressions over the the 0.7.0-1ubuntu3
package in Ubuntu.
[PATCH 1*]
patch_1a_reenable-nonfile-labels.patch:
When James Morris originally submitted his sVirt patches (as seen in
libvirt 0.6.1), he did not require on disk labelling for
virSecurityDomainRestoreImageLabel. A later commit[2] changed this
behavior to assume on disk labelling, which halts implementations for
path-based MAC systems such as AppArmor and TOMOYO where
vm->def->seclabel is required to obtain the label. This patch simply
adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel.
patch_1b_optional.patch:
Due to the above change, 'make syntax-check' fails because
SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr
vm'. patch_1b_optional.patch is a simple patch to fix this by checking
if vm->def->seclabel == NULL and returns with error if it does. I
realize this may not be desired in the long term, but it should be
harmless enough to include.
[PATCH 2]
patch_2_security_c.patch:
Updates src/security.c for AppArmor
[PATCH 3]
patch_3_security_apparmor.patch:
Adds security_apparmor.c, security_apparmor.h, virt-aa-helper.c and
updates po/POTFILES.in. virt-aa-helper.c is a new binary which is used
exclusively by the AppArmor security driver to manipulate AppArmor.
These files compile without warning and pass syntax-check.
[PATCH 4]
patch_4_tests.patch:
Adds tests for virt-aa-helper and the security driver. secaatest.c is
identical to seclabeltest.c except it initializes the 'apparmor' driver
instead of 'selinux'. These tests are integrated into 'make check' and
pass.
[PATCH 5]
patch_5_docs.patch:
Updates docs/drvqemu.html.in for AppArmor and adds profile examples to
examples/apparmor.
[PATCH 6]
patch_6_autoconf.patch:
Updates Makefile.am and configure.in for AppArmor. It is based on and
should operate the same as the SELinux configuration.
Caveats and known issues:
1. it does not take advantage of the recent host device labelling
functionality yet
2. it does not properly handle hot-plugging of devices yet
3. qemu:///session runs unconfined (see above)
Thanks!
Jamie (jdstrand on Freenode and OFTC)
[1] https://wiki.ubuntu.com/SecurityTeam/Specifications/AppArmorLibvirtProfile
[2]
http://libvirt.org/git/?p=libvirt.git;a=commit;h=c86afc85ee0d1ec6d76c2d25...
--
Jamie Strandboge | http://www.canonical.com
4:19 p.m.
New subject: [libvirt] Resubmission: [PATCH 1/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 1*]
> patch_1a_reenable-nonfile-labels.patch:
> When James Morris originally submitted his sVirt patches (as seen in
> libvirt 0.6.1), he did not require on disk labelling for
> virSecurityDomainRestoreImageLabel. A later commit[2] changed this
> behavior to assume on disk labelling, which halts implementations for
> path-based MAC systems such as AppArmor and TOMOYO where
> vm->def->seclabel is required to obtain the label. This patch simply
> adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel.
>
> patch_1b_optional.patch:
> Due to the above change, 'make syntax-check' fails because
> SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr
> vm'. patch_1b_optional.patch is a simple patch to fix this by checking
> if vm->def->seclabel == NULL and returns with error if it does. I
> realize this may not be desired in the long term, but it should be
> harmless enough to include.
>
--
Jamie Strandboge | http://www.canonical.com
6:41 a.m.
New subject: [libvirt] Resubmission: [PATCH 1/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:19:59PM -0500, Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> > [PATCH 1*]
> > patch_1a_reenable-nonfile-labels.patch:
> > When James Morris originally submitted his sVirt patches (as seen in
> > libvirt 0.6.1), he did not require on disk labelling for
> > virSecurityDomainRestoreImageLabel. A later commit[2] changed this
> > behavior to assume on disk labelling, which halts implementations for
> > path-based MAC systems such as AppArmor and TOMOYO where
> > vm->def->seclabel is required to obtain the label. This patch simply
> > adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel.
> >
> > patch_1b_optional.patch:
> > Due to the above change, 'make syntax-check' fails because
> > SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr
> > vm'. patch_1b_optional.patch is a simple patch to fix this by checking
> > if vm->def->seclabel == NULL and returns with error if it does. I
> > realize this may not be desired in the long term, but it should be
> > harmless enough to include.
The 'ATTRIBUTE_UNUSED' anotation takes care of this much better.
diff -Nurp ./libvirt.orig/src/qemu_driver.c
./libvirt/src/qemu_driver.c
--- ./libvirt.orig/src/qemu_driver.c 2009-09-08 13:00:00.000000000 -0500
+++ ./libvirt/src/qemu_driver.c 2009-09-08 15:32:22.000000000 -0500
@@ -6144,7 +6144,7 @@ static int qemudDomainDetachDevice(virDo
dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) {
ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev);
if (driver->securityDriver)
- driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn,
dev->data.disk);
+ driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn,
vm, dev->data.disk);
if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0)
VIR_WARN0("Fail to restore disk device ownership");
} else if (dev->type == VIR_DOMAIN_DEVICE_NET) {
diff -Nurp ./libvirt.orig/src/security.h ./libvirt/src/security.h
--- ./libvirt.orig/src/security.h 2009-08-17 11:00:40.000000000 -0500
+++ ./libvirt/src/security.h 2009-09-08 15:32:22.000000000 -0500
@@ -32,6 +32,7 @@ typedef virSecurityDriverStatus (*virSec
typedef int (*virSecurityDriverOpen) (virConnectPtr conn,
virSecurityDriverPtr drv);
typedef int (*virSecurityDomainRestoreImageLabel) (virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDiskDefPtr disk);
typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn,
virDomainObjPtr vm,
diff -Nurp ./libvirt.orig/src/security_selinux.c ./libvirt/src/security_selinux.c
--- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500
+++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500
@@ -354,6 +354,7 @@ SELinuxSetFilecon(virConnectPtr conn, co
static int
SELinuxRestoreSecurityImageLabel(virConnectPtr conn,
+ virDomainObjPtr vm,
virDomainDiskDefPtr disk)
{
struct stat buf;
@@ -423,7 +429,8 @@ SELinuxRestoreSecurityLabel(virConnectPt
int rc = 0;
if (secdef->imagelabel) {
for (i = 0 ; i < vm->def->ndisks ; i++) {
- if (SELinuxRestoreSecurityImageLabel(conn, vm->def->disks[i]) < 0)
+ if (SELinuxRestoreSecurityImageLabel(conn, vm,
+ vm->def->disks[i]) < 0)
rc = -1;
}
VIR_FREE(secdef->model);
ACK to this part
diff -Nurp ./libvirt.orig/src/security_selinux.c
./libvirt/src/security_selinux.c
--- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500
+++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500
@@ -364,6 +365,11 @@ SELinuxRestoreSecurityImageLabel(virConn
char *newpath = NULL;
const char *path = disk->src;
+ if (&vm->def->seclabel == NULL) {
+ virSecurityReportError(conn, VIR_ERR_ERROR, _("seclabel is NULL"));
+ return rc;
+ }
+
/* Don't restore labels on readoly/shared disks, because
* other VMs may still be accessing these
* Alternatively we could iterate over all running
This should have ATTRIBUTE_USED instead
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:58 a.m.
New subject: [libvirt] Resubmission: [PATCH 1/6] sVirt AppArmor security driver
Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
>> [PATCH 1*]
>> patch_1a_reenable-nonfile-labels.patch:
>> When James Morris originally submitted his sVirt patches (as seen in
>> libvirt 0.6.1), he did not require on disk labelling for
>> virSecurityDomainRestoreImageLabel. A later commit[2] changed this
>> behavior to assume on disk labelling, which halts implementations for
>> path-based MAC systems such as AppArmor and TOMOYO where
>> vm->def->seclabel is required to obtain the label. This patch simply
>> adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel.
>>
>> patch_1b_optional.patch:
>> Due to the above change, 'make syntax-check' fails because
>> SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr
>> vm'. patch_1b_optional.patch is a simple patch to fix this by checking
>> if vm->def->seclabel == NULL and returns with error if it does. I
>> realize this may not be desired in the long term, but it should be
>> harmless enough to include.
>>
It looks fine, except instead of using the optional patch, just use
"ATTRIBUTE_UNUSED" instead. (Oh, I see danpb gave you the same feedback :).
--
Chris Lalancette
4:20 p.m.
New subject: [libvirt] Resubmission: [PATCH 2/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 2]
> patch_2_security_c.patch:
> Updates src/security.c for AppArmor
>
--
Jamie Strandboge | http://www.canonical.com
6:42 a.m.
New subject: [libvirt] Resubmission: [PATCH 2/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:20:43PM -0500, Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> > [PATCH 2]
> > patch_2_security_c.patch:
> > Updates src/security.c for AppArmor
> >
--
Jamie Strandboge | http://www.canonical.com
diff -Nurp ./libvirt.orig/src/security.c ./libvirt/src/security.c
--- ./libvirt.orig/src/security.c 2009-08-17 11:00:40.000000000 -0500
+++ ./libvirt/src/security.c 2009-09-08 15:32:22.000000000 -0500
@@ -20,10 +20,17 @@
#include "security_selinux.h"
#endif
+#ifdef WITH_SECDRIVER_APPARMOR
+#include "security_apparmor.h"
+#endif
+
static virSecurityDriverPtr security_drivers[] = {
#ifdef WITH_SECDRIVER_SELINUX
&virSELinuxSecurityDriver,
#endif
+#ifdef WITH_SECDRIVER_APPARMOR
+ &virAppArmorSecurityDriver,
+#endif
NULL
};
ACK, but this part should really be part of the next patch, since you
can't apply this & still compile without also applying the next patch
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:21 p.m.
New subject: [libvirt] Resubmission: [PATCH 3/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 3]
> patch_3_security_apparmor.patch:
> Adds security_apparmor.c, security_apparmor.h, virt-aa-helper.c and
> updates po/POTFILES.in. virt-aa-helper.c is a new binary which is used
> exclusively by the AppArmor security driver to manipulate AppArmor.
> These files compile without warning and pass syntax-check.
>
--
Jamie Strandboge | http://www.canonical.com
7:07 a.m.
New subject: [libvirt] Resubmission: [PATCH 3/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:21:23PM -0500, Jamie Strandboge wrote:
diff -Nurp ./libvirt.orig/src/security_apparmor.c
./libvirt/src/security_apparmor.c
--- ./libvirt.orig/src/security_apparmor.c 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/src/security_apparmor.c 2009-09-08 15:32:42.000000000 -0500
+/*
+ * profile_status returns '-1' on error, '0' if loaded
+ *
+ * If check_enforcing is set to '1', then returns '-1' on error,
'0' if
+ * loaded in complain mode, and '1' if loaded in enforcing mode.
+ */
+static int
+profile_status(const char *str, const int check_enforcing)
+{
+ char ebuf[1024];
+ char *content = NULL;
+ char *tmp = NULL;
+ char *etmp = NULL;
+ int rc = -1;
+
+ /* create string that is '<str> \0' for accurate matching */
+ if (VIR_ALLOC_N(tmp, strlen(str) + 2) < 0) {
+ virSecurityReportError(NULL, VIR_ERR_ERROR,
+ _
+ ("%s: could not allocate memory for string"),
+ __func__);
+ return rc;
+ }
+ sprintf(tmp, "%s ", str);
+
+ if (check_enforcing != 0) {
+ /* create string that is '<str> (enforce)\0' for accurate matching
*/
+ if (VIR_ALLOC_N(etmp, strlen(str) + 11) < 0) {
+ virSecurityReportError(NULL, VIR_ERR_ERROR,
+ _("%s: could not allocate memory for "
+ "string"), __func__);
+ VIR_FREE(tmp);
+ return rc;
+ }
+ sprintf(etmp, "%s (enforce)", str);
+ }
Can you replace the VIR_ALLOC_N + sprintf calls with just virAsprintf()
which is safer wrt future changes because it is guarenteed to automatically
allocate the correct amount of memory without us needing to hardcode it.
Likewise for future instances of ALLOC+printf in this patch.
+
+ if (virFileReadAll(APPARMOR_PROFILES_PATH, MAX_FILE_LEN, &content) < 0) {
+ virSecurityReportError(NULL, VIR_ERR_ERROR,
+ _
+ ("%s: Failed to read AppArmor profiles list "
+ "%s: %s"), __func__,
+ APPARMOR_PROFILES_PATH, virStrerror(errno,
+ ebuf,
+ sizeof
+ ebuf));
I realize you're just copying the existing SELinux code style here, but
unfortunately the SELinux driver code was not following our best practices
for error reporting. Any place where you have an 'errno' value should call
the generic virReportSystemError() function, and not use virStrerror().
Also, there's no need to ever pass __func__ to any error reporting APIs
since we capture function, filename & lineno automatically in all cases.
+static int
+load_profile(const char *profile, virDomainObjPtr vm,
+ const char *skip_disk)
+{
+ const char **argv;
+ int rc = -1;
+ int offset;
+ size_t i;
+ size_t len;
+ int create;
+ int j;
+
+ create = 1;
+ offset = 6;
+ if (profile_status_file(profile) >= 0)
+ create = 0;
+ len = offset + vm->def->ndisks + 1;
+
+ if (VIR_ALLOC_N(argv, len) < 0) {
+ virSecurityReportError(NULL, VIR_ERR_ERROR,
+ _("%s: could not allocate memory"),
+ __func__);
+ return rc;
+ }
+
+ argv[0] = VIRT_AA_HELPER_PATH;
+ argv[2] = (char *) "-u";
+ argv[3] = profile;
+ argv[4] = (char *) "-n";
+ argv[5] = vm->def->name;
+
+ if (create == 0)
+ argv[1] = (char *) "-r";
+ else
+ argv[1] = (char *) "-c";
+
+ /* add disks to argv, skipping skip_disk if it is defined. */
+ j = 0;
+ for (i = 0; i < vm->def->ndisks; i++) {
+ if (vm->def->disks[i]->src == NULL || (skip_disk != NULL &&
+ STREQ(skip_disk,
+ vm->def->
+ disks[i]->src))) {
+ continue;
+ }
+ if (offset + j >= len - 1)
+ break;
+ argv[offset + j] = vm->def->disks[i]->src;
+ j++;
+ }
+ argv[offset + j] = NULL;
+
+ if (virRun(NULL, argv, NULL) == 0)
+ rc = 0;
+
+ VIR_FREE(argv);
+ return rc;
+}
IMHO the way data is passed from the security driver to the aa-helper program
is sub-optimal & requires too much unneccessary code. As you add in support
for USB & PCI device labelling you'll have a large number of additional args
to pass to the helper. We'll likely add even more later for serial/parallel
device assignment & other config options. In effect we'll end up inventing a
entirely new custom data interchange format for this helper, when we already
have a perfect one for the job.... the domain XML.
Thus I'd prefer to see this code simply take the virDomainDefPtr object,
call virDomainDefFormat(), and feed the resulting XML string to the
helper program on its STDIN file descriptor (or any other FD if desired).
The helper program can then simply read the XML off the FD, and call
virDomainDefParseString() to get back a virDomainDefPtr object. That
way guarentees the aa-helper always has all the data it will ever need
about the geust config without us needing to add more & more command
line options to aa-helper to deal with new config params.
+
+/*
+ * profile_name is buffer to hold name and len is how many bytes in the
+ * buffer
+ */
+static int
+get_profile_name(virConnectPtr conn,
+ virDomainObjPtr vm, char *profile_name, const size_t len)
+{
+ virDomainPtr dom = NULL;
+ int rc = -1;
+ char *prefix = (char *) "libvirt-";
Don't cast away const-ness, declare the variable with the neccessary
const-ness eg
const char *prefix = "libvirt-";
+
+ if (len < PROFILE_NAME_SIZE) {
+ virSecurityReportError(conn, VIR_ERR_ERROR,
+ _("%s: profile_name has wrong size"),
+ __func__);
+ return rc;
+ }
+ profile_name[0] = '\0';
+ strcat(profile_name, prefix);
+
+ /* generate the profile name */
+ dom = virGetDomain(conn, vm->def->name, vm->def->uuid);
+ if (!dom) {
+ virSecurityReportError(conn, VIR_ERR_ERROR,
+ _("%s: could not get domain for VM"),
+ __func__);
There's no need to raise an error when virGetDomain calls fail - it
will already have done that for you.
+typedef struct {
+ char uuid[PROFILE_NAME_SIZE]; /* UUID of vm */
+ char name[PATH_MAX]; /* name of vm */
+ bool dryrun; /* dry run */
+ char cmd; /* 'c' create
+ * 'a' add (load)
+ * 'r' replace
+ * 'R' remove */
+ int ndisks; /* number of disks */
+ char **disks; /* list of disks */
+} vahControl;
+
+static int
+vahDeinit(vahControl * ctl)
+{
+ int i;
+
+ if (ctl->disks != NULL) {
+ for (i = 0; i < ctl->ndisks; i++) {
+ if (ctl->disks[i] == NULL)
+ continue;
+ free(ctl->disks[i]);
+ }
+ VIR_FREE(ctl->disks);
+ }
+ return 0;
+}
+
+static void
+vah_error(vahControl * ctl, int doexit, const char *str)
+{
+ fprintf(stderr, _("%s: error: %s\n"), progname, str);
+
+ if (doexit) {
+ if (ctl != NULL)
+ vahDeinit(ctl);
+ exit(EXIT_FAILURE);
+ }
+}
+
+static void
+vah_warning(const char *str)
+{
+ fprintf(stderr, _("%s: warning: %s\n"), progname, str);
+}
+
+static void
+vah_info(const char *str)
+{
+ fprintf(stderr, _("%s:\n%s\n"), progname, str);
+}
+
+/*
+ * Replace @oldstr in @orig with @repstr
+ * @len is number of bytes allocated for @orig. Assumes @orig, @oldstr and
+ * @repstr are null terminated
+ */
+static int
+replace_string(char *orig, const size_t len, const char *oldstr,
+ const char *repstr)
+{
+ int idx;
+ char *pos = NULL;
+ char *tmp = NULL;
+
+ if ((pos = strstr(orig, oldstr)) == NULL) {
+ vah_error(NULL, 0, "could not find replacement string");
+ return -1;
+ }
+
+ if (VIR_ALLOC_N(tmp, len) < 0) {
+ vah_error(NULL, 0, "could not allocate memory for string");
+ return -1;
+ }
+ tmp[0] = '\0';
+
+ idx = abs(pos - orig);
+
+ /* copy everything up to oldstr */
+ strncat(tmp, orig, idx);
+
+ /* add the replacement string */
+ if (strlen(tmp) + strlen(repstr) > len - 1) {
+ vah_error(NULL, 0, "not enough space in target buffer");
+ VIR_FREE(tmp);
+ return -1;
+ }
+ strcat(tmp, repstr);
+
+ /* add everything after oldstr */
+ if (strlen(tmp) + strlen(orig) - (idx + strlen(oldstr)) > len - 1) {
+ vah_error(NULL, 0, "not enough space in target buffer");
+ VIR_FREE(tmp);
+ return -1;
+ }
+ strncat(tmp, orig + idx + strlen(oldstr),
+ strlen(orig) - (idx + strlen(oldstr)));
+
+ strncpy(orig, tmp, len);
+ orig[len - 1] = '\0';
+ VIR_FREE(tmp);
+
+ return 0;
+}
+
+/*
+ * run an apparmor_parser command
+ */
+static int
+parserCommand(const char *profile_name, const char cmd)
+{
+ const char *argv[4];
+ char flag[3];
+ char profile[PATH_MAX];
+
+ if (strchr("arR", cmd) == NULL) {
+ vah_error(NULL, 0, "invalid flag");
+ return -1;
+ }
+
+ snprintf(flag, 3, "-%c", cmd);
+
+ if (snprintf(profile, PATH_MAX, "%s/%s",
+ APPARMOR_DIR "/libvirt", profile_name) > PATH_MAX - 1) {
+ vah_error(NULL, 0, "profile name exceeds maximum length");
+ return -1;
+ }
+
+ if (!virFileExists(profile)) {
+ vah_error(NULL, 0, "profile does not exist");
+ return -1;
+ }
+
+ argv[0] = (char *) "/sbin/apparmor_parser";
+ argv[1] = flag;
+ argv[2] = profile;
+ argv[3] = NULL;
+
+ if (virRun(NULL, argv, NULL) != 0) {
+ vah_error(NULL, 0, "failed to run apparmor_parser");
+ return -1;
+ }
+
+ return 0;
+}
+
+/*
+ * Update the disks file
+ */
+static int
+update_include_file(const char *include_file, const char *included_files)
+{
+ int plen;
+ int fd;
+ char *pcontent = NULL;
+ char *existing = NULL;
+ char *warning = (char *)
+ "# DO NOT EDIT THIS FILE DIRECTLY. IT IS MANAGED BY LIBVIRT.\n";
More const-ness oissues here.
+ /* While 3*PATH_MAX is not accurate when thinking about
PATH_MAX,
+ * it is good enough since the #include line is a relative path
+ * and we are just trying to keep tmp from overflowing here anyway
+ */
+ if (snprintf(tmp, 3 * PATH_MAX,
+ " %s/log/libvirt/**/%s.log w,\n"
+ " %s/lib/libvirt/**/%s.monitor rw,\n"
+ " %s/run/libvirt/**/%s.pid rwk,\n",
+ LOCAL_STATE_DIR, ctl->name, LOCAL_STATE_DIR, ctl->name,
+ LOCAL_STATE_DIR, ctl->name) > 3 * PATH_MAX - 1) {
+ vah_error(ctl, 0, "added rules exceed maximum length");
+ goto clean;
+ }
It is much safer to just use virAsprintf() here instead of a huge static
buffer
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:22 p.m.
New subject: [libvirt] Resubmission: [PATCH 4/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 4]
> patch_4_tests.patch:
> Adds tests for virt-aa-helper and the security driver. secaatest.c is
> identical to seclabeltest.c except it initializes the 'apparmor' driver
> instead of 'selinux'. These tests are integrated into 'make check'
and
> pass.
>
--
Jamie Strandboge | http://www.canonical.com
6:51 a.m.
New subject: [libvirt] Resubmission: [PATCH 4/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:22:14PM -0500, Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> > [PATCH 4]
> > patch_4_tests.patch:
> > Adds tests for virt-aa-helper and the security driver. secaatest.c is
> > identical to seclabeltest.c except it initializes the 'apparmor'
driver
> > instead of 'selinux'. These tests are integrated into 'make
check' and
> > pass.
> >
--
Jamie Strandboge | http://www.canonical.com
diff -Nurp ./libvirt.orig/tests/Makefile.am
./libvirt/tests/Makefile.am
--- ./libvirt.orig/tests/Makefile.am 2009-08-17 11:00:40.000000000 -0500
+++ ./libvirt/tests/Makefile.am 2009-09-08 15:32:22.000000000 -0500
@@ -77,6 +77,10 @@ if WITH_SECDRIVER_SELINUX
noinst_PROGRAMS += seclabeltest
endif
+if WITH_SECDRIVER_APPARMOR
+noinst_PROGRAMS += secaatest
+endif
+
if WITH_CIL
noinst_PROGRAMS += object-locking
endif
@@ -112,6 +116,9 @@ test_scripts += \
virsh-synopsis
endif
+if WITH_SECDRIVER_APPARMOR
+test_scripts += virt-aa-helper-test
+endif
EXTRA_DIST += $(test_scripts)
TESTS = virshtest \
@@ -138,6 +145,10 @@ if WITH_SECDRIVER_SELINUX
TESTS += seclabeltest
endif
+if WITH_SECDRIVER_APPARMOR
+TESTS += secaatest
+endif
+
if WITH_LIBVIRTD
noinst_PROGRAMS += eventtest
TESTS += eventtest
@@ -255,6 +266,14 @@ else
EXTRA_DIST += seclabeltest.c
endif
+if WITH_SECDRIVER_APPARMOR
+secaatest_SOURCES = \
+ secaatest.c
+secaatest_LDADD = ../src/libvirt_driver_security.la $(LDADDS)
+else
+EXTRA_DIST += secaatest.c
+endif
+
qparamtest_SOURCES = \
qparamtest.c testutils.h testutils.c
qparamtest_LDADD = $(LDADDS)
diff -Nurp ./libvirt.orig/tests/secaatest.c ./libvirt/tests/secaatest.c
--- ./libvirt.orig/tests/secaatest.c 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/tests/secaatest.c 2009-09-08 15:32:22.000000000 -0500
@@ -0,0 +1,45 @@
+#include <config.h>
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include "security.h"
+
+int
+main (int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
+{
+ int ret;
+
+ const char *doi, *model;
+ virSecurityDriverPtr security_drv;
+
+ ret = virSecurityDriverStartup (&security_drv, "apparmor");
+ if (ret == -1)
+ {
+ fprintf (stderr, "Failed to start security driver");
+ exit (-1);
+ }
+ /* No security driver wanted to be enabled: just return */
+ if (ret == -2)
+ return 0;
+
+ model = virSecurityDriverGetModel (security_drv);
+ if (!model)
+ {
+ fprintf (stderr, "Failed to copy secModel model: %s",
+ strerror (errno));
+ exit (-1);
+ }
+
+ doi = virSecurityDriverGetDOI (security_drv);
+ if (!doi)
+ {
+ fprintf (stderr, "Failed to copy secModel DOI: %s",
+ strerror (errno));
+ exit (-1);
+ }
+
+ return 0;
+}
diff -Nurp ./libvirt.orig/tests/virt-aa-helper-test ./libvirt/tests/virt-aa-helper-test
--- ./libvirt.orig/tests/virt-aa-helper-test 1969-12-31 18:00:00.000000000 -0600
+++ ./libvirt/tests/virt-aa-helper-test 2009-09-08 15:32:22.000000000 -0500
@@ -0,0 +1,100 @@
+#!/bin/sh
+set -e
+
+output="/dev/null"
+use_valgrind=""
+ld_library_path=""
+if [ ! -z "$1" ] && [ "$1" = "-d" ]; then
+ output="/dev/stdout"
+ shift
+fi
+
+exe="../src/virt-aa-helper"
+if [ ! -z "$1" ]; then
+ if [ "$1" = "-v" ]; then
+ use_valgrind="yes"
+ exe="./src/.libs/virt-aa-helper"
+ ld_library_path="./src/.libs"
+ else
+ exe="$1"
+ fi
+ shift
+fi
+
+if [ ! -x "$exe" ]; then
+ echo "Could not find '$exe'"
+ exit 1
+fi
+
+echo "testing `basename $exe`" >$output
+if [ "$use_valgrind" = "yes" ]; then
+ exe="valgrind --error-exitcode=2 --track-origins=yes $exe"
+fi
+
+extra_args="--dryrun"
+errors=0
+
+tmpdir=`mktemp -d`
+trap "rm -rf $tmpdir" EXIT HUP INT QUIT TERM
+
+disk1="$tmpdir/1.img"
+disk2="$tmpdir/2.img"
+relative_disk1="$tmpdir/./../`basename $tmpdir`//./1.img"
+nonexistent="$tmpdir/nonexistant.img"
+bad_disk="/etc/passwd"
+valid_uuid="libvirt-00000000-0000-0000-0000-0123456789ab"
+valid_name="foo"
+nonexistent_uuid="libvirt-00000000-0000-0000-0000-000000000001"
+touch "$disk1" "$disk2"
+
+testme() {
+ expected="$1"
+ outstr="$2"
+ args="$3"
+ echo -n " $outstr: " >$output
+ echo " '$extra_args $args': " >$output
+ set +e
+ LD_LIBRARY_PATH="$ld_library_path" $exe $extra_args $args >$output
2>&1
+ rc="$?"
+ set -e
+ if [ "$rc" = "$expected" ]; then
+ echo "pass" >$output
+ else
+ echo "FAIL: exited with '$rc'" >$output
+ errors=$(($errors + 1))
+ fi
+}
+
+# Expected failures
+echo "Expected failures:" >$output
+testme "1" "invalid arg" "-z"
+testme "1" "invalid case" "-A"
+testme "1" "not enough args" "-c"
+testme "1" "missing name" "-c -n -u $valid_uuid $disk1"
+testme "1" "bad name" "-c -n foo[a-z] -u $valid_uuid
$disk1"
+testme "1" "no -u with -c" "-c -n $valid_name $disk1"
+testme "1" "bad uuid (bad digit)" "-c -n $valid_name -u
libvirt-00000000-0000-0000-0000-00000000000g $disk1"
+testme "1" "bad uuid (too long)" "-c -n $valid_name -u
${valid_uuid}abcdef $disk1"
+testme "1" "bad uuid (too short)" "-c -n $valid_name -u
libvirt-00000000-0000-0000-0000-0123456789a $disk1"
+testme "1" "missing uuid" "-c -n $valid_name -u $disk1"
+testme "1" "no -u with -R" "-R"
+testme "1" "non-existent uuid" "-R -u $nonexistent_uuid"
+testme "1" "no -u with -r" "-r"
+testme "1" "no name with -r" "-r -u $valid_uuid $disk1"
+testme "1" "bad disk" "-c -n $valid_name -u $valid_uuid
$bad_disk"
+testme "1" "bad disk2" "-c -n $valid_name -u $valid_uuid $disk1
$bad_disk $disk2"
+
+echo "Expected pass:" >$output
+testme "0" "create" "-c -n foo -u $valid_uuid $disk1"
+testme "0" "create (non-existent disk)" "-c -n foo -u
$valid_uuid $nonexistent"
+testme "0" "create (relative path)" "-c -n foo -u $valid_uuid
$relative_disk1"
+testme "0" "replace" "-r -n foo -u $valid_uuid $disk2"
+testme "0" "replace (non-existent disk)" "-r -n foo -u
$valid_uuid $nonexistent"
+testme "0" "help" "-h"
+
+echo "" >$output
+if [ "$errors" != "0" ]; then
+ echo "FAIL: $errors error(s)" >$output
+ exit 1
+fi
+echo PASS >$output
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:22 p.m.
New subject: [libvirt] Resubmission: [PATCH 5/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 5]
> patch_5_docs.patch:
> Updates docs/drvqemu.html.in for AppArmor and adds profile examples to
> examples/apparmor.
>
--
Jamie Strandboge | http://www.canonical.com
6:47 a.m.
New subject: [libvirt] Resubmission: [PATCH 5/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:22:56PM -0500, Jamie Strandboge wrote:
diff -Nurp ./libvirt.orig/examples/apparmor/usr.sbin.libvirtd
./libvirt/examples/apparmor/usr.sbin.libvirtd
--- ./libvirt.orig/examples/apparmor/usr.sbin.libvirtd 1969-12-31 18:00:00.000000000
-0600
+++ ./libvirt/examples/apparmor/usr.sbin.libvirtd 2009-09-08 15:32:22.000000000 -0500
@@ -0,0 +1,39 @@
+# Last Modified: Mon Jul 6 17:23:58 2009
+#include <tunables/global>
+@{LIBVIRT}="libvirt"
+
+/usr/sbin/libvirtd {
+ #include <abstractions/base>
+
+ capability kill,
+ capability net_admin,
+ capability net_raw,
+ capability setgid,
+ capability sys_admin,
+ capability sys_module,
+ capability sys_ptrace,
I'm fairly sure libvirtd will need more than this set of capabilities.
We tried to limit this in the C code a few months back, but gave up
because we ended up requiring about 2/3s of all capabilities and once
you allow net_admin & sys_admin its game over for security benefits.
You'll certainly have broken functionality without sys_nice, sys_chroot,
setuid, setpcap, mknod, dac_override, dac_read_search, fowner, chown
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
4:23 p.m.
New subject: [libvirt] Resubmission: [PATCH 6/6] sVirt AppArmor security driver
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> [PATCH 6]
> patch_6_autoconf.patch:
> Updates Makefile.am and configure.in for AppArmor. It is based on and
> should operate the same as the SELinux configuration.
>
--
Jamie Strandboge | http://www.canonical.com
6:50 a.m.
New subject: [libvirt] Resubmission: [PATCH 6/6] sVirt AppArmor security driver
On Tue, Sep 08, 2009 at 04:23:34PM -0500, Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
> > [PATCH 6]
> > patch_6_autoconf.patch:
> > Updates Makefile.am and configure.in for AppArmor. It is based on and
> > should operate the same as the SELinux configuration.
> >
--
Jamie Strandboge | http://www.canonical.com
diff -Nurp ./libvirt.orig/configure.in ./libvirt/configure.in
--- ./libvirt.orig/configure.in 2009-09-08 12:59:59.000000000 -0500
+++ ./libvirt/configure.in 2009-09-08 15:32:22.000000000 -0500
@@ -799,6 +799,84 @@ fi
AM_CONDITIONAL([WITH_SECDRIVER_SELINUX], [test "$with_secdriver_selinux" !=
"no"])
+dnl AppArmor
+AC_ARG_WITH([apparmor],
+ [ --with-apparmor use AppArmor to manage security],
+ [],
+ [with_apparmor=check])
+
+APPARMOR_CFLAGS=
+APPARMOR_LIBS=
+if test "$with_apparmor" != "no"; then
+ old_cflags="$CFLAGS"
+ old_libs="$LIBS"
+ if test "$with_apparmor" = "check"; then
+ AC_CHECK_HEADER([sys/apparmor.h],[],[with_apparmor=no])
+ AC_CHECK_LIB([apparmor], [aa_change_profile],[],[with_apparmor=no])
+ AC_CHECK_LIB([apparmor], [aa_change_hat],[],[with_apparmor=no])
+ if test "$with_apparmor" != "no"; then
+ with_apparmor="yes"
+ fi
+ else
+ fail=0
+ AC_CHECK_HEADER([sys/apparmor.h],[],[fail=1])
+ AC_CHECK_LIB([apparmor], [aa_change_profile],[],[fail=1])
+ AC_CHECK_LIB([apparmor], [aa_change_hat],[],[fail=1])
+ test $fail = 1 &&
+ AC_MSG_ERROR([You must install the AppArmor development package in order to
compile libvirt])
+ fi
+ CFLAGS="$old_cflags"
+ LIBS="$old_libs"
+fi
+if test "$with_apparmor" = "yes"; then
+ APPARMOR_LIBS="-lapparmor"
+ AC_DEFINE_UNQUOTED([HAVE_APPARMOR], 1, [whether AppArmor is available for security])
+ AC_DEFINE_UNQUOTED([APPARMOR_DIR], "/etc/apparmor.d", [path to apparmor
directory])
+ AC_DEFINE_UNQUOTED([APPARMOR_PROFILES_PATH],
"/sys/kernel/security/apparmor/profiles", [path to kernel profiles])
+ AC_DEFINE_UNQUOTED([VIRT_AA_HELPER_PATH], "$prefix/bin/virt-aa-helper",
[path to virt-aa-helper])
+fi
+AM_CONDITIONAL([HAVE_APPARMOR], [test "$with_apparmor" != "no"])
+AC_SUBST([APPARMOR_CFLAGS])
+AC_SUBST([APPARMOR_LIBS])
+
+
+AC_ARG_WITH([secdriver-apparmor],
+ [ --with-secdriver-apparmor use AppArmor security driver],
+ [],
+ [with_secdriver_apparmor=check])
+
+if test "$with_apparmor" != "yes" ; then
+ if test "$with_secdriver_apparmor" = "check" ; then
+ with_secdriver_apparmor=no
+ else
+ AC_MSG_ERROR([You must install the AppArmor development package in order to compile
libvirt])
+ fi
+else
+ old_cflags="$CFLAGS"
+ old_libs="$LIBS"
+ CFLAGS="$CFLAGS $APPARMOR_CFLAGS"
+ LIBS="$CFLAGS $APPARMOR_LIBS"
+
+ fail=0
+ AC_CHECK_FUNC([change_hat], [], [fail=1])
+ AC_CHECK_FUNC([aa_change_profile], [], [fail=1])
+ CFLAGS="$old_cflags"
+ LIBS="$old_libs"
+
+ if test "$fail" = "1" ; then
+ if test "$with_secdriver_apparmor" = "check" ; then
+ with_secdriver_apparmor=no
+ else
+ AC_MSG_ERROR([You must install the AppArmor development package in order to
compile libvirt])
+ fi
+ else
+ with_secdriver_apparmor=yes
+ AC_DEFINE_UNQUOTED([WITH_SECDRIVER_APPARMOR], 1, [whether AppArmor security driver
is available])
+ fi
+fi
+AM_CONDITIONAL([WITH_SECDRIVER_APPARMOR], [test "$with_secdriver_apparmor" !=
"no"])
+
+
dnl NUMA lib
AC_ARG_WITH([numactl],
@@ -1706,6 +1784,7 @@ AC_MSG_NOTICE([])
AC_MSG_NOTICE([Security Drivers])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([ SELinux: $with_secdriver_selinux])
+AC_MSG_NOTICE([ AppArmor: $with_secdriver_apparmor])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Driver Loadable Modules])
AC_MSG_NOTICE([])
@@ -1753,6 +1832,11 @@ AC_MSG_NOTICE([ selinux: $SELINUX_CFLAGS
else
AC_MSG_NOTICE([ selinux: no])
fi
+if test "$with_apparmor" = "yes" ; then
+AC_MSG_NOTICE([ apparmor: $APPARMOR_CFLAGS $APPARMOR_LIBS])
+else
+AC_MSG_NOTICE([ apparmor: no])
+fi
if test "$with_numactl" = "yes" ; then
AC_MSG_NOTICE([ numactl: $NUMACTL_CFLAGS $NUMACTL_LIBS])
else
diff -Nurp ./libvirt.orig/src/Makefile.am ./libvirt/src/Makefile.am
--- ./libvirt.orig/src/Makefile.am 2009-09-08 13:00:00.000000000 -0500
+++ ./libvirt/src/Makefile.am 2009-09-08 15:32:22.000000000 -0500
@@ -9,6 +9,7 @@ INCLUDES = \
$(LIBSSH2_CFLAGS) \
$(XEN_CFLAGS) \
$(SELINUX_CFLAGS) \
+ $(APPARMOR_CFLAGS) \
$(DRIVER_MODULE_CFLAGS) \
-DLIBDIR=\""$(libdir)"\" \
-DBINDIR=\""$(libexecdir)"\" \
@@ -216,6 +217,8 @@ SECURITY_DRIVER_SOURCES = \
SECURITY_DRIVER_SELINUX_SOURCES = \
security_selinux.h security_selinux.c
+SECURITY_DRIVER_APPARMOR_SOURCES = \
+ security_apparmor.h security_apparmor.c
NODE_DEVICE_DRIVER_SOURCES = \
node_device.c node_device.h
@@ -527,6 +530,9 @@ libvirt_la_LIBADD += libvirt_driver_secu
if WITH_SECDRIVER_SELINUX
libvirt_driver_security_la_SOURCES += $(SECURITY_DRIVER_SELINUX_SOURCES)
endif
+if WITH_SECDRIVER_APPARMOR
+libvirt_driver_security_la_SOURCES += $(SECURITY_DRIVER_APPARMOR_SOURCES)
+endif
# Add all conditional sources just in case...
EXTRA_DIST += \
@@ -615,7 +621,7 @@ libvirt_la_LIBADD += \
libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)libvirt.syms \
-version-info @LIBVIRT_VERSION_INFO@ \
$(COVERAGE_CFLAGS:-f%=-Wc,-f%) \
- $(LIBXML_LIBS) $(SELINUX_LIBS) \
+ $(LIBXML_LIBS) $(SELINUX_LIBS) $(APPARMOR_LIBS) \
$(XEN_LIBS) $(DRIVER_MODULE_LIBS) \
$(DEVMAPPER_LIBS) \
@CYGWIN_EXTRA_LDFLAGS@ @MINGW_EXTRA_LDFLAGS@
@@ -654,6 +660,23 @@ virsh_LDADD = \
../gnulib/lib/libgnu.la \
$(VIRSH_LIBS)
virsh_CFLAGS = $(COVERAGE_CFLAGS) $(READLINE_CFLAGS) $(NUMACTL_CFLAGS)
+
+if WITH_SECDRIVER_APPARMOR
+bin_PROGRAMS += virt-aa-helper
+
+virt_aa_helper_SOURCES = \
+ virt-aa-helper.c
+
+virt_aa_helper_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS)
+virt_aa_helper_LDADD = \
+ $(STATIC_BINARIES) \
+ $(WARN_CFLAGS) \
+ libvirt.la \
+ ../gnulib/lib/libgnu.la \
+ $(VIRSH_LIBS)
+virt_aa_helper_CFLAGS = $(COVERAGE_CFLAGS)
+endif
+
BUILT_SOURCES = virsh-net-edit.c virsh-pool-edit.c libvirt.syms
virsh-net-edit.c: virsh.c Makefile.am
ACK, though again this patch needs to be adjusted wrt the others, since
earlier patches look like they have build time depends on this one.
It is probably counter-productive to try & split up this series since
it is all one big feature. Probably just have the first generic cleanup,
then the apparmour implementation, and then the documentation patch
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
3:52 a.m.
On Tue, Sep 08, 2009 at 04:17:26PM -0500, Jamie Strandboge wrote:
Resubmitting based on feedback from this list. Notably, *alloc calls
have been removed and syntax-check completes without error for all
files.
Thanks ! I think this is an item for post 0.7.1, so most likely you
won't get much comment before early next week, as focuse is now for the
upcoming release,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
5549
days inactive
5557
days old
14 comments
4 participants
participants (4)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Jamie Strandboge