Re: [PATCH v1 3/9] virt: Implement Heki common code

2023-05-29 Thread Mickaël Salaün



On 17/05/2023 14:47, Madhavan T. Venkataraman wrote:

Sorry for the delay. See inline...

On 5/8/23 12:29, Wei Liu wrote:

On Fri, May 05, 2023 at 05:20:40PM +0200, Mickaël Salaün wrote:

From: Madhavan T. Venkataraman 

Hypervisor Enforced Kernel Integrity (Heki) is a feature that will use
the hypervisor to enhance guest virtual machine security.

Configuration
=

Define the config variables for the feature. This feature depends on
support from the architecture as well as the hypervisor.

Enabling HEKI
=

Define a kernel command line parameter "heki" to turn the feature on or
off. By default, Heki is on.


For such a newfangled feature can we have it off by default? Especially
when there are unsolved issues around dynamically loaded code.



Yes. We can certainly do that.


By default the Kconfig option should definitely be off. We also need to 
change the Kconfig option to only be set if kernel module, JIT, kprobes 
and other dynamic text change feature are disabled at build time  (see 
discussion with Sean).


With this new Kconfig option for the static case, I think the boot 
option should be on by default because otherwise it would not really be 
possible to switch back to on later without taking the risk to silently 
breaking users' machines. However, we should rename this option to 
something like "heki_static" to be in line with the new Kconfig option.


The goal of Heki is to improve and complement kernel self-protection 
mechanisms (which don't have boot time options), and to make it 
available to everyone, see 
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Recommended_Settings
In practice, it would then be kind of useless to be required to set a 
boot option to enable Heki (rather than to disable it).








[...]

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..5cf5a7a97811 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -297,6 +297,7 @@ config X86
select FUNCTION_ALIGNMENT_4B
imply IMA_SECURE_AND_OR_TRUSTED_BOOTif EFI
select HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
+   select ARCH_SUPPORTS_HEKI   if X86_64


Why is there a restriction on X86_64?



We want to get the PoC working and reviewed on X64 first. We have tested this 
only on X64 so far.


X86_64 includes Intel CPUs, which can support EPT and MBEC, which are a 
requirement for Heki. ARM might have similar features but we're focused 
on x86 for now.


As a side note, I only have access to an Intel machine, which means that 
I cannot work on AMD support. However, I'll be pleased to implement such 
support if I get access to a machine with a recent AMD CPU.





  
  config INSTRUCTION_DECODER

def_bool y
diff --git a/arch/x86/include/asm/sections.h b/arch/x86/include/asm/sections.h
index a6e8373a5170..42ef1e33b8a5 100644
--- a/arch/x86/include/asm/sections.h
+++ b/arch/x86/include/asm/sections.h

[...]
  
+#ifdef CONFIG_HEKI

+
+/*
+ * Gather all of the statically defined sections so heki_late_init() can
+ * protect these sections in the host page table.
+ *
+ * The sections are defined under "SECTIONS" in vmlinux.lds.S
+ * Keep this array in sync with SECTIONS.
+ */


This seems a bit fragile, because it requires constant attention from
people who care about this functionality. Can this table be
automatically generated?



We realize that. But I don't know of a way this can be automatically generated. 
Also, the permissions for
each section is specific to the use of that section. The developer who 
introduces a new section is the
one who will know what the permissions should be.

If any one has any ideas of how we can generate this table automatically or 
even just add a build time check
of some sort, please let us know.


One clean solution might be to parse the vmlinux.lds.S file, extract 
section and their permission, and fill that into an automatically 
generated header file.


Another way to do it would be to extract sections and associated 
permissions with objdump, but that could be an issue because of longer 
build time.


A better solution would be to extract such sections and associated 
permissions at boot time. I guess the kernel already has such helpers 
used in early boot.




Re: [PATCH v1 3/9] virt: Implement Heki common code

2023-05-17 Thread Madhavan T. Venkataraman
Sorry for the delay. See inline...

On 5/8/23 12:29, Wei Liu wrote:
> On Fri, May 05, 2023 at 05:20:40PM +0200, Mickaël Salaün wrote:
>> From: Madhavan T. Venkataraman 
>>
>> Hypervisor Enforced Kernel Integrity (Heki) is a feature that will use
>> the hypervisor to enhance guest virtual machine security.
>>
>> Configuration
>> =
>>
>> Define the config variables for the feature. This feature depends on
>> support from the architecture as well as the hypervisor.
>>
>> Enabling HEKI
>> =
>>
>> Define a kernel command line parameter "heki" to turn the feature on or
>> off. By default, Heki is on.
> 
> For such a newfangled feature can we have it off by default? Especially
> when there are unsolved issues around dynamically loaded code.
> 

Yes. We can certainly do that.

>>
> [...]
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 3604074a878b..5cf5a7a97811 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -297,6 +297,7 @@ config X86
>>  select FUNCTION_ALIGNMENT_4B
>>  imply IMA_SECURE_AND_OR_TRUSTED_BOOTif EFI
>>  select HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
>> +select ARCH_SUPPORTS_HEKI   if X86_64
> 
> Why is there a restriction on X86_64?
> 

We want to get the PoC working and reviewed on X64 first. We have tested this 
only on X64 so far.

>>  
>>  config INSTRUCTION_DECODER
>>  def_bool y
>> diff --git a/arch/x86/include/asm/sections.h 
>> b/arch/x86/include/asm/sections.h
>> index a6e8373a5170..42ef1e33b8a5 100644
>> --- a/arch/x86/include/asm/sections.h
>> +++ b/arch/x86/include/asm/sections.h
> [...]
>>  
>> +#ifdef CONFIG_HEKI
>> +
>> +/*
>> + * Gather all of the statically defined sections so heki_late_init() can
>> + * protect these sections in the host page table.
>> + *
>> + * The sections are defined under "SECTIONS" in vmlinux.lds.S
>> + * Keep this array in sync with SECTIONS.
>> + */
> 
> This seems a bit fragile, because it requires constant attention from
> people who care about this functionality. Can this table be
> automatically generated?
> 

We realize that. But I don't know of a way this can be automatically generated. 
Also, the permissions for
each section is specific to the use of that section. The developer who 
introduces a new section is the
one who will know what the permissions should be.

If any one has any ideas of how we can generate this table automatically or 
even just add a build time check
of some sort, please let us know.

Thanks.

Madhavan

> Thanks,
> Wei.
> 
>> +struct heki_va_range __initdata heki_va_ranges[] = {
>> +{
>> +.va_start = _stext,
>> +.va_end = _etext,
>> +.attributes = HEKI_ATTR_MEM_NOWRITE | HEKI_ATTR_MEM_EXEC,
>> +},
>> +{
>> +.va_start = __start_rodata,
>> +.va_end = __end_rodata,
>> +.attributes = HEKI_ATTR_MEM_NOWRITE,
>> +},
>> +#ifdef CONFIG_UNWINDER_ORC
>> +{
>> +.va_start = __start_orc_unwind_ip,
>> +.va_end = __stop_orc_unwind_ip,
>> +.attributes = HEKI_ATTR_MEM_NOWRITE,
>> +},
>> +{
>> +.va_start = __start_orc_unwind,
>> +.va_end = __stop_orc_unwind,
>> +.attributes = HEKI_ATTR_MEM_NOWRITE,
>> +},
>> +{
>> +.va_start = orc_lookup,
>> +.va_end = orc_lookup_end,
>> +.attributes = HEKI_ATTR_MEM_NOWRITE,
>> +},
>> +#endif /* CONFIG_UNWINDER_ORC */
>> +};
>> +



Re: [PATCH v1 3/9] virt: Implement Heki common code

2023-05-08 Thread Wei Liu
On Fri, May 05, 2023 at 05:20:40PM +0200, Mickaël Salaün wrote:
> From: Madhavan T. Venkataraman 
> 
> Hypervisor Enforced Kernel Integrity (Heki) is a feature that will use
> the hypervisor to enhance guest virtual machine security.
> 
> Configuration
> =
> 
> Define the config variables for the feature. This feature depends on
> support from the architecture as well as the hypervisor.
> 
> Enabling HEKI
> =
> 
> Define a kernel command line parameter "heki" to turn the feature on or
> off. By default, Heki is on.

For such a newfangled feature can we have it off by default? Especially
when there are unsolved issues around dynamically loaded code.

> 
[...]
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 3604074a878b..5cf5a7a97811 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -297,6 +297,7 @@ config X86
>   select FUNCTION_ALIGNMENT_4B
>   imply IMA_SECURE_AND_OR_TRUSTED_BOOTif EFI
>   select HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
> + select ARCH_SUPPORTS_HEKI   if X86_64

Why is there a restriction on X86_64?

>  
>  config INSTRUCTION_DECODER
>   def_bool y
> diff --git a/arch/x86/include/asm/sections.h b/arch/x86/include/asm/sections.h
> index a6e8373a5170..42ef1e33b8a5 100644
> --- a/arch/x86/include/asm/sections.h
> +++ b/arch/x86/include/asm/sections.h
[...]
>  
> +#ifdef CONFIG_HEKI
> +
> +/*
> + * Gather all of the statically defined sections so heki_late_init() can
> + * protect these sections in the host page table.
> + *
> + * The sections are defined under "SECTIONS" in vmlinux.lds.S
> + * Keep this array in sync with SECTIONS.
> + */

This seems a bit fragile, because it requires constant attention from
people who care about this functionality. Can this table be
automatically generated?

Thanks,
Wei.

> +struct heki_va_range __initdata heki_va_ranges[] = {
> + {
> + .va_start = _stext,
> + .va_end = _etext,
> + .attributes = HEKI_ATTR_MEM_NOWRITE | HEKI_ATTR_MEM_EXEC,
> + },
> + {
> + .va_start = __start_rodata,
> + .va_end = __end_rodata,
> + .attributes = HEKI_ATTR_MEM_NOWRITE,
> + },
> +#ifdef CONFIG_UNWINDER_ORC
> + {
> + .va_start = __start_orc_unwind_ip,
> + .va_end = __stop_orc_unwind_ip,
> + .attributes = HEKI_ATTR_MEM_NOWRITE,
> + },
> + {
> + .va_start = __start_orc_unwind,
> + .va_end = __stop_orc_unwind,
> + .attributes = HEKI_ATTR_MEM_NOWRITE,
> + },
> + {
> + .va_start = orc_lookup,
> + .va_end = orc_lookup_end,
> + .attributes = HEKI_ATTR_MEM_NOWRITE,
> + },
> +#endif /* CONFIG_UNWINDER_ORC */
> +};
> +



[PATCH v1 3/9] virt: Implement Heki common code

2023-05-05 Thread Mickaël Salaün
From: Madhavan T. Venkataraman 

Hypervisor Enforced Kernel Integrity (Heki) is a feature that will use
the hypervisor to enhance guest virtual machine security.

Configuration
=

Define the config variables for the feature. This feature depends on
support from the architecture as well as the hypervisor.

Enabling HEKI
=

Define a kernel command line parameter "heki" to turn the feature on or
off. By default, Heki is on.

Feature initialization
==

The linker script, vmlinux.lds.S, defines a number of sections that are
loaded in kernel memory. Each of these sections has its own permissions.
For instance, .text has HEKI_ATTR_MEM_EXEC | HEKI_ATTR_MEM_NOWRITE, and
.rodata has HEKI_ATTR_MEM_NOWRITE.

Define an architecture specific init function, heki_arch_init(). In this
function, collect the ranges of all of the sections. These sections will
be protected in the host page table with their respective permissions so
that even if the guest kernel is compromised, their permissions cannot
be changed.

Define heki_early_init() to initialize the feature. For now, this
function just checks if the feature is enabled and calls
heki_arch_init().

Define heki_late_init() that protects the sections in the host page
table. This needs hypervisor support which will be introduced in the
future.  This function is called at the end of kernel init.

Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: H. Peter Anvin 
Cc: Ingo Molnar 
Cc: Kees Cook 
Cc: Madhavan T. Venkataraman 
Cc: Mickaël Salaün 
Cc: Paolo Bonzini 
Cc: Sean Christopherson 
Cc: Thomas Gleixner 
Cc: Vitaly Kuznetsov 
Cc: Wanpeng Li 
Signed-off-by: Madhavan T. Venkataraman 
Link: https://lore.kernel.org/r/20230505152046.6575-4-...@digikod.net
---
 Kconfig |   2 +
 arch/x86/Kconfig|   1 +
 arch/x86/include/asm/sections.h |   4 +
 arch/x86/kernel/setup.c |  49 
 include/linux/heki.h|  90 +
 init/main.c |   3 +
 virt/Makefile   |   1 +
 virt/heki/Kconfig   |  22 ++
 virt/heki/Makefile  |   3 +
 virt/heki/heki.c| 135 
 10 files changed, 310 insertions(+)
 create mode 100644 include/linux/heki.h
 create mode 100644 virt/heki/Kconfig
 create mode 100644 virt/heki/Makefile
 create mode 100644 virt/heki/heki.c

diff --git a/Kconfig b/Kconfig
index 745bc773f567..0c844d9bcb03 100644
--- a/Kconfig
+++ b/Kconfig
@@ -29,4 +29,6 @@ source "lib/Kconfig"
 
 source "lib/Kconfig.debug"
 
+source "virt/heki/Kconfig"
+
 source "Documentation/Kconfig"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..5cf5a7a97811 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -297,6 +297,7 @@ config X86
select FUNCTION_ALIGNMENT_4B
imply IMA_SECURE_AND_OR_TRUSTED_BOOTif EFI
select HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
+   select ARCH_SUPPORTS_HEKI   if X86_64
 
 config INSTRUCTION_DECODER
def_bool y
diff --git a/arch/x86/include/asm/sections.h b/arch/x86/include/asm/sections.h
index a6e8373a5170..42ef1e33b8a5 100644
--- a/arch/x86/include/asm/sections.h
+++ b/arch/x86/include/asm/sections.h
@@ -18,6 +18,10 @@ extern char __end_of_kernel_reserve[];
 
 extern unsigned long _brk_start, _brk_end;
 
+extern int __start_orc_unwind_ip[], __stop_orc_unwind_ip[];
+extern struct orc_entry __start_orc_unwind[], __stop_orc_unwind[];
+extern unsigned int orc_lookup[], orc_lookup_end[];
+
 static inline bool arch_is_kernel_initmem_freed(unsigned long addr)
 {
/*
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 88188549647c..f0ddaf24ab63 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -850,6 +851,54 @@ static void __init x86_report_nx(void)
}
 }
 
+#ifdef CONFIG_HEKI
+
+/*
+ * Gather all of the statically defined sections so heki_late_init() can
+ * protect these sections in the host page table.
+ *
+ * The sections are defined under "SECTIONS" in vmlinux.lds.S
+ * Keep this array in sync with SECTIONS.
+ */
+struct heki_va_range __initdata heki_va_ranges[] = {
+   {
+   .va_start = _stext,
+   .va_end = _etext,
+   .attributes = HEKI_ATTR_MEM_NOWRITE | HEKI_ATTR_MEM_EXEC,
+   },
+   {
+   .va_start = __start_rodata,
+   .va_end = __end_rodata,
+   .attributes = HEKI_ATTR_MEM_NOWRITE,
+   },
+#ifdef CONFIG_UNWINDER_ORC
+   {
+   .va_start = __start_orc_unwind_ip,
+   .va_end = __stop_orc_unwind_ip,
+   .attributes = HEKI_ATTR_MEM_NOWRITE,
+   },
+   {
+   .va_start = __start_orc_unwind,
+   .va_end = __stop_orc_unwind,
+   .attributes = HEKI_ATTR_MEM_NOWRITE,
+   },
+   {
+