5:04 a.m.
Peter Krempa (6):
util: storage: Add helper to determine whether storage is local
conf: audit: rng: Reorder new and old RNG device definitions
conf: audit: Split out common steps to audit domain devices
audit: disk: Refactor disk auditing to avoid auditing remote storage
audit: Add auditing for serial/parallel/channel/console characted devs
audit: Audit smartcard devices
src/conf/domain_audit.c | 281 +++++++++++++++++++++++++---------------------
src/conf/domain_audit.h | 11 +-
src/libvirt_private.syms | 2 +
src/lxc/lxc_driver.c | 6 +-
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 38 +++----
src/util/virstoragefile.c | 7 ++
src/util/virstoragefile.h | 1 +
8 files changed, 198 insertions(+), 152 deletions(-)
--
1.9.3
5:04 a.m.
New subject: [libvirt] [PATCH 1/6] util: storage: Add helper to determine whether storage is local
There's a lot of places where we skip doing actions based on the
locality of given storage type. The usual pattern is to skip it if:
virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK
Add a simple helper to simplify the pattern to
virStorageSourceIsLocalStorage(src)
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 7 +++++++
src/util/virstoragefile.h | 1 +
3 files changed, 9 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ed56103..067dcad 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1910,6 +1910,7 @@ virStorageSourceClear;
virStorageSourceFree;
virStorageSourceGetActualType;
virStorageSourceGetSecurityLabelDef;
+virStorageSourceIsLocalStorage;
virStorageSourceNewFromBacking;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 0c50de1..7ab640f 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1550,6 +1550,13 @@ virStorageSourceGetActualType(virStorageSourcePtr def)
}
+bool
+virStorageSourceIsLocalStorage(virStorageSourcePtr src)
+{
+ return virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK;
+}
+
+
/**
* virStorageSourceBackingStoreClear:
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 48c7e02..6609023 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -320,6 +320,7 @@ void virStorageSourceAuthClear(virStorageSourcePtr def);
void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def);
void virStorageSourceClear(virStorageSourcePtr def);
int virStorageSourceGetActualType(virStorageSourcePtr def);
+bool virStorageSourceIsLocalStorage(virStorageSourcePtr src);
void virStorageSourceFree(virStorageSourcePtr def);
void virStorageSourceBackingStoreClear(virStorageSourcePtr def);
virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);
--
1.9.3
8:25 a.m.
New subject: [libvirt] [PATCH 1/6] util: storage: Add helper to determine whether storage is local
On 07/03/2014 12:04 PM, Peter Krempa wrote:
There's a lot of places where we skip doing actions based on the
locality of given storage type. The usual pattern is to skip it if:
virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK
Add a simple helper to simplify the pattern to
virStorageSourceIsLocalStorage(src)
Are you planning to use it anywhere out of those "lot of places"? :)
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 7 +++++++
src/util/virstoragefile.h | 1 +
3 files changed, 9 insertions(+)
ACK
Jan
2:17 a.m.
New subject: [libvirt] [PATCH 1/6] util: storage: Add helper to determine whether storage is local
On 07/03/14 15:25, Ján Tomko wrote:
On 07/03/2014 12:04 PM, Peter Krempa wrote:
> There's a lot of places where we skip doing actions based on the
> locality of given storage type. The usual pattern is to skip it if:
>
> virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK
>
> Add a simple helper to simplify the pattern to
> virStorageSourceIsLocalStorage(src)
Are you planning to use it anywhere out of those "lot of places"? :)
I stole this patch from my snapshot source handling refactor :
"[libvirt] [PATCHv4 00/29] (for 1.2.7) qemu: Refactor handling of disk
image metadata"
That series didn't get any reviews and I needed that functionality. It's
getting a lot of love in that series :)
> ---
> src/libvirt_private.syms | 1 +
> src/util/virstoragefile.c | 7 +++++++
> src/util/virstoragefile.h | 1 +
> 3 files changed, 9 insertions(+)
ACK
Jan
4:09 a.m.
New subject: [libvirt] [PATCH 1/6] util: storage: Add helper to determine whether storage is local
On 07/03/14 15:25, Ján Tomko wrote:
On 07/03/2014 12:04 PM, Peter Krempa wrote:
> There's a lot of places where we skip doing actions based on the
> locality of given storage type. The usual pattern is to skip it if:
>
> virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_NETWORK
>
> Add a simple helper to simplify the pattern to
> virStorageSourceIsLocalStorage(src)
Are you planning to use it anywhere out of those "lot of places"? :)
> ---
> src/libvirt_private.syms | 1 +
> src/util/virstoragefile.c | 7 +++++++
> src/util/virstoragefile.h | 1 +
> 3 files changed, 9 insertions(+)
ACK
Pushed now; Thanks.
Peter
5:04 a.m.
New subject: [libvirt] [PATCH 2/6] conf: audit: rng: Reorder new and old RNG device definitions
The audit functions usually take the old definition before the new one
in the argument list. Unify RNG device to use the same order.
---
src/conf/domain_audit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 52c643d..91095b1 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -138,7 +138,7 @@ virDomainAuditDisk(virDomainObjPtr vm,
static void
virDomainAuditRNG(virDomainObjPtr vm,
- virDomainRNGDefPtr newDef, virDomainRNGDefPtr oldDef,
+ virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef,
const char *reason, bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -827,7 +827,7 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
}
if (vm->def->rng)
- virDomainAuditRNG(vm, vm->def->rng, NULL, "start", true);
+ virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true);
if (vm->def->tpm)
virDomainAuditTPM(vm, vm->def->tpm, "start", true);
--
1.9.3
8:30 a.m.
New subject: [libvirt] [PATCH 2/6] conf: audit: rng: Reorder new and old RNG device definitions
On 07/03/2014 12:04 PM, Peter Krempa wrote:
The audit functions usually take the old definition before the new
one
in the argument list. Unify RNG device to use the same order.
---
src/conf/domain_audit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK
4:10 a.m.
New subject: [libvirt] [PATCH 2/6] conf: audit: rng: Reorder new and old RNG device definitions
On 07/03/14 15:30, Ján Tomko wrote:
On 07/03/2014 12:04 PM, Peter Krempa wrote:
> The audit functions usually take the old definition before the new one
> in the argument list. Unify RNG device to use the same order.
> ---
> src/conf/domain_audit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
ACK
Pushed; Thanks.
Peter
5:04 a.m.
New subject: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
Extract common operations done when creating an audit message to a
separate generic function that can be reused and convert RNG, disk, FS
and net audit to use it.
---
src/conf/domain_audit.c | 175 ++++++++++++++++--------------------------------
1 file changed, 57 insertions(+), 118 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 91095b1..4c4290c 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -93,46 +93,73 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr)
}
-void
-virDomainAuditDisk(virDomainObjPtr vm,
- const char *oldDef, const char *newDef,
- const char *reason, bool success)
+static void
+virDomainAuditGenericDev(virDomainObjPtr vm,
+ const char *type,
+ const char *oldsrcpath,
+ const char *newsrcpath,
+ const char *reason,
+ bool success)
{
+ char *newdev = NULL;
+ char *olddev = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
char *oldsrc = NULL;
char *newsrc = NULL;
const char *virt;
- virUUIDFormat(vm->def->uuid, uuidstr);
- if (!(vmname = virAuditEncode("vm", vm->def->name))) {
- VIR_WARN("OOM while encoding audit message");
+ /* if both new and old source aren't provided don't log anything */
+ if (!newsrcpath && !oldsrcpath)
return;
- }
+
+ if (virAsprintfQuiet(&newdev, "new-%s", type) < 0)
+ goto no_memory;
+
+ if (virAsprintfQuiet(&olddev, "old-%s", type) < 0)
+ goto no_memory;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+
+ if (!(vmname = virAuditEncode("vm", vm->def->name)))
+ goto no_memory;
if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
- VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
+ VIR_WARN("Unexpected virt type %d while encoding audit message",
+ vm->def->virtType);
virt = "?";
}
- if (!(oldsrc = virAuditEncode("old-disk", VIR_AUDIT_STR(oldDef)))) {
- VIR_WARN("OOM while encoding audit message");
- goto cleanup;
- }
- if (!(newsrc = virAuditEncode("new-disk", VIR_AUDIT_STR(newDef)))) {
- VIR_WARN("OOM while encoding audit message");
- goto cleanup;
- }
+ if (!(newsrc = virAuditEncode(newdev, VIR_AUDIT_STR(newsrcpath))))
+ goto no_memory;
+
+ if (!(oldsrc = virAuditEncode(olddev, VIR_AUDIT_STR(oldsrcpath))))
+ goto no_memory;
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "virt=%s resrc=disk reason=%s %s uuid=%s %s %s",
- virt, reason, vmname, uuidstr,
- oldsrc, newsrc);
+ "virt=%s resrc=%s reason=%s %s uuid=%s %s %s",
+ virt, type, reason, vmname, uuidstr, oldsrc, newsrc);
cleanup:
+ VIR_FREE(newdev);
+ VIR_FREE(olddev);
VIR_FREE(vmname);
VIR_FREE(oldsrc);
VIR_FREE(newsrc);
+ return;
+
+ no_memory:
+ VIR_WARN("OOM while encoding audit message");
+ goto cleanup;
+}
+
+
+void
+virDomainAuditDisk(virDomainObjPtr vm,
+ const char *oldDef, const char *newDef,
+ const char *reason, bool success)
+{
+ virDomainAuditGenericDev(vm, "disk", oldDef, newDef, reason, success);
}
@@ -141,13 +168,8 @@ virDomainAuditRNG(virDomainObjPtr vm,
virDomainRNGDefPtr oldDef, virDomainRNGDefPtr newDef,
const char *reason, bool success)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- char *vmname;
const char *newsrcpath = NULL;
const char *oldsrcpath = NULL;
- char *oldsrc = NULL;
- char *newsrc = NULL;
- const char *virt;
if (newDef) {
switch ((virDomainRNGBackend) newDef->backend) {
@@ -185,40 +207,7 @@ virDomainAuditRNG(virDomainObjPtr vm,
}
}
- /* don't audit the RNG device if it doesn't use local resources */
- if (!oldsrcpath && !newsrcpath)
- return;
-
- virUUIDFormat(vm->def->uuid, uuidstr);
- if (!(vmname = virAuditEncode("vm", vm->def->name)))
- goto no_memory;
-
- if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
- VIR_WARN("Unexpected virt type %d while encoding audit message",
- vm->def->virtType);
- virt = "?";
- }
-
- if (!(newsrc = virAuditEncode("new-rng", VIR_AUDIT_STR(newsrcpath))))
- goto no_memory;
-
- if (!(oldsrc = virAuditEncode("old-rng", VIR_AUDIT_STR(oldsrcpath))))
- goto no_memory;
-
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "virt=%s resrc=rng reason=%s %s uuid=%s %s %s",
- virt, reason, vmname, uuidstr,
- oldsrc, newsrc);
-
- cleanup:
- VIR_FREE(vmname);
- VIR_FREE(oldsrc);
- VIR_FREE(newsrc);
- return;
-
- no_memory:
- VIR_WARN("OOM while encoding audit message");
- goto cleanup;
+ virDomainAuditGenericDev(vm, "rng", oldsrcpath, newsrcpath, reason,
success);
}
@@ -227,45 +216,10 @@ virDomainAuditFS(virDomainObjPtr vm,
virDomainFSDefPtr oldDef, virDomainFSDefPtr newDef,
const char *reason, bool success)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- char *vmname;
- char *oldsrc = NULL;
- char *newsrc = NULL;
- const char *virt;
-
- virUUIDFormat(vm->def->uuid, uuidstr);
- if (!(vmname = virAuditEncode("vm", vm->def->name))) {
- VIR_WARN("OOM while encoding audit message");
- return;
- }
-
- if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
- VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
- virt = "?";
- }
-
- if (!(oldsrc = virAuditEncode("old-fs",
- oldDef && oldDef->src ?
- oldDef->src : "?"))) {
- VIR_WARN("OOM while encoding audit message");
- goto cleanup;
- }
- if (!(newsrc = virAuditEncode("new-fs",
- newDef && newDef->src ?
- newDef->src : "?"))) {
- VIR_WARN("OOM while encoding audit message");
- goto cleanup;
- }
-
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "virt=%s resrc=fs reason=%s %s uuid=%s %s %s",
- virt, reason, vmname, uuidstr,
- oldsrc, newsrc);
-
- cleanup:
- VIR_FREE(vmname);
- VIR_FREE(oldsrc);
- VIR_FREE(newsrc);
+ virDomainAuditGenericDev(vm, "fs",
+ oldDef ? oldDef->src : NULL,
+ newDef ? newDef->src : NULL,
+ reason, success);
}
@@ -274,34 +228,19 @@ virDomainAuditNet(virDomainObjPtr vm,
virDomainNetDefPtr oldDef, virDomainNetDefPtr newDef,
const char *reason, bool success)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
char newMacstr[VIR_MAC_STRING_BUFLEN];
char oldMacstr[VIR_MAC_STRING_BUFLEN];
- char *vmname;
- const char *virt;
- virUUIDFormat(vm->def->uuid, uuidstr);
if (oldDef)
virMacAddrFormat(&oldDef->mac, oldMacstr);
+
if (newDef)
virMacAddrFormat(&newDef->mac, newMacstr);
- if (!(vmname = virAuditEncode("vm", vm->def->name))) {
- VIR_WARN("OOM while encoding audit message");
- return;
- }
- if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
- VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
- virt = "?";
- }
-
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "virt=%s resrc=net reason=%s %s uuid=%s old-net=%s new-net=%s",
- virt, reason, vmname, uuidstr,
- oldDef ? oldMacstr : "?",
- newDef ? newMacstr : "?");
-
- VIR_FREE(vmname);
+ virDomainAuditGenericDev(vm, "fs",
+ oldDef ? oldMacstr : NULL,
+ newDef ? newMacstr : NULL,
+ reason, success);
}
/**
--
1.9.3
8:29 a.m.
New subject: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
On 07/03/2014 12:04 PM, Peter Krempa wrote:
Extract common operations done when creating an audit message to a
separate generic function that can be reused and convert RNG, disk, FS
and net audit to use it.
---
src/conf/domain_audit.c | 175 ++++++++++++++++--------------------------------
1 file changed, 57 insertions(+), 118 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 91095b1..4c4290c 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -93,46 +93,73 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr)
}
-void
-virDomainAuditDisk(virDomainObjPtr vm,
- const char *oldDef, const char *newDef,
- const char *reason, bool success)
+static void
+virDomainAuditGenericDev(virDomainObjPtr vm,
+ const char *type,
+ const char *oldsrcpath,
+ const char *newsrcpath,
+ const char *reason,
+ bool success)
{
+ char *newdev = NULL;
+ char *olddev = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
vmname can be used unitialized in the cleanup section on OOM
char *oldsrc = NULL;
char *newsrc = NULL;
const char *virt;
- virUUIDFormat(vm->def->uuid, uuidstr);
- if (!(vmname = virAuditEncode("vm", vm->def->name))) {
- VIR_WARN("OOM while encoding audit message");
+ /* if both new and old source aren't provided don't log
anything */
+ if (!newsrcpath && !oldsrcpath)
Please move this to the next commit and let this one be just code movement.
return;
- }
+
+ if (virAsprintfQuiet(&newdev, "new-%s", type) < 0)
+ goto no_memory;
+
+ if (virAsprintfQuiet(&olddev, "old-%s", type) < 0)
+ goto no_memory;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+
+ if (!(vmname = virAuditEncode("vm", vm->def->name)))
+ goto no_memory;
if (!(virt = virDomainVirtTypeToString(vm->def->virtType))) {
- VIR_WARN("Unexpected virt type %d while encoding audit message",
vm->def->virtType);
+ VIR_WARN("Unexpected virt type %d while encoding audit message",
+ vm->def->virtType);
virt = "?";
}
@@ -274,34 +228,19 @@ virDomainAuditNet(virDomainObjPtr vm,
virDomainNetDefPtr oldDef, virDomainNetDefPtr newDef,
const char *reason, bool success)
{
-
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "virt=%s resrc=net reason=%s %s uuid=%s old-net=%s new-net=%s",
- virt, reason, vmname, uuidstr,
- oldDef ? oldMacstr : "?",
- newDef ? newMacstr : "?");
-
- VIR_FREE(vmname);
+ virDomainAuditGenericDev(vm, "fs",
The resrc should be "net" here, not "fs".
+ oldDef ? oldMacstr : NULL,
+ newDef ? newMacstr : NULL,
+ reason, success);
}
/**
ACK with the nits fixed.
Jan
4:08 a.m.
New subject: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
On 07/03/14 15:29, Ján Tomko wrote:
On 07/03/2014 12:04 PM, Peter Krempa wrote:
> Extract common operations done when creating an audit message to a
> separate generic function that can be reused and convert RNG, disk, FS
> and net audit to use it.
> ---
> src/conf/domain_audit.c | 175 ++++++++++++++++--------------------------------
> 1 file changed, 57 insertions(+), 118 deletions(-)
>
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index 91095b1..4c4290c 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -93,46 +93,73 @@ virDomainAuditChardevPath(virDomainChrSourceDefPtr chr)
> }
>
>
> -void
> -virDomainAuditDisk(virDomainObjPtr vm,
> - const char *oldDef, const char *newDef,
> - const char *reason, bool success)
> +static void
> +virDomainAuditGenericDev(virDomainObjPtr vm,
> + const char *type,
> + const char *oldsrcpath,
> + const char *newsrcpath,
> + const char *reason,
> + bool success)
> {
> + char *newdev = NULL;
> + char *olddev = NULL;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> char *vmname;
vmname can be used unitialized in the cleanup section on OOM
> char *oldsrc = NULL;
> char *newsrc = NULL;
> const char *virt;
>
> - virUUIDFormat(vm->def->uuid, uuidstr);
> - if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> - VIR_WARN("OOM while encoding audit message");
> + /* if both new and old source aren't provided don't log anything */
> + if (!newsrcpath && !oldsrcpath)
Please move this to the next commit and let this one be just code movement.
That condition is also necessary for this patch as auditing with both
new and old definition missing doesn't make sense. Some of the other
refactored functions rely on that.
Peter
4:11 a.m.
New subject: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
On 07/04/2014 11:08 AM, Peter Krempa wrote:
On 07/03/14 15:29, Ján Tomko wrote:
> On 07/03/2014 12:04 PM, Peter Krempa wrote:
>> Extract common operations done when creating an audit message to a
>> separate generic function that can be reused and convert RNG, disk, FS
>> and net audit to use it.
>> ---
>> src/conf/domain_audit.c | 175 ++++++++++++++++--------------------------------
>> 1 file changed, 57 insertions(+), 118 deletions(-)
>>
>> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
>> index 91095b1..4c4290c 100644
>> --- a/src/conf/domain_audit.c
>> +++ b/src/conf/domain_audit.c
>
>> char *oldsrc = NULL;
>> char *newsrc = NULL;
>> const char *virt;
>>
>> - virUUIDFormat(vm->def->uuid, uuidstr);
>> - if (!(vmname = virAuditEncode("vm", vm->def->name))) {
>> - VIR_WARN("OOM while encoding audit message");
>
>> + /* if both new and old source aren't provided don't log anything */
>> + if (!newsrcpath && !oldsrcpath)
>
> Please move this to the next commit and let this one be just code movement.
>
That condition is also necessary for this patch as auditing with both
new and old definition missing doesn't make sense. Some of the other
refactored functions rely on that.
Right, I missed the condition in virDomainAuditRNG.
Jan
5:36 a.m.
New subject: [libvirt] [PATCH 3/6] conf: audit: Split out common steps to audit domain devices
On 07/04/14 11:11, Ján Tomko wrote:
On 07/04/2014 11:08 AM, Peter Krempa wrote:
> On 07/03/14 15:29, Ján Tomko wrote:
>> On 07/03/2014 12:04 PM, Peter Krempa wrote:
>>> Extract common operations done when creating an audit message to a
>>> separate generic function that can be reused and convert RNG, disk, FS
>>> and net audit to use it.
>>> ---
>>> src/conf/domain_audit.c | 175
++++++++++++++++--------------------------------
>>> 1 file changed, 57 insertions(+), 118 deletions(-)
>>>
>>> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
>>> index 91095b1..4c4290c 100644
>>> --- a/src/conf/domain_audit.c
>>> +++ b/src/conf/domain_audit.c
>>
>>> char *oldsrc = NULL;
>>> char *newsrc = NULL;
>>> const char *virt;
>>>
>>> - virUUIDFormat(vm->def->uuid, uuidstr);
>>> - if (!(vmname = virAuditEncode("vm", vm->def->name))) {
>>> - VIR_WARN("OOM while encoding audit message");
>>
>>> + /* if both new and old source aren't provided don't log anything
*/
>>> + if (!newsrcpath && !oldsrcpath)
>>
>> Please move this to the next commit and let this one be just code movement.
>>
>
> That condition is also necessary for this patch as auditing with both
> new and old definition missing doesn't make sense. Some of the other
> refactored functions rely on that.
>
Right, I missed the condition in virDomainAuditRNG.
Jan
I've fixed the uninitialized variable and the resource name for
networks and I'm about to push this one in a minute.
Thanks.
Peter
5:05 a.m.
New subject: [libvirt] [PATCH 4/6] audit: disk: Refactor disk auditing to avoid auditing remote storage
Pass the virStorageSource struct to the auditing function and check if
storage is local before auditting.
---
src/conf/domain_audit.c | 25 ++++++++++++++++---------
src/conf/domain_audit.h | 4 ++--
src/lxc/lxc_driver.c | 6 +++---
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_hotplug.c | 21 ++++++++-------------
5 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 4c4290c..c4dcfa5 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -156,10 +156,21 @@ virDomainAuditGenericDev(virDomainObjPtr vm,
void
virDomainAuditDisk(virDomainObjPtr vm,
- const char *oldDef, const char *newDef,
- const char *reason, bool success)
+ virStorageSourcePtr oldDef,
+ virStorageSourcePtr newDef,
+ const char *reason,
+ bool success)
{
- virDomainAuditGenericDev(vm, "disk", oldDef, newDef, reason, success);
+ const char *oldsrc = NULL;
+ const char *newsrc = NULL;
+
+ if (oldDef && virStorageSourceIsLocalStorage(oldDef))
+ oldsrc = oldDef->path;
+
+ if (newDef && virStorageSourceIsLocalStorage(newDef))
+ newsrc = newDef->path;
+
+ virDomainAuditGenericDev(vm, "disk", oldsrc, newsrc, reason, success);
}
@@ -738,12 +749,8 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
{
size_t i;
- for (i = 0; i < vm->def->ndisks; i++) {
- const char *src = virDomainDiskGetSource(vm->def->disks[i]);
-
- if (src) /* Skips CDROM without media initially inserted */
- virDomainAuditDisk(vm, NULL, src, "start", true);
- }
+ for (i = 0; i < vm->def->ndisks; i++)
+ virDomainAuditDisk(vm, NULL, vm->def->disks[i]->src, "start",
true);
for (i = 0; i < vm->def->nfss; i++) {
virDomainFSDefPtr fs = vm->def->fss[i];
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h
index 70b09e5..58d25a4 100644
--- a/src/conf/domain_audit.h
+++ b/src/conf/domain_audit.h
@@ -39,8 +39,8 @@ void virDomainAuditStop(virDomainObjPtr vm,
const char *reason)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virDomainAuditDisk(virDomainObjPtr vm,
- const char *oldDef,
- const char *newDef,
+ virStorageSourcePtr oldDef,
+ virStorageSourcePtr newDef,
const char *reason,
bool success)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a9a87ea..251817d 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4120,7 +4120,7 @@ lxcDomainAttachDeviceDiskLive(virLXCDriverPtr driver,
cleanup:
if (src)
- virDomainAuditDisk(vm, NULL, src, "attach", ret == 0);
+ virDomainAuditDisk(vm, NULL, def->src, "attach", ret == 0);
VIR_FREE(file);
return ret;
}
@@ -4608,10 +4608,10 @@ lxcDomainDetachDeviceDiskLive(virDomainObjPtr vm,
}
if (lxcDomainAttachDeviceUnlink(vm, dst) < 0) {
- virDomainAuditDisk(vm, src, NULL, "detach", false);
+ virDomainAuditDisk(vm, def->src, NULL, "detach", false);
goto cleanup;
}
- virDomainAuditDisk(vm, src, NULL, "detach", true);
+ virDomainAuditDisk(vm, def->src, NULL, "detach", true);
if (virCgroupDenyDevicePath(priv->cgroup, src, VIR_CGROUP_DEVICE_RWM) != 0)
VIR_WARN("cannot deny device %s for domain %s",
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d45a161..b39c405 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12969,7 +12969,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
}
}
- virDomainAuditDisk(vm, disk->src->path, source, "snapshot", ret >=
0);
+ virDomainAuditDisk(vm, disk->src, snap->src, "snapshot", ret >=
0);
if (ret < 0)
goto cleanup;
@@ -15400,7 +15400,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorDriveMirror(priv->mon, device, dest, format, bandwidth,
flags);
- virDomainAuditDisk(vm, NULL, dest, "mirror", ret >= 0);
+ virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0);
qemuDomainObjExitMonitor(driver, vm);
if (ret < 0) {
qemuDomainPrepareDiskChainElementPath(driver, vm, disk, dest,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5e8aa4e..8d37813 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -154,9 +154,7 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
}
audit:
- if (src)
- virDomainAuditDisk(vm, virDomainDiskGetSource(origdisk),
- src, "update", ret >= 0);
+ virDomainAuditDisk(vm, origdisk->src, disk->src, "update", ret >=
0);
if (ret < 0)
goto error;
@@ -330,7 +328,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
}
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0);
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -583,7 +581,7 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn,
}
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0);
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -677,7 +675,7 @@ qemuDomainAttachUSBMassstorageDevice(virConnectPtr conn,
}
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, NULL, src, "attach", ret >= 0);
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
if (ret < 0)
goto error;
@@ -2489,7 +2487,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjExitMonitor(driver, vm);
VIR_FREE(drivestr);
- virDomainAuditDisk(vm, src, NULL, "detach", true);
+ virDomainAuditDisk(vm, disk->src, NULL, "detach", true);
event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias);
if (event)
@@ -2942,16 +2940,14 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, virDomainDiskGetSource(detach),
- NULL, "detach", false);
+ virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
goto cleanup;
}
} else {
if (qemuMonitorRemovePCIDevice(priv->mon,
&detach->info.addr.pci) < 0) {
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, virDomainDiskGetSource(detach),
- NULL, "detach", false);
+ virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
goto cleanup;
}
}
@@ -2996,8 +2992,7 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorDelDevice(priv->mon, detach->info.alias) < 0) {
qemuDomainObjExitMonitor(driver, vm);
- virDomainAuditDisk(vm, virDomainDiskGetSource(detach),
- NULL, "detach", false);
+ virDomainAuditDisk(vm, detach->src, NULL, "detach", false);
goto cleanup;
}
qemuDomainObjExitMonitor(driver, vm);
--
1.9.3
8:30 a.m.
New subject: [libvirt] [PATCH 4/6] audit: disk: Refactor disk auditing to avoid auditing remote storage
On 07/03/2014 12:05 PM, Peter Krempa wrote:
Pass the virStorageSource struct to the auditing function and check
if
storage is local before auditting.
*auditing
This would be nice to document in auditlog.html
---
src/conf/domain_audit.c | 25 ++++++++++++++++---------
src/conf/domain_audit.h | 4 ++--
src/lxc/lxc_driver.c | 6 +++---
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_hotplug.c | 21 ++++++++-------------
5 files changed, 31 insertions(+), 29 deletions(-)
ACK
Jan
5:32 a.m.
New subject: [libvirt] [PATCH 4/6] audit: disk: Refactor disk auditing to avoid auditing remote storage
On 07/03/14 15:30, Ján Tomko wrote:
On 07/03/2014 12:05 PM, Peter Krempa wrote:
> Pass the virStorageSource struct to the auditing function and check if
> storage is local before auditting.
*auditing
This would be nice to document in auditlog.html
Well the auditlog doc states that we audit only *host resources*, thus
auditing (sometimes) incomplete info about a remote storage volume is
technically a bug.
> ---
> src/conf/domain_audit.c | 25 ++++++++++++++++---------
> src/conf/domain_audit.h | 4 ++--
> src/lxc/lxc_driver.c | 6 +++---
> src/qemu/qemu_driver.c | 4 ++--
> src/qemu/qemu_hotplug.c | 21 ++++++++-------------
> 5 files changed, 31 insertions(+), 29 deletions(-)
>
ACK
Thanks, I'll push this one shortly.
Peter
5:05 a.m.
New subject: [libvirt] [PATCH 5/6] audit: Add auditing for serial/parallel/channel/console characted devs
Add startup auditing and also hotplug auditing for said devices
---
src/conf/domain_audit.c | 35 +++++++++++++++++++++++++++++++++++
src/conf/domain_audit.h | 7 +++++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++------
4 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index c4dcfa5..b7f8123 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -155,6 +155,29 @@ virDomainAuditGenericDev(virDomainObjPtr vm,
void
+virDomainAuditChardev(virDomainObjPtr vm,
+ virDomainChrDefPtr oldDef,
+ virDomainChrDefPtr newDef,
+ const char *reason,
+ bool success)
+{
+ virDomainChrSourceDefPtr oldsrc = NULL;
+ virDomainChrSourceDefPtr newsrc = NULL;
+
+ if (oldDef)
+ oldsrc = &oldDef->source;
+
+ if (newDef)
+ newsrc = &newDef->source;
+
+ virDomainAuditGenericDev(vm, "chardev",
+ virDomainAuditChardevPath(oldsrc),
+ virDomainAuditChardevPath(newsrc),
+ reason, success);
+}
+
+
+void
virDomainAuditDisk(virDomainObjPtr vm,
virStorageSourcePtr oldDef,
virStorageSourcePtr newDef,
@@ -772,6 +795,18 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
virDomainAuditRedirdev(vm, redirdev, "start", true);
}
+ for (i = 0; i < vm->def->nserials; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->serials[i], "start",
true);
+
+ for (i = 0; i < vm->def->nparallels; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->parallels[i], "start",
true);
+
+ for (i = 0; i < vm->def->nchannels; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->channels[i], "start",
true);
+
+ for (i = 0; i < vm->def->nconsoles; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start",
true);
+
if (vm->def->rng)
virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true);
diff --git a/src/conf/domain_audit.h b/src/conf/domain_audit.h
index 58d25a4..3434feb 100644
--- a/src/conf/domain_audit.h
+++ b/src/conf/domain_audit.h
@@ -111,4 +111,11 @@ void virDomainAuditRedirdev(virDomainObjPtr vm,
bool success)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void virDomainAuditChardev(virDomainObjPtr vm,
+ virDomainChrDefPtr oldDef,
+ virDomainChrDefPtr newDef,
+ const char *reason,
+ bool success)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
+
#endif /* __VIR_DOMAIN_AUDIT_H__ */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 067dcad..b04b099 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -116,6 +116,7 @@ virDomainPCIAddressValidate;
virDomainAuditCgroup;
virDomainAuditCgroupMajor;
virDomainAuditCgroupPath;
+virDomainAuditChardev;
virDomainAuditDisk;
virDomainAuditFS;
virDomainAuditHostdev;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 8d37813..5451118 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1460,18 +1460,20 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
qemuDomainObjEnterMonitor(driver, vm);
if (qemuMonitorAttachCharDev(priv->mon, charAlias, &chr->source) < 0) {
qemuDomainObjExitMonitor(driver, vm);
- goto cleanup;
+ goto audit;
}
if (devstr && qemuMonitorAddDevice(priv->mon, devstr) < 0) {
/* detach associated chardev on error */
qemuMonitorDetachCharDev(priv->mon, charAlias);
qemuDomainObjExitMonitor(driver, vm);
- goto cleanup;
+ goto audit;
}
qemuDomainObjExitMonitor(driver, vm);
ret = 0;
+ audit:
+ virDomainAuditChardev(vm, NULL, chr, "attach", ret == 0);
cleanup:
if (ret < 0 && need_remove)
qemuDomainChrRemove(vmdef, chr);
@@ -2751,6 +2753,7 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
char *charAlias = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
+ int rc;
VIR_DEBUG("Removing character device %s from domain %p %s",
chr->info.alias, vm, vm->def->name);
@@ -2759,12 +2762,14 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0) {
- qemuDomainObjExitMonitor(driver, vm);
- goto cleanup;
- }
+ rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
qemuDomainObjExitMonitor(driver, vm);
+ virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0);
+
+ if (rc < 0)
+ goto cleanup;
+
event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias);
if (event)
qemuDomainEventQueue(driver, event);
--
1.9.3
8:28 a.m.
New subject: [libvirt] [PATCH 5/6] audit: Add auditing for serial/parallel/channel/console characted devs
On 07/03/2014 12:05 PM, Peter Krempa wrote:
Add startup auditing and also hotplug auditing for said devices
---
src/conf/domain_audit.c | 35 +++++++++++++++++++++++++++++++++++
src/conf/domain_audit.h | 7 +++++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_hotplug.c | 17 +++++++++++------
4 files changed, 54 insertions(+), 6 deletions(-)
Missing changes in docs/auditlog.html.in
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index c4dcfa5..b7f8123 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
virDomainAuditDisk(virDomainObjPtr vm,
virStorageSourcePtr oldDef,
virStorageSourcePtr newDef,
@@ -772,6 +795,18 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
virDomainAuditRedirdev(vm, redirdev, "start", true);
}
+ for (i = 0; i < vm->def->nserials; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->serials[i], "start",
true);
+
+ for (i = 0; i < vm->def->nparallels; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->parallels[i], "start",
true);
+
+ for (i = 0; i < vm->def->nchannels; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->channels[i], "start",
true);
+
+ for (i = 0; i < vm->def->nconsoles; i++)
+ virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start",
true);
+
I wonder if working around the first console aliased to the first serial port
(or was it the other way around?) is worth it to prevent logging the same
device twice.
ACK with the docs added.
Jan
5:05 a.m.
New subject: [libvirt] [PATCH 6/6] audit: Audit smartcard devices
---
src/conf/domain_audit.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index b7f8123..a906d88 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -177,6 +177,51 @@ virDomainAuditChardev(virDomainObjPtr vm,
}
+static void
+virDomainAuditSmartcard(virDomainObjPtr vm,
+ virDomainSmartcardDefPtr def,
+ const char *reason,
+ bool success)
+{
+ const char *database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
+ size_t i;
+
+ if (def) {
+ switch ((virDomainSmartcardType) def->type) {
+ case VIR_DOMAIN_SMARTCARD_TYPE_HOST:
+ virDomainAuditGenericDev(vm, "smartcard",
+ NULL, "nss-smartcard-device",
+ reason, success);
+ break;
+
+ case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
+ for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
+ virDomainAuditGenericDev(vm, "smartcard", NULL,
+ def->data.cert.file[i],
+ reason, success);
+ }
+
+ if (def->data.cert.database)
+ database = def->data.cert.database;
+
+ virDomainAuditGenericDev(vm, "smartcard",
+ NULL, database,
+ reason, success);
+ break;
+
+ case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
+ virDomainAuditGenericDev(vm, "smartcard", NULL,
+
virDomainAuditChardevPath(&def->data.passthru),
+ reason, success);
+ break;
+
+ case VIR_DOMAIN_SMARTCARD_TYPE_LAST:
+ break;
+ }
+ }
+}
+
+
void
virDomainAuditDisk(virDomainObjPtr vm,
virStorageSourcePtr oldDef,
@@ -807,6 +852,9 @@ virDomainAuditStart(virDomainObjPtr vm, const char *reason, bool
success)
for (i = 0; i < vm->def->nconsoles; i++)
virDomainAuditChardev(vm, NULL, vm->def->consoles[i], "start",
true);
+ for (i = 0; i < vm->def->nsmartcards; i++)
+ virDomainAuditSmartcard(vm, vm->def->smartcards[i], "start",
true);
+
if (vm->def->rng)
virDomainAuditRNG(vm, NULL, vm->def->rng, "start", true);
--
1.9.3
3794
days inactive
3795
days old
19 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa