[libvirt] [PATCH] AppArmor: allow QEMU to set_process_name.

Jamie Strandboge jamie at canonical.com
Tue Dec 6 16:40:39 UTC 2016

On Tue, 2016-12-06 at 10:17 -0600, Jamie Strandboge wrote:
> On Mon, 2016-12-05 at 17:30 +0100, Christian Ehrhardt wrote:
> > 
> > On Mon, Dec 5, 2016 at 12:21 PM, intrigeri <intrigeri+libvirt at boum.org>
> > wrote:
> > 
> > > 
> > > 
> > > +  @{PROC}/@{pid}/task/@{tid}/comm rw,
> > > 
> > Hi,
> > we have used the following for now that we planned to submit soon:
> > owner @{PROC}/@{pid}/task/[0-9]*/comm rw
> > 
> > But I like yours more since you are adding the explicit TID instead of a
> > pattern.
> > I'm convinced you confirmed your fix working, but I wonder if might want to
> > consider the "owner" part we had.
> > 
> Note that @{pid} and @{tid} may be causing some confusion. Today, these are
> simple pattern matches in anticipation of AppArmor kernel variables. From
> /etc/apparmor.d/tunables/kernelvars:
> # until kernel vars are implemented
> # and until the parser supports nested groupings like
> #   @{pid}=[1-9]{[0-9]{[0-9]{[0-9]{[0-9]{[0-9],},},},},}
> # use
> @{pid}={[1-9],[1-9][0-9],[1-9][0-9][0-9],[1-9][0-9][0-9][0-9],[1-9][0-9][0-
> 9][0-
> 9][0-9],[1-9][0-9][0-9][0-9][0-9][0-9]}
> #same pattern as @{pid} for now
> @{tid}=@{pid}
> As such, while intrigeri's use of @{tid} is more correct for the long term, it
> is not (appreciably) different from Christian's use of [0-9]*.
> This means that either rule will allow modifying other process' 'comm' value.
> Since libvirt is *usually* configured to launch VMs as non-root (eg. the
> 'libvirt-qemu' group on Debian and its derivatives) there is some mitigation
> with owner match, but it does mean that any libvirt managed qemu process can
> update the 'comm' value of any other libvirt managed qemu process. For systems
> that are configured to launch qemu under root, then the rule means a qemu
> process is able to modify the 'comm' value of any other root process. Either
> way, this breaks qemu isolation and as a result I think it is wrong to add it
> to
> the libvirt-qemu abstraction until kernel variables are available.
> The best we can do at this point is conditionally add the owner rule per VM or
> per invocation of the command that runs set_process_name. The safest would be
> to
> add the rule when set_process_name is called, then remove it after with a
> profile reload. I'm unfamiliar with set_process_name and when it is run so I
> can't say if this is a viable approach or not.
> If it is not possible to update the profile in this manner, then at a minimum
> there should be a FIXME comment above this rule discussing all of this.

I forgot to reiterate: the above is true *unless* there is another non-DAC, non-
MAC kernel mediation (eg, does the kernel only allow modifying the 'comm' value
of its own threads? If so, then the rule would be safe to add to the default
abstraction (though we should document that it is safe)).

Jamie Strandboge             | http://www.canonical.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161206/fe170df2/attachment-0001.sig>

More information about the libvir-list mailing list