On Wed, Jun 30, 2021 at 12:04:27 +0100, Daniel P. Berrangé wrote:
On Wed, Jun 30, 2021 at 12:50:29PM +0200, Peter Krempa wrote:
> On Wed, Jun 30, 2021 at 11:03:12 +0100, Daniel P. Berrangé wrote:
> > A process can access a file if the set of MCS categories
> > for the file is equal-to *or* a subset-of, the set of
> > MCS categories for the process.
> >
> > If there are two VMs:
> >
> > a) svirt_t:s0:c117
> > b) svirt_t:s0:c117,c720
> >
> > Then VM (b) is able to access files labelled for VM (a).
> >
> > IOW, we must discard case where the categories are equal
> > because that is a subset of many other valid category pairs.
> >
> > Fixes:
https://gitlab.com/libvirt/libvirt/-/issues/153
> > CVE-2021-xxxx - tbd before pushing
> > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> > ---
> > src/security/security_selinux.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> > index b50f4463cc..c98dab0d6f 100644
> > --- a/src/security/security_selinux.c
> > +++ b/src/security/security_selinux.c
> > @@ -383,7 +383,15 @@ virSecuritySELinuxMCSFind(virSecurityManager *mgr,
> > VIR_DEBUG("Try cat %s:c%d,c%d", sens, c1 + catMin, c2 +
catMin);
> >
> > if (c1 == c2) {
> > - mcs = g_strdup_printf("%s:c%d", sens, catMin + c1);
> > + /*
> > + * A process can access a file if the set of MCS categories
> > + * for the file is equal-to *or* a subset-of, the set of
> > + * MCS categories for the process.
> > + *
> > + * IOW, we must discard case where the categories are equal
> > + * because that is a subset of other category pairs.
> > + */
> > + continue
>
> Missing ';'
>
> > } else {
> > if (c1 > c2) {
> > int t = c1;
>
> This algorithm seems to be susceptible to infinite loops in case when
> 'catMin' and 'catMax' are too close or there's already enough of
them
> taken. Not a problem with this patch per-se, but it makes it more
> likely.
Categories must be ordered, and we can't use matching categories, so
with the range 0->1023, we have something like (1024*1024/2)-1024
total unique pairs. aka 523264.
I'll be impressed if someone has enough VMs on a single host to use
more than 1% of that total space.
So you're right that its tehcnically an inifinite loop but in practice
we can ignore the problem (for now).
Okay, I had to go back and read actually how the catMin/Max values are
obtained, and it's based on the process label that libvirtd gets. So I
guess users could shoot themselves in the foot when messing with the
label of libvirtd, but by default they get 0-1023.
I guess a failsafe in this regard would be to allow e.g. max 1000
iterations and then fail.
Either way that's for a different patch.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
once you add the semicolon.