UBUNTU: config: enable DEBUG_CREDENTIALS

This adds a few bytes of overhead to each credential and adds a tiny
amount of CPU overhead when changing credentials. It can catch some
types of credential manipulation attacks, so turn it on.

Signed-off-by: Kees Cook <kees@ubuntu.com>
---
 debian.master/config/config.common.ubuntu |    2 +-
 debian.master/config/enforce              |    1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

On 07/20/2012 01:57 PM, Kees Cook wrote:
> This adds a few bytes of overhead to each credential and adds a tiny
> amount of CPU overhead when changing credentials. It can catch some
> types of credential manipulation attacks, so turn it on.
> 
Hey kees, Its a great debug option, however I am still not sure its
worth the admittedly minor cost of turning it on for our kernels and
I am still looking into it

thanks
john

> Signed-off-by: Kees Cook <kees@ubuntu.com>
> ---
>  debian.master/config/config.common.ubuntu |    2 +-
>  debian.master/config/enforce              |    1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
> index a1bcec2..e24e3d00 100644
> --- a/debian.master/config/config.common.ubuntu
> +++ b/debian.master/config/config.common.ubuntu
> @@ -1241,7 +1241,7 @@ CONFIG_DEBUGGER=y
>  # CONFIG_DEBUG_BLOCK_EXT_DEVT is not set
>  # CONFIG_DEBUG_BOOT_PARAMS is not set
>  CONFIG_DEBUG_BUGVERBOSE=y
> -# CONFIG_DEBUG_CREDENTIALS is not set
> +CONFIG_DEBUG_CREDENTIALS=y
>  # CONFIG_DEBUG_DEVRES is not set
>  # CONFIG_DEBUG_DRIVER is not set
>  # CONFIG_DEBUG_FORCE_WEAK_PER_CPU is not set
> diff --git a/debian.master/config/enforce b/debian.master/config/enforce
> index 89c9497..1cb6270 100644
> --- a/debian.master/config/enforce
> +++ b/debian.master/config/enforce
> @@ -20,6 +20,7 @@ value CONFIG_DEFAULT_SECURITY_APPARMOR y
>  !exists CONFIG_DEBUG_RODATA | value CONFIG_DEBUG_RODATA y
>  !exists CONFIG_DEBUG_SET_MODULE_RONX | value CONFIG_DEBUG_SET_MODULE_RONX y
>  !exists CONFIG_STRICT_DEVMEM | value CONFIG_STRICT_DEVMEM y
> +!exists CONFIG_DEBUG_CREDENTIALS | value CONFIG_DEBUG_CREDENTIALS y
>  # For architectures which support this option ensure it is disabled.
>  !exists CONFIG_COMPAT_VDSO | value CONFIG_COMPAT_VDSO n
>  !exists CONFIG_ACPI_CUSTOM_METHOD | value CONFIG_ACPI_CUSTOM_METHOD n
>

On 07/20/2012 02:57 PM, Kees Cook wrote:
> This adds a few bytes of overhead to each credential and adds a tiny
> amount of CPU overhead when changing credentials. It can catch some
> types of credential manipulation attacks, so turn it on.
>
> Signed-off-by: Kees Cook <kees@ubuntu.com>
> ---

Kees - I'm curious how a credential attack might be attempted? If a 
successful credential attack is possible, then this option provides an 
excellent DOS since AFAICT every credential check failure ultimately 
ends up in a BUG() statement. I think DEBUG_CREDENTIALS was designed as 
a coding aid to detect reference counting and RCU mistakes, not as a 
protection method against attacks.

rtg

On 07/26/2012 05:34 AM, Tim Gardner wrote:
> On 07/20/2012 02:57 PM, Kees Cook wrote:
>> This adds a few bytes of overhead to each credential and adds a tiny
>> amount of CPU overhead when changing credentials. It can catch some
>> types of credential manipulation attacks, so turn it on.
>>
>> Signed-off-by: Kees Cook <kees@ubuntu.com>
>> ---
> 
> Kees - I'm curious how a credential attack might be attempted? If a successful credential attack is possible, then this option provides an excellent DOS since AFAICT every credential check failure ultimately ends up in a BUG() statement. I think DEBUG_CREDENTIALS was designed as a coding aid to detect reference counting and RCU mistakes, not as a protection method against attacks.
> 
There are a couple of different vectors to attack creds but I can't see how
the current DEBUG_CREDENTIALS would stop the attacks, though it would
possibly make the overwrite attack harder.

1. is a use after free attack. Basically there is an uncounted reference
   still in use or an extra put ..

   You exploit it by getting an unprivileged task/object to "free" the
   cred and getting a privileged task to allocate the freed cred,
   reinitializing it into a privileged cred.

   However the debug code will only help in the race window, if the
   cred is referenced while it is freed. Once it has been reinitialized
   the cred looks good and the error won't be caught.

2. some kind of memory overwrite attack, where the attacker has some
   limited ability to overwrite the cred. The debug code won't catch
   most over writes, unless it results in a wrong magic value or
   the task ref count > than the cred ref count


At the moment I am leaning towards saying No to turning this on for our
default kernels, because while the cost is small (noise with the limited
testing I have done). I don't see it actually stopping most actual
attacks.

I will poke some more and hopefully kees can slap some sense into me,
and make me see what I am missing.

Hi John,

On Wed, Aug 01, 2012 at 12:47:58PM -0700, John Johansen wrote:
> There are a couple of different vectors to attack creds but I can't see how
> the current DEBUG_CREDENTIALS would stop the attacks, though it would
> possibly make the overwrite attack harder.

It was the "harder" part I was thinking of.

> 1. is a use after free attack. Basically there is an uncounted reference
>    still in use or an extra put ..
> 
>    You exploit it by getting an unprivileged task/object to "free" the
>    cred and getting a privileged task to allocate the freed cred,
>    reinitializing it into a privileged cred.
> 
>    However the debug code will only help in the race window, if the
>    cred is referenced while it is freed. Once it has been reinitialized
>    the cred looks good and the error won't be caught.
> 
> 2. some kind of memory overwrite attack, where the attacker has some
>    limited ability to overwrite the cred. The debug code won't catch
>    most over writes, unless it results in a wrong magic value or
>    the task ref count > than the cred ref count
> 
> 
> At the moment I am leaning towards saying No to turning this on for our
> default kernels, because while the cost is small (noise with the limited
> testing I have done). I don't see it actually stopping most actual
> attacks.
> 
> I will poke some more and hopefully kees can slap some sense into me,
> and make me see what I am missing.

There's not anything more obvious that stands out to me. It just seemed
like an added hurdle to rare cred allocation flaws. Not all kernel
bugs allow arbitrary control over what's being written, so adding the
requirement of the magic + usage counter (at the correct location)
seemed worth it to me to get a small defense.

I won't cry if it's not enabled -- it just seemed like a good trade off
to me.

-Kees