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
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 <[email protected]>
---
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
On Sun, Jul 03 2022 at 8:02P -0400,
Daniil Lunev <[email protected]> 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 <[email protected]>
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
>
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 <[email protected]> wrote:
>
> On Sun, Jul 03 2022 at 8:02P -0400,
> Daniil Lunev <[email protected]> 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 <[email protected]>
>
> 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
> >
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
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
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
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 <[email protected]> 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
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 <[email protected]> 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