[PATCH] sysctl: add config to make randomize_va_space RO

[PATCH] sysctl: add config to make randomize_va_space RO

[Michael McCracken]

Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
sysctl to 0444 to disallow all runtime changes. This will prevent
accidental changing of this value by a root service.

The config is disabled by default to avoid surprises.

Signed-off-by: Michael McCracken <michael.mccracken@gmail.com>
---
 kernel/sysctl.c | 4 ++++
 mm/Kconfig      | 7 +++++++
 2 files changed, 11 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index bfe53e835524..c5aafb734abe 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1913,7 +1913,11 @@ static struct ctl_table kern_table[] = {
 		.procname	= "randomize_va_space",
 		.data		= &randomize_va_space,
 		.maxlen		= sizeof(int),
+#if defined(CONFIG_RO_RANDMAP_SYSCTL)
+		.mode		= 0444,
+#else
 		.mode		= 0644,
+#endif
 		.proc_handler	= proc_dointvec,
 	},
 #endif
diff --git a/mm/Kconfig b/mm/Kconfig
index 7672a22647b4..91a4a86d70e0 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -1206,6 +1206,13 @@ config PER_VMA_LOCK
 	  This feature allows locking each virtual memory area separately when
 	  handling page faults instead of taking mmap_lock.
 
+config RO_RANDMAP_SYSCTL
+    bool "Make randomize_va_space sysctl 0444"
+    depends on MMU
+    default n
+    help
+      Set file mode of /proc/sys/kernel/randomize_va_space to 0444 to disallow runtime changes in ASLR.
+
 source "mm/damon/Kconfig"
 
 endmenu
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[David Hildenbrand]

On 04.05.23 23:30, Michael McCracken wrote:
> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> sysctl to 0444 to disallow all runtime changes. This will prevent
> accidental changing of this value by a root service.
> 
> The config is disabled by default to avoid surprises.

Can you elaborate why we care about "accidental changing of this value 
by a root service"?

We cannot really stop root from doing a lot of stupid things (e.g., 
erase the root fs), so why do we particularly care here?

-- 
Thanks,

David / dhildenb

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 842 bytes --]


David Hildenbrand <david@redhat.com> writes:

> On 04.05.23 23:30, Michael McCracken wrote:
>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
>> sysctl to 0444 to disallow all runtime changes. This will prevent
>> accidental changing of this value by a root service.
>> The config is disabled by default to avoid surprises.
>
> Can you elaborate why we care about "accidental changing of this value
> by a root service"?
>
> We cannot really stop root from doing a lot of stupid things (e.g.,
> erase the root fs), so why do we particularly care here?

(I'm really not defending the utility of this, fwiw).

In the past, I've seen fuzzing tools and other debuggers try to set
it, and it might be that an admin doesn't realise that. But they could
easily set other dangerous settings unsuitable for production, so...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 377 bytes --]

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[David Hildenbrand]

On 05.05.23 09:46, Sam James wrote:
> 
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 04.05.23 23:30, Michael McCracken wrote:
>>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
>>> sysctl to 0444 to disallow all runtime changes. This will prevent
>>> accidental changing of this value by a root service.
>>> The config is disabled by default to avoid surprises.
>>
>> Can you elaborate why we care about "accidental changing of this value
>> by a root service"?
>>
>> We cannot really stop root from doing a lot of stupid things (e.g.,
>> erase the root fs), so why do we particularly care here?
> 
> (I'm really not defending the utility of this, fwiw).
> 
> In the past, I've seen fuzzing tools and other debuggers try to set
> it, and it might be that an admin doesn't realise that. But they could
> easily set other dangerous settings unsuitable for production, so...

At least fuzzing tools randomly toggling it could actually find real 
problems. Debugging tools ... makes sense that they might be using it.

What I understand is, that it's more of a problem that the system 
continues running and the disabled randomization isn't revealed to an 
admin easily.

If we really care, not sure what's better: maybe we want to disallow 
disabling it only in a security lockdown kernel? Or at least warn the 
user when disabling it? (WARN_TAINT?)

-- 
Thanks,

David / dhildenb

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[David Hildenbrand]

On 05.05.23 17:15, David Hildenbrand wrote:
> On 05.05.23 09:46, Sam James wrote:
>>
>> David Hildenbrand <david@redhat.com> writes:
>>
>>> On 04.05.23 23:30, Michael McCracken wrote:
>>>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
>>>> sysctl to 0444 to disallow all runtime changes. This will prevent
>>>> accidental changing of this value by a root service.
>>>> The config is disabled by default to avoid surprises.
>>>
>>> Can you elaborate why we care about "accidental changing of this value
>>> by a root service"?
>>>
>>> We cannot really stop root from doing a lot of stupid things (e.g.,
>>> erase the root fs), so why do we particularly care here?
>>
>> (I'm really not defending the utility of this, fwiw).
>>
>> In the past, I've seen fuzzing tools and other debuggers try to set
>> it, and it might be that an admin doesn't realise that. But they could
>> easily set other dangerous settings unsuitable for production, so...
> 
> At least fuzzing tools randomly toggling it could actually find real
> problems. Debugging tools ... makes sense that they might be using it.
> 
> What I understand is, that it's more of a problem that the system
> continues running and the disabled randomization isn't revealed to an
> admin easily.
> 
> If we really care, not sure what's better: maybe we want to disallow
> disabling it only in a security lockdown kernel? Or at least warn the
> user when disabling it? (WARN_TAINT?)

Sorry, not WARN_TAINT. pr_warn() maybe. Tainting the kernel is probably 
a bit too much as well.

-- 
Thanks,

David / dhildenb

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Paul Moore]

On Fri, May 5, 2023 at 11:15 AM David Hildenbrand <david@redhat.com> wrote:
> On 05.05.23 09:46, Sam James wrote:
> > David Hildenbrand <david@redhat.com> writes:
> >> On 04.05.23 23:30, Michael McCracken wrote:
> >>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> >>> sysctl to 0444 to disallow all runtime changes. This will prevent
> >>> accidental changing of this value by a root service.
> >>> The config is disabled by default to avoid surprises.

...

> If we really care, not sure what's better: maybe we want to disallow
> disabling it only in a security lockdown kernel?

If we're bringing up the idea of Lockdown, controlling access to
randomize_va_space is possible with the use of LSMs.  One could easily
remove write access to randomize_va_space, even for tasks running as
root.

(On my Rawhide system with SELinux enabled)
% ls -Z /proc/sys/kernel/randomize_va_space
system_u:object_r:proc_security_t:s0 /proc/sys/kernel/randomize_va_space

-- 
paul-moore.com

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Kaiwan N Billimoria]

On Fri, May 5, 2023 at 8:53 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, May 5, 2023 at 11:15 AM David Hildenbrand <david@redhat.com> wrote:
> > On 05.05.23 09:46, Sam James wrote:
> > > David Hildenbrand <david@redhat.com> writes:
> > >> On 04.05.23 23:30, Michael McCracken wrote:
> > >>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> > >>> sysctl to 0444 to disallow all runtime changes. This will prevent
> > >>> accidental changing of this value by a root service.
> > >>> The config is disabled by default to avoid surprises.
>
> ...
>
> > If we really care, not sure what's better: maybe we want to disallow
> > disabling it only in a security lockdown kernel?
>
> If we're bringing up the idea of Lockdown, controlling access to
> randomize_va_space is possible with the use of LSMs.  One could easily
> remove write access to randomize_va_space, even for tasks running as
> root.
IMO, don't _move_ the sysctl to LSM(s). There are legitimate scenarios
(typically debugging) where root needs to disable/enable ASLR.
I think the key thing is the file ownership; being root-writable takes
care of security concerns... (as David says, if root screws around we
can't do much)..
If one argues for changing the mode from 0644 to 0444, what prevents
all the other dozens of sysctls - owned by root mind you - from not
wanting the same treatment?
Where does one draw the line?
- Kaiwan.
>
> (On my Rawhide system with SELinux enabled)
> % ls -Z /proc/sys/kernel/randomize_va_space
> system_u:object_r:proc_security_t:s0 /proc/sys/kernel/randomize_va_space
>
> --
> paul-moore.com

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Paul Moore]

On Sat, May 6, 2023 at 3:05 AM Kaiwan N Billimoria
<kaiwan@kaiwantech.com> wrote:
> On Fri, May 5, 2023 at 8:53 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Fri, May 5, 2023 at 11:15 AM David Hildenbrand <david@redhat.com> wrote:
> > > On 05.05.23 09:46, Sam James wrote:
> > > > David Hildenbrand <david@redhat.com> writes:
> > > >> On 04.05.23 23:30, Michael McCracken wrote:
> > > >>> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> > > >>> sysctl to 0444 to disallow all runtime changes. This will prevent
> > > >>> accidental changing of this value by a root service.
> > > >>> The config is disabled by default to avoid surprises.
> >
> > ...
> >
> > > If we really care, not sure what's better: maybe we want to disallow
> > > disabling it only in a security lockdown kernel?
> >
> > If we're bringing up the idea of Lockdown, controlling access to
> > randomize_va_space is possible with the use of LSMs.  One could easily
> > remove write access to randomize_va_space, even for tasks running as
> > root.
>
> IMO, don't _move_ the sysctl to LSM(s).

There is nothing to move, the ability to restrict access to
randomize_va_space exists today, it is simply a matter of if the
security policy author or admin wants to enable it.

If you are like Michael and you want to block write access, even when
running as root, you can do so with an LSM.  You can also allow write
access.  With SELinux you can allow/disallow the privilege on a
task-by-task basis to meet individual usability and security
requirements.

-- 
paul-moore.com

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Serge Hallyn]

On Fri, May 05, 2023 at 09:35:59AM +0200, David Hildenbrand wrote:
> On 04.05.23 23:30, Michael McCracken wrote:
> > Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> > sysctl to 0444 to disallow all runtime changes. This will prevent
> > accidental changing of this value by a root service.
> > 
> > The config is disabled by default to avoid surprises.
> 
> Can you elaborate why we care about "accidental changing of this value by a
> root service"?

Accidental... malicious...  Note that when people run programs as root with
reduced or no capabilities they can still write this file.

> We cannot really stop root from doing a lot of stupid things (e.g., erase
> the root fs), so why do we particularly care here?

Regardless of the "real value" of it, I know for a fact there are lots
of teams out there adding kernel patches to just change the mode of that
file.  Why?  Possibly to satisfy a scanner, because another team says
it's important.

The problem with lockdown is it's all or nothing.  The problem with LSM
for this purpose is that everyone will have to configure their policy
differently.

So I do think it was worth testing the waters with this patch, to reduce
the number of duplicate patches people run with.

-serge

Re: [PATCH] sysctl: add config to make randomize_va_space RO

[Kees Cook]

On Thu, May 04, 2023 at 02:30:02PM -0700, Michael McCracken wrote:
> Add config RO_RANDMAP_SYSCTL to set the mode of the randomize_va_space
> sysctl to 0444 to disallow all runtime changes. This will prevent
> accidental changing of this value by a root service.
> 
> The config is disabled by default to avoid surprises.
> 
> Signed-off-by: Michael McCracken <michael.mccracken@gmail.com>
> ---
>  kernel/sysctl.c | 4 ++++
>  mm/Kconfig      | 7 +++++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index bfe53e835524..c5aafb734abe 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1913,7 +1913,11 @@ static struct ctl_table kern_table[] = {
>  		.procname	= "randomize_va_space",
>  		.data		= &randomize_va_space,
>  		.maxlen		= sizeof(int),
> +#if defined(CONFIG_RO_RANDMAP_SYSCTL)
> +		.mode		= 0444,
> +#else
>  		.mode		= 0644,
> +#endif

The way we've dealt with this in the past for similarly sensitive
sysctl variables to was set a "locked" position. (e.g. 0==off, 1==on,
2==cannot be turned off). In this case, we could make it, 0, 1, 2,
3==forced on full.

I note that there is actually no min/max (extra1/extra2) for this sysctl,
which is itself a bug, IMO. And there is just a magic "> 1" test that
should be a define or enum:

fs/binfmt_elf.c:        if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1)) {

I think much of this should be improved.

Regardless, take a look at yama_dointvec_minmax(), which could, perhaps,
be generalized and used here.

Then we have a run-time way to manage this bit, without needing full
kernel rebuilds, etc, etc.

-Kees

-- 
Kees Cook