[dm-devel] [PATCH 0/1] Signal to disallow open of a dm device

[dm-devel] [PATCH 0/1] Signal to disallow open of a dm device

[Daniil Lunev]

There was a previuous discussion upstream about this kind of
functionality (see https://lkml.org/lkml/2022/1/24/633). The previous
patchset was rejected with concerns about how the feature was
integrated. This patch takes a different approach, and instead of
tying itself to unrelated mechanisms (e.g. deffer remove), this one
allows an explicit signal via message interface to signalize device
mapper shall block any further access to the device. The primary use
case is to restrict userspace access to decrypted data after a device
was setup with a kernel facility (e.g. swap).


Daniil Lunev (1):
  dm: add message command to disallow device open

 drivers/md/dm-core.h          |  1 +
 drivers/md/dm-ioctl.c         | 10 ++++++++++
 drivers/md/dm.c               | 12 ++++++++++++
 drivers/md/dm.h               | 10 ++++++++++
 include/uapi/linux/dm-ioctl.h |  5 +++++
 5 files changed, 38 insertions(+)

-- 
2.31.0

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

A message can be passed to device mapper to prohibit open on a certain
mapped device. This makes possible to disallow userspace access to
raw swapped data if the system uses device mapper to encrypt it at rest.

Signed-off-by: Daniil Lunev <dlunev@chromium.org>
---

 drivers/md/dm-core.h          |  1 +
 drivers/md/dm-ioctl.c         | 10 ++++++++++
 drivers/md/dm.c               | 12 ++++++++++++
 drivers/md/dm.h               | 10 ++++++++++
 include/uapi/linux/dm-ioctl.h |  5 +++++
 5 files changed, 38 insertions(+)

diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index 4277853c75351..37529b605b7c4 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -140,6 +140,7 @@ struct mapped_device {
 #define DMF_SUSPENDED_INTERNALLY 7
 #define DMF_POST_SUSPENDING 8
 #define DMF_EMULATE_ZONE_APPEND 9
+#define DMF_DISALLOW_OPEN 10
 
 void disable_discard(struct mapped_device *md);
 void disable_write_zeroes(struct mapped_device *md);
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 87310fceb0d86..e35d560aa2ff3 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -815,6 +815,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
 	if (dm_test_deferred_remove_flag(md))
 		param->flags |= DM_DEFERRED_REMOVE;
 
+	if (dm_test_disallow_open_flag(md))
+		param->flags |= DM_DISALLOWED_OPEN;
+
 	param->dev = huge_encode_dev(disk_devt(disk));
 
 	/*
@@ -1656,6 +1659,13 @@ static int message_for_md(struct mapped_device *md, unsigned argc, char **argv,
 		}
 		return dm_cancel_deferred_remove(md);
 	}
+	if (!strcasecmp(argv[0], "@disallow_open")) {
+		if (argc != 1) {
+			DMERR("Invalid arguments for @disallow_open");
+			return -EINVAL;
+		}
+		return dm_disallow_open(md);
+	}
 
 	r = dm_stats_message(md, argc, argv, result, maxlen);
 	if (r < 2)
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 82957bd460e89..3e53d1bd40f0c 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -327,6 +327,7 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
 		goto out;
 
 	if (test_bit(DMF_FREEING, &md->flags) ||
+	    test_bit(DMF_DISALLOW_OPEN, &md->flags) ||
 	    dm_deleting_md(md)) {
 		md = NULL;
 		goto out;
@@ -403,6 +404,12 @@ int dm_cancel_deferred_remove(struct mapped_device *md)
 	return r;
 }
 
+int dm_disallow_open(struct mapped_device *md)
+{
+	set_bit(DMF_DISALLOW_OPEN, &md->flags);
+	return 0;
+}
+
 static void do_deferred_remove(struct work_struct *w)
 {
 	dm_deferred_remove();
@@ -2883,6 +2890,11 @@ int dm_test_deferred_remove_flag(struct mapped_device *md)
 	return test_bit(DMF_DEFERRED_REMOVE, &md->flags);
 }
 
+int dm_test_disallow_open_flag(struct mapped_device *md)
+{
+	return test_bit(DMF_DISALLOW_OPEN, &md->flags);
+}
+
 int dm_suspended(struct dm_target *ti)
 {
 	return dm_suspended_md(ti->table->md);
diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 9013dc1a7b002..da27f9dfe1413 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -163,6 +163,16 @@ int dm_test_deferred_remove_flag(struct mapped_device *md);
  */
 void dm_deferred_remove(void);
 
+/*
+ * Test if the device is openable.
+ */
+int dm_test_disallow_open_flag(struct mapped_device *md);
+
+/*
+ * Prevent new open request on the device.
+ */
+int dm_disallow_open(struct mapped_device *md);
+
 /*
  * The device-mapper can be driven through one of two interfaces;
  * ioctl or filesystem, depending which patch you have applied.
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index 2e9550fef90fa..3b4d12d09c005 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -382,4 +382,9 @@ enum {
  */
 #define DM_IMA_MEASUREMENT_FLAG	(1 << 19) /* In */
 
+/*
+ * If set, the device can not be opened.
+ */
+#define DM_DISALLOWED_OPEN	(1 << 20) /* Out */
+
 #endif				/* _LINUX_DM_IOCTL_H */
-- 
2.31.0

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Mike Snitzer]

On Sun, Jul 03 2022 at  8:02P -0400,
Daniil Lunev <dlunev@chromium.org> wrote:

> A message can be passed to device mapper to prohibit open on a certain
> mapped device. This makes possible to disallow userspace access to
> raw swapped data if the system uses device mapper to encrypt it at rest.
> 
> Signed-off-by: Daniil Lunev <dlunev@chromium.org>

This commit header and patch make little sense to me.

If you're concerned about a normal (non-root) user having read access
to the swap device then disallow non-root user access permissions on
the swap device.

Why is an encrypted swap device any different than any other encrypted
device?

As is, this patch seems to be the wrong way to achieve your desired
result.  If you or someone else on the chromium team can better
defend/explain the need for this change please do so.

Thanks,
Mike


> ---
> 
>  drivers/md/dm-core.h          |  1 +
>  drivers/md/dm-ioctl.c         | 10 ++++++++++
>  drivers/md/dm.c               | 12 ++++++++++++
>  drivers/md/dm.h               | 10 ++++++++++
>  include/uapi/linux/dm-ioctl.h |  5 +++++
>  5 files changed, 38 insertions(+)
> 
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index 4277853c75351..37529b605b7c4 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -140,6 +140,7 @@ struct mapped_device {
>  #define DMF_SUSPENDED_INTERNALLY 7
>  #define DMF_POST_SUSPENDING 8
>  #define DMF_EMULATE_ZONE_APPEND 9
> +#define DMF_DISALLOW_OPEN 10
>  
>  void disable_discard(struct mapped_device *md);
>  void disable_write_zeroes(struct mapped_device *md);
> diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> index 87310fceb0d86..e35d560aa2ff3 100644
> --- a/drivers/md/dm-ioctl.c
> +++ b/drivers/md/dm-ioctl.c
> @@ -815,6 +815,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
>  	if (dm_test_deferred_remove_flag(md))
>  		param->flags |= DM_DEFERRED_REMOVE;
>  
> +	if (dm_test_disallow_open_flag(md))
> +		param->flags |= DM_DISALLOWED_OPEN;
> +
>  	param->dev = huge_encode_dev(disk_devt(disk));
>  
>  	/*
> @@ -1656,6 +1659,13 @@ static int message_for_md(struct mapped_device *md, unsigned argc, char **argv,
>  		}
>  		return dm_cancel_deferred_remove(md);
>  	}
> +	if (!strcasecmp(argv[0], "@disallow_open")) {
> +		if (argc != 1) {
> +			DMERR("Invalid arguments for @disallow_open");
> +			return -EINVAL;
> +		}
> +		return dm_disallow_open(md);
> +	}
>  
>  	r = dm_stats_message(md, argc, argv, result, maxlen);
>  	if (r < 2)
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 82957bd460e89..3e53d1bd40f0c 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -327,6 +327,7 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
>  		goto out;
>  
>  	if (test_bit(DMF_FREEING, &md->flags) ||
> +	    test_bit(DMF_DISALLOW_OPEN, &md->flags) ||
>  	    dm_deleting_md(md)) {
>  		md = NULL;
>  		goto out;
> @@ -403,6 +404,12 @@ int dm_cancel_deferred_remove(struct mapped_device *md)
>  	return r;
>  }
>  
> +int dm_disallow_open(struct mapped_device *md)
> +{
> +	set_bit(DMF_DISALLOW_OPEN, &md->flags);
> +	return 0;
> +}
> +
>  static void do_deferred_remove(struct work_struct *w)
>  {
>  	dm_deferred_remove();
> @@ -2883,6 +2890,11 @@ int dm_test_deferred_remove_flag(struct mapped_device *md)
>  	return test_bit(DMF_DEFERRED_REMOVE, &md->flags);
>  }
>  
> +int dm_test_disallow_open_flag(struct mapped_device *md)
> +{
> +	return test_bit(DMF_DISALLOW_OPEN, &md->flags);
> +}
> +
>  int dm_suspended(struct dm_target *ti)
>  {
>  	return dm_suspended_md(ti->table->md);
> diff --git a/drivers/md/dm.h b/drivers/md/dm.h
> index 9013dc1a7b002..da27f9dfe1413 100644
> --- a/drivers/md/dm.h
> +++ b/drivers/md/dm.h
> @@ -163,6 +163,16 @@ int dm_test_deferred_remove_flag(struct mapped_device *md);
>   */
>  void dm_deferred_remove(void);
>  
> +/*
> + * Test if the device is openable.
> + */
> +int dm_test_disallow_open_flag(struct mapped_device *md);
> +
> +/*
> + * Prevent new open request on the device.
> + */
> +int dm_disallow_open(struct mapped_device *md);
> +
>  /*
>   * The device-mapper can be driven through one of two interfaces;
>   * ioctl or filesystem, depending which patch you have applied.
> diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
> index 2e9550fef90fa..3b4d12d09c005 100644
> --- a/include/uapi/linux/dm-ioctl.h
> +++ b/include/uapi/linux/dm-ioctl.h
> @@ -382,4 +382,9 @@ enum {
>   */
>  #define DM_IMA_MEASUREMENT_FLAG	(1 << 19) /* In */
>  
> +/*
> + * If set, the device can not be opened.
> + */
> +#define DM_DISALLOWED_OPEN	(1 << 20) /* Out */
> +
>  #endif				/* _LINUX_DM_IOCTL_H */
> -- 
> 2.31.0
> 

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

Hi Mike,
Thank you for your response. I should have probably added more context
to the commit message that I specified in the cover letter. The idea is to
prohibit access of all userspace, including the root. The main concern here
is potential system applications' vulnerabilities that can trick the system to
operate on non-intended files with elevated permissions. While those could
also be exploited to get more access to the regular file systems, those firstly
has to be useable by userspace for normal system operation (e.g. to store
user data), secondly, never contain plain text secrets. Swap content is a
different story - access to it can leak very sensitive information, which
otherwise is never available as plaintext on any persistent media - e.g. raw
user secrets, raw disk encryption keys etc, other security related tokens.
Thus we propose a mechanism to enable such a lockdown after necessary
configuration has been done to the device at boot time.
--Daniil

On Fri, Jul 15, 2022 at 6:13 AM Mike Snitzer <snitzer@kernel.org> wrote:
>
> On Sun, Jul 03 2022 at  8:02P -0400,
> Daniil Lunev <dlunev@chromium.org> wrote:
>
> > A message can be passed to device mapper to prohibit open on a certain
> > mapped device. This makes possible to disallow userspace access to
> > raw swapped data if the system uses device mapper to encrypt it at rest.
> >
> > Signed-off-by: Daniil Lunev <dlunev@chromium.org>
>
> This commit header and patch make little sense to me.
>
> If you're concerned about a normal (non-root) user having read access
> to the swap device then disallow non-root user access permissions on
> the swap device.
>
> Why is an encrypted swap device any different than any other encrypted
> device?
>
> As is, this patch seems to be the wrong way to achieve your desired
> result.  If you or someone else on the chromium team can better
> defend/explain the need for this change please do so.
>
> Thanks,
> Mike
>
>
> > ---
> >
> >  drivers/md/dm-core.h          |  1 +
> >  drivers/md/dm-ioctl.c         | 10 ++++++++++
> >  drivers/md/dm.c               | 12 ++++++++++++
> >  drivers/md/dm.h               | 10 ++++++++++
> >  include/uapi/linux/dm-ioctl.h |  5 +++++
> >  5 files changed, 38 insertions(+)
> >
> > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> > index 4277853c75351..37529b605b7c4 100644
> > --- a/drivers/md/dm-core.h
> > +++ b/drivers/md/dm-core.h
> > @@ -140,6 +140,7 @@ struct mapped_device {
> >  #define DMF_SUSPENDED_INTERNALLY 7
> >  #define DMF_POST_SUSPENDING 8
> >  #define DMF_EMULATE_ZONE_APPEND 9
> > +#define DMF_DISALLOW_OPEN 10
> >
> >  void disable_discard(struct mapped_device *md);
> >  void disable_write_zeroes(struct mapped_device *md);
> > diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
> > index 87310fceb0d86..e35d560aa2ff3 100644
> > --- a/drivers/md/dm-ioctl.c
> > +++ b/drivers/md/dm-ioctl.c
> > @@ -815,6 +815,9 @@ static void __dev_status(struct mapped_device *md, struct dm_ioctl *param)
> >       if (dm_test_deferred_remove_flag(md))
> >               param->flags |= DM_DEFERRED_REMOVE;
> >
> > +     if (dm_test_disallow_open_flag(md))
> > +             param->flags |= DM_DISALLOWED_OPEN;
> > +
> >       param->dev = huge_encode_dev(disk_devt(disk));
> >
> >       /*
> > @@ -1656,6 +1659,13 @@ static int message_for_md(struct mapped_device *md, unsigned argc, char **argv,
> >               }
> >               return dm_cancel_deferred_remove(md);
> >       }
> > +     if (!strcasecmp(argv[0], "@disallow_open")) {
> > +             if (argc != 1) {
> > +                     DMERR("Invalid arguments for @disallow_open");
> > +                     return -EINVAL;
> > +             }
> > +             return dm_disallow_open(md);
> > +     }
> >
> >       r = dm_stats_message(md, argc, argv, result, maxlen);
> >       if (r < 2)
> > diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> > index 82957bd460e89..3e53d1bd40f0c 100644
> > --- a/drivers/md/dm.c
> > +++ b/drivers/md/dm.c
> > @@ -327,6 +327,7 @@ static int dm_blk_open(struct block_device *bdev, fmode_t mode)
> >               goto out;
> >
> >       if (test_bit(DMF_FREEING, &md->flags) ||
> > +         test_bit(DMF_DISALLOW_OPEN, &md->flags) ||
> >           dm_deleting_md(md)) {
> >               md = NULL;
> >               goto out;
> > @@ -403,6 +404,12 @@ int dm_cancel_deferred_remove(struct mapped_device *md)
> >       return r;
> >  }
> >
> > +int dm_disallow_open(struct mapped_device *md)
> > +{
> > +     set_bit(DMF_DISALLOW_OPEN, &md->flags);
> > +     return 0;
> > +}
> > +
> >  static void do_deferred_remove(struct work_struct *w)
> >  {
> >       dm_deferred_remove();
> > @@ -2883,6 +2890,11 @@ int dm_test_deferred_remove_flag(struct mapped_device *md)
> >       return test_bit(DMF_DEFERRED_REMOVE, &md->flags);
> >  }
> >
> > +int dm_test_disallow_open_flag(struct mapped_device *md)
> > +{
> > +     return test_bit(DMF_DISALLOW_OPEN, &md->flags);
> > +}
> > +
> >  int dm_suspended(struct dm_target *ti)
> >  {
> >       return dm_suspended_md(ti->table->md);
> > diff --git a/drivers/md/dm.h b/drivers/md/dm.h
> > index 9013dc1a7b002..da27f9dfe1413 100644
> > --- a/drivers/md/dm.h
> > +++ b/drivers/md/dm.h
> > @@ -163,6 +163,16 @@ int dm_test_deferred_remove_flag(struct mapped_device *md);
> >   */
> >  void dm_deferred_remove(void);
> >
> > +/*
> > + * Test if the device is openable.
> > + */
> > +int dm_test_disallow_open_flag(struct mapped_device *md);
> > +
> > +/*
> > + * Prevent new open request on the device.
> > + */
> > +int dm_disallow_open(struct mapped_device *md);
> > +
> >  /*
> >   * The device-mapper can be driven through one of two interfaces;
> >   * ioctl or filesystem, depending which patch you have applied.
> > diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
> > index 2e9550fef90fa..3b4d12d09c005 100644
> > --- a/include/uapi/linux/dm-ioctl.h
> > +++ b/include/uapi/linux/dm-ioctl.h
> > @@ -382,4 +382,9 @@ enum {
> >   */
> >  #define DM_IMA_MEASUREMENT_FLAG      (1 << 19) /* In */
> >
> > +/*
> > + * If set, the device can not be opened.
> > + */
> > +#define DM_DISALLOWED_OPEN   (1 << 20) /* Out */
> > +
> >  #endif                               /* _LINUX_DM_IOCTL_H */
> > --
> > 2.31.0
> >

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Mikulas Patocka]

On Fri, 15 Jul 2022, Daniil Lunev wrote:

> Hi Mike,
> Thank you for your response. I should have probably added more context
> to the commit message that I specified in the cover letter. The idea is to
> prohibit access of all userspace, including the root. The main concern here
> is potential system applications' vulnerabilities that can trick the system to
> operate on non-intended files with elevated permissions. While those could
> also be exploited to get more access to the regular file systems, those firstly
> has to be useable by userspace for normal system operation (e.g. to store
> user data), secondly, never contain plain text secrets. Swap content is a
> different story - access to it can leak very sensitive information, which
> otherwise is never available as plaintext on any persistent media - e.g. raw
> user secrets, raw disk encryption keys etc, other security related tokens.
> Thus we propose a mechanism to enable such a lockdown after necessary
> configuration has been done to the device at boot time.
> --Daniil

If someone gains root, he can do anything on the system.

I'm quite skeptical about these attempts; protecting the system from the 
root user is never-ending whack-a-mole game.

Mikulas
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Zdenek Kabelac]

Dne 15. 07. 22 v 11:36 Mikulas Patocka napsal(a):
>
> On Fri, 15 Jul 2022, Daniil Lunev wrote:
>
>> Hi Mike,
>> Thank you for your response. I should have probably added more context
>> to the commit message that I specified in the cover letter. The idea is to
>> prohibit access of all userspace, including the root. The main concern here
>> is potential system applications' vulnerabilities that can trick the system to
>> operate on non-intended files with elevated permissions. While those could
>> also be exploited to get more access to the regular file systems, those firstly
>> has to be useable by userspace for normal system operation (e.g. to store
>> user data), secondly, never contain plain text secrets. Swap content is a
>> different story - access to it can leak very sensitive information, which
>> otherwise is never available as plaintext on any persistent media - e.g. raw
>> user secrets, raw disk encryption keys etc, other security related tokens.
>> Thus we propose a mechanism to enable such a lockdown after necessary
>> configuration has been done to the device at boot time.
>> --Daniil
> If someone gains root, he can do anything on the system.
>
> I'm quite skeptical about these attempts; protecting the system from the
> root user is never-ending whack-a-mole game.


It's in fact a 'design feature' of whole DM  that root can always open any 
device in device stack (although cause some troubles to i.e. some lvm2 logic) 
such feature is useful i.e. for debugging device problems. There was never an 
intention to prohibit root user from 'seeing' all stacked devices.

Regards

Zdenek

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

We understand that if someone acquires root it is a game over. The intent of
this mechanism is to reduce the attack surface. The exposure might be a
certain system daemon that is exploited into accessing a wrong node in
the filesystem. And exposing modifiable system memory is a pathway for
further escalation and leaks of secrets. This is a defense in depth mechanism,
that is intended to make attackers' lives harder even if they find an
exploitable
vulnerability.
We understand that in regular situations people may not want the behaviour,
that is why the mechanism is controlled via a side channel - if a message is
never sent - the behaviour is not altered.
--Daniil

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

Hello all
To signal boost here. What can we do to advance the discussion on this
topic? Can we move forward with the approach or are there any
alternative suggestions how the desired behaviour can be achieved?
Thanks,
--Daniil

On Tue, Jul 19, 2022 at 9:42 AM Daniil Lunev <dlunev@chromium.org> wrote:
>
> We understand that if someone acquires root it is a game over. The intent of
> this mechanism is to reduce the attack surface. The exposure might be a
> certain system daemon that is exploited into accessing a wrong node in
> the filesystem. And exposing modifiable system memory is a pathway for
> further escalation and leaks of secrets. This is a defense in depth mechanism,
> that is intended to make attackers' lives harder even if they find an
> exploitable
> vulnerability.
> We understand that in regular situations people may not want the behaviour,
> that is why the mechanism is controlled via a side channel - if a message is
> never sent - the behaviour is not altered.
> --Daniil

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Eric Biggers]

On Wed, Aug 03, 2022 at 02:12:26PM +1000, Daniil Lunev wrote:
> Hello all
> To signal boost here. What can we do to advance the discussion on this
> topic? Can we move forward with the approach or are there any
> alternative suggestions how the desired behaviour can be achieved?
> Thanks,
> --Daniil
> 
> On Tue, Jul 19, 2022 at 9:42 AM Daniil Lunev <dlunev@chromium.org> wrote:
> >
> > We understand that if someone acquires root it is a game over. The intent of
> > this mechanism is to reduce the attack surface. The exposure might be a
> > certain system daemon that is exploited into accessing a wrong node in
> > the filesystem. And exposing modifiable system memory is a pathway for
> > further escalation and leaks of secrets. This is a defense in depth mechanism,
> > that is intended to make attackers' lives harder even if they find an
> > exploitable
> > vulnerability.
> > We understand that in regular situations people may not want the behaviour,
> > that is why the mechanism is controlled via a side channel - if a message is
> > never sent - the behaviour is not altered.
> > --Daniil

This seems like an access control policy, which the Linux kernel already has a
lot of mechanisms for.  Chrome OS already uses SELinux.  Couldn't this be solved
by giving the device node an SELinux label that no one has permission to open?

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

> This seems like an access control policy, which the Linux kernel already has a
> lot of mechanisms for.  Chrome OS already uses SELinux.  Couldn't this be solved
> by giving the device node an SELinux label that no one has permission to open?
That would be the ideal solution, but there is a number of challenges
that prevent
us enabling enforcement on all SELinux domains unfortunately. While in the long
run that would be a preferred option, in the short run this doesn't
seem feasible. I
would assume the problem of enabling full SELInux enforcement would plague
any big project that didn't have them enabled from the get going.
--Daniil

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Mike Snitzer]

On Wed, Aug 03 2022 at 12:29P -0400,
Daniil Lunev <dlunev@chromium.org> wrote:

> > This seems like an access control policy, which the Linux kernel already has a
> > lot of mechanisms for.  Chrome OS already uses SELinux.  Couldn't this be solved
> > by giving the device node an SELinux label that no one has permission to open?
> That would be the ideal solution, but there is a number of challenges
> that prevent
> us enabling enforcement on all SELinux domains unfortunately. While in the long
> run that would be a preferred option, in the short run this doesn't
> seem feasible. I
> would assume the problem of enabling full SELInux enforcement would plague
> any big project that didn't have them enabled from the get going.
> --Daniil

I'm not going to take this patch. It isn't the proper way to handle
preventing use of a DM device. In addition, the patch's header doesn't
speak to a proper review/audit of implications this change would have
on all aspects of a DM device's capabilities. If Chrome OS needs this
as a stop-gap then please carry it as needed.

Regards,
Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

> I'm not going to take this patch. It isn't the proper way to handle
> preventing use of a DM device.
Can you suggest a better mechanism that would be acceptable
from your perspective?

> In addition, the patch's header doesn't speak to a proper
> review/audit of implications this change would have
> on all aspects of a DM device's capabilities.
I would gladly clarify the commit message, and I am sorry
for making it terse in the beginning. Can you please
clarify, what capabilities are you concerned about? The
change shouldn't change any existing semantics if the
mechanism is never used on a specific system, and only
alters "open" behaviour in the cases where the specific
message was issued, but I am happy to cover any
additional aspects you are concerned about

Thanks,
Daniil

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Eric Biggers]

On Wed, Aug 03, 2022 at 02:29:40PM +1000, Daniil Lunev wrote:
> > This seems like an access control policy, which the Linux kernel already has a
> > lot of mechanisms for.  Chrome OS already uses SELinux.  Couldn't this be solved
> > by giving the device node an SELinux label that no one has permission to open?
> That would be the ideal solution, but there is a number of challenges
> that prevent
> us enabling enforcement on all SELinux domains unfortunately. While in the long
> run that would be a preferred option, in the short run this doesn't
> seem feasible. I
> would assume the problem of enabling full SELInux enforcement would plague
> any big project that didn't have them enabled from the get going.
> --Daniil

Have you also considered unlinking the device node (/dev/dm-$idx) from the
filesystem after it has been set up for swap?

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

> Have you also considered unlinking the device node (/dev/dm-$idx) from the
> filesystem after it has been set up for swap?
Yes, the node can be re-linked with mknod, thus is not a suitable solution.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Eric Biggers]

On Thu, Aug 04, 2022 at 06:44:53AM +1000, Daniil Lunev wrote:
> > Have you also considered unlinking the device node (/dev/dm-$idx) from the
> > filesystem after it has been set up for swap?
> Yes, the node can be re-linked with mknod, thus is not a suitable solution.

I thought you were trying to defend against path traversal attacks, not
arbitrary code execution?  If your threat model includes arbitrary code
execution by root, you really need to be using SELinux.

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 1/1] dm: add message command to disallow device open

[Daniil Lunev]

> I thought you were trying to defend against path traversal attacks, not
> arbitrary code execution?  If your threat model includes arbitrary code
> execution by root, you really need to be using SELinux.
Hm, this is actually a very good point which we somehow missed, hm.
Thanks for pointing that out, let me think on that

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel