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@xxxxxxxxxx> > > > --- > > > 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@xxxxxxxxxx> once you add the semicolon.
