On 03/30/2012 06:25 AM, Daniel P. Berrange wrote:
On Fri, Mar 30, 2012 at 03:13:31AM -0400, Laine Stump wrote:
On 03/29/2012 05:52 PM, Daniel P. Berrange wrote:
On Thu, Mar 29, 2012 at 04:38:50PM -0400, Laine Stump wrote:
+ if (oldbridge && + virNetDevBridgeRemovePort(oldbridge, olddev->ifname) < 0) { + return -1; + } + if (virNetDevBridgeAddPort(newbridge, olddev->ifname) < 0) { + if (virNetDevBridgeAddPort(oldbridge, olddev->ifname) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("unable to recover former state by adding port" + "to bridge %s"), oldbridge); + } + return -1; + } I think you need to emit 2 audit notifications here, one for the bridge being removed and one for the bridge being added. That does sound like a good idea, but the current virDomainAuditNet() function only reports MAC address, and virDomainAuditNetDevice() only reports "/dev/net/tun" - neither of them gives any information about the name of tap device or which bridge it is being attached to.
Right now, here are the audit messages that are logged when I do a full device detach/attach of a network device:
type=VIRT_RESOURCE msg=audit(1333090567.694:1051): pid=0 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net reason=detach vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd old-net=52:54:00:00:01:81 new-net=?: exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success'
type=VIRT_RESOURCE msg=audit(1333090573.195:1053): pid=0 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net reason=open vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd net=52:54:00:00:01:81 path="/dev/net/tun" rdev=0A:C8: exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success' type=VIRT_RESOURCE msg=audit(1333090573.196:1054): pid=0 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net reason=open vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd net=52:54:00:00:01:81 path="/dev/vhost-net" rdev=0A:EE: exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success' type=VIRT_RESOURCE msg=audit(1333090574.092:1055): pid=0 uid=0 auid=4294967295 ses=4294967295 subj=system_u:system_r:virtd_t:s0-s0:c0.c1023 msg='virt=kvm resrc=net reason=attach vm="F14" uuid=de3aa186-be64-088e-b64a-a1a03e023ffd old-net=? new-net=52:54:00:00:01:81: exe="/usr/sbin/libvirtd" hostname=? addr=? terminal=? res=success'
It does a good job of telling me the MAC address that's going to be used by the domain, but nothing about how it's connected to the network. Hmm, this seems flawed to me. The intent of the auditing code we added was to provide information about any host resource that the VM is associated with. So I'm really surprised we're not providing any info about the host network interface. I suspect this should be fixed.
If we're staying within the current boundaries of reporting, is there really value in logging a pair of messages that are ultimately just telling us that the same mac address was detached then immediately reattached, but not saying anything about what it was connected to? Alternately, if we're going to start reporting about changes in network connection, shouldn't we also be reporting what those connections are to begin with? (I think that's a change in scope of what's being audited, and requires a separate patch if we decide we want to do that). I think we should still audit this change, even though current audit rules appear broken.
Okay. Would the patch I've attached here be adequate? If so, I'll squash it into the rest of the patch. Beyond that, to fix the problem of general inadequacy in the current audits, would it be enough to add tap device and bridge names to the current attach and detach log messages? Or is the audit message format very strict, and we would need to do a separate log message for tap device and for bridge device?