From e101c9fd9fc06998534a4f26529a79ba9f324639 Mon Sep 17 00:00:00 2001 From: zseri Date: Fri, 30 Sep 2022 12:35:08 +0200 Subject: [PATCH] GENERAL BREAK: get rid of signed pointers --- Cargo.lock | 7 -- crates/fogtix-bytecode/Cargo.toml | 4 - crates/fogtix-bytecode/src/instr.rs | 4 +- crates/fogtix-bytecode/src/lib.rs | 2 - crates/fogtix-bytecode/src/pointer.rs | 150 -------------------------- crates/fogtix-vm/src/lib.rs | 16 +-- docs/design.md | 24 +++-- 7 files changed, 26 insertions(+), 181 deletions(-) delete mode 100644 crates/fogtix-bytecode/src/pointer.rs diff --git a/Cargo.lock b/Cargo.lock index b9e5305..8af3d53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -93,7 +93,6 @@ version = "0.0.0" dependencies = [ "int-enum", "proptest", - "siphasher", ] [[package]] @@ -328,12 +327,6 @@ dependencies = [ "lazy_static", ] -[[package]] -name = "siphasher" -version = "0.3.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de" - [[package]] name = "smallvec" version = "1.9.0" diff --git a/crates/fogtix-bytecode/Cargo.toml b/crates/fogtix-bytecode/Cargo.toml index fc4b3e0..95b0bdb 100644 --- a/crates/fogtix-bytecode/Cargo.toml +++ b/crates/fogtix-bytecode/Cargo.toml @@ -11,10 +11,6 @@ license = "LGPL-2.1+" version = "0.4" default-features = false -[dependencies.siphasher] -version = "0.3" -default-features = false - [dev-dependencies.proptest] version = "1.0" default-features = false diff --git a/crates/fogtix-bytecode/src/instr.rs b/crates/fogtix-bytecode/src/instr.rs index 47ad9eb..2c4c919 100644 --- a/crates/fogtix-bytecode/src/instr.rs +++ b/crates/fogtix-bytecode/src/instr.rs @@ -32,7 +32,7 @@ pub enum Instr { // basic stack operations /// pushes the given value to the stack - Push(u128), + Push(u64), /// pops the top $0+1 values from the stack Pop(u16), @@ -105,7 +105,7 @@ impl<'a> crate::Parse<'a> for Instr { OpType::CallLocal => u64::parse(inp).map(|(inp, val)| (inp, Instr::CallLocal(val))), OpType::CallLDefer => u64::parse(inp).map(|(inp, val)| (inp, Instr::CallLDefer(val))), OpType::JumpCond => u64::parse(inp).map(|(inp, val)| (inp, Instr::JumpCond(val))), - OpType::Push => u128::parse(inp).map(|(inp, val)| (inp, Instr::Push(val))), + OpType::Push => u64::parse(inp).map(|(inp, val)| (inp, Instr::Push(val))), OpType::Pop => u16::parse(inp).map(|(inp, val)| (inp, Instr::Pop(val))), OpType::Dup => u16::parse(inp).map(|(inp, val)| (inp, Instr::Dup(val))), OpType::Swap => u16::parse(inp).map(|(inp, val)| (inp, Instr::Swap(val))), diff --git a/crates/fogtix-bytecode/src/lib.rs b/crates/fogtix-bytecode/src/lib.rs index c966fab..0ddd8a7 100644 --- a/crates/fogtix-bytecode/src/lib.rs +++ b/crates/fogtix-bytecode/src/lib.rs @@ -8,5 +8,3 @@ mod instr; pub use instr::{Instr, ParseError as InstrParseError}; mod parse; pub use parse::Parse; -mod pointer; -pub use pointer::Pointer; diff --git a/crates/fogtix-bytecode/src/pointer.rs b/crates/fogtix-bytecode/src/pointer.rs deleted file mode 100644 index 5299c4a..0000000 --- a/crates/fogtix-bytecode/src/pointer.rs +++ /dev/null @@ -1,150 +0,0 @@ -use siphasher::sip::SipHasher24 as SipHasher; - -/// A signed pointer, the `payload` should never by dereferenced -/// without verifying the pointer first -#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -#[repr(C, align(16))] -pub struct Pointer(pub [u8; 16]); - -impl From for Pointer { - #[inline] - fn from(x: u128) -> Self { - Self(x.to_be_bytes()) - } -} - -impl From<(u64, u64)> for Pointer { - #[inline] - fn from((a, b): (u64, u64)) -> Self { - let mut this = Self([0; 16]); - { - let (a_, b_) = this.0.split_at_mut(8); - a_.copy_from_slice(&a.to_be_bytes()); - b_.copy_from_slice(&b.to_be_bytes()); - } - this - } -} - -impl From for u128 { - #[inline] - fn from(Pointer(x): Pointer) -> u128 { - u128::from_be_bytes(x) - } -} - -impl From for (u64, u64) { - #[inline] - fn from(Pointer(x): Pointer) -> (u64, u64) { - let (a, b) = x.split_at(8); - ( - u64::from_be_bytes(a.try_into().unwrap()), - u64::from_be_bytes(b.try_into().unwrap()), - ) - } -} - -impl Pointer { - fn calculate_hmac(payload: u64, key: &u128) -> u64 { - use core::hash::Hasher; - let mut h = SipHasher::new_with_key(&key.to_be_bytes()); - h.write_u64(payload & ((1 << 48) - 1)); - h.finish() - } - - /// SECURITY NOTE: the upper two bytes of `payload` (`origin`) aren't taken - /// into account when calculating the HMAC because they're node-specific - pub fn new_with_key(payload: u64, key: &u128) -> Pointer { - let hmac = Self::calculate_hmac(payload, key); - Self::from((hmac, payload)) - } - - #[inline] - pub fn verify(&self, key: &u128) -> Option { - let (hmac, payload) = (*self).into(); - if hmac == Self::calculate_hmac(payload, key) { - Some(payload) - } else { - None - } - } - - #[inline] - pub fn set_origin(&mut self, origin: u16) { - self.0[8..10].copy_from_slice(&origin.to_be_bytes()); - } - - #[inline] - pub fn origin(&self) -> u16 { - u16::from_be_bytes(self.0[8..10].try_into().unwrap()) - } -} - -impl crate::Parse<'_> for Pointer { - type Err = (); - fn parse(inp: &[u8]) -> Result<(&[u8], Self), ()> { - if inp.len() < 16 { - Err(()) - } else { - let (this, next) = inp.split_at(16); - Ok((next, Self(this.try_into().unwrap()))) - } - } -} -#[cfg(any(test, feature = "std"))] -impl Pointer { - #[inline] - pub fn write_to(&self, mut writer: W) -> std::io::Result<()> { - writer.write_all(&self.0) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - proptest::proptest! { - #[test] - fn u128_roundtrip(x in 0..u128::MAX) { - let ptr = Pointer::from(x); - assert_eq!(u128::from(ptr), x); - } - } - - #[test] - fn pointer_repr() { - use core::mem; - assert_eq!(mem::size_of::(), 16); - assert_eq!(mem::align_of::(), 16); - } - - #[test] - fn pointer_usage() { - let k: u128 = 0x000000000000dead000000000000beef; - let p = Pointer::new_with_key(0xfefe, &k); - // verify that this is the same value on all systems - assert_eq!(p.0[0..8], [42, 115, 131, 215, 127, 235, 147, 241]); - assert_eq!(p.verify(&k), Some(0xfefe)); - assert_eq!(p.origin(), 0); - } - - #[test] - fn pointer_usage2() { - let k: u128 = 0x000000000000dead000000000000beef; - let p = Pointer::new_with_key(0x0508deadbeeffefe, &k); - // verify that this is the same value on all systems - assert_eq!(p.0[0..8], [191, 23, 107, 0, 61, 74, 249, 219]); - assert_eq!(p.verify(&k), Some(0x0508deadbeeffefe)); - assert_eq!(p.origin(), 0x0508); - } - - #[test] - fn pointer_usage3() { - let k: u128 = 0x000000000000dead000000000000beef; - let p = Pointer::new_with_key(0xf7d8deadbeeffefe, &k); - // verify that this is the same value on all systems - assert_eq!(p.0[0..8], [191, 23, 107, 0, 61, 74, 249, 219]); - assert_eq!(p.verify(&k), Some(0xf7d8deadbeeffefe)); - assert_eq!(p.origin(), 0xf7d8); - } -} diff --git a/crates/fogtix-vm/src/lib.rs b/crates/fogtix-vm/src/lib.rs index 8d09c5f..7566bbd 100644 --- a/crates/fogtix-vm/src/lib.rs +++ b/crates/fogtix-vm/src/lib.rs @@ -5,7 +5,7 @@ extern crate alloc; use alloc::{sync::Arc, vec::Vec}; use core::fmt; -use fogtix_bytecode::{consts, Instr, Parse, Pointer}; +use fogtix_bytecode::{consts, Instr, Parse}; pub type Module = Arc; @@ -61,7 +61,7 @@ impl InstrPtr { } } -pub type StackEntValue = u128; +pub type StackEntValue = u64; #[derive(Clone, Debug)] pub enum Error { @@ -75,7 +75,7 @@ pub enum Error { OutOfFuel, NotEnoughStacked, DivisionByZero, - RemoteCall(Pointer), + RemoteCall(u64), } impl fmt::Display for Error { @@ -97,7 +97,7 @@ impl fmt::Display for Error { Self::OutOfFuel => write!(f, "out of fuel"), Self::NotEnoughStacked => write!(f, "not enough operands on stack"), Self::DivisionByZero => write!(f, "tried to divide by zero"), - Self::RemoteCall(ptr) => write!(f, "tried to call remote @ {:x?}", ptr.0), + Self::RemoteCall(ptr) => write!(f, "tried to call remote @ {:x?}", ptr), } } } @@ -157,7 +157,7 @@ impl Process { if !self.instrp.is_call2jump() { self.callstack.push(self.instrp.clone()); } - return Err(Error::RemoteCall(Pointer::from(self.stpop()?))); + return Err(Error::RemoteCall(self.stpop()?)); } Instr::CallLocal(x) => { if !self.instrp.is_call2jump() { @@ -282,8 +282,8 @@ impl Process { Some(x) => x, None => return Err(Error::DivisionByZero), }, - B::Srem => match (a as i128).checked_rem(b as i128) { - Some(x) => x as u128, + B::Srem => match (a as i64).checked_rem(b as i64) { + Some(x) => x as u64, None => return Err(Error::DivisionByZero), }, }); @@ -299,7 +299,7 @@ mod tests { #[test] fn stack_item_size() { - assert_eq!(core::mem::size_of::(), 16); + assert_eq!(core::mem::size_of::(), 8); } proptest::proptest! { diff --git a/docs/design.md b/docs/design.md index 60b5cab..9e2950a 100644 --- a/docs/design.md +++ b/docs/design.md @@ -1,15 +1,8 @@ # basic ideas -* use tagged, signed pointers for memory addresses * use a separate call stack to prevent ROP * implement message passing - -# values - -* atoms (u128): atomic values, also used as pointer keys to sign pointers -* bytes ([]u8): arbitrary byte strings -* int (u64): the default integer type -* pointer (u128): signed pointer (can only be accessed with matching key) +* integers as only internal data type # entry points and libraries @@ -17,3 +10,18 @@ each module starts with the entry point, this avoids the need for a separate "en the entry point of the main module is what's initially called by the VM to start execution. the entry point of library modules are called to build the library descriptor table, which should leave an additional pointer on the stack which can be passed to `call-r`. + +# historic + +## signed pointers + +Initially, this project tried to implement an idea of signed pointers, so that a +party can verify that it authored a pointer (via HMAC, e.g. SipHash); but some +problems remain, which make this impractical + +* either it is insecure because the pointer is too small to store a resistant-enough hash +* thus, the pointer is so large that it gets unwieldly (e.g. `u128`) +* how to find out which node or process owns a pointer if you aren't that node + (the hash only indicates if you own it or not, not who owns it) +* how to find out if the pointer is still valid (you need some hashmap or so to store that), + but then you have basically reinvented file descriptors.