From f2ccd9f5931f0733c069040e28b0cef1940f4792 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 15:04:07 +0200 Subject: [PATCH 01/14] ASLR: Increase KERNEL_STACK_SIZE to 0x20000 The previous value, 0x8000, caused problems. The minimum value required for relocatable images to load (starting from NULL) is 0x13000. For additional buffer, the temporary hack here is to increase that said value. --- src/consts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/consts.rs b/src/consts.rs index b33727f3..a5480546 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -20,7 +20,7 @@ pub const EFER_LMA: u64 = 1 << 10; /* Long mode active (read-only) */ pub const EFER_NXE: u64 = 1 << 11; /* PTE No-Execute bit enable */ pub const IOAPIC_BASE: u64 = 0xfec00000; pub const IOAPIC_SIZE: u64 = 0x1000; -pub const KERNEL_STACK_SIZE: u64 = 32_768; +pub const KERNEL_STACK_SIZE: u64 = 0x20000; pub const SHAREDQUEUE_START: usize = 0x80000; pub const UHYVE_NET_MTU: usize = 1500; pub const UHYVE_QUEUE_SIZE: usize = 8; From e44672d0ef33764d605279eaa35f8cd9761f033a Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 15:07:33 +0200 Subject: [PATCH 02/14] ASLR: Introduce START_ADDRESS_OFFSET variable Will be utilized later. --- src/consts.rs | 5 +++++ src/vm.rs | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/consts.rs b/src/consts.rs index a5480546..0d4241cb 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -20,6 +20,11 @@ pub const EFER_LMA: u64 = 1 << 10; /* Long mode active (read-only) */ pub const EFER_NXE: u64 = 1 << 11; /* PTE No-Execute bit enable */ pub const IOAPIC_BASE: u64 = 0xfec00000; pub const IOAPIC_SIZE: u64 = 0x1000; +// TODO: Change to 0x100000 once ASLR is implemented. +// START_ADDRESS_OFFSET + KERNEL_STACK_SIZE = 0x400000, which was +// the set address that all binaries, relocatable or not, would previously +// be loaded on. This maintains "compatibility" for now. +pub const START_ADDRESS_OFFSET: u64 = 0x380000; pub const KERNEL_STACK_SIZE: u64 = 0x20000; pub const SHAREDQUEUE_START: usize = 0x80000; pub const UHYVE_NET_MTU: usize = 1500; diff --git a/src/vm.rs b/src/vm.rs index 8f0ba51a..c0f8d974 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -180,7 +180,7 @@ impl UhyveVm { let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; // TODO: should be a random start address, if we have a relocatable executable - let kernel_start_address = object.start_addr().unwrap_or(0x400000) as usize; + let kernel_start_address = object.start_addr().unwrap_or(KERNEL_STACK_SIZE + START_ADDRESS_OFFSET) as usize; let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; From f8e99e4ad844ff0dd1c44f69920e3d6f8b23c06f Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 16:02:47 +0200 Subject: [PATCH 03/14] ASLR: Super rough proof of concept --- Cargo.lock | 1 + Cargo.toml | 1 + src/consts.rs | 6 +----- src/vm.rs | 14 ++++++++++++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 523cb53c..432f0568 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1430,6 +1430,7 @@ dependencies = [ "log", "mac_address", "nix 0.29.0", + "rand", "raw-cpuid", "rftrace", "rftrace-frontend", diff --git a/Cargo.toml b/Cargo.toml index 840198ba..f6c0923d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ uhyve-interface = { version = "0.1.1", path = "uhyve-interface", features = ["st virtio-bindings = { version = "0.2", features = ["virtio-v4_14_0"] } rftrace = { version = "0.1", optional = true } rftrace-frontend = { version = "0.1", optional = true } +rand = "0.8.5" [target.'cfg(target_os = "linux")'.dependencies] kvm-bindings = "0.8" diff --git a/src/consts.rs b/src/consts.rs index 0d4241cb..152de0b4 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -20,11 +20,7 @@ pub const EFER_LMA: u64 = 1 << 10; /* Long mode active (read-only) */ pub const EFER_NXE: u64 = 1 << 11; /* PTE No-Execute bit enable */ pub const IOAPIC_BASE: u64 = 0xfec00000; pub const IOAPIC_SIZE: u64 = 0x1000; -// TODO: Change to 0x100000 once ASLR is implemented. -// START_ADDRESS_OFFSET + KERNEL_STACK_SIZE = 0x400000, which was -// the set address that all binaries, relocatable or not, would previously -// be loaded on. This maintains "compatibility" for now. -pub const START_ADDRESS_OFFSET: u64 = 0x380000; +pub const START_ADDRESS_OFFSET: u64 = 0x100000; pub const KERNEL_STACK_SIZE: u64 = 0x20000; pub const SHAREDQUEUE_START: usize = 0x80000; pub const UHYVE_NET_MTU: usize = 1500; diff --git a/src/vm.rs b/src/vm.rs index c0f8d974..46a93177 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -9,6 +9,8 @@ use std::{ time::SystemTime, }; +use rand::Rng; + use hermit_entry::{ boot_info::{BootInfo, HardwareInfo, PlatformInfo, RawBootInfo, SerialPortBase}, elf::{KernelObject, LoadedKernel, ParseKernelError}, @@ -179,11 +181,19 @@ impl UhyveVm { let elf = fs::read(self.kernel_path())?; let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; - // TODO: should be a random start address, if we have a relocatable executable - let kernel_start_address = object.start_addr().unwrap_or(KERNEL_STACK_SIZE + START_ADDRESS_OFFSET) as usize; + // TODO: If rand::Rng should not be used, use `0x400000` instead. + // TODO: Is the value generated properly? Are we using rand properly? + let mut rng = rand::thread_rng(); + // 0xFFFFF0 maintains the generated address, minus the last 4 bits, which are required for paging. + // TODO: Find the upper boundary, and decuce it from the max possible address. Remove 0x891230. + // TODO: What if we don't have enough space? + // TODO: Uhyve should be informed if the value returned by `start_addr()` is equal to zero. + let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..0x891230) & 0xFFFFF0; + let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; + println!("{}", self.mem.guest_address.as_u64()); if kernel_end_address > self.mem.memory_size - self.mem.guest_address.as_u64() as usize { return Err(LoadKernelError::InsufficientMemory); } From fd9a9847ea5e775b2fc0df0350d83bffc8a430b9 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 19:26:37 +0200 Subject: [PATCH 04/14] ASLR: Flesh things out a bit more, add TODOs - Introduce end_address_upper_bound. - Add mask. - Some additional investigation and testing is necessary. - Right now, it feels like the mask just works because of a magic spell called "Works on my machine.". I didn't get creative with the spell name (which does not exist), because I don't know how spells sound like. Sorrry. - Some architecture-specific work may be needed. - Some fine-tuning and debugging information for ASLR is necessary. --- src/vm.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index 46a93177..12cc5517 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -9,6 +9,7 @@ use std::{ time::SystemTime, }; +use libc::STATX__RESERVED; use rand::Rng; use hermit_entry::{ @@ -181,19 +182,25 @@ impl UhyveVm { let elf = fs::read(self.kernel_path())?; let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; - // TODO: If rand::Rng should not be used, use `0x400000` instead. + // TODO: If rand::Rng should and cannot not be used, use `0x400000` instead. // TODO: Is the value generated properly? Are we using rand properly? let mut rng = rand::thread_rng(); - // 0xFFFFF0 maintains the generated address, minus the last 4 bits, which are required for paging. - // TODO: Find the upper boundary, and decuce it from the max possible address. Remove 0x891230. - // TODO: What if we don't have enough space? - // TODO: Uhyve should be informed if the value returned by `start_addr()` is equal to zero. - let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..0x891230) & 0xFFFFF0; + let end_address_upper_bound: u64 = self.mem.memory_size as u64 - self.mem.guest_address.as_u64(); + + // The heavily caffeinated author artificially modified the range from end_address_upper_bound-0x000001 + // to end_address_upper_bound+0x000001, so as to check the soundness of this implementation. + + // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. + // TODO: Is 0xFFFFF0 sound and cross-architecture? Should the mask be set as an architecture-specific constant? + // - Go over the paging implementation. + let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFF0; let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; + + // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. + let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; - println!("{}", self.mem.guest_address.as_u64()); if kernel_end_address > self.mem.memory_size - self.mem.guest_address.as_u64() as usize { return Err(LoadKernelError::InsufficientMemory); } From be472f71d261f966ed06208387c4144c660c9585 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 20:25:25 +0200 Subject: [PATCH 05/14] ASLR: Fix mask - 0xFFFFF0 cut off the upper 4 bits. --- src/vm.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index 12cc5517..4ca3397f 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -191,9 +191,8 @@ impl UhyveVm { // to end_address_upper_bound+0x000001, so as to check the soundness of this implementation. // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. - // TODO: Is 0xFFFFF0 sound and cross-architecture? Should the mask be set as an architecture-specific constant? - // - Go over the paging implementation. - let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFF0; + // TODO: Use some proper bitwise arithemtic instead of whatever this is supposed to be. + let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFF0; let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. From 705e6e8cc19d50532465dce74919b496189c9844 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sat, 15 Jun 2024 20:33:58 +0200 Subject: [PATCH 06/14] ASLR: Stop using dumb values for the mask - Some sleep is required. - Some bitwise arithmetic was avoided. - Why over-engineer a literal mask value? - I should take a break. --- src/vm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm.rs b/src/vm.rs index 4ca3397f..f67ba411 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -192,7 +192,7 @@ impl UhyveVm { // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. // TODO: Use some proper bitwise arithemtic instead of whatever this is supposed to be. - let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFF0; + let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFFFFFFFFFFF0; let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. From 67528e57c3100f49fdec0b72c2d69a9bafa84963 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sun, 16 Jun 2024 00:32:56 +0200 Subject: [PATCH 07/14] ASLR: cargo fmt run --- src/vm.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index f67ba411..cb65ff5b 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -9,14 +9,13 @@ use std::{ time::SystemTime, }; -use libc::STATX__RESERVED; -use rand::Rng; - use hermit_entry::{ boot_info::{BootInfo, HardwareInfo, PlatformInfo, RawBootInfo, SerialPortBase}, elf::{KernelObject, LoadedKernel, ParseKernelError}, }; +use libc::STATX__RESERVED; use log::{error, warn}; +use rand::Rng; use thiserror::Error; #[cfg(target_arch = "x86_64")] @@ -185,14 +184,16 @@ impl UhyveVm { // TODO: If rand::Rng should and cannot not be used, use `0x400000` instead. // TODO: Is the value generated properly? Are we using rand properly? let mut rng = rand::thread_rng(); - let end_address_upper_bound: u64 = self.mem.memory_size as u64 - self.mem.guest_address.as_u64(); + let end_address_upper_bound: u64 = + self.mem.memory_size as u64 - self.mem.guest_address.as_u64(); // The heavily caffeinated author artificially modified the range from end_address_upper_bound-0x000001 // to end_address_upper_bound+0x000001, so as to check the soundness of this implementation. // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. // TODO: Use some proper bitwise arithemtic instead of whatever this is supposed to be. - let kernel_random_address: u64 = rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFFFFFFFFFFF0; + let kernel_random_address: u64 = + rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFFFFFFFFFFF0; let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. From 842982a801d7ca034e7323eb752fca56833fd8f6 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sun, 16 Jun 2024 00:40:27 +0200 Subject: [PATCH 08/14] ASLR: Use proper constant. --- src/vm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vm.rs b/src/vm.rs index cb65ff5b..ebba605c 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -193,7 +193,7 @@ impl UhyveVm { // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. // TODO: Use some proper bitwise arithemtic instead of whatever this is supposed to be. let kernel_random_address: u64 = - rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xFFFFFFFFFFFFFFF0; + rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xffff_ffff_ffff_fff0; let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. From fe8cefacfb6dc759ba5c93a8bf0ac7ec4807e60b Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sun, 16 Jun 2024 09:32:47 +0200 Subject: [PATCH 09/14] ASLR: Distinguish relocatable objects, improve upper bound? There is something wrong going on here in this current revision. Not sure why. --- src/vm.rs | 49 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/vm.rs b/src/vm.rs index ebba605c..e54a2d7e 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -84,6 +84,7 @@ pub struct UhyveVm { boot_info: *const RawBootInfo, verbose: bool, pub virtio_device: Arc>, + aslr_status: bool, #[allow(dead_code)] // gdb is not supported on macos pub(super) gdb_port: Option, _vcpu_type: PhantomData, @@ -127,6 +128,7 @@ impl UhyveVm { args: params.kernel_args, boot_info: ptr::null(), verbose: params.verbose, + aslr_status: false, virtio_device, gdb_port: params.gdb_port, _vcpu_type: PhantomData, @@ -181,22 +183,36 @@ impl UhyveVm { let elf = fs::read(self.kernel_path())?; let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; - // TODO: If rand::Rng should and cannot not be used, use `0x400000` instead. - // TODO: Is the value generated properly? Are we using rand properly? - let mut rng = rand::thread_rng(); - let end_address_upper_bound: u64 = - self.mem.memory_size as u64 - self.mem.guest_address.as_u64(); - - // The heavily caffeinated author artificially modified the range from end_address_upper_bound-0x000001 - // to end_address_upper_bound+0x000001, so as to check the soundness of this implementation. - - // TODO: Move kernel address calculations, introduce tests that allow returning a stub to the range. - // TODO: Use some proper bitwise arithemtic instead of whatever this is supposed to be. - let kernel_random_address: u64 = - rng.gen_range(START_ADDRESS_OFFSET..end_address_upper_bound) & 0xffff_ffff_ffff_fff0; - let kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; - - // TODO: Check if kernel_start_address is equal to kernel_random_address. If None, change internal state. + let kernel_start_address; + + // Although Hermit is supposed to be relocatable by default, at the moment, some edge cases (Hermit C images) exist. + // We let hermit_entry do the hard work of establishing whether this is the case. If it is, + // the user *will* be warned, and the random value generation using ThreadRng will be _avoided_. + if !object.start_addr().is_none() { + info!("ASLR disabled: ELF not relocatable."); + + // As the ELF object is not relocatable, given that the object has a pre-defined start address, + // we don't have to choose an address. + kernel_start_address = object.start_addr().unwrap() as usize; + } else { + let mut rng = rand::thread_rng(); + // TODO: This breaks sometimes, causing the kernel to return InsufficientMemory errors despite the + // checks preceeding ASLR support passing. Why? + let start_address_upper_bound: u64 = self.mem.memory_size as u64 + - self.mem.guest_address.as_u64() + - object.mem_size() as u64; + + // TODO: Add test. (from end_address_upper_bound-0x000001 to end_address_upper_bound+0x000001) + // TODO: Is the mask alright? + + let kernel_random_address: u64 = rng + .gen_range(START_ADDRESS_OFFSET..start_address_upper_bound) + & 0xffff_ffff_ffff_fff0; + kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; + + // TODO: Actually use this variable somewhere for something or completely remove it. + self.aslr_status = true; + } let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; @@ -262,6 +278,7 @@ impl fmt::Debug for UhyveVm { .field("boot_info", &self.boot_info) .field("verbose", &self.verbose) .field("virtio_device", &self.virtio_device) + .field("aslr_status", &self.aslr_status) .finish() } } From 30e578f273c41df9d9033819605254dc7bbeb0d8 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Sun, 16 Jun 2024 11:36:58 +0200 Subject: [PATCH 10/14] ASLR: Add TODO comment --- src/vm.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vm.rs b/src/vm.rs index e54a2d7e..73f733bc 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -188,6 +188,8 @@ impl UhyveVm { // Although Hermit is supposed to be relocatable by default, at the moment, some edge cases (Hermit C images) exist. // We let hermit_entry do the hard work of establishing whether this is the case. If it is, // the user *will* be warned, and the random value generation using ThreadRng will be _avoided_. + + // TODO: Put everything in this if..else block in unwrap_or instead. if !object.start_addr().is_none() { info!("ASLR disabled: ELF not relocatable."); From b26ab261ef9fb27e7bb2e953ecaee45cd4eb0674 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Mon, 17 Jun 2024 15:15:00 +0200 Subject: [PATCH 11/14] ASLR: Massive refactor --- Cargo.toml | 6 +++-- src/consts.rs | 1 - src/vm.rs | 70 ++++++++++++++++++++++++++++----------------------- 3 files changed, 42 insertions(+), 35 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f6c0923d..c0cce904 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,9 @@ path = "benches/benchmarks.rs" harness = false [features] -default = [] +default = ["disable-aslr"] +disable-aslr = [] +enable-aslr = [] instrument = ["rftrace", "rftrace-frontend"] [dependencies] @@ -80,4 +82,4 @@ bitflags = "2.4" [dev-dependencies] assert_fs = "1" -criterion = "0.5" +criterion = "0.5" \ No newline at end of file diff --git a/src/consts.rs b/src/consts.rs index 152de0b4..a5480546 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -20,7 +20,6 @@ pub const EFER_LMA: u64 = 1 << 10; /* Long mode active (read-only) */ pub const EFER_NXE: u64 = 1 << 11; /* PTE No-Execute bit enable */ pub const IOAPIC_BASE: u64 = 0xfec00000; pub const IOAPIC_SIZE: u64 = 0x1000; -pub const START_ADDRESS_OFFSET: u64 = 0x100000; pub const KERNEL_STACK_SIZE: u64 = 0x20000; pub const SHAREDQUEUE_START: usize = 0x80000; pub const UHYVE_NET_MTU: usize = 1500; diff --git a/src/vm.rs b/src/vm.rs index 73f733bc..128f1f19 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -13,8 +13,8 @@ use hermit_entry::{ boot_info::{BootInfo, HardwareInfo, PlatformInfo, RawBootInfo, SerialPortBase}, elf::{KernelObject, LoadedKernel, ParseKernelError}, }; -use libc::STATX__RESERVED; use log::{error, warn}; +#[cfg(feature = "enable-aslr")] use rand::Rng; use thiserror::Error; @@ -179,41 +179,47 @@ impl UhyveVm { ); } + #[cfg(feature = "enable-aslr")] + fn generate_start_address(&mut self, object_mem_size: u64) -> u64 { + let mut rng = rand::thread_rng(); + + // TODO: This breaks sometimes, causing the _kernel_ to return InsufficientMemory errors, despite the + // checks the boot process passing. Why? + // + // This seems to be a problem on the side of hermit-os/kernel, which does not behave very well if the + // start address is way too high. For some mysterious reason, this problem never occurs if we do not + // go past 0x3000000. + let start_address_upper_bound: u64 = std::cmp::min( + self.mem.memory_size as u64 - self.mem.guest_address.as_u64() - object_mem_size, + 0x3000000, + ); + + // TODO: Add test. (from end_address_upper_bound-0x000001 to end_address_upper_bound+0x000001) + // TODO: Is the mask alright? + // + // We use 0x100000 as the offset for the start address so as to not use the zero page. + let kernel_random_address: u64 = + rng.gen_range(0x100000..start_address_upper_bound) & 0xffff_ffff_ffff_fff0; + + // TODO: Actually use this variable somewhere for something or completely remove it. + self.aslr_status = true; + + kernel_random_address + } + pub fn load_kernel(&mut self) -> LoadKernelResult<()> { let elf = fs::read(self.kernel_path())?; let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; - let kernel_start_address; - - // Although Hermit is supposed to be relocatable by default, at the moment, some edge cases (Hermit C images) exist. - // We let hermit_entry do the hard work of establishing whether this is the case. If it is, - // the user *will* be warned, and the random value generation using ThreadRng will be _avoided_. - - // TODO: Put everything in this if..else block in unwrap_or instead. - if !object.start_addr().is_none() { - info!("ASLR disabled: ELF not relocatable."); - - // As the ELF object is not relocatable, given that the object has a pre-defined start address, - // we don't have to choose an address. - kernel_start_address = object.start_addr().unwrap() as usize; - } else { - let mut rng = rand::thread_rng(); - // TODO: This breaks sometimes, causing the kernel to return InsufficientMemory errors despite the - // checks preceeding ASLR support passing. Why? - let start_address_upper_bound: u64 = self.mem.memory_size as u64 - - self.mem.guest_address.as_u64() - - object.mem_size() as u64; - - // TODO: Add test. (from end_address_upper_bound-0x000001 to end_address_upper_bound+0x000001) - // TODO: Is the mask alright? - - let kernel_random_address: u64 = rng - .gen_range(START_ADDRESS_OFFSET..start_address_upper_bound) - & 0xffff_ffff_ffff_fff0; - kernel_start_address = object.start_addr().unwrap_or(kernel_random_address) as usize; - - // TODO: Actually use this variable somewhere for something or completely remove it. - self.aslr_status = true; + #[cfg(all(feature = "enable-aslr", feature = "disable-aslr"))] + compile_error!("\"enable-aslr\" and \"disable-aslr\" cannot be enabled at the same time"); + + #[cfg(feature = "disable-aslr")] + let kernel_start_address = object.start_addr().unwrap_or(0x400000) as usize; + + #[cfg(feature = "enable-aslr")] + { + let kernel_start_address = self.generate_start_address(object.mem_size() as u64) as usize; } let kernel_end_address = kernel_start_address + object.mem_size(); From cecd8f8de4643d750c1ba72b42898a0b8d06c0cf Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Mon, 17 Jun 2024 15:25:16 +0200 Subject: [PATCH 12/14] ASLR: Only use one feature - It's #[cfg(not(feature = aslr))], not #[not(cfg(feature = aslr))]. - Some additional typo-induced bug was fixed. .-. --- Cargo.toml | 5 ++--- src/vm.rs | 15 +++++---------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c0cce904..60cbfac8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,9 +36,8 @@ path = "benches/benchmarks.rs" harness = false [features] -default = ["disable-aslr"] -disable-aslr = [] -enable-aslr = [] +default = ["aslr"] +aslr = [] instrument = ["rftrace", "rftrace-frontend"] [dependencies] diff --git a/src/vm.rs b/src/vm.rs index 128f1f19..f6fe814d 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -14,7 +14,7 @@ use hermit_entry::{ elf::{KernelObject, LoadedKernel, ParseKernelError}, }; use log::{error, warn}; -#[cfg(feature = "enable-aslr")] +#[cfg(feature = "aslr")] use rand::Rng; use thiserror::Error; @@ -179,7 +179,7 @@ impl UhyveVm { ); } - #[cfg(feature = "enable-aslr")] + #[cfg(feature = "aslr")] fn generate_start_address(&mut self, object_mem_size: u64) -> u64 { let mut rng = rand::thread_rng(); @@ -211,16 +211,11 @@ impl UhyveVm { let elf = fs::read(self.kernel_path())?; let object = KernelObject::parse(&elf).map_err(LoadKernelError::ParseKernelError)?; - #[cfg(all(feature = "enable-aslr", feature = "disable-aslr"))] - compile_error!("\"enable-aslr\" and \"disable-aslr\" cannot be enabled at the same time"); - - #[cfg(feature = "disable-aslr")] + #[cfg(not(feature = "aslr"))] let kernel_start_address = object.start_addr().unwrap_or(0x400000) as usize; - #[cfg(feature = "enable-aslr")] - { - let kernel_start_address = self.generate_start_address(object.mem_size() as u64) as usize; - } + #[cfg(feature = "aslr")] + let kernel_start_adress = self.generate_start_address(object.mem_size() as u64) as usize; let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; From 6eb3a18d25ef5b9f7339752a6b1de60d9691ecae Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Mon, 17 Jun 2024 15:28:16 +0200 Subject: [PATCH 13/14] ASLR: Disable aslr feature by default --- Cargo.toml | 2 +- src/vm.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 60cbfac8..824478de 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ path = "benches/benchmarks.rs" harness = false [features] -default = ["aslr"] +default = [] aslr = [] instrument = ["rftrace", "rftrace-frontend"] diff --git a/src/vm.rs b/src/vm.rs index f6fe814d..c30360d2 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -215,7 +215,7 @@ impl UhyveVm { let kernel_start_address = object.start_addr().unwrap_or(0x400000) as usize; #[cfg(feature = "aslr")] - let kernel_start_adress = self.generate_start_address(object.mem_size() as u64) as usize; + let kernel_start_address = self.generate_start_address(object.mem_size() as u64) as usize; let kernel_end_address = kernel_start_address + object.mem_size(); self.offset = kernel_start_address as u64; From 1b613510a91d00dda2d6cef005610d50696bd258 Mon Sep 17 00:00:00 2001 From: "Panagiotis \"Ivory\" Vasilopoulos" Date: Mon, 17 Jun 2024 16:13:35 +0200 Subject: [PATCH 14/14] ASLR: Set self.aslr_status to True later We should probably tell Uhyve that it is enabled once we are sure that nothing has gone wrong. What we should do when something goes wrong (can anything go wrong?) is an open question. --- Cargo.toml | 2 +- src/vm.rs | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 824478de..60cbfac8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ path = "benches/benchmarks.rs" harness = false [features] -default = [] +default = ["aslr"] aslr = [] instrument = ["rftrace", "rftrace-frontend"] diff --git a/src/vm.rs b/src/vm.rs index c30360d2..b9e96205 100644 --- a/src/vm.rs +++ b/src/vm.rs @@ -194,17 +194,10 @@ impl UhyveVm { 0x3000000, ); - // TODO: Add test. (from end_address_upper_bound-0x000001 to end_address_upper_bound+0x000001) - // TODO: Is the mask alright? + // TODO: Add test. (from start_address_upper_bound-0x000001 to start_address_upper_bound+0x000001) // // We use 0x100000 as the offset for the start address so as to not use the zero page. - let kernel_random_address: u64 = - rng.gen_range(0x100000..start_address_upper_bound) & 0xffff_ffff_ffff_fff0; - - // TODO: Actually use this variable somewhere for something or completely remove it. - self.aslr_status = true; - - kernel_random_address + rng.gen_range(0x100000..start_address_upper_bound) & 0xffff_ffff_ffff_fff0 } pub fn load_kernel(&mut self) -> LoadKernelResult<()> { @@ -234,6 +227,10 @@ impl UhyveVm { kernel_start_address as u64, ); self.entry_point = entry_point; + #[cfg(feature = "aslr")] + { + self.aslr_status = true; + } let boot_info = BootInfo { hardware_info: HardwareInfo {