[PATCH] Documentation: dt: chosen property for kaslr-seed

[PATCH] Documentation: dt: chosen property for kaslr-seed

[Kees Cook]

Document then /chosen/kaslr-seed property (and its interaction with the
EFI_RNG_PROTOCOL API).

Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
index dee3f5d9df26..0cdb43b268e5 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
 for passing data between firmware and the operating system, like boot
 arguments. Data in the chosen node does not represent the hardware.
 
+The following properties are recognized:
 
-stdout-path property
---------------------
+
+kaslr-seed
+-----------
+
+This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
+the entropy used to randomize the kernel image base address location. It
+is parsed as a u64 value, e.g.
+
+/ {
+	chosen {
+		kaslr-seed = <0xfeedbeef 0xc0def00d>;
+	};
+};
+
+Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
+this value will be overwritten by the EFI stub.
+
+stdout-path
+-----------
 
 Device trees may specify the device to be used for boot console output
 with a stdout-path property under /chosen, as described in the Devicetree
-- 
2.7.4


-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Ard Biesheuvel]

(+ Mark, Will, Catalin)

On 15 July 2017 at 01:38, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Document then /chosen/kaslr-seed property (and its interaction with the
> EFI_RNG_PROTOCOL API).
>
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>

For the textual changes:

Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

*However*, documenting the /chosen/kaslr-seed property promotes it
from a stub<->kernel private interface to an external ABI between the
kernel and the bootloader, and we need to reach agreement on whether
doing so is desirable first IMHO.


> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index dee3f5d9df26..0cdb43b268e5 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>  for passing data between firmware and the operating system, like boot
>  arguments. Data in the chosen node does not represent the hardware.
>
> +The following properties are recognized:
>
> -stdout-path property
> ---------------------
> +
> +kaslr-seed
> +-----------
> +
> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
> +the entropy used to randomize the kernel image base address location. It
> +is parsed as a u64 value, e.g.
> +
> +/ {
> +       chosen {
> +               kaslr-seed = <0xfeedbeef 0xc0def00d>;
> +       };
> +};
> +
> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
> +this value will be overwritten by the EFI stub.
> +
> +stdout-path
> +-----------
>
>  Device trees may specify the device to be used for boot console output
>  with a stdout-path property under /chosen, as described in the Devicetree
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Kees Cook]

On Sat, Jul 15, 2017 at 5:42 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> (+ Mark, Will, Catalin)
>
> On 15 July 2017 at 01:38, Kees Cook <keescook@chromium.org> wrote:
>> Document then /chosen/kaslr-seed property (and its interaction with the
>> EFI_RNG_PROTOCOL API).
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>
> For the textual changes:
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> *However*, documenting the /chosen/kaslr-seed property promotes it
> from a stub<->kernel private interface to an external ABI between the
> kernel and the bootloader, and we need to reach agreement on whether
> doing so is desirable first IMHO.
>

Oh! I thought that was the point (having a bootloader provide kaslr
entropy). And that in the EFI case, it was the stub doing it.

-Kees

-- 
Kees Cook
Pixel Security

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Ard Biesheuvel]

On 16 July 2017 at 03:13, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Sat, Jul 15, 2017 at 5:42 AM, Ard Biesheuvel
> <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> (+ Mark, Will, Catalin)
>>
>> On 15 July 2017 at 01:38, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>>> Document then /chosen/kaslr-seed property (and its interaction with the
>>> EFI_RNG_PROTOCOL API).
>>>
>>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>> ---
>>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>
>> For the textual changes:
>>
>> Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> *However*, documenting the /chosen/kaslr-seed property promotes it
>> from a stub<->kernel private interface to an external ABI between the
>> kernel and the bootloader, and we need to reach agreement on whether
>> doing so is desirable first IMHO.
>>
>
> Oh! I thought that was the point (having a bootloader provide kaslr
> entropy). And that in the EFI case, it was the stub doing it.
>

It was the opposite, actually,  The /chosen node is the most
appropriate way for the EFI stub to communicate a seed value to the
kernel proper, given how it is needed extremely early in the boot.
(Using UEFI config tables like we do for the /dev/random seed is not
possible for this)

So as a side effect, other bootloaders can use the same mechanism. I'm
fine with that, but it needs to be an explicit decision by the
maintainers imo.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Mark Rutland]

On Sun, Jul 16, 2017 at 05:42:25PM +0100, Ard Biesheuvel wrote:
> On 16 July 2017 at 03:13, Kees Cook <keescook@chromium.org> wrote:
> > On Sat, Jul 15, 2017 at 5:42 AM, Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> >> (+ Mark, Will, Catalin)
> >>
> >> On 15 July 2017 at 01:38, Kees Cook <keescook@chromium.org> wrote:
> >>> Document then /chosen/kaslr-seed property (and its interaction with the
> >>> EFI_RNG_PROTOCOL API).
> >>>
> >>> Signed-off-by: Kees Cook <keescook@chromium.org>
> >>> ---
> >>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
> >>>  1 file changed, 20 insertions(+), 2 deletions(-)
> >>
> >> For the textual changes:
> >>
> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> *However*, documenting the /chosen/kaslr-seed property promotes it
> >> from a stub<->kernel private interface to an external ABI between the
> >> kernel and the bootloader, and we need to reach agreement on whether
> >> doing so is desirable first IMHO.
> >
> > Oh! I thought that was the point (having a bootloader provide kaslr
> > entropy). And that in the EFI case, it was the stub doing it.
> 
> It was the opposite, actually,  The /chosen node is the most
> appropriate way for the EFI stub to communicate a seed value to the
> kernel proper, given how it is needed extremely early in the boot.
> (Using UEFI config tables like we do for the /dev/random seed is not
> possible for this)
> 
> So as a side effect, other bootloaders can use the same mechanism. I'm
> fine with that, but it needs to be an explicit decision by the
> maintainers imo.

I was under the impression that we'd already assumed other bootloaders could
set this, so I don't have a problem promoting this to a defined public
interface.

I guess we just need Will and Catalin to agree.

FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

As an aside, we might want to make a split between /chosen properties which are
Linux-specific (e.g. this), and those which are somewhat generic (e.g.
stdout-path), since other OSs may/should respect those generic ones.

Mark.

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Will Deacon]

On Mon, Jul 17, 2017 at 12:56:10PM +0100, Mark Rutland wrote:
> On Sun, Jul 16, 2017 at 05:42:25PM +0100, Ard Biesheuvel wrote:
> > On 16 July 2017 at 03:13, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > > On Sat, Jul 15, 2017 at 5:42 AM, Ard Biesheuvel
> > > <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > >> (+ Mark, Will, Catalin)
> > >>
> > >> On 15 July 2017 at 01:38, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> > >>> Document then /chosen/kaslr-seed property (and its interaction with the
> > >>> EFI_RNG_PROTOCOL API).
> > >>>
> > >>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> > >>> ---
> > >>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
> > >>>  1 file changed, 20 insertions(+), 2 deletions(-)
> > >>
> > >> For the textual changes:
> > >>
> > >> Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > >>
> > >> *However*, documenting the /chosen/kaslr-seed property promotes it
> > >> from a stub<->kernel private interface to an external ABI between the
> > >> kernel and the bootloader, and we need to reach agreement on whether
> > >> doing so is desirable first IMHO.
> > >
> > > Oh! I thought that was the point (having a bootloader provide kaslr
> > > entropy). And that in the EFI case, it was the stub doing it.
> > 
> > It was the opposite, actually,  The /chosen node is the most
> > appropriate way for the EFI stub to communicate a seed value to the
> > kernel proper, given how it is needed extremely early in the boot.
> > (Using UEFI config tables like we do for the /dev/random seed is not
> > possible for this)
> > 
> > So as a side effect, other bootloaders can use the same mechanism. I'm
> > fine with that, but it needs to be an explicit decision by the
> > maintainers imo.
> 
> I was under the impression that we'd already assumed other bootloaders could
> set this, so I don't have a problem promoting this to a defined public
> interface.
> 
> I guess we just need Will and Catalin to agree.

If we expose an undocumented property, then I think it's ABI the moment
somebody starts using it, irrespective of whether or not we document it
later. For example, if somebody outside of the stub was using this and we
changed the ABI in a way that broke things for them, I'd have a hard time
defending that.

So Documentation is good, but I don't think it really changes anything wrt
ABI guarantees.

Acked-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

(I'm assuming this goes via some DT tree).

Will
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Rob Herring]

On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote:
> Document then /chosen/kaslr-seed property (and its interaction with the

s/then/the/

> EFI_RNG_PROTOCOL API).

"dt-bindings: chosen: ..." for the subject.

> 
> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
> index dee3f5d9df26..0cdb43b268e5 100644
> --- a/Documentation/devicetree/bindings/chosen.txt
> +++ b/Documentation/devicetree/bindings/chosen.txt
> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>  for passing data between firmware and the operating system, like boot
>  arguments. Data in the chosen node does not represent the hardware.
>  
> +The following properties are recognized:
>  
> -stdout-path property
> ---------------------
> +
> +kaslr-seed
> +-----------

Is there some reason we would not feed this to other things needing 
entropy and therefore should have a more generic name?

> +
> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
> +the entropy used to randomize the kernel image base address location. It
> +is parsed as a u64 value, e.g.

Why limit the size to 64-bit and why does it matter how the data is 
interpretted? 

> +
> +/ {
> +	chosen {
> +		kaslr-seed = <0xfeedbeef 0xc0def00d>;
> +	};
> +};
> +
> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
> +this value will be overwritten by the EFI stub.

Isn't this always true? The bootloader will overwrite. Just in the EFI 
case, the EFI stub is part of the bootloader from a functional (and not 
packaging) standpoint.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Kees Cook]

On Mon, Jul 17, 2017 at 12:32 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote:
>> Document then /chosen/kaslr-seed property (and its interaction with the
>
> s/then/the/
>
>> EFI_RNG_PROTOCOL API).
>
> "dt-bindings: chosen: ..." for the subject.

I'll send a v2 with these fixed and the Acks added.

>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>> index dee3f5d9df26..0cdb43b268e5 100644
>> --- a/Documentation/devicetree/bindings/chosen.txt
>> +++ b/Documentation/devicetree/bindings/chosen.txt
>> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>>  for passing data between firmware and the operating system, like boot
>>  arguments. Data in the chosen node does not represent the hardware.
>>
>> +The following properties are recognized:
>>
>> -stdout-path property
>> ---------------------
>> +
>> +kaslr-seed
>> +-----------
>
> Is there some reason we would not feed this to other things needing
> entropy and therefore should have a more generic name?

I'll let Ard answer this better than me, but IIRC, he wanted a narrow use.

>> +
>> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
>> +the entropy used to randomize the kernel image base address location. It
>> +is parsed as a u64 value, e.g.
>
> Why limit the size to 64-bit and why does it matter how the data is
> interpretted?

IIRC, u64 is sufficient. (And note I'm just documenting what exists...)

>> +
>> +/ {
>> +     chosen {
>> +             kaslr-seed = <0xfeedbeef 0xc0def00d>;
>> +     };
>> +};
>> +
>> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
>> +this value will be overwritten by the EFI stub.
>
> Isn't this always true? The bootloader will overwrite. Just in the EFI
> case, the EFI stub is part of the bootloader from a functional (and not
> packaging) standpoint.

I thought it best to call this out so that no one could get confused
if they wanted to understand how to use kaslr-seed with an EFI system.
(i.e. just implement EFI_RNG_PROTOCOL instead.)

-Kees

-- 
Kees Cook
Pixel Security
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Ard Biesheuvel]

On 17 July 2017 at 20:54, Kees Cook <keescook@chromium.org> wrote:
> On Mon, Jul 17, 2017 at 12:32 PM, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote:
>>> Document then /chosen/kaslr-seed property (and its interaction with the
>>
>> s/then/the/
>>
>>> EFI_RNG_PROTOCOL API).
>>
>> "dt-bindings: chosen: ..." for the subject.
>
> I'll send a v2 with these fixed and the Acks added.
>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>>> index dee3f5d9df26..0cdb43b268e5 100644
>>> --- a/Documentation/devicetree/bindings/chosen.txt
>>> +++ b/Documentation/devicetree/bindings/chosen.txt
>>> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>>>  for passing data between firmware and the operating system, like boot
>>>  arguments. Data in the chosen node does not represent the hardware.
>>>
>>> +The following properties are recognized:
>>>
>>> -stdout-path property
>>> ---------------------
>>> +
>>> +kaslr-seed
>>> +-----------
>>
>> Is there some reason we would not feed this to other things needing
>> entropy and therefore should have a more generic name?
>
> I'll let Ard answer this better than me, but IIRC, he wanted a narrow use.
>

The reason is that the seed is not used to feed a DRBG (because it is
way to early for that when we lay out the VA space), but different
slices of the u64 are used for randomizing different parts of the
address space, i.e., the top 16 bits are used to randomize the linear
region, bits 48 - 21 for the kernel itself, and the bits below that
for the module region. (The virtual kernel offset is randomized
further by the physical placement modulo 2 MB, but this is done by the
stub, which calls EFI_RNG_PROTOCOL itself so it does not use
/chosen/kaslr-seed)

This means any use of this seed beyond its intended purpose may leak
information.

For UEFI systems, we do pass a random seed via a UEFI configuration
table, but this is deliberately kept separate (and uses a UEFI
specific interface, which seemed more appropriate)

>>> +
>>> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
>>> +the entropy used to randomize the kernel image base address location. It
>>> +is parsed as a u64 value, e.g.
>>
>> Why limit the size to 64-bit and why does it matter how the data is
>> interpretted?
>
> IIRC, u64 is sufficient. (And note I'm just documenting what exists...)
>

See above. Seeding a DRBG typically requires more than that, but for
KASLR it is sufficient.

>>> +
>>> +/ {
>>> +     chosen {
>>> +             kaslr-seed = <0xfeedbeef 0xc0def00d>;
>>> +     };
>>> +};
>>> +
>>> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
>>> +this value will be overwritten by the EFI stub.
>>
>> Isn't this always true? The bootloader will overwrite. Just in the EFI
>> case, the EFI stub is part of the bootloader from a functional (and not
>> packaging) standpoint.
>
> I thought it best to call this out so that no one could get confused
> if they wanted to understand how to use kaslr-seed with an EFI system.
> (i.e. just implement EFI_RNG_PROTOCOL instead.)
>

Well, I guess the point being made is that setting this property from
UEFI (or u-boot in EFI mode) is pointless is EFI_RNG_PROTOCOL is
implemented as well.

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Rob Herring]

On Mon, Jul 17, 2017 at 3:22 PM, Ard Biesheuvel
<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On 17 July 2017 at 20:54, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> On Mon, Jul 17, 2017 at 12:32 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>>> On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote:
>>>> Document then /chosen/kaslr-seed property (and its interaction with the
>>>
>>> s/then/the/
>>>
>>>> EFI_RNG_PROTOCOL API).
>>>
>>> "dt-bindings: chosen: ..." for the subject.
>>
>> I'll send a v2 with these fixed and the Acks added.
>>
>>>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>>>> index dee3f5d9df26..0cdb43b268e5 100644
>>>> --- a/Documentation/devicetree/bindings/chosen.txt
>>>> +++ b/Documentation/devicetree/bindings/chosen.txt
>>>> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>>>>  for passing data between firmware and the operating system, like boot
>>>>  arguments. Data in the chosen node does not represent the hardware.
>>>>
>>>> +The following properties are recognized:
>>>>
>>>> -stdout-path property
>>>> ---------------------
>>>> +
>>>> +kaslr-seed
>>>> +-----------
>>>
>>> Is there some reason we would not feed this to other things needing
>>> entropy and therefore should have a more generic name?
>>
>> I'll let Ard answer this better than me, but IIRC, he wanted a narrow use.
>>
>
> The reason is that the seed is not used to feed a DRBG (because it is
> way to early for that when we lay out the VA space), but different
> slices of the u64 are used for randomizing different parts of the
> address space, i.e., the top 16 bits are used to randomize the linear
> region, bits 48 - 21 for the kernel itself, and the bits below that
> for the module region. (The virtual kernel offset is randomized
> further by the physical placement modulo 2 MB, but this is done by the
> stub, which calls EFI_RNG_PROTOCOL itself so it does not use
> /chosen/kaslr-seed)
>
> This means any use of this seed beyond its intended purpose may leak
> information.

Is having this value in /proc/device-tree/chosen/kaslr-seed a leak?
Should the kernel remove it?

> For UEFI systems, we do pass a random seed via a UEFI configuration
> table, but this is deliberately kept separate (and uses a UEFI
> specific interface, which seemed more appropriate)

So if we wanted a seed for non-UEFI systems, we should define a
separate property?

>>>> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
>>>> +the entropy used to randomize the kernel image base address location. It
>>>> +is parsed as a u64 value, e.g.
>>>
>>> Why limit the size to 64-bit and why does it matter how the data is
>>> interpretted?
>>
>> IIRC, u64 is sufficient. (And note I'm just documenting what exists...)
>>
>
> See above. Seeding a DRBG typically requires more than that, but for
> KASLR it is sufficient.
>
>>>> +
>>>> +/ {
>>>> +     chosen {
>>>> +             kaslr-seed = <0xfeedbeef 0xc0def00d>;
>>>> +     };
>>>> +};
>>>> +
>>>> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
>>>> +this value will be overwritten by the EFI stub.
>>>
>>> Isn't this always true? The bootloader will overwrite. Just in the EFI
>>> case, the EFI stub is part of the bootloader from a functional (and not
>>> packaging) standpoint.
>>
>> I thought it best to call this out so that no one could get confused
>> if they wanted to understand how to use kaslr-seed with an EFI system.
>> (i.e. just implement EFI_RNG_PROTOCOL instead.)
>>
>
> Well, I guess the point being made is that setting this property from
> UEFI (or u-boot in EFI mode) is pointless is EFI_RNG_PROTOCOL is
> implemented as well.

Okay.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Documentation: dt: chosen property for kaslr-seed

[Ard Biesheuvel]

On 17 July 2017 at 22:05, Rob Herring <robh@kernel.org> wrote:
> On Mon, Jul 17, 2017 at 3:22 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 17 July 2017 at 20:54, Kees Cook <keescook@chromium.org> wrote:
>>> On Mon, Jul 17, 2017 at 12:32 PM, Rob Herring <robh@kernel.org> wrote:
>>>> On Fri, Jul 14, 2017 at 05:38:36PM -0700, Kees Cook wrote:
>>>>> Document then /chosen/kaslr-seed property (and its interaction with the
>>>>
>>>> s/then/the/
>>>>
>>>>> EFI_RNG_PROTOCOL API).
>>>>
>>>> "dt-bindings: chosen: ..." for the subject.
>>>
>>> I'll send a v2 with these fixed and the Acks added.
>>>
>>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/chosen.txt | 22 ++++++++++++++++++++--
>>>>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt
>>>>> index dee3f5d9df26..0cdb43b268e5 100644
>>>>> --- a/Documentation/devicetree/bindings/chosen.txt
>>>>> +++ b/Documentation/devicetree/bindings/chosen.txt
>>>>> @@ -5,9 +5,27 @@ The chosen node does not represent a real device, but serves as a place
>>>>>  for passing data between firmware and the operating system, like boot
>>>>>  arguments. Data in the chosen node does not represent the hardware.
>>>>>
>>>>> +The following properties are recognized:
>>>>>
>>>>> -stdout-path property
>>>>> ---------------------
>>>>> +
>>>>> +kaslr-seed
>>>>> +-----------
>>>>
>>>> Is there some reason we would not feed this to other things needing
>>>> entropy and therefore should have a more generic name?
>>>
>>> I'll let Ard answer this better than me, but IIRC, he wanted a narrow use.
>>>
>>
>> The reason is that the seed is not used to feed a DRBG (because it is
>> way to early for that when we lay out the VA space), but different
>> slices of the u64 are used for randomizing different parts of the
>> address space, i.e., the top 16 bits are used to randomize the linear
>> region, bits 48 - 21 for the kernel itself, and the bits below that
>> for the module region. (The virtual kernel offset is randomized
>> further by the physical placement modulo 2 MB, but this is done by the
>> stub, which calls EFI_RNG_PROTOCOL itself so it does not use
>> /chosen/kaslr-seed)
>>
>> This means any use of this seed beyond its intended purpose may leak
>> information.
>
> Is having this value in /proc/device-tree/chosen/kaslr-seed a leak?

Yes

> Should the kernel remove it?
>

It already does.

>> For UEFI systems, we do pass a random seed via a UEFI configuration
>> table, but this is deliberately kept separate (and uses a UEFI
>> specific interface, which seemed more appropriate)
>
> So if we wanted a seed for non-UEFI systems, we should define a
> separate property?
>

Yes. This was being discussed a while ago, though, and IIRC the
preferred method was to pass a physical memory address containing the
seed instead, since it would be compatible with bootloaders that are
not capable of manipulating the DT

Some of the discussion here:
https://patchwork.kernel.org/patch/9197865/


>>>>> +This property is used when booting with CONFIG_RANDOMIZE_BASE to seed
>>>>> +the entropy used to randomize the kernel image base address location. It
>>>>> +is parsed as a u64 value, e.g.
>>>>
>>>> Why limit the size to 64-bit and why does it matter how the data is
>>>> interpretted?
>>>
>>> IIRC, u64 is sufficient. (And note I'm just documenting what exists...)
>>>
>>
>> See above. Seeding a DRBG typically requires more than that, but for
>> KASLR it is sufficient.
>>
>>>>> +
>>>>> +/ {
>>>>> +     chosen {
>>>>> +             kaslr-seed = <0xfeedbeef 0xc0def00d>;
>>>>> +     };
>>>>> +};
>>>>> +
>>>>> +Note that when booting through EFI when EFI_RNG_PROTOCOL is supported,
>>>>> +this value will be overwritten by the EFI stub.
>>>>
>>>> Isn't this always true? The bootloader will overwrite. Just in the EFI
>>>> case, the EFI stub is part of the bootloader from a functional (and not
>>>> packaging) standpoint.
>>>
>>> I thought it best to call this out so that no one could get confused
>>> if they wanted to understand how to use kaslr-seed with an EFI system.
>>> (i.e. just implement EFI_RNG_PROTOCOL instead.)
>>>
>>
>> Well, I guess the point being made is that setting this property from
>> UEFI (or u-boot in EFI mode) is pointless is EFI_RNG_PROTOCOL is
>> implemented as well.
>
> Okay.
>
> Rob