1. Patchew
  2. Libvirt
  3. View series

[PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets

David Michael posted 1 patch 1 year, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/871qv9ezmi.fsf@bigbadwolfsecurity.com
src/security/security_selinux.c            | 6 ++++--
tests/securityselinuxlabeldata/chardev.txt | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
[PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by David Michael 1 year, 10 months ago 
This supports sockets created by libvirt and passed by FD using the
same method as in security_dac.c.

Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
---

Hi,

Custom SELinux labels are not applied to sockets when they have
mode="bind", but other security models (DAC) allow changing these
sockets.  Can the same method be used to support SELinux?

Thanks.

David

 src/security/security_selinux.c            | 6 ++++--
 tests/securityselinuxlabeldata/chardev.txt | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e2f34a27dc..8b258c9e36 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_UNIX:
-        if (!dev_source->data.nix.listen) {
+        if (!dev_source->data.nix.listen ||
+            (dev_source->data.nix.path &&
+             virFileExists(dev_source->data.nix.path))) {
             if (virSecuritySELinuxSetFilecon(mgr,
                                              dev_source->data.nix.path,
                                              imagelabel,
@@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
     case VIR_DOMAIN_CHR_TYPE_UNIX:
         if (!dev_source->data.nix.listen) {
             if (virSecuritySELinuxRestoreFileLabel(mgr,
-                                                   dev_source->data.file.path,
+                                                   dev_source->data.nix.path,
                                                    true) < 0)
                 goto done;
         }
diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
index 3f4b6302b9..bdb367f7a5 100644
--- a/tests/securityselinuxlabeldata/chardev.txt
+++ b/tests/securityselinuxlabeldata/chardev.txt
@@ -2,6 +2,6 @@
 /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
 /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
 /nolabel.sock;
-/plain.sock;
+/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
 /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
 /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264
-- 
2.36.1
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by Michal Prívozník 1 year, 10 months ago 
On 6/28/22 14:33, David Michael wrote:
> This supports sockets created by libvirt and passed by FD using the
> same method as in security_dac.c.
> 
> Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> ---
> 
> Hi,
> 
> Custom SELinux labels are not applied to sockets when they have
> mode="bind", but other security models (DAC) allow changing these
> sockets.  Can the same method be used to support SELinux?
> 
> Thanks.
> 
> David
> 
>  src/security/security_selinux.c            | 6 ++++--
>  tests/securityselinuxlabeldata/chardev.txt | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index e2f34a27dc..8b258c9e36 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
>          break;
>  
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
> -        if (!dev_source->data.nix.listen) {
> +        if (!dev_source->data.nix.listen ||
> +            (dev_source->data.nix.path &&
> +             virFileExists(dev_source->data.nix.path))) {

I've copied the comment from corresponding _dac.c function, so that it's
obvious why we are relabelling in this case too.

>              if (virSecuritySELinuxSetFilecon(mgr,
>                                               dev_source->data.nix.path,
>                                               imagelabel,
> @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
>          if (!dev_source->data.nix.listen) {
>              if (virSecuritySELinuxRestoreFileLabel(mgr,
> -                                                   dev_source->data.file.path,
> +                                                   dev_source->data.nix.path,
>                                                     true) < 0)
>                  goto done;
>          }
> diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
> index 3f4b6302b9..bdb367f7a5 100644
> --- a/tests/securityselinuxlabeldata/chardev.txt
> +++ b/tests/securityselinuxlabeldata/chardev.txt
> @@ -2,6 +2,6 @@
>  /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
>  /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
>  /nolabel.sock;
> -/plain.sock;
> +/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>  /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>  /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed. Congratulations on your first libvirt contribution!

Michal
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by Michal Prívozník 1 year, 10 months ago 
On 6/28/22 14:33, David Michael wrote:
> This supports sockets created by libvirt and passed by FD using the
> same method as in security_dac.c.
> 
> Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> ---
> 
> Hi,
> 
> Custom SELinux labels are not applied to sockets when they have
> mode="bind", but other security models (DAC) allow changing these
> sockets.  Can the same method be used to support SELinux?
> 
> Thanks.
> 
> David
> 
>  src/security/security_selinux.c            | 6 ++++--
>  tests/securityselinuxlabeldata/chardev.txt | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index e2f34a27dc..8b258c9e36 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c

> @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
>          if (!dev_source->data.nix.listen) {
>              if (virSecuritySELinuxRestoreFileLabel(mgr,
> -                                                   dev_source->data.file.path,
> +                                                   dev_source->data.nix.path,
>                                                     true) < 0)
>                  goto done;
>          }

Regardless of the fate of the rest of the patch, this hunk is a bug fix
and thus should be merged. It's just a coincidence that data.file.path
maps onto data.nix.path in the union.

Michal
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by Daniel P. Berrangé 1 year, 10 months ago 
On Tue, Jun 28, 2022 at 08:33:41AM -0400, David Michael wrote:
> This supports sockets created by libvirt and passed by FD using the
> same method as in security_dac.c.
> 
> Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> ---
> 
> Hi,
> 
> Custom SELinux labels are not applied to sockets when they have
> mode="bind", but other security models (DAC) allow changing these
> sockets.  Can the same method be used to support SELinux?

This is rather intriguing. There must have been some compelling
reason why we intentionally skipped listener sockets for SELinux
labelling originally, but I'm struggling to recall what it could
have been. Conceptually it makes sense to want to label the
listener sockets with the per-VM label.


How did you come across this issue ?  Is there a particular
deployment/usage sceanrio where you're tripping up over this
flaw ?

> 
> Thanks.
> 
> David
> 
>  src/security/security_selinux.c            | 6 ++++--
>  tests/securityselinuxlabeldata/chardev.txt | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index e2f34a27dc..8b258c9e36 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
>          break;
>  
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
> -        if (!dev_source->data.nix.listen) {
> +        if (!dev_source->data.nix.listen ||
> +            (dev_source->data.nix.path &&
> +             virFileExists(dev_source->data.nix.path))) {
>              if (virSecuritySELinuxSetFilecon(mgr,
>                                               dev_source->data.nix.path,
>                                               imagelabel,
> @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
>          if (!dev_source->data.nix.listen) {
>              if (virSecuritySELinuxRestoreFileLabel(mgr,
> -                                                   dev_source->data.file.path,
> +                                                   dev_source->data.nix.path,
>                                                     true) < 0)
>                  goto done;
>          }
> diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
> index 3f4b6302b9..bdb367f7a5 100644
> --- a/tests/securityselinuxlabeldata/chardev.txt
> +++ b/tests/securityselinuxlabeldata/chardev.txt
> @@ -2,6 +2,6 @@
>  /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
>  /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
>  /nolabel.sock;
> -/plain.sock;
> +/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>  /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>  /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264
> -- 
> 2.36.1
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by David Michael 1 year, 10 months ago 
On Tue, Jun 28, 2022 at 10:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Tue, Jun 28, 2022 at 08:33:41AM -0400, David Michael wrote:
> > This supports sockets created by libvirt and passed by FD using the
> > same method as in security_dac.c.
> >
> > Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> > ---
> >
> > Hi,
> >
> > Custom SELinux labels are not applied to sockets when they have
> > mode="bind", but other security models (DAC) allow changing these
> > sockets.  Can the same method be used to support SELinux?
>
> This is rather intriguing. There must have been some compelling
> reason why we intentionally skipped listener sockets for SELinux
> labelling originally, but I'm struggling to recall what it could
> have been. Conceptually it makes sense to want to label the
> listener sockets with the per-VM label.
>
> How did you come across this issue ?  Is there a particular
> deployment/usage sceanrio where you're tripping up over this
> flaw ?

Yes, setting custom MLS labels on the VMs (replacing the default sVirt
type and random categories) resulted in sockets being created with the
system policy's type and SystemLow.  So then there are denials for
QEMU and user processes running at specific MLS levels trying to work
with the sockets' unexpected type and level.  It's a customized
appliance-specific SELinux policy.

> > Thanks.
> >
> > David
> >
> >  src/security/security_selinux.c            | 6 ++++--
> >  tests/securityselinuxlabeldata/chardev.txt | 2 +-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> > index e2f34a27dc..8b258c9e36 100644
> > --- a/src/security/security_selinux.c
> > +++ b/src/security/security_selinux.c
> > @@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
> >          break;
> >
> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
> > -        if (!dev_source->data.nix.listen) {
> > +        if (!dev_source->data.nix.listen ||
> > +            (dev_source->data.nix.path &&
> > +             virFileExists(dev_source->data.nix.path))) {
> >              if (virSecuritySELinuxSetFilecon(mgr,
> >                                               dev_source->data.nix.path,
> >                                               imagelabel,
> > @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
> >          if (!dev_source->data.nix.listen) {
> >              if (virSecuritySELinuxRestoreFileLabel(mgr,
> > -                                                   dev_source->data.file.path,
> > +                                                   dev_source->data.nix.path,
> >                                                     true) < 0)
> >                  goto done;
> >          }
> > diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
> > index 3f4b6302b9..bdb367f7a5 100644
> > --- a/tests/securityselinuxlabeldata/chardev.txt
> > +++ b/tests/securityselinuxlabeldata/chardev.txt
> > @@ -2,6 +2,6 @@
> >  /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
> >  /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
> >  /nolabel.sock;
> > -/plain.sock;
> > +/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
> >  /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
> >  /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264
> > --
> > 2.36.1
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by Martin Kletzander 1 year, 10 months ago 
On Tue, Jun 28, 2022 at 10:18:25AM -0400, David Michael wrote:
>On Tue, Jun 28, 2022 at 10:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Tue, Jun 28, 2022 at 08:33:41AM -0400, David Michael wrote:
>> > This supports sockets created by libvirt and passed by FD using the
>> > same method as in security_dac.c.
>> >
>> > Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
>> > ---
>> >
>> > Hi,
>> >
>> > Custom SELinux labels are not applied to sockets when they have
>> > mode="bind", but other security models (DAC) allow changing these
>> > sockets.  Can the same method be used to support SELinux?
>>
>> This is rather intriguing. There must have been some compelling
>> reason why we intentionally skipped listener sockets for SELinux
>> labelling originally, but I'm struggling to recall what it could
>> have been. Conceptually it makes sense to want to label the
>> listener sockets with the per-VM label.
>>

Could it be that we only thought about the scenario of someone
connecting to the socket (and the fact that it matters more what the
actual socket label is rather than its file representation) but did not
think about other possibilities, e.g. QEMU rewriting the socket (because
we remove it before starting or any other reason) or some custom policy?

>> How did you come across this issue ?  Is there a particular
>> deployment/usage sceanrio where you're tripping up over this
>> flaw ?
>
>Yes, setting custom MLS labels on the VMs (replacing the default sVirt
>type and random categories) resulted in sockets being created with the
>system policy's type and SystemLow.  So then there are denials for
>QEMU and user processes running at specific MLS levels trying to work
>with the sockets' unexpected type and level.  It's a customized
>appliance-specific SELinux policy.
>
>> > Thanks.
>> >
>> > David
>> >
>> >  src/security/security_selinux.c            | 6 ++++--
>> >  tests/securityselinuxlabeldata/chardev.txt | 2 +-
>> >  2 files changed, 5 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
>> > index e2f34a27dc..8b258c9e36 100644
>> > --- a/src/security/security_selinux.c
>> > +++ b/src/security/security_selinux.c
>> > @@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
>> >          break;
>> >
>> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
>> > -        if (!dev_source->data.nix.listen) {
>> > +        if (!dev_source->data.nix.listen ||
>> > +            (dev_source->data.nix.path &&
>> > +             virFileExists(dev_source->data.nix.path))) {
>> >              if (virSecuritySELinuxSetFilecon(mgr,
>> >                                               dev_source->data.nix.path,
>> >                                               imagelabel,
>> > @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
>> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
>> >          if (!dev_source->data.nix.listen) {
>> >              if (virSecuritySELinuxRestoreFileLabel(mgr,
>> > -                                                   dev_source->data.file.path,
>> > +                                                   dev_source->data.nix.path,
>> >                                                     true) < 0)
>> >                  goto done;
>> >          }
>> > diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
>> > index 3f4b6302b9..bdb367f7a5 100644
>> > --- a/tests/securityselinuxlabeldata/chardev.txt
>> > +++ b/tests/securityselinuxlabeldata/chardev.txt
>> > @@ -2,6 +2,6 @@
>> >  /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
>> >  /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
>> >  /nolabel.sock;
>> > -/plain.sock;
>> > +/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>> >  /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
>> >  /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264
>> > --
>> > 2.36.1
>> >
>>
>> With regards,
>> Daniel
>> --
>> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by David Michael 1 year, 10 months ago 
On Tue, Jun 28, 2022 at 4:14 PM Martin Kletzander <mkletzan@redhat.com> wrote:
> On Tue, Jun 28, 2022 at 10:18:25AM -0400, David Michael wrote:
> >On Tue, Jun 28, 2022 at 10:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >> On Tue, Jun 28, 2022 at 08:33:41AM -0400, David Michael wrote:
> >> > This supports sockets created by libvirt and passed by FD using the
> >> > same method as in security_dac.c.
> >> >
> >> > Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> >> > ---
> >> >
> >> > Hi,
> >> >
> >> > Custom SELinux labels are not applied to sockets when they have
> >> > mode="bind", but other security models (DAC) allow changing these
> >> > sockets.  Can the same method be used to support SELinux?
> >>
> >> This is rather intriguing. There must have been some compelling
> >> reason why we intentionally skipped listener sockets for SELinux
> >> labelling originally, but I'm struggling to recall what it could
> >> have been. Conceptually it makes sense to want to label the
> >> listener sockets with the per-VM label.
> >>
>
> Could it be that we only thought about the scenario of someone
> connecting to the socket (and the fact that it matters more what the
> actual socket label is rather than its file representation) but did not
> think about other possibilities, e.g. QEMU rewriting the socket (because
> we remove it before starting or any other reason) or some custom policy?

I checked the Git history for when DAC made the change, which has some
further links for context:

https://gitlab.com/libvirt/libvirt/-/commit/d6b8838dd83697f721fe0706068df765148154de

It looks like socket DAC relabeling was added shortly after libvirt
started creating the sockets to pass as FDs within the last few years,
but the related SELinux socket code hasn't been modified since it was
added a decade ago.  Seems like it just got left behind after that
change?

> >> How did you come across this issue ?  Is there a particular
> >> deployment/usage sceanrio where you're tripping up over this
> >> flaw ?
> >
> >Yes, setting custom MLS labels on the VMs (replacing the default sVirt
> >type and random categories) resulted in sockets being created with the
> >system policy's type and SystemLow.  So then there are denials for
> >QEMU and user processes running at specific MLS levels trying to work
> >with the sockets' unexpected type and level.  It's a customized
> >appliance-specific SELinux policy.
> >
> >> > Thanks.
> >> >
> >> > David
> >> >
> >> >  src/security/security_selinux.c            | 6 ++++--
> >> >  tests/securityselinuxlabeldata/chardev.txt | 2 +-
> >> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> >> > index e2f34a27dc..8b258c9e36 100644
> >> > --- a/src/security/security_selinux.c
> >> > +++ b/src/security/security_selinux.c
> >> > @@ -2541,7 +2541,9 @@ virSecuritySELinuxSetChardevLabel(virSecurityManager *mgr,
> >> >          break;
> >> >
> >> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
> >> > -        if (!dev_source->data.nix.listen) {
> >> > +        if (!dev_source->data.nix.listen ||
> >> > +            (dev_source->data.nix.path &&
> >> > +             virFileExists(dev_source->data.nix.path))) {
> >> >              if (virSecuritySELinuxSetFilecon(mgr,
> >> >                                               dev_source->data.nix.path,
> >> >                                               imagelabel,
> >> > @@ -2618,7 +2620,7 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManager *mgr,
> >> >      case VIR_DOMAIN_CHR_TYPE_UNIX:
> >> >          if (!dev_source->data.nix.listen) {
> >> >              if (virSecuritySELinuxRestoreFileLabel(mgr,
> >> > -                                                   dev_source->data.file.path,
> >> > +                                                   dev_source->data.nix.path,
> >> >                                                     true) < 0)
> >> >                  goto done;
> >> >          }
> >> > diff --git a/tests/securityselinuxlabeldata/chardev.txt b/tests/securityselinuxlabeldata/chardev.txt
> >> > index 3f4b6302b9..bdb367f7a5 100644
> >> > --- a/tests/securityselinuxlabeldata/chardev.txt
> >> > +++ b/tests/securityselinuxlabeldata/chardev.txt
> >> > @@ -2,6 +2,6 @@
> >> >  /plain.dev;system_u:object_r:svirt_image_t:s0:c41,c264
> >> >  /plain.fifo;system_u:object_r:svirt_image_t:s0:c41,c264
> >> >  /nolabel.sock;
> >> > -/plain.sock;
> >> > +/plain.sock;system_u:object_r:svirt_image_t:s0:c41,c264
> >> >  /yeslabel.sock;system_u:object_r:svirt_image_t:s0:c41,c264
> >> >  /altlabel.sock;system_u:object_r:svirt_image_custom_t:s0:c41,c264
> >> > --
> >> > 2.36.1
> >> >
> >>
> >> With regards,
> >> Daniel
> >> --
> >> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> >> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> >> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
> >
Re: [PATCH] security_selinux.c: Relabel existing mode="bind" UNIX sockets
Posted by Daniel P. Berrangé 1 year, 10 months ago 
On Tue, Jun 28, 2022 at 05:00:06PM -0400, David Michael wrote:
> On Tue, Jun 28, 2022 at 4:14 PM Martin Kletzander <mkletzan@redhat.com> wrote:
> > On Tue, Jun 28, 2022 at 10:18:25AM -0400, David Michael wrote:
> > >On Tue, Jun 28, 2022 at 10:03 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >> On Tue, Jun 28, 2022 at 08:33:41AM -0400, David Michael wrote:
> > >> > This supports sockets created by libvirt and passed by FD using the
> > >> > same method as in security_dac.c.
> > >> >
> > >> > Signed-off-by: David Michael <david@bigbadwolfsecurity.com>
> > >> > ---
> > >> >
> > >> > Hi,
> > >> >
> > >> > Custom SELinux labels are not applied to sockets when they have
> > >> > mode="bind", but other security models (DAC) allow changing these
> > >> > sockets.  Can the same method be used to support SELinux?
> > >>
> > >> This is rather intriguing. There must have been some compelling
> > >> reason why we intentionally skipped listener sockets for SELinux
> > >> labelling originally, but I'm struggling to recall what it could
> > >> have been. Conceptually it makes sense to want to label the
> > >> listener sockets with the per-VM label.
> > >>
> >
> > Could it be that we only thought about the scenario of someone
> > connecting to the socket (and the fact that it matters more what the
> > actual socket label is rather than its file representation) but did not
> > think about other possibilities, e.g. QEMU rewriting the socket (because
> > we remove it before starting or any other reason) or some custom policy?
> 
> I checked the Git history for when DAC made the change, which has some
> further links for context:
> 
> https://gitlab.com/libvirt/libvirt/-/commit/d6b8838dd83697f721fe0706068df765148154de
> 
> It looks like socket DAC relabeling was added shortly after libvirt
> started creating the sockets to pass as FDs within the last few years,
> but the related SELinux socket code hasn't been modified since it was
> added a decade ago.  Seems like it just got left behind after that
> change?

Yeah that looks like the cause. Originally when QEMU invoked the
bind() + listen(), it would inherit labelling, so we wouldn't need
todo anything special ourselves. When we switched to creating it
ourselves & passing the FD we broke that implicit behaviour we
relied on.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.