From 1e35346c7463dc505ab453e8621b6b9efd53761a Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Sat, 15 May 2021 12:15:01 +0200 Subject: [PATCH 01/29] deps(rtc_tenclave): add serde & serde_json (sgx and std versions, for testing) --- rtc_auth_enclave/Cargo.lock | 47 +++++++++++++++++++++--- rtc_data_enclave/Cargo.lock | 41 +++++++++++++++++---- rtc_tenclave/Cargo.lock | 72 +++++++++++++++++++++++++++++++++++-- rtc_tenclave/Cargo.toml | 6 +++- rtc_tenclave/src/lib.rs | 2 ++ 5 files changed, 154 insertions(+), 14 deletions(-) diff --git a/rtc_auth_enclave/Cargo.lock b/rtc_auth_enclave/Cargo.lock index 02e76914..f2235a33 100644 --- a/rtc_auth_enclave/Cargo.lock +++ b/rtc_auth_enclave/Cargo.lock @@ -50,8 +50,8 @@ dependencies = [ "log", "proc-macro2", "quote", - "serde", - "serde_json", + "serde 1.0.125", + "serde_json 1.0.64", "syn", "tempfile", "toml", @@ -157,6 +157,14 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "itoa" +version = "0.4.5" +source = "git+https://github.com/mesalock-linux/itoa-sgx#295ee451f5ec74f25c299552b481beb445ea3eb7" +dependencies = [ + "sgx_tstd", +] + [[package]] name = "itoa" version = "0.4.7" @@ -352,6 +360,8 @@ dependencies = [ "ring", "rtc_types", "secrecy", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde_json 1.0.60", "sgx_tcrypto", "sgx_tse", "sgx_tstd", @@ -386,6 +396,22 @@ dependencies = [ "zeroize", ] +[[package]] +name = "serde" +version = "1.0.118" +source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" +dependencies = [ + "sgx_tstd", +] + +[[package]] +name = "serde" +version = "1.0.118" +source = "git+https://github.com/mesalock-linux/serde-sgx#db0226f1d5d70fca6b96af2c285851502204e21c" +dependencies = [ + "sgx_tstd", +] + [[package]] name = "serde" version = "1.0.125" @@ -406,15 +432,26 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_json" +version = "1.0.60" +source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380893814ad2a057758d825bab798aa117f7362a" +dependencies = [ + "itoa 0.4.5", + "ryu", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "sgx_tstd", +] + [[package]] name = "serde_json" version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "799e97dc9fdae36a5c8b8f2cae9ce2ee9fdce2058c57a93e6099d919fd982f79" dependencies = [ - "itoa", + "itoa 0.4.7", "ryu", - "serde", + "serde 1.0.125", ] [[package]] @@ -613,7 +650,7 @@ version = "0.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a31142970826733df8241ef35dc040ef98c679ab14d7c3e54d827099b3acecaa" dependencies = [ - "serde", + "serde 1.0.125", ] [[package]] diff --git a/rtc_data_enclave/Cargo.lock b/rtc_data_enclave/Cargo.lock index d53d93de..cf4562a1 100644 --- a/rtc_data_enclave/Cargo.lock +++ b/rtc_data_enclave/Cargo.lock @@ -38,7 +38,7 @@ version = "1.3.1" source = "git+https://github.com/mesalock-linux/bincode-sgx.git#58b004eb5bd7ab8e974a02b70344afcd9724bb2d" dependencies = [ "byteorder", - "serde 1.0.118", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", "sgx_tstd", ] @@ -75,7 +75,7 @@ dependencies = [ "proc-macro2", "quote", "serde 1.0.125", - "serde_json", + "serde_json 1.0.64", "syn", "tempfile", "toml", @@ -206,6 +206,14 @@ dependencies = [ "either", ] +[[package]] +name = "itoa" +version = "0.4.5" +source = "git+https://github.com/mesalock-linux/itoa-sgx#295ee451f5ec74f25c299552b481beb445ea3eb7" +dependencies = [ + "sgx_tstd", +] + [[package]] name = "itoa" version = "0.4.7" @@ -422,7 +430,7 @@ dependencies = [ "rtc_tenclave", "rtc_types", "secrecy", - "serde 1.0.118", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", "serde-big-array", "serde_derive 1.0.118", "sgx_tcrypto", @@ -449,6 +457,8 @@ dependencies = [ "ring", "rtc_types", "secrecy", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde_json 1.0.60", "sgx_tcrypto", "sgx_tse", "sgx_tstd", @@ -483,6 +493,14 @@ dependencies = [ "zeroize", ] +[[package]] +name = "serde" +version = "1.0.118" +source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" +dependencies = [ + "sgx_tstd", +] + [[package]] name = "serde" version = "1.0.118" @@ -506,7 +524,7 @@ name = "serde-big-array" version = "0.3.0" source = "git+https://github.com/mesalock-linux/serde-big-array-sgx#94122c5167aee38b39b09a620a60db2c28cf7428" dependencies = [ - "serde 1.0.118", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", "serde_derive 1.0.118", ] @@ -531,13 +549,24 @@ dependencies = [ "syn", ] +[[package]] +name = "serde_json" +version = "1.0.60" +source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380893814ad2a057758d825bab798aa117f7362a" +dependencies = [ + "itoa 0.4.5", + "ryu", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "sgx_tstd", +] + [[package]] name = "serde_json" version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "799e97dc9fdae36a5c8b8f2cae9ce2ee9fdce2058c57a93e6099d919fd982f79" dependencies = [ - "itoa", + "itoa 0.4.7", "ryu", "serde 1.0.125", ] @@ -568,7 +597,7 @@ version = "1.1.3" source = "git+https://github.com/apache/teaclave-sgx-sdk.git?rev=v1.1.3#a6a172e652b4db4eaa17e4faa078fda8922abdd0" dependencies = [ "itertools", - "serde 1.0.118", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", "serde-big-array", "serde_derive 1.0.118", "sgx_tcrypto", diff --git a/rtc_tenclave/Cargo.lock b/rtc_tenclave/Cargo.lock index 3dd970af..72d13d7a 100644 --- a/rtc_tenclave/Cargo.lock +++ b/rtc_tenclave/Cargo.lock @@ -57,6 +57,20 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4161ceaf2f41b6cd3f6502f5da085d4ad4393a51e0c70ed2fce1d5698d798fae" +[[package]] +name = "itoa" +version = "0.4.5" +source = "git+https://github.com/mesalock-linux/itoa-sgx#295ee451f5ec74f25c299552b481beb445ea3eb7" +dependencies = [ + "sgx_tstd", +] + +[[package]] +name = "itoa" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd25036021b0de88a0aff6b850051563c6516d0bf53f8638938edbb9de732736" + [[package]] name = "js-sys" version = "0.3.50" @@ -242,6 +256,10 @@ dependencies = [ "ring", "rtc_types", "secrecy", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde 1.0.126", + "serde_json 1.0.60", + "serde_json 1.0.64", "sgx_tcrypto", "sgx_tse", "sgx_tstd", @@ -263,6 +281,12 @@ dependencies = [ "thiserror 1.0.9", ] +[[package]] +name = "ryu" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" + [[package]] name = "secrecy" version = "0.7.0" @@ -272,6 +296,50 @@ dependencies = [ "zeroize", ] +[[package]] +name = "serde" +version = "1.0.118" +source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" +dependencies = [ + "sgx_tstd", +] + +[[package]] +name = "serde" +version = "1.0.118" +source = "git+https://github.com/mesalock-linux/serde-sgx#db0226f1d5d70fca6b96af2c285851502204e21c" +dependencies = [ + "sgx_tstd", +] + +[[package]] +name = "serde" +version = "1.0.126" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec7505abeacaec74ae4778d9d9328fe5a5d04253220a85c4ee022239fc996d03" + +[[package]] +name = "serde_json" +version = "1.0.60" +source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380893814ad2a057758d825bab798aa117f7362a" +dependencies = [ + "itoa 0.4.5", + "ryu", + "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "sgx_tstd", +] + +[[package]] +name = "serde_json" +version = "1.0.64" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "799e97dc9fdae36a5c8b8f2cae9ce2ee9fdce2058c57a93e6099d919fd982f79" +dependencies = [ + "itoa 0.4.7", + "ryu", + "serde 1.0.126", +] + [[package]] name = "sgx_alloc" version = "1.1.3" @@ -308,7 +376,7 @@ dependencies = [ [[package]] name = "sgx_tcrypto" version = "1.1.3" -source = "git+https://github.com/apache/teaclave-sgx-sdk.git#c2698dc2685f8dcd9550086c62077bceff15ded0" +source = "git+https://github.com/apache/teaclave-sgx-sdk.git#d1e2a909ee0a4e95437ba75149d12ab9d07fcfe1" dependencies = [ "sgx_types", ] @@ -363,7 +431,7 @@ source = "git+https://github.com/apache/incubator-teaclave-sgx-sdk.git#c2698dc26 [[package]] name = "sgx_ucrypto" version = "1.1.3" -source = "git+https://github.com/apache/teaclave-sgx-sdk.git#c2698dc2685f8dcd9550086c62077bceff15ded0" +source = "git+https://github.com/apache/teaclave-sgx-sdk.git#d1e2a909ee0a4e95437ba75149d12ab9d07fcfe1" dependencies = [ "libc", "rand_core 0.3.1", diff --git a/rtc_tenclave/Cargo.toml b/rtc_tenclave/Cargo.toml index d7bc98a0..235aa683 100644 --- a/rtc_tenclave/Cargo.toml +++ b/rtc_tenclave/Cargo.toml @@ -11,7 +11,7 @@ doctest = false crate-type = ["lib"] [features] -default = ["sgx_tstd", "sgx_tse", "rtc_types/teaclave_sgx", "rand", "thiserror", "sgx_tcrypto"] +default = ["sgx_tstd", "sgx_tse", "rtc_types/teaclave_sgx", "rand", "thiserror", "sgx_tcrypto", "serde", "serde_json"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html @@ -22,6 +22,8 @@ sgx_tse = { git = "https://github.com/apache/teaclave-sgx-sdk.git" , rev = "v1.1 rand = { git = "https://github.com/mesalock-linux/rand-sgx", tag = "v0.7.3_sgx1.1.3", optional = true } thiserror = { git = "https://github.com/mesalock-linux/thiserror-sgx.git", optional = true } sgx_tcrypto = { git = "https://github.com/apache/teaclave-sgx-sdk.git", optional = true } +serde = { git = "https://github.com/mesalock-linux/serde-sgx", tag = "sgx_1.1.3", optional = true } +serde_json = { git = "https://github.com/mesalock-linux/serde-json-sgx", tag = "sgx_1.1.3", optional = true } rtc_types = { path = "../rtc_types" } sgx_types = { git = "https://github.com/apache/teaclave-sgx-sdk.git", features = ["extra_traits"]} @@ -38,6 +40,8 @@ cfg-if = "1.0.0" thiserror_std = { package = "thiserror", version = "1.0.9" } rand_std = { package = "rand", version = "0.7.3" } sgx_ucrypto = { git = "https://github.com/apache/teaclave-sgx-sdk.git" } +serde_std = { package = "serde", version = "1.0.0" } +serde_json_std = { package = "serde_json", version = "1.0.0" } [patch."https://github.com/apache/teaclave-sgx-sdk.git"] diff --git a/rtc_tenclave/src/lib.rs b/rtc_tenclave/src/lib.rs index db0d3873..2f16ad01 100644 --- a/rtc_tenclave/src/lib.rs +++ b/rtc_tenclave/src/lib.rs @@ -16,6 +16,8 @@ cfg_if::cfg_if! { extern crate thiserror_std as thiserror; extern crate rand_std as rand; extern crate sgx_ucrypto as sgx_tcrypto; + extern crate serde_std as serde; + extern crate serde_json_std as serde_json; } } From f2c45402729c62e13f6a39da6f1b5766fb49f01b Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Sat, 15 May 2021 22:47:18 +0200 Subject: [PATCH 02/29] deps(rtc_tenclave): work around serde-json-sgx / Cargo version handling issue See the comment in rtc_tenclave/Cargo.toml --- rtc_auth_enclave/Cargo.lock | 12 ++---------- rtc_data_enclave/Cargo.lock | 20 ++++++-------------- rtc_data_enclave/Cargo.toml | 2 +- rtc_tenclave/Cargo.lock | 12 ++---------- rtc_tenclave/Cargo.toml | 27 ++++++++++++++++++++++++++- 5 files changed, 37 insertions(+), 36 deletions(-) diff --git a/rtc_auth_enclave/Cargo.lock b/rtc_auth_enclave/Cargo.lock index f2235a33..fd33c9f3 100644 --- a/rtc_auth_enclave/Cargo.lock +++ b/rtc_auth_enclave/Cargo.lock @@ -360,7 +360,7 @@ dependencies = [ "ring", "rtc_types", "secrecy", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde 1.0.118", "serde_json 1.0.60", "sgx_tcrypto", "sgx_tse", @@ -396,14 +396,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "serde" -version = "1.0.118" -source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" -dependencies = [ - "sgx_tstd", -] - [[package]] name = "serde" version = "1.0.118" @@ -439,7 +431,7 @@ source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380 dependencies = [ "itoa 0.4.5", "ryu", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "sgx_tstd", ] diff --git a/rtc_data_enclave/Cargo.lock b/rtc_data_enclave/Cargo.lock index cf4562a1..c4ecc698 100644 --- a/rtc_data_enclave/Cargo.lock +++ b/rtc_data_enclave/Cargo.lock @@ -38,7 +38,7 @@ version = "1.3.1" source = "git+https://github.com/mesalock-linux/bincode-sgx.git#58b004eb5bd7ab8e974a02b70344afcd9724bb2d" dependencies = [ "byteorder", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "sgx_tstd", ] @@ -430,7 +430,7 @@ dependencies = [ "rtc_tenclave", "rtc_types", "secrecy", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "serde-big-array", "serde_derive 1.0.118", "sgx_tcrypto", @@ -457,7 +457,7 @@ dependencies = [ "ring", "rtc_types", "secrecy", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde 1.0.118", "serde_json 1.0.60", "sgx_tcrypto", "sgx_tse", @@ -493,14 +493,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "serde" -version = "1.0.118" -source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" -dependencies = [ - "sgx_tstd", -] - [[package]] name = "serde" version = "1.0.118" @@ -524,7 +516,7 @@ name = "serde-big-array" version = "0.3.0" source = "git+https://github.com/mesalock-linux/serde-big-array-sgx#94122c5167aee38b39b09a620a60db2c28cf7428" dependencies = [ - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "serde_derive 1.0.118", ] @@ -556,7 +548,7 @@ source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380 dependencies = [ "itoa 0.4.5", "ryu", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "sgx_tstd", ] @@ -597,7 +589,7 @@ version = "1.1.3" source = "git+https://github.com/apache/teaclave-sgx-sdk.git?rev=v1.1.3#a6a172e652b4db4eaa17e4faa078fda8922abdd0" dependencies = [ "itertools", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "serde-big-array", "serde_derive 1.0.118", "sgx_tcrypto", diff --git a/rtc_data_enclave/Cargo.toml b/rtc_data_enclave/Cargo.toml index 6ece2fb4..3c1da1e3 100644 --- a/rtc_data_enclave/Cargo.toml +++ b/rtc_data_enclave/Cargo.toml @@ -29,7 +29,7 @@ rtc_types = { path = "../rtc_types", features = ["teaclave_sgx"]} rtc_tenclave = { path = "../rtc_tenclave" } [dependencies] -# The build fails when specifying rev here, I don't know what is causing it. +# XXX: See comment about serde-sgx dependency versioning in rtc_tenclave/Cargo.toml serde_derive = { git = "https://github.com/mesalock-linux/serde-sgx" } serde = { git = "https://github.com/mesalock-linux/serde-sgx", features = ["derive"]} bincode = { git = "https://github.com/mesalock-linux/bincode-sgx.git" } diff --git a/rtc_tenclave/Cargo.lock b/rtc_tenclave/Cargo.lock index 72d13d7a..97e5cc2c 100644 --- a/rtc_tenclave/Cargo.lock +++ b/rtc_tenclave/Cargo.lock @@ -256,7 +256,7 @@ dependencies = [ "ring", "rtc_types", "secrecy", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3)", + "serde 1.0.118", "serde 1.0.126", "serde_json 1.0.60", "serde_json 1.0.64", @@ -296,14 +296,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "serde" -version = "1.0.118" -source = "git+https://github.com/mesalock-linux/serde-sgx?tag=sgx_1.1.3#db0226f1d5d70fca6b96af2c285851502204e21c" -dependencies = [ - "sgx_tstd", -] - [[package]] name = "serde" version = "1.0.118" @@ -325,7 +317,7 @@ source = "git+https://github.com/mesalock-linux/serde-json-sgx?tag=sgx_1.1.3#380 dependencies = [ "itoa 0.4.5", "ryu", - "serde 1.0.118 (git+https://github.com/mesalock-linux/serde-sgx)", + "serde 1.0.118", "sgx_tstd", ] diff --git a/rtc_tenclave/Cargo.toml b/rtc_tenclave/Cargo.toml index 235aa683..693695c7 100644 --- a/rtc_tenclave/Cargo.toml +++ b/rtc_tenclave/Cargo.toml @@ -22,7 +22,32 @@ sgx_tse = { git = "https://github.com/apache/teaclave-sgx-sdk.git" , rev = "v1.1 rand = { git = "https://github.com/mesalock-linux/rand-sgx", tag = "v0.7.3_sgx1.1.3", optional = true } thiserror = { git = "https://github.com/mesalock-linux/thiserror-sgx.git", optional = true } sgx_tcrypto = { git = "https://github.com/apache/teaclave-sgx-sdk.git", optional = true } -serde = { git = "https://github.com/mesalock-linux/serde-sgx", tag = "sgx_1.1.3", optional = true } + +# XXX: Work around serde-json-sgx / Cargo version handling issue +# +# We want to specify tag = "sgx_1.1.3" for the serde dependency here, +# but this makes Cargo resolve the dependency to a different crate +# version identity than what serde-json-sgx's dependency resolves to, +# even though both "master" and "sgx_1.1.3" point to same git revision. +# +# This causes build failures due to serde_json referring to a different +# versions of the Serialize / Deserialize traits than this project's code. +# +# We cannot use [patch] to override serde-json-sgx's dependency to use +# the same tag for serde-sgx as here, because of this Cargo limitation: +# +# * "Cannot patch underspecified git dependency" +# https://github.com/rust-lang/cargo/issues/7670 +# +# Comment: https://github.com/rust-lang/cargo/issues/7670#issuecomment-841722488 +# +# To work around this problem, the git reference here must match +# the upstream dependency line exactly: +# +# * https://github.com/mesalock-linux/serde-json-sgx/blob/sgx_1.1.3/Cargo.toml#L17 +# +serde = { git = "https://github.com/mesalock-linux/serde-sgx", optional = true } + serde_json = { git = "https://github.com/mesalock-linux/serde-json-sgx", tag = "sgx_1.1.3", optional = true } rtc_types = { path = "../rtc_types" } From 063f43e7e372a31f476856a6f060ec009c3b701b Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Sat, 15 May 2021 12:20:44 +0200 Subject: [PATCH 03/29] deps(rtc_tenclave): add proptest for dev --- rtc_tenclave/Cargo.lock | 210 +++++++++++++++++++++++++++++++++++++++- rtc_tenclave/Cargo.toml | 2 + 2 files changed, 210 insertions(+), 2 deletions(-) diff --git a/rtc_tenclave/Cargo.lock b/rtc_tenclave/Cargo.lock index 97e5cc2c..5c406c9e 100644 --- a/rtc_tenclave/Cargo.lock +++ b/rtc_tenclave/Cargo.lock @@ -1,11 +1,44 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +[[package]] +name = "autocfg" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdb031dd78e28731d87d56cc8ffef4a8f36ca26c38fe2de700543e627f8a464a" + +[[package]] +name = "bit-set" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e11e16035ea35e4e5997b393eacbf6f63983188f7a2ad25bfb13465f5ad59de" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "349f9b6a179ed607305526ca489b34ad0a41aed5f7980fa90eb03160b69598fb" + +[[package]] +name = "bitflags" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" + [[package]] name = "bumpalo" version = "3.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63396b8a4b9de3f4fdfb320ab6080762242f66a8ef174c49d8e19b674db4cdbe" +[[package]] +name = "byteorder" +version = "1.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" + [[package]] name = "cc" version = "1.0.67" @@ -24,6 +57,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "getrandom" version = "0.1.14" @@ -43,7 +82,18 @@ checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" dependencies = [ "cfg-if 1.0.0", "libc", - "wasi", + "wasi 0.9.0+wasi-snapshot-preview1", +] + +[[package]] +name = "getrandom" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c9495705279e7140bf035dde1f6e750c162df8b625267cd52cc44e0b156732c8" +dependencies = [ + "cfg-if 1.0.0", + "libc", + "wasi 0.10.2+wasi-snapshot-preview1", ] [[package]] @@ -101,6 +151,15 @@ dependencies = [ "cfg-if 1.0.0", ] +[[package]] +name = "num-traits" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a64b1ec5cda2586e284722486d802acf1f7dbdc623e2bfc57e65ca1cd099290" +dependencies = [ + "autocfg", +] + [[package]] name = "once_cell" version = "1.7.2" @@ -127,6 +186,38 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "proptest" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e0d9cc07f18492d879586c92b485def06bc850da3118075cd45d50e9c95b0e5" +dependencies = [ + "bit-set", + "bitflags", + "byteorder", + "lazy_static", + "num-traits", + "quick-error 2.0.1", + "rand 0.8.3", + "rand_chacha 0.3.0", + "rand_xorshift", + "regex-syntax", + "rusty-fork", + "tempfile", +] + +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + +[[package]] +name = "quick-error" +version = "2.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" + [[package]] name = "quote" version = "1.0.9" @@ -146,7 +237,7 @@ dependencies = [ "libc", "rand_chacha 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", - "rand_hc", + "rand_hc 0.2.0", ] [[package]] @@ -160,6 +251,18 @@ dependencies = [ "sgx_tstd", ] +[[package]] +name = "rand" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ef9e7e66b4468674bfcb0c81af8b7fa0bb154fa9f28eb840da5c447baeb8d7e" +dependencies = [ + "libc", + "rand_chacha 0.3.0", + "rand_core 0.6.2", + "rand_hc 0.3.0", +] + [[package]] name = "rand_chacha" version = "0.2.2" @@ -180,6 +283,16 @@ dependencies = [ "sgx_tstd", ] +[[package]] +name = "rand_chacha" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e12735cf05c9e10bf21534da50a147b924d555dc7a547c42e6bb2d5b6017ae0d" +dependencies = [ + "ppv-lite86 0.2.10", + "rand_core 0.6.2", +] + [[package]] name = "rand_core" version = "0.3.1" @@ -213,6 +326,15 @@ dependencies = [ "sgx_tstd", ] +[[package]] +name = "rand_core" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34cf66eb183df1c5876e2dcf6b13d57340741e8dc255b48e40a26de954d06ae7" +dependencies = [ + "getrandom 0.2.2", +] + [[package]] name = "rand_hc" version = "0.2.0" @@ -222,6 +344,24 @@ dependencies = [ "rand_core 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rand_hc" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3190ef7066a446f2e7f42e239d161e905420ccab01eb967c9eb27d21b2322a73" +dependencies = [ + "rand_core 0.6.2", +] + +[[package]] +name = "rand_xorshift" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d25bf25ec5ae4a3f1b92f929810509a2f53d7dca2f50b794ff57e3face536c8f" +dependencies = [ + "rand_core 0.6.2", +] + [[package]] name = "rdrand" version = "0.6.0" @@ -231,6 +371,30 @@ dependencies = [ "rand_core 0.4.2", ] +[[package]] +name = "redox_syscall" +version = "0.2.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "742739e41cd49414de871ea5e549afb7e2a3ac77b589bcbebe8c82fab37147fc" +dependencies = [ + "bitflags", +] + +[[package]] +name = "regex-syntax" +version = "0.6.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f497285884f3fcff424ffc933e56d7cbca511def0c9831a7f9b5f6153e3cc89b" + +[[package]] +name = "remove_dir_all" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" +dependencies = [ + "winapi", +] + [[package]] name = "ring" version = "0.17.0-alpha.10" @@ -251,6 +415,7 @@ name = "rtc_tenclave" version = "0.1.0" dependencies = [ "cfg-if 1.0.0", + "proptest", "rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.3 (git+https://github.com/mesalock-linux/rand-sgx?tag=v0.7.3_sgx1.1.3)", "ring", @@ -281,6 +446,18 @@ dependencies = [ "thiserror 1.0.9", ] +[[package]] +name = "rusty-fork" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb3dcc6e454c328bb824492db107ab7c0ae8fcffe4ad210136ef014458c1bc4f" +dependencies = [ + "fnv", + "quick-error 1.2.3", + "tempfile", + "wait-timeout", +] + [[package]] name = "ryu" version = "1.0.5" @@ -465,6 +642,20 @@ dependencies = [ "unicode-xid", ] +[[package]] +name = "tempfile" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22" +dependencies = [ + "cfg-if 1.0.0", + "libc", + "rand 0.8.3", + "redox_syscall", + "remove_dir_all", + "winapi", +] + [[package]] name = "thiserror" version = "1.0.9" @@ -516,12 +707,27 @@ version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a156c684c91ea7d62626509bce3cb4e1d9ed5c4d978f7b4352658f96a4c26b4a" +[[package]] +name = "wait-timeout" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f200f5b12eb75f8c1ed65abd4b2db8a6e1b138a20de009dacee265a2498f3f6" +dependencies = [ + "libc", +] + [[package]] name = "wasi" version = "0.9.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" +[[package]] +name = "wasi" +version = "0.10.2+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" + [[package]] name = "wasm-bindgen" version = "0.2.73" diff --git a/rtc_tenclave/Cargo.toml b/rtc_tenclave/Cargo.toml index 693695c7..d29b653b 100644 --- a/rtc_tenclave/Cargo.toml +++ b/rtc_tenclave/Cargo.toml @@ -68,6 +68,8 @@ sgx_ucrypto = { git = "https://github.com/apache/teaclave-sgx-sdk.git" } serde_std = { package = "serde", version = "1.0.0" } serde_json_std = { package = "serde_json", version = "1.0.0" } +proptest = "1.0.0" + [patch."https://github.com/apache/teaclave-sgx-sdk.git"] sgx_types = { git = "https://github.com/apache/incubator-teaclave-sgx-sdk.git" } From 9aa846ed49eb8db199a2a7fbc0fff9b33c1b2ede Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Sat, 15 May 2021 15:41:32 +0200 Subject: [PATCH 04/29] feat(rtc_tenclave,kv_store): add key-value store module This contains in-memory implementations and a property test, so far. --- rtc_tenclave/src/kv_store/in_memory.rs | 51 ++++++++++++++++++++++++++ rtc_tenclave/src/kv_store/inspect.rs | 39 ++++++++++++++++++++ rtc_tenclave/src/kv_store/mod.rs | 37 +++++++++++++++++++ rtc_tenclave/src/kv_store/tests.rs | 46 +++++++++++++++++++++++ rtc_tenclave/src/lib.rs | 1 + 5 files changed, 174 insertions(+) create mode 100644 rtc_tenclave/src/kv_store/in_memory.rs create mode 100644 rtc_tenclave/src/kv_store/inspect.rs create mode 100644 rtc_tenclave/src/kv_store/mod.rs create mode 100644 rtc_tenclave/src/kv_store/tests.rs diff --git a/rtc_tenclave/src/kv_store/in_memory.rs b/rtc_tenclave/src/kv_store/in_memory.rs new file mode 100644 index 00000000..3a945139 --- /dev/null +++ b/rtc_tenclave/src/kv_store/in_memory.rs @@ -0,0 +1,51 @@ +//! In-memory implementations of [`KvStore`] (for testing) + +use serde::de::DeserializeOwned; +use serde::Serialize; +use std::collections::HashMap; +use std::prelude::v1::*; + +use super::{KvStore, StoreResult}; + +/// In-memory [`KvStore`] using [`HashMap`] +#[derive(Default)] +pub struct InMemoryStore { + pub(crate) map: HashMap, +} + +impl KvStore for InMemoryStore +where + V: Clone, +{ + fn load(&self, key: &str) -> StoreResult> { + Ok(self.map.get(key).cloned()) + } + + fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + self.map.insert(key.to_string(), value); + Ok(()) + } +} + +/// In-memory [`KvStore`] using [`HashMap`] and [`serde_json`] serialization +#[derive(Default)] +pub struct InMemoryJsonStore { + pub(crate) map: HashMap>, +} + +impl KvStore for InMemoryJsonStore +where + V: Serialize + DeserializeOwned, +{ + fn load(&self, key: &str) -> StoreResult> { + let loaded: Option<&[u8]> = self.map.get(key).map(|v| v.as_slice()); + let deserialized: Option = loaded.map(serde_json::from_slice).transpose()?; + Ok(deserialized) + } + + fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + let serialized = serde_json::to_vec(&value)?; + self.map.insert(key.to_string(), serialized); + Ok(()) + } +} diff --git a/rtc_tenclave/src/kv_store/inspect.rs b/rtc_tenclave/src/kv_store/inspect.rs new file mode 100644 index 00000000..5e7fc4ae --- /dev/null +++ b/rtc_tenclave/src/kv_store/inspect.rs @@ -0,0 +1,39 @@ +//! Support for inspecting [`KvStore`] instances (for testing and debugging) + +use serde::de::DeserializeOwned; +use serde::Serialize; +use std::borrow::ToOwned; +use std::collections::HashMap; +use std::iter::Iterator; + +use super::in_memory::{InMemoryJsonStore, InMemoryStore}; +use super::KvStore; + +pub trait InspectStore { + fn as_map(&self) -> HashMap; +} + +impl InspectStore for InMemoryStore +where + V: Clone, +{ + fn as_map(&self) -> HashMap { + self.map.clone() + } +} + +impl InspectStore for InMemoryJsonStore +where + V: Serialize + DeserializeOwned, +{ + fn as_map(&self) -> HashMap { + self.map + .keys() + .map(|k| { + let loaded: Option = self.load(&k).expect(&format!("load {:?} failed!", k)); + let v: V = loaded.expect(&format!("key missing! {:?}", k)); + (k.to_owned(), v) + }) + .collect() + } +} diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs new file mode 100644 index 00000000..7b9e32d2 --- /dev/null +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -0,0 +1,37 @@ +//! Simple key-value store abstraction + +use std::boxed::Box; +use std::error::Error; + +type StoreResult = Result>; + +/// A key-value store. +/// +/// These methods borrow key references, and +/// +pub trait KvStore { + // TODO: Use associated type for V? + + /// Load the saved value for `key`, if any. + /// + /// Return [`None`] if `key` has no previous value. + /// + fn load(&self, key: &str) -> StoreResult>; + + /// Save a new value for `key`. + /// + /// This will replace any existing value. + /// + fn save(&mut self, key: &str, value: V) -> StoreResult<()>; + + // TODO: add update() +} + +#[cfg(test)] +mod in_memory; + +#[cfg(test)] +mod inspect; + +#[cfg(test)] +mod tests; diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs new file mode 100644 index 00000000..ada68f76 --- /dev/null +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -0,0 +1,46 @@ +//! Tests for [`rtc_tenclave::kv_store`] + +use std::collections::HashMap; + +use proptest::prelude::*; + +use super::in_memory::{InMemoryJsonStore, InMemoryStore}; +use super::inspect::InspectStore; +use super::KvStore; + +/// Verify that executing a sequence of store operations matches a simple model. +#[test] +fn prop_store_ops_match_model() { + // Strategy to generate store operations: currently, just key / value pairs to save. + let store_ops_strategy = { + let keys = prop_oneof!(r"[a-z]{0,5}", ".*"); + let values = prop_oneof!(keys.clone()); // TODO: Non-string values + + // This strategy will generate key / value pairs with multiple values per key, + // and shuffle them to have some interleaving, for bugs that depend on that. + proptest::collection::hash_map(keys, proptest::collection::vec(values, 0..10), 0..10) + .prop_map(flatten_key_values) + .prop_shuffle() + }; + + proptest!(|(store_ops_vec in store_ops_strategy)| { + type V = String; + let mut store_model: InMemoryStore = InMemoryStore::default(); + let mut store_model_json: InMemoryJsonStore = InMemoryJsonStore::default(); + // TODO: FS-based store + + for (k, v) in store_ops_vec { + store_model.save(&k, v.clone()).expect("InMemoryStore save failed!"); + store_model_json.save(&k, v.clone()).expect("InMemoryJsonStore save failed!"); + + prop_assert_eq!(store_model.as_map(), store_model_json.as_map()); + } + }); +} + +/// Helper: Flatten `{K => [V, …], …}` to `[(K, V), …]`, cloning `K` for each `V`. +fn flatten_key_values(kvs: HashMap>) -> Vec<(K, V)> { + kvs.into_iter() + .flat_map(|(k, vs)| vs.into_iter().map(move |v| (k.clone(), v))) + .collect() +} diff --git a/rtc_tenclave/src/lib.rs b/rtc_tenclave/src/lib.rs index 2f16ad01..4cfb33f7 100644 --- a/rtc_tenclave/src/lib.rs +++ b/rtc_tenclave/src/lib.rs @@ -23,4 +23,5 @@ cfg_if::cfg_if! { pub mod crypto; pub mod enclave; +pub mod kv_store; pub mod util; From e8b785fb1e29b0e7498b9849e56b5ed178d06103 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Mon, 17 May 2021 10:44:37 +0200 Subject: [PATCH 05/29] deps(rtc_tenclave): add hex (no_std) --- rtc_auth_enclave/Cargo.lock | 7 +++++++ rtc_data_enclave/Cargo.lock | 7 +++++++ rtc_tenclave/Cargo.lock | 7 +++++++ rtc_tenclave/Cargo.toml | 1 + 4 files changed, 22 insertions(+) diff --git a/rtc_auth_enclave/Cargo.lock b/rtc_auth_enclave/Cargo.lock index fd33c9f3..b5fb647e 100644 --- a/rtc_auth_enclave/Cargo.lock +++ b/rtc_auth_enclave/Cargo.lock @@ -141,6 +141,12 @@ dependencies = [ "libc", ] +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "index-fixed" version = "0.3.1" @@ -356,6 +362,7 @@ name = "rtc_tenclave" version = "0.1.0" dependencies = [ "cfg-if 1.0.0", + "hex", "rand 0.7.3", "ring", "rtc_types", diff --git a/rtc_data_enclave/Cargo.lock b/rtc_data_enclave/Cargo.lock index c4ecc698..3c86c441 100644 --- a/rtc_data_enclave/Cargo.lock +++ b/rtc_data_enclave/Cargo.lock @@ -181,6 +181,12 @@ dependencies = [ "libc", ] +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "index-fixed" version = "0.3.1" @@ -453,6 +459,7 @@ name = "rtc_tenclave" version = "0.1.0" dependencies = [ "cfg-if 1.0.0", + "hex", "rand 0.7.3", "ring", "rtc_types", diff --git a/rtc_tenclave/Cargo.lock b/rtc_tenclave/Cargo.lock index 5c406c9e..e5c2ef25 100644 --- a/rtc_tenclave/Cargo.lock +++ b/rtc_tenclave/Cargo.lock @@ -101,6 +101,12 @@ name = "hashbrown_tstd" version = "0.9.0" source = "git+https://github.com/apache/teaclave-sgx-sdk.git?rev=v1.1.3#a6a172e652b4db4eaa17e4faa078fda8922abdd0" +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "index-fixed" version = "0.3.1" @@ -415,6 +421,7 @@ name = "rtc_tenclave" version = "0.1.0" dependencies = [ "cfg-if 1.0.0", + "hex", "proptest", "rand 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.3 (git+https://github.com/mesalock-linux/rand-sgx?tag=v0.7.3_sgx1.1.3)", diff --git a/rtc_tenclave/Cargo.toml b/rtc_tenclave/Cargo.toml index d29b653b..76d8dcd8 100644 --- a/rtc_tenclave/Cargo.toml +++ b/rtc_tenclave/Cargo.toml @@ -59,6 +59,7 @@ secrecy = { version = "0.7.0", default-features = false } ring = { version = "0.17.0-alpha.8", default-features = false } sodalite = { version = "0.4.0", default-features = false } cfg-if = "1.0.0" +hex = { version = "0.4.3", default-features = false, features = ["alloc"] } [dev-dependencies] From 23486e1f5355487b49d92f7bb9903366f79ed91f Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Mon, 17 May 2021 13:01:56 +0200 Subject: [PATCH 06/29] feat(rtc_tenclave): add filesystem-based FsStore and SgxFsStore --- rtc_tenclave/src/kv_store/fs.rs | 114 +++++++++++++++++++++++++++ rtc_tenclave/src/kv_store/inspect.rs | 55 +++++++++++-- rtc_tenclave/src/kv_store/mod.rs | 6 +- rtc_tenclave/src/kv_store/sgxfs.rs | 102 ++++++++++++++++++++++++ rtc_tenclave/src/kv_store/tests.rs | 30 ++++++- rtc_tenclave/src/lib.rs | 1 + 6 files changed, 299 insertions(+), 9 deletions(-) create mode 100644 rtc_tenclave/src/kv_store/fs.rs create mode 100644 rtc_tenclave/src/kv_store/sgxfs.rs diff --git a/rtc_tenclave/src/kv_store/fs.rs b/rtc_tenclave/src/kv_store/fs.rs new file mode 100644 index 00000000..f80194a6 --- /dev/null +++ b/rtc_tenclave/src/kv_store/fs.rs @@ -0,0 +1,114 @@ +//! [`KvStore`] implementation based on [`fs`] + +#[cfg(not(test))] +use std::prelude::v1::*; + +use std::io; +use std::io::prelude::*; +use std::path::{Path, PathBuf}; + +use serde::de::DeserializeOwned; +use serde::Serialize; + +#[cfg(not(test))] +use std::untrusted::{fs, fs::File}; +#[cfg(test)] +use std::{fs, fs::File}; + +use super::{KvStore, StoreResult}; + +/// Filesystem-based [`KvStore`] +pub struct FsStore { + pub(crate) root_dir: PathBuf, +} + +impl FsStore { + /// Validate that `root_dir` exists as a directory. + #[cfg_attr(not(test), allow(dead_code))] + pub fn new(root: impl AsRef) -> StoreResult { + let root = root.as_ref(); + + fs::create_dir_all(root) + .map_err(|err| format!("FsStore: create_dir_all({:?}) failed: {:?}", root, err))?; + + Ok(FsStore { + root_dir: root.to_path_buf(), + }) + } + + /// Resolve file name for the value of `key`. + fn value_path(&self, key: &str) -> PathBuf { + // XXX: Escaping / encoding? + let file_name = Self::encode_key(key); + self.root_dir.join(file_name) + } + + pub(crate) fn encode_key(key: &str) -> String { + let encoded = hex::encode(key); + format!("x{}", encoded) + } + + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn decode_key(file_name: &str) -> StoreResult { + let encoded: &str = file_name + .strip_prefix("x") + .ok_or_else(|| format!("FsStore::decode_key: missing x prefix for {:?}", file_name))?; + // FIXME: Dodgy err.to_string() + let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; + String::from_utf8(bytes).map_err(|err| err.into()) + } +} + +impl KvStore for FsStore +where + V: Serialize + DeserializeOwned, +{ + fn load(&self, key: &str) -> StoreResult> { + let value_file_name = self.value_path(key); + + // TODO: Handle NotFound + let value_file = File::open(&value_file_name) + .map_err(|err| format!("FsStore: open {:?} failed: {}", value_file_name, err))?; + + // Note: Read all the data into memory first, then deserialize, for efficiency. + // See the docs for [`serde_json::de::from_reader`], + // and https://github.com/serde-rs/json/issues/160 + let serialised: Vec = read_all(value_file) + .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; + + let deserialized: V = serde_json::from_slice(serialised.as_slice())?; + Ok(Some(deserialized)) + } + + fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + let serialized: Vec = serde_json::to_vec(&value)?; + + let value_file_name = self.value_path(key); + + let mut value_file = File::create(&value_file_name) + .map_err(|err| format!("open {:?} failed: {}", value_file_name, err))?; + + value_file.write_all(serialized.as_slice()).map_err(|err| { + format!( + "FsStore: write_all to {:?} failed: {}", + value_file_name, err + ) + })?; + Ok(()) + } +} + +/// Helper: Like [`fs::read`], but take an open file. +fn read_all(mut file: File) -> io::Result> { + let mut bytes = Vec::with_capacity(initial_buffer_size(&file)); + file.read_to_end(&mut bytes)?; + Ok(bytes) +} + +/// Indicates how large a buffer to pre-allocate before reading the entire file. +fn initial_buffer_size(file: &File) -> usize { + // Allocate one extra byte so the buffer doesn't need to grow before the + // final `read` call at the end of the file. Don't worry about `usize` + // overflow because reading will fail regardless in that case. + file.metadata().map(|m| m.len() as usize + 1).unwrap_or(0) +} diff --git a/rtc_tenclave/src/kv_store/inspect.rs b/rtc_tenclave/src/kv_store/inspect.rs index 5e7fc4ae..8f3b5310 100644 --- a/rtc_tenclave/src/kv_store/inspect.rs +++ b/rtc_tenclave/src/kv_store/inspect.rs @@ -1,10 +1,13 @@ //! Support for inspecting [`KvStore`] instances (for testing and debugging) -use serde::de::DeserializeOwned; -use serde::Serialize; +#[cfg(not(test))] +use std::prelude::v1::*; + use std::borrow::ToOwned; use std::collections::HashMap; -use std::iter::Iterator; + +use serde::de::DeserializeOwned; +use serde::Serialize; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::KvStore; @@ -30,10 +33,52 @@ where self.map .keys() .map(|k| { - let loaded: Option = self.load(&k).expect(&format!("load {:?} failed!", k)); - let v: V = loaded.expect(&format!("key missing! {:?}", k)); + let loaded: Option = self + .load(k) + .unwrap_or_else(|_| panic!("load {:?} failed!", k)); + let v: V = loaded.unwrap_or_else(|| panic!("key missing! {:?}", k)); (k.to_owned(), v) }) .collect() } } + +// sgx_tstd (v1.1.3) does not support `fs::read_dir`, so limit the following to tests, for now. +// +// See: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/v1.1.3/release_notes.md#partially-supported-modstraits-in-sgx_tstd + +#[cfg(test)] +use std::{ffi::OsStr, fs::DirEntry, io, iter::Iterator}; + +#[cfg(test)] +use super::fs::FsStore; + +#[cfg(test)] +impl InspectStore for FsStore +where + V: Serialize + DeserializeOwned, +{ + fn as_map(&self) -> HashMap { + let entries: impl Iterator> = self + .root_dir + .read_dir() + .expect(&format!("read_dir {:?} failed", self.root_dir)); + + let keys: impl Iterator = entries.map(|entry: io::Result| { + let entry: DirEntry = entry.expect("read_dir entry failed"); + let file_path = entry.path(); + let os_file_name: &OsStr = file_path + .file_name() + .expect(&format!("directory entry lacks file_name: {:?}", file_path)); + let file_name: &str = os_file_name.to_str().expect("OsStr.to_str failed"); + FsStore::decode_key(file_name).expect("FsStore::decode_key failed") + }); + + keys.map(|k| { + let loaded: Option = self.load(&k).expect(&format!("load {:?} failed!", k)); + let v: V = loaded.expect(&format!("key missing! {:?}", k)); + (k, v) + }) + .collect() + } +} diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 7b9e32d2..d7c378d9 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -27,11 +27,11 @@ pub trait KvStore { // TODO: add update() } -#[cfg(test)] +mod fs; mod in_memory; - -#[cfg(test)] mod inspect; +#[cfg(not(test))] +mod sgxfs; #[cfg(test)] mod tests; diff --git a/rtc_tenclave/src/kv_store/sgxfs.rs b/rtc_tenclave/src/kv_store/sgxfs.rs new file mode 100644 index 00000000..7d735a97 --- /dev/null +++ b/rtc_tenclave/src/kv_store/sgxfs.rs @@ -0,0 +1,102 @@ +//! [`KvStore`] implementation based on [`sgx_tstd::sgxfs`] (using the Intel SGX Protected FS Library) + +use std::prelude::v1::*; + +use std::io; +use std::io::prelude::*; +use std::path::{Path, PathBuf}; + +use serde::de::DeserializeOwned; +use serde::Serialize; +use sgx_tstd::sgxfs::SgxFile; + +use super::{KvStore, StoreResult}; + +/// Filesystem-based [`KvStore`], using [`SgxFile`] +/// +/// TODO: Document security guarantees. +/// +struct SgxFsStore { + pub(crate) root_dir: PathBuf, +} + +impl SgxFsStore { + /// Validate that `root_dir` exists as a directory. + pub fn new(root: impl AsRef) -> StoreResult { + let root = root.as_ref(); + + // XXX: no create_dir_all() + + Ok(SgxFsStore { + root_dir: root.to_path_buf(), + }) + } + + /// Resolve file name for the value of `key`. + fn value_path(&self, key: &str) -> PathBuf { + // XXX: Escaping / encoding? + let file_name = Self::encode_key(key); + self.root_dir.join(file_name) + } + + pub(crate) fn encode_key(key: &str) -> String { + let encoded = hex::encode(key); + format!("x{}", encoded) + } + pub(crate) fn decode_key(file_name: &str) -> StoreResult { + let encoded: &str = file_name + .strip_prefix("x") + .ok_or_else(|| format!("FsStore::decode_key: missing x prefix for {:?}", file_name))?; + // FIXME: Dodgy err.to_string() + let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; + String::from_utf8(bytes).map_err(|err| err.into()) + } +} + +impl KvStore for SgxFsStore +where + V: Serialize + DeserializeOwned, +{ + fn load(&self, key: &str) -> StoreResult> { + let value_file_name = self.value_path(key); + + // TODO: Handle NotFound + // TODO: open_ex() with key + let value_file = SgxFile::open(&value_file_name) + .map_err(|err| format!("FsStore: open {:?} failed: {}", value_file_name, err))?; + + // Note: Read all the data into memory first, then deserialize, for efficiency. + // See the docs for [`serde_json::de::from_reader`], + // and https://github.com/serde-rs/json/issues/160 + let serialised: Vec = read_all(value_file) + .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; + + let deserialized: V = serde_json::from_slice(serialised.as_slice())?; + Ok(Some(deserialized)) + } + + fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + let serialized: Vec = serde_json::to_vec(&value)?; + + let value_file_name = self.value_path(key); + + let mut value_file = SgxFile::create(&value_file_name) + .map_err(|err| format!("open {:?} failed: {}", value_file_name, err))?; + + value_file.write_all(serialized.as_slice()).map_err(|err| { + format!( + "FsStore: write_all to {:?} failed: {}", + value_file_name, err + ) + })?; + Ok(()) + } +} + +/// Helper: Like [`fs::read`], but take an open file. +fn read_all(mut file: SgxFile) -> io::Result> { + // XXX: No metadata for initial_buffer_size in sgxfs + let mut bytes = Vec::new(); + file.read_to_end(&mut bytes)?; + Ok(bytes) +} diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index ada68f76..ec4e2fbb 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -1,9 +1,15 @@ //! Tests for [`rtc_tenclave::kv_store`] +#[cfg(not(test))] +use std::prelude::v1::*; + use std::collections::HashMap; +use std::fs::remove_dir_all; +use std::path::Path; use proptest::prelude::*; +use super::fs::FsStore; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::inspect::InspectStore; use super::KvStore; @@ -23,18 +29,40 @@ fn prop_store_ops_match_model() { .prop_shuffle() }; + // XXX: hacky clearing + pub fn clear_dir(path: &Path) { + if path.is_dir() { + remove_dir_all(path).expect("remove_dir_all failed"); + }; + } + proptest!(|(store_ops_vec in store_ops_strategy)| { + // FIXME: This value type parameter needs better handling. type V = String; + + // Init the models let mut store_model: InMemoryStore = InMemoryStore::default(); let mut store_model_json: InMemoryJsonStore = InMemoryJsonStore::default(); - // TODO: FS-based store + + // Init the store under test + let path = Path::new("store_test"); + clear_dir(path); // Clear before each test + let mut store_fs: FsStore = FsStore::new(path).expect("FsStore::new failed"); for (k, v) in store_ops_vec { store_model.save(&k, v.clone()).expect("InMemoryStore save failed!"); store_model_json.save(&k, v.clone()).expect("InMemoryJsonStore save failed!"); + store_fs.save(&k, v.clone()).expect("FsStore save failed!"); + // Models match each other prop_assert_eq!(store_model.as_map(), store_model_json.as_map()); + // Models match store_fs + prop_assert_eq!(store_model.as_map(), store_fs.as_map()); + // FIXME: explicit coercion for as_map() + prop_assert_eq!(store_model_json.as_map() as HashMap, store_fs.as_map()); } + + clear_dir(path); // Clear after successful tests, just to keep the workdir clean }); } diff --git a/rtc_tenclave/src/lib.rs b/rtc_tenclave/src/lib.rs index 4cfb33f7..9c4dddcd 100644 --- a/rtc_tenclave/src/lib.rs +++ b/rtc_tenclave/src/lib.rs @@ -6,6 +6,7 @@ #![feature(const_evaluatable_checked)] #![deny(clippy::mem_forget)] #![cfg_attr(not(test), no_std)] +#![feature(impl_trait_in_bindings)] #[cfg(not(test))] #[macro_use] From b3eea8ebc7b65d00f220ab0c923273b581b19f42 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 02:36:34 +0200 Subject: [PATCH 07/29] refactor(rtc_tenclave,kv_store): rework FsStore, extract Filer trait This should be a lot simpler to work with. --- rtc_tenclave/src/kv_store/fs.rs | 114 ------------------ rtc_tenclave/src/kv_store/fs/inspect.rs | 48 ++++++++ .../src/kv_store/{sgxfs.rs => fs/mod.rs} | 94 +++++++-------- rtc_tenclave/src/kv_store/fs/sgx_filer.rs | 43 +++++++ rtc_tenclave/src/kv_store/fs/std_filer.rs | 30 +++++ rtc_tenclave/src/kv_store/inspect.rs | 40 ------ rtc_tenclave/src/kv_store/mod.rs | 2 - rtc_tenclave/src/kv_store/tests.rs | 6 +- 8 files changed, 172 insertions(+), 205 deletions(-) delete mode 100644 rtc_tenclave/src/kv_store/fs.rs create mode 100644 rtc_tenclave/src/kv_store/fs/inspect.rs rename rtc_tenclave/src/kv_store/{sgxfs.rs => fs/mod.rs} (51%) create mode 100644 rtc_tenclave/src/kv_store/fs/sgx_filer.rs create mode 100644 rtc_tenclave/src/kv_store/fs/std_filer.rs diff --git a/rtc_tenclave/src/kv_store/fs.rs b/rtc_tenclave/src/kv_store/fs.rs deleted file mode 100644 index f80194a6..00000000 --- a/rtc_tenclave/src/kv_store/fs.rs +++ /dev/null @@ -1,114 +0,0 @@ -//! [`KvStore`] implementation based on [`fs`] - -#[cfg(not(test))] -use std::prelude::v1::*; - -use std::io; -use std::io::prelude::*; -use std::path::{Path, PathBuf}; - -use serde::de::DeserializeOwned; -use serde::Serialize; - -#[cfg(not(test))] -use std::untrusted::{fs, fs::File}; -#[cfg(test)] -use std::{fs, fs::File}; - -use super::{KvStore, StoreResult}; - -/// Filesystem-based [`KvStore`] -pub struct FsStore { - pub(crate) root_dir: PathBuf, -} - -impl FsStore { - /// Validate that `root_dir` exists as a directory. - #[cfg_attr(not(test), allow(dead_code))] - pub fn new(root: impl AsRef) -> StoreResult { - let root = root.as_ref(); - - fs::create_dir_all(root) - .map_err(|err| format!("FsStore: create_dir_all({:?}) failed: {:?}", root, err))?; - - Ok(FsStore { - root_dir: root.to_path_buf(), - }) - } - - /// Resolve file name for the value of `key`. - fn value_path(&self, key: &str) -> PathBuf { - // XXX: Escaping / encoding? - let file_name = Self::encode_key(key); - self.root_dir.join(file_name) - } - - pub(crate) fn encode_key(key: &str) -> String { - let encoded = hex::encode(key); - format!("x{}", encoded) - } - - #[cfg_attr(not(test), allow(dead_code))] - pub(crate) fn decode_key(file_name: &str) -> StoreResult { - let encoded: &str = file_name - .strip_prefix("x") - .ok_or_else(|| format!("FsStore::decode_key: missing x prefix for {:?}", file_name))?; - // FIXME: Dodgy err.to_string() - let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; - String::from_utf8(bytes).map_err(|err| err.into()) - } -} - -impl KvStore for FsStore -where - V: Serialize + DeserializeOwned, -{ - fn load(&self, key: &str) -> StoreResult> { - let value_file_name = self.value_path(key); - - // TODO: Handle NotFound - let value_file = File::open(&value_file_name) - .map_err(|err| format!("FsStore: open {:?} failed: {}", value_file_name, err))?; - - // Note: Read all the data into memory first, then deserialize, for efficiency. - // See the docs for [`serde_json::de::from_reader`], - // and https://github.com/serde-rs/json/issues/160 - let serialised: Vec = read_all(value_file) - .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; - - let deserialized: V = serde_json::from_slice(serialised.as_slice())?; - Ok(Some(deserialized)) - } - - fn save(&mut self, key: &str, value: V) -> StoreResult<()> { - let serialized: Vec = serde_json::to_vec(&value)?; - - let value_file_name = self.value_path(key); - - let mut value_file = File::create(&value_file_name) - .map_err(|err| format!("open {:?} failed: {}", value_file_name, err))?; - - value_file.write_all(serialized.as_slice()).map_err(|err| { - format!( - "FsStore: write_all to {:?} failed: {}", - value_file_name, err - ) - })?; - Ok(()) - } -} - -/// Helper: Like [`fs::read`], but take an open file. -fn read_all(mut file: File) -> io::Result> { - let mut bytes = Vec::with_capacity(initial_buffer_size(&file)); - file.read_to_end(&mut bytes)?; - Ok(bytes) -} - -/// Indicates how large a buffer to pre-allocate before reading the entire file. -fn initial_buffer_size(file: &File) -> usize { - // Allocate one extra byte so the buffer doesn't need to grow before the - // final `read` call at the end of the file. Don't worry about `usize` - // overflow because reading will fail regardless in that case. - file.metadata().map(|m| m.len() as usize + 1).unwrap_or(0) -} diff --git a/rtc_tenclave/src/kv_store/fs/inspect.rs b/rtc_tenclave/src/kv_store/fs/inspect.rs new file mode 100644 index 00000000..07f3c845 --- /dev/null +++ b/rtc_tenclave/src/kv_store/fs/inspect.rs @@ -0,0 +1,48 @@ +//! [`InspectStore`] for [`FsStore`] + +use std::collections::HashMap; +use std::ffi::OsStr; +use std::fs::DirEntry; +use std::io; +use std::iter::Iterator; + +use serde::de::DeserializeOwned; +use serde::Serialize; + +use crate::kv_store::inspect::InspectStore; +use crate::kv_store::KvStore; + +use super::std_filer::StdFiler; +use super::FsStore; +use std::path::PathBuf; + +impl InspectStore for FsStore +where + V: Serialize + DeserializeOwned, +{ + fn as_map(&self) -> HashMap { + let entries: impl Iterator> = self + .root_dir + .read_dir() + .unwrap_or_else(|_| panic!("read_dir {:?} failed", self.root_dir)); + + let keys: impl Iterator = entries.map(|entry: io::Result| { + let entry: DirEntry = entry.expect("read_dir entry failed"); + let file_path: PathBuf = entry.path(); + let os_file_name: &OsStr = file_path + .file_name() + .unwrap_or_else(|| panic!("directory entry lacks file_name: {:?}", file_path)); + let file_name: &str = os_file_name.to_str().expect("OsStr.to_str failed"); + Self::decode_key(file_name).expect("FsStore::decode_key failed") + }); + + keys.map(|k| { + let loaded: Option = self + .load(&k) + .unwrap_or_else(|_| panic!("load {:?} failed!", k)); + let v: V = loaded.unwrap_or_else(|| panic!("key missing! {:?}", k)); + (k, v) + }) + .collect() + } +} diff --git a/rtc_tenclave/src/kv_store/sgxfs.rs b/rtc_tenclave/src/kv_store/fs/mod.rs similarity index 51% rename from rtc_tenclave/src/kv_store/sgxfs.rs rename to rtc_tenclave/src/kv_store/fs/mod.rs index 7d735a97..40286f52 100644 --- a/rtc_tenclave/src/kv_store/sgxfs.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -1,48 +1,65 @@ -//! [`KvStore`] implementation based on [`sgx_tstd::sgxfs`] (using the Intel SGX Protected FS Library) +//! Filesystem-based [`KvStore`] implementation +#[cfg(test)] +pub(crate) mod inspect; +#[cfg(not(test))] +pub mod sgx_filer; +pub mod std_filer; + +// sgx_tstd (v1.1.3) does not support `fs::read_dir`, so limit the following to tests, for now. +// +// See: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/v1.1.3/release_notes.md#partially-supported-modstraits-in-sgx_tstd + +#[cfg(not(test))] use std::prelude::v1::*; use std::io; -use std::io::prelude::*; use std::path::{Path, PathBuf}; use serde::de::DeserializeOwned; use serde::Serialize; -use sgx_tstd::sgxfs::SgxFile; use super::{KvStore, StoreResult}; -/// Filesystem-based [`KvStore`], using [`SgxFile`] -/// -/// TODO: Document security guarantees. -/// -struct SgxFsStore { +/// Simplified interface for reading and writing files. +pub trait Filer { + /// Read content of `path`. + fn get(&self, path: impl AsRef) -> io::Result>; + + /// Write `content` to `path`. Discard any existing content. + fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> io::Result<()>; +} + +/// [`KvStore`] using a file per key under `root_dir`. +pub struct FsStore { pub(crate) root_dir: PathBuf, + pub(crate) filer: F, } -impl SgxFsStore { +impl FsStore +where + F: Filer, +{ /// Validate that `root_dir` exists as a directory. - pub fn new(root: impl AsRef) -> StoreResult { - let root = root.as_ref(); - - // XXX: no create_dir_all() - - Ok(SgxFsStore { - root_dir: root.to_path_buf(), - }) + pub fn new(root: impl AsRef, filer: F) -> StoreResult { + let root_dir = root.as_ref().to_path_buf(); + Ok(FsStore { root_dir, filer }) } /// Resolve file name for the value of `key`. fn value_path(&self, key: &str) -> PathBuf { - // XXX: Escaping / encoding? let file_name = Self::encode_key(key); self.root_dir.join(file_name) } + // Make keys filesystem-safe using hex (conservative, but effective): + pub(crate) fn encode_key(key: &str) -> String { let encoded = hex::encode(key); format!("x{}", encoded) } + + #[cfg_attr(not(test), allow(dead_code))] pub(crate) fn decode_key(file_name: &str) -> StoreResult { let encoded: &str = file_name .strip_prefix("x") @@ -53,50 +70,33 @@ impl SgxFsStore { } } -impl KvStore for SgxFsStore +impl KvStore for FsStore where + F: Filer, V: Serialize + DeserializeOwned, { fn load(&self, key: &str) -> StoreResult> { let value_file_name = self.value_path(key); - // TODO: Handle NotFound - // TODO: open_ex() with key - let value_file = SgxFile::open(&value_file_name) - .map_err(|err| format!("FsStore: open {:?} failed: {}", value_file_name, err))?; - // Note: Read all the data into memory first, then deserialize, for efficiency. // See the docs for [`serde_json::de::from_reader`], // and https://github.com/serde-rs/json/issues/160 - let serialised: Vec = read_all(value_file) + let serialised: Vec = self + .filer + .get(&value_file_name) + // TODO: Handle NotFound .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; - let deserialized: V = serde_json::from_slice(serialised.as_slice())?; - Ok(Some(deserialized)) + let value: V = serde_json::from_slice(serialised.as_slice())?; + Ok(Some(value)) } fn save(&mut self, key: &str, value: V) -> StoreResult<()> { - let serialized: Vec = serde_json::to_vec(&value)?; - let value_file_name = self.value_path(key); - - let mut value_file = SgxFile::create(&value_file_name) - .map_err(|err| format!("open {:?} failed: {}", value_file_name, err))?; - - value_file.write_all(serialized.as_slice()).map_err(|err| { - format!( - "FsStore: write_all to {:?} failed: {}", - value_file_name, err - ) - })?; + let serialized: Vec = serde_json::to_vec(&value)?; + self.filer + .put(&value_file_name, serialized) + .map_err(|err| format!("FsStore: write to {:?} failed: {}", value_file_name, err))?; Ok(()) } } - -/// Helper: Like [`fs::read`], but take an open file. -fn read_all(mut file: SgxFile) -> io::Result> { - // XXX: No metadata for initial_buffer_size in sgxfs - let mut bytes = Vec::new(); - file.read_to_end(&mut bytes)?; - Ok(bytes) -} diff --git a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs new file mode 100644 index 00000000..c660e3e9 --- /dev/null +++ b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs @@ -0,0 +1,43 @@ +//! [`SgxFile`] support + +use std::prelude::v1::Vec; + +use std::io::Read; +use std::io::Result; +use std::io::Write; +use std::path::Path; + +use sgx_tstd::sgxfs::SgxFile; + +use super::Filer; + +/// TODO: key management policy +pub struct SgxFiler; + +// Default key management: +// +// * `protected_fs_file::generate_random_meta_data_key` +// https://github.com/intel/linux-sgx/blob/sgx_2.13.3/sdk/protected_fs/sgx_tprotected_fs/file_crypto.cpp#L197 +// +impl Filer for SgxFiler { + fn get(&self, path: impl AsRef) -> Result> { + // TODO: open_ex with key + let value_file = SgxFile::open(path)?; + read_all(value_file) + } + + fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { + let contents: &[u8] = content.as_ref(); + // TODO: create_ex with key + let mut value_file = SgxFile::create(path)?; + value_file.write_all(contents) + } +} + +/// Helper: Like [`fs::read`], but take an open file. +fn read_all(mut file: SgxFile) -> Result> { + // XXX: No metadata for initial_buffer_size in sgxfs + let mut buf = Vec::new(); + file.read_to_end(&mut buf)?; + Ok(buf) +} diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs new file mode 100644 index 00000000..32f5cdc8 --- /dev/null +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -0,0 +1,30 @@ +//! [`std::fs::File`] support + +use std::prelude::v1::Vec; + +use std::io::Result; +use std::io::Write; + +use std::path::Path; + +// Under sgx_tstd, fs needs the std::untrusted prefix: +#[cfg(not(test))] +use std::untrusted::{fs, fs::File}; +#[cfg(test)] +use std::{fs, fs::File}; + +use super::Filer; + +pub struct StdFiler; + +impl Filer for StdFiler { + fn get(&self, path: impl AsRef) -> Result> { + fs::read(path) + } + + fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { + let contents: &[u8] = content.as_ref(); + let mut value_file = File::create(path)?; + value_file.write_all(contents) + } +} diff --git a/rtc_tenclave/src/kv_store/inspect.rs b/rtc_tenclave/src/kv_store/inspect.rs index 8f3b5310..aaac2d66 100644 --- a/rtc_tenclave/src/kv_store/inspect.rs +++ b/rtc_tenclave/src/kv_store/inspect.rs @@ -42,43 +42,3 @@ where .collect() } } - -// sgx_tstd (v1.1.3) does not support `fs::read_dir`, so limit the following to tests, for now. -// -// See: https://github.com/apache/incubator-teaclave-sgx-sdk/blob/v1.1.3/release_notes.md#partially-supported-modstraits-in-sgx_tstd - -#[cfg(test)] -use std::{ffi::OsStr, fs::DirEntry, io, iter::Iterator}; - -#[cfg(test)] -use super::fs::FsStore; - -#[cfg(test)] -impl InspectStore for FsStore -where - V: Serialize + DeserializeOwned, -{ - fn as_map(&self) -> HashMap { - let entries: impl Iterator> = self - .root_dir - .read_dir() - .expect(&format!("read_dir {:?} failed", self.root_dir)); - - let keys: impl Iterator = entries.map(|entry: io::Result| { - let entry: DirEntry = entry.expect("read_dir entry failed"); - let file_path = entry.path(); - let os_file_name: &OsStr = file_path - .file_name() - .expect(&format!("directory entry lacks file_name: {:?}", file_path)); - let file_name: &str = os_file_name.to_str().expect("OsStr.to_str failed"); - FsStore::decode_key(file_name).expect("FsStore::decode_key failed") - }); - - keys.map(|k| { - let loaded: Option = self.load(&k).expect(&format!("load {:?} failed!", k)); - let v: V = loaded.expect(&format!("key missing! {:?}", k)); - (k, v) - }) - .collect() - } -} diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index d7c378d9..93a1ce66 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -30,8 +30,6 @@ pub trait KvStore { mod fs; mod in_memory; mod inspect; -#[cfg(not(test))] -mod sgxfs; #[cfg(test)] mod tests; diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index ec4e2fbb..4f273bd7 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -4,12 +4,13 @@ use std::prelude::v1::*; use std::collections::HashMap; +use std::fs::create_dir_all; use std::fs::remove_dir_all; use std::path::Path; use proptest::prelude::*; -use super::fs::FsStore; +use super::fs::{std_filer::StdFiler, FsStore}; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::inspect::InspectStore; use super::KvStore; @@ -47,7 +48,8 @@ fn prop_store_ops_match_model() { // Init the store under test let path = Path::new("store_test"); clear_dir(path); // Clear before each test - let mut store_fs: FsStore = FsStore::new(path).expect("FsStore::new failed"); + create_dir_all(path).expect("create_dir_all failed"); + let mut store_fs: FsStore = FsStore::new(path, StdFiler).expect("FsStore::new failed"); for (k, v) in store_ops_vec { store_model.save(&k, v.clone()).expect("InMemoryStore save failed!"); From dc12fe6b928d23ffcd75c1c3de83324592f1f3f4 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 15:44:15 +0200 Subject: [PATCH 08/29] style(rtc_tenclave,tests): whitespace inside macro --- rtc_tenclave/src/kv_store/tests.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 4f273bd7..1a4a13f8 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -47,13 +47,18 @@ fn prop_store_ops_match_model() { // Init the store under test let path = Path::new("store_test"); - clear_dir(path); // Clear before each test + clear_dir(path); // Clear before each test create_dir_all(path).expect("create_dir_all failed"); - let mut store_fs: FsStore = FsStore::new(path, StdFiler).expect("FsStore::new failed"); + let mut store_fs: FsStore = + FsStore::new(path, StdFiler).expect("FsStore::new failed"); for (k, v) in store_ops_vec { - store_model.save(&k, v.clone()).expect("InMemoryStore save failed!"); - store_model_json.save(&k, v.clone()).expect("InMemoryJsonStore save failed!"); + store_model + .save(&k, v.clone()) + .expect("InMemoryStore save failed!"); + store_model_json + .save(&k, v.clone()) + .expect("InMemoryJsonStore save failed!"); store_fs.save(&k, v.clone()).expect("FsStore save failed!"); // Models match each other @@ -61,10 +66,13 @@ fn prop_store_ops_match_model() { // Models match store_fs prop_assert_eq!(store_model.as_map(), store_fs.as_map()); // FIXME: explicit coercion for as_map() - prop_assert_eq!(store_model_json.as_map() as HashMap, store_fs.as_map()); + prop_assert_eq!( + store_model_json.as_map() as HashMap, + store_fs.as_map() + ); } - clear_dir(path); // Clear after successful tests, just to keep the workdir clean + clear_dir(path); // Clear after successful tests, just to keep the workdir clean }); } From 4d1a8c889c179dbe7acf03bb93b9f52ec4e3785b Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 15:47:01 +0200 Subject: [PATCH 09/29] test(rtc_tenclave): pull function out of proptest macro This lets formatting and analysis work better. --- rtc_tenclave/src/kv_store/tests.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 1a4a13f8..3b3ae9e6 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -9,6 +9,7 @@ use std::fs::remove_dir_all; use std::path::Path; use proptest::prelude::*; +use proptest::test_runner::TestCaseResult; use super::fs::{std_filer::StdFiler, FsStore}; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; @@ -37,7 +38,7 @@ fn prop_store_ops_match_model() { }; } - proptest!(|(store_ops_vec in store_ops_strategy)| { + fn test(store_ops_vec: Vec<(String, String)>) -> TestCaseResult { // FIXME: This value type parameter needs better handling. type V = String; @@ -73,6 +74,11 @@ fn prop_store_ops_match_model() { } clear_dir(path); // Clear after successful tests, just to keep the workdir clean + Ok(()) + } + + proptest!(|(store_ops_vec in store_ops_strategy)| { + test(store_ops_vec)?; }); } From 80a1d5ea89a05e6395215e0d1d91575815da38e3 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 15:08:21 +0200 Subject: [PATCH 10/29] deps(rtc_tenclave): add tempfile for tests --- rtc_tenclave/Cargo.lock | 1 + rtc_tenclave/Cargo.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/rtc_tenclave/Cargo.lock b/rtc_tenclave/Cargo.lock index e5c2ef25..7d9e4657 100644 --- a/rtc_tenclave/Cargo.lock +++ b/rtc_tenclave/Cargo.lock @@ -438,6 +438,7 @@ dependencies = [ "sgx_types", "sgx_ucrypto", "sodalite", + "tempfile", "thiserror 1.0.24", "thiserror 1.0.9", "zeroize", diff --git a/rtc_tenclave/Cargo.toml b/rtc_tenclave/Cargo.toml index 76d8dcd8..19104a54 100644 --- a/rtc_tenclave/Cargo.toml +++ b/rtc_tenclave/Cargo.toml @@ -69,7 +69,9 @@ sgx_ucrypto = { git = "https://github.com/apache/teaclave-sgx-sdk.git" } serde_std = { package = "serde", version = "1.0.0" } serde_json_std = { package = "serde_json", version = "1.0.0" } +# Test-only dependencies proptest = "1.0.0" +tempfile = "3.2.0" [patch."https://github.com/apache/teaclave-sgx-sdk.git"] From 5f8bbcce6384dbfc99f23d9d7763d0a934628692 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 15:51:50 +0200 Subject: [PATCH 11/29] test(rtc_tenclave): simplify using tempfile::TempDir --- rtc_tenclave/src/kv_store/tests.rs | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 3b3ae9e6..5f06daf3 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -4,12 +4,10 @@ use std::prelude::v1::*; use std::collections::HashMap; -use std::fs::create_dir_all; -use std::fs::remove_dir_all; -use std::path::Path; use proptest::prelude::*; use proptest::test_runner::TestCaseResult; +use tempfile::TempDir; use super::fs::{std_filer::StdFiler, FsStore}; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; @@ -31,13 +29,6 @@ fn prop_store_ops_match_model() { .prop_shuffle() }; - // XXX: hacky clearing - pub fn clear_dir(path: &Path) { - if path.is_dir() { - remove_dir_all(path).expect("remove_dir_all failed"); - }; - } - fn test(store_ops_vec: Vec<(String, String)>) -> TestCaseResult { // FIXME: This value type parameter needs better handling. type V = String; @@ -47,11 +38,9 @@ fn prop_store_ops_match_model() { let mut store_model_json: InMemoryJsonStore = InMemoryJsonStore::default(); // Init the store under test - let path = Path::new("store_test"); - clear_dir(path); // Clear before each test - create_dir_all(path).expect("create_dir_all failed"); + let temp_dir = TempDir::new().unwrap(); let mut store_fs: FsStore = - FsStore::new(path, StdFiler).expect("FsStore::new failed"); + FsStore::new(&temp_dir, StdFiler).expect("FsStore::new failed"); for (k, v) in store_ops_vec { store_model @@ -73,7 +62,7 @@ fn prop_store_ops_match_model() { ); } - clear_dir(path); // Clear after successful tests, just to keep the workdir clean + temp_dir.close()?; Ok(()) } From 674a42883fd9a99084989c5b26daab27276103fa Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 17:06:59 +0200 Subject: [PATCH 12/29] test(rtc_tenclave,kv_store,std_filer): basic test coverage --- rtc_tenclave/src/kv_store/fs/std_filer.rs | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs index 32f5cdc8..7c77a630 100644 --- a/rtc_tenclave/src/kv_store/fs/std_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -28,3 +28,42 @@ impl Filer for StdFiler { value_file.write_all(contents) } } + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::*; + + // Helper: Run `f` with a non-existent file path inside a temporary directory. + fn with_temp_path(f: impl FnOnce(&Path)) { + let temp_dir = TempDir::new().unwrap(); + f(&temp_dir.path().join("foo")); + temp_dir.close().unwrap() + } + + #[test] + fn get_empty() { + with_temp_path(|path: &Path| { + File::create(path).unwrap(); + assert_eq!(StdFiler.get(path).unwrap(), "".as_bytes()); + }) + } + + #[test] + fn put_get() { + with_temp_path(|path| { + StdFiler.put(path, "spam").unwrap(); + assert_eq!(StdFiler.get(path).unwrap(), "spam".as_bytes()) + }) + } + + #[test] + fn put_get_overwrite() { + with_temp_path(|path| { + StdFiler.put(path, "spam").unwrap(); + StdFiler.put(path, "ham").unwrap(); + assert_eq!(StdFiler.get(path).unwrap(), "ham".as_bytes()) + }) + } +} From 6d08635fb3f36a1ad0f75356536877b24dcac62b Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 17:52:43 +0200 Subject: [PATCH 13/29] refactor(rtc_tenclave,kv_store): add Option to Filer.get() result type --- rtc_tenclave/src/kv_store/fs/mod.rs | 17 ++++++++++------- rtc_tenclave/src/kv_store/fs/sgx_filer.rs | 4 ++-- rtc_tenclave/src/kv_store/fs/std_filer.rs | 10 +++++----- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index 40286f52..45c4900c 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -23,8 +23,11 @@ use super::{KvStore, StoreResult}; /// Simplified interface for reading and writing files. pub trait Filer { - /// Read content of `path`. - fn get(&self, path: impl AsRef) -> io::Result>; + /// Read content of `path`, if any. + /// + /// Return [`None`] if `path` doesn't exist. + /// + fn get(&self, path: impl AsRef) -> io::Result>>; /// Write `content` to `path`. Discard any existing content. fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> io::Result<()>; @@ -81,14 +84,14 @@ where // Note: Read all the data into memory first, then deserialize, for efficiency. // See the docs for [`serde_json::de::from_reader`], // and https://github.com/serde-rs/json/issues/160 - let serialised: Vec = self + let loaded: Option> = self .filer .get(&value_file_name) - // TODO: Handle NotFound .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; - - let value: V = serde_json::from_slice(serialised.as_slice())?; - Ok(Some(value)) + let value: Option = loaded + .map(|serialised: Vec| serde_json::from_slice(serialised.as_slice())) + .transpose()?; + Ok(value) } fn save(&mut self, key: &str, value: V) -> StoreResult<()> { diff --git a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs index c660e3e9..b8a991f4 100644 --- a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs @@ -20,10 +20,10 @@ pub struct SgxFiler; // https://github.com/intel/linux-sgx/blob/sgx_2.13.3/sdk/protected_fs/sgx_tprotected_fs/file_crypto.cpp#L197 // impl Filer for SgxFiler { - fn get(&self, path: impl AsRef) -> Result> { + fn get(&self, path: impl AsRef) -> Result>> { // TODO: open_ex with key let value_file = SgxFile::open(path)?; - read_all(value_file) + read_all(value_file).map(Some) } fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs index 7c77a630..e8b9c799 100644 --- a/rtc_tenclave/src/kv_store/fs/std_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -18,8 +18,8 @@ use super::Filer; pub struct StdFiler; impl Filer for StdFiler { - fn get(&self, path: impl AsRef) -> Result> { - fs::read(path) + fn get(&self, path: impl AsRef) -> Result>> { + fs::read(path).map(Some) } fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { @@ -46,7 +46,7 @@ mod tests { fn get_empty() { with_temp_path(|path: &Path| { File::create(path).unwrap(); - assert_eq!(StdFiler.get(path).unwrap(), "".as_bytes()); + assert_eq!(StdFiler.get(path).unwrap().unwrap(), "".as_bytes()); }) } @@ -54,7 +54,7 @@ mod tests { fn put_get() { with_temp_path(|path| { StdFiler.put(path, "spam").unwrap(); - assert_eq!(StdFiler.get(path).unwrap(), "spam".as_bytes()) + assert_eq!(StdFiler.get(path).unwrap().unwrap(), "spam".as_bytes()) }) } @@ -63,7 +63,7 @@ mod tests { with_temp_path(|path| { StdFiler.put(path, "spam").unwrap(); StdFiler.put(path, "ham").unwrap(); - assert_eq!(StdFiler.get(path).unwrap(), "ham".as_bytes()) + assert_eq!(StdFiler.get(path).unwrap().unwrap(), "ham".as_bytes()) }) } } From 94fb6d32fdae293d4410e12b1cc82e2e89fa58ba Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 18:03:58 +0200 Subject: [PATCH 14/29] feat(rtc_tenclave,kv_store): handle NotFound in Filer --- rtc_tenclave/src/kv_store/fs/sgx_filer.rs | 7 ++++++- rtc_tenclave/src/kv_store/fs/std_filer.rs | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs index b8a991f4..86ee4fb4 100644 --- a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs @@ -2,6 +2,7 @@ use std::prelude::v1::Vec; +use std::io::ErrorKind::NotFound; use std::io::Read; use std::io::Result; use std::io::Write; @@ -23,7 +24,11 @@ impl Filer for SgxFiler { fn get(&self, path: impl AsRef) -> Result>> { // TODO: open_ex with key let value_file = SgxFile::open(path)?; - read_all(value_file).map(Some) + match read_all(value_file) { + Ok(contents) => Ok(Some(contents)), + Err(error) if error.kind() == NotFound => Ok(None), + Err(error) => Err(error), + } } fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs index e8b9c799..8e15036b 100644 --- a/rtc_tenclave/src/kv_store/fs/std_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -2,6 +2,7 @@ use std::prelude::v1::Vec; +use std::io::ErrorKind::NotFound; use std::io::Result; use std::io::Write; @@ -19,7 +20,11 @@ pub struct StdFiler; impl Filer for StdFiler { fn get(&self, path: impl AsRef) -> Result>> { - fs::read(path).map(Some) + match fs::read(path) { + Ok(contents) => Ok(Some(contents)), + Err(error) if error.kind() == NotFound => Ok(None), + Err(error) => Err(error), + } } fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> Result<()> { @@ -42,6 +47,14 @@ mod tests { temp_dir.close().unwrap() } + #[test] + fn get_not_found() { + with_temp_path(|path: &Path| { + assert!(!path.exists()); + assert_eq!(StdFiler.get(path).unwrap(), None); + }) + } + #[test] fn get_empty() { with_temp_path(|path: &Path| { From 35fad2c5d03b7c4bc28a2fd1a031d38fbd27d99d Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 20:42:12 +0200 Subject: [PATCH 15/29] check(rtc_tenclave,kv_store): mark functions currently only referenced in tests --- rtc_tenclave/src/kv_store/fs/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index 45c4900c..f4b18e21 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -44,6 +44,7 @@ where F: Filer, { /// Validate that `root_dir` exists as a directory. + #[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests pub fn new(root: impl AsRef, filer: F) -> StoreResult { let root_dir = root.as_ref().to_path_buf(); Ok(FsStore { root_dir, filer }) @@ -62,7 +63,7 @@ where format!("x{}", encoded) } - #[cfg_attr(not(test), allow(dead_code))] + #[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests pub(crate) fn decode_key(file_name: &str) -> StoreResult { let encoded: &str = file_name .strip_prefix("x") From 89d8eb2c8419937ae0705fc89c1372f8e2303314 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 20:45:24 +0200 Subject: [PATCH 16/29] refactor(rtc_tenclave,kv_store): take save() value by reference This fits better with how the copying store implementations work. --- rtc_tenclave/src/kv_store/fs/mod.rs | 2 +- rtc_tenclave/src/kv_store/in_memory.rs | 6 +++--- rtc_tenclave/src/kv_store/mod.rs | 5 +++-- rtc_tenclave/src/kv_store/tests.rs | 6 +++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index f4b18e21..c9b08cd8 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -95,7 +95,7 @@ where Ok(value) } - fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { let value_file_name = self.value_path(key); let serialized: Vec = serde_json::to_vec(&value)?; self.filer diff --git a/rtc_tenclave/src/kv_store/in_memory.rs b/rtc_tenclave/src/kv_store/in_memory.rs index 3a945139..319dce7c 100644 --- a/rtc_tenclave/src/kv_store/in_memory.rs +++ b/rtc_tenclave/src/kv_store/in_memory.rs @@ -21,8 +21,8 @@ where Ok(self.map.get(key).cloned()) } - fn save(&mut self, key: &str, value: V) -> StoreResult<()> { - self.map.insert(key.to_string(), value); + fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { + self.map.insert(key.to_string(), value.clone()); Ok(()) } } @@ -43,7 +43,7 @@ where Ok(deserialized) } - fn save(&mut self, key: &str, value: V) -> StoreResult<()> { + fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { let serialized = serde_json::to_vec(&value)?; self.map.insert(key.to_string(), serialized); Ok(()) diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 93a1ce66..3db0815f 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -7,7 +7,8 @@ type StoreResult = Result>; /// A key-value store. /// -/// These methods borrow key references, and +/// These methods borrow key and value references, +/// to suit cloning / serialising implementations. /// pub trait KvStore { // TODO: Use associated type for V? @@ -22,7 +23,7 @@ pub trait KvStore { /// /// This will replace any existing value. /// - fn save(&mut self, key: &str, value: V) -> StoreResult<()>; + fn save(&mut self, key: &str, value: &V) -> StoreResult<()>; // TODO: add update() } diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 5f06daf3..8b816555 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -44,12 +44,12 @@ fn prop_store_ops_match_model() { for (k, v) in store_ops_vec { store_model - .save(&k, v.clone()) + .save(&k, &v) .expect("InMemoryStore save failed!"); store_model_json - .save(&k, v.clone()) + .save(&k, &v) .expect("InMemoryJsonStore save failed!"); - store_fs.save(&k, v.clone()).expect("FsStore save failed!"); + store_fs.save(&k, &v).expect("FsStore save failed!"); // Models match each other prop_assert_eq!(store_model.as_map(), store_model_json.as_map()); From fe1440523df493b729bf374aece87d95997410ed Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Wed, 19 May 2021 21:15:34 +0200 Subject: [PATCH 17/29] feat(rtc_tenclave,kv_store): add delete operation --- rtc_tenclave/src/kv_store/fs/mod.rs | 9 ++++++++ rtc_tenclave/src/kv_store/fs/sgx_filer.rs | 8 +++++++ rtc_tenclave/src/kv_store/fs/std_filer.rs | 27 +++++++++++++++++++++++ rtc_tenclave/src/kv_store/in_memory.rs | 10 +++++++++ rtc_tenclave/src/kv_store/mod.rs | 3 ++- 5 files changed, 56 insertions(+), 1 deletion(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index c9b08cd8..11915295 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -31,6 +31,9 @@ pub trait Filer { /// Write `content` to `path`. Discard any existing content. fn put(&self, path: impl AsRef, content: impl AsRef<[u8]>) -> io::Result<()>; + + /// Delete `path`. Discard any existing content. + fn delete(&self, path: impl AsRef) -> io::Result<()>; } /// [`KvStore`] using a file per key under `root_dir`. @@ -103,4 +106,10 @@ where .map_err(|err| format!("FsStore: write to {:?} failed: {}", value_file_name, err))?; Ok(()) } + + fn delete(&mut self, key: &str) -> StoreResult<()> { + let path = self.value_path(key); + self.filer.delete(path)?; + Ok(()) + } } diff --git a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs index 86ee4fb4..fa50689f 100644 --- a/rtc_tenclave/src/kv_store/fs/sgx_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/sgx_filer.rs @@ -8,6 +8,7 @@ use std::io::Result; use std::io::Write; use std::path::Path; +use sgx_tstd::sgxfs; use sgx_tstd::sgxfs::SgxFile; use super::Filer; @@ -37,6 +38,13 @@ impl Filer for SgxFiler { let mut value_file = SgxFile::create(path)?; value_file.write_all(contents) } + + fn delete(&self, path: impl AsRef) -> Result<()> { + match sgxfs::remove(path) { + Err(error) if error.kind() == NotFound => Ok(()), + result => result, + } + } } /// Helper: Like [`fs::read`], but take an open file. diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs index 8e15036b..92f059d1 100644 --- a/rtc_tenclave/src/kv_store/fs/std_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -32,6 +32,13 @@ impl Filer for StdFiler { let mut value_file = File::create(path)?; value_file.write_all(contents) } + + fn delete(&self, path: impl AsRef) -> Result<()> { + match fs::remove_file(path) { + Err(error) if error.kind() == NotFound => Ok(()), + result => result, + } + } } #[cfg(test)] @@ -79,4 +86,24 @@ mod tests { assert_eq!(StdFiler.get(path).unwrap().unwrap(), "ham".as_bytes()) }) } + + #[test] + fn delete_missing() { + with_temp_path(|path: &Path| { + assert!(!path.exists()); + assert_eq!(StdFiler.delete(path).unwrap(), ()); + assert!(!path.exists()); + }) + } + + #[test] + fn delete_present() { + with_temp_path(|path: &Path| { + StdFiler.put(path, "spam").unwrap(); + assert_eq!(StdFiler.get(path).unwrap().unwrap(), "spam".as_bytes()); + assert!(path.exists()); + assert_eq!(StdFiler.delete(path).unwrap(), ()); + assert!(!path.exists()); + }) + } } diff --git a/rtc_tenclave/src/kv_store/in_memory.rs b/rtc_tenclave/src/kv_store/in_memory.rs index 319dce7c..ec596653 100644 --- a/rtc_tenclave/src/kv_store/in_memory.rs +++ b/rtc_tenclave/src/kv_store/in_memory.rs @@ -25,6 +25,11 @@ where self.map.insert(key.to_string(), value.clone()); Ok(()) } + + fn delete(&mut self, key: &str) -> StoreResult<()> { + self.map.remove(key); + Ok(()) + } } /// In-memory [`KvStore`] using [`HashMap`] and [`serde_json`] serialization @@ -48,4 +53,9 @@ where self.map.insert(key.to_string(), serialized); Ok(()) } + + fn delete(&mut self, key: &str) -> StoreResult<()> { + self.map.remove(key); + Ok(()) + } } diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 3db0815f..9eab74c3 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -25,7 +25,8 @@ pub trait KvStore { /// fn save(&mut self, key: &str, value: &V) -> StoreResult<()>; - // TODO: add update() + /// Delete the saved value for `key`. + fn delete(&mut self, key: &str) -> StoreResult<()>; } mod fs; From b864be9991f72bb65e084e233cd66b5a10409791 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 00:58:49 +0200 Subject: [PATCH 18/29] test(rtc_tenclave,kv_store): clean up, and test delete operations too --- rtc_tenclave/src/kv_store/tests.rs | 89 +++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 26 deletions(-) diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 8b816555..cd3eae81 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -12,54 +12,91 @@ use tempfile::TempDir; use super::fs::{std_filer::StdFiler, FsStore}; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::inspect::InspectStore; -use super::KvStore; +use super::{KvStore, StoreResult}; /// Verify that executing a sequence of store operations matches a simple model. #[test] fn prop_store_ops_match_model() { - // Strategy to generate store operations: currently, just key / value pairs to save. + // FIXME: This value type parameter needs better handling. + type V = String; + + /// Helper: Represent store operations. + #[derive(Debug)] + enum StoreOp { + Save { key: String, value: V }, + Delete { key: String }, + } + use StoreOp::*; + impl StoreOp { + /// Apply operation, and also check some invariants. + fn apply(&self, store: &mut impl KvStore) -> StoreResult<()> { + match self { + Save { key, value } => { + store.save(key, value)?; + assert_eq!(store.load(key)?.as_ref(), Some(value)); + } + Delete { key } => { + store.delete(key)?; + assert_eq!(store.load(key)?, None); + } + }; + Ok(()) + } + } + + // Strategy to generate lists of store operations. let store_ops_strategy = { let keys = prop_oneof!(r"[a-z]{0,5}", ".*"); let values = prop_oneof!(keys.clone()); // TODO: Non-string values + let half_ops = prop_oneof!( + (Just("Save"), values.clone().prop_map(Some)), + (Just("Delete"), Just(None)), + ); // This strategy will generate key / value pairs with multiple values per key, // and shuffle them to have some interleaving, for bugs that depend on that. - proptest::collection::hash_map(keys, proptest::collection::vec(values, 0..10), 0..10) + proptest::collection::hash_map(keys, proptest::collection::vec(half_ops, 0..10), 0..10) .prop_map(flatten_key_values) + .prop_map(|pairs: Vec<_>| -> Vec { + pairs + .into_iter() + .map(|(key, half_op)| -> StoreOp { + match half_op { + ("Save", Some(value)) => Save { key, value }, + ("Delete", None) => Delete { key }, + unexpected => panic!("unexpected: {:?}", unexpected), + } + }) + .collect() + }) .prop_shuffle() }; - fn test(store_ops_vec: Vec<(String, String)>) -> TestCaseResult { - // FIXME: This value type parameter needs better handling. - type V = String; + /// Helper: Check that store state matches model. + fn check_state(store1: &impl InspectStore, store2: &impl InspectStore) -> TestCaseResult { + prop_assert_eq!(store1.as_map(), store2.as_map()); + Ok(()) + } + fn test(store_ops_vec: Vec) -> TestCaseResult { // Init the models - let mut store_model: InMemoryStore = InMemoryStore::default(); - let mut store_model_json: InMemoryJsonStore = InMemoryJsonStore::default(); + let ref mut store_model = InMemoryStore::default(); + let ref mut store_model_json = InMemoryJsonStore::default(); // Init the store under test let temp_dir = TempDir::new().unwrap(); - let mut store_fs: FsStore = - FsStore::new(&temp_dir, StdFiler).expect("FsStore::new failed"); - - for (k, v) in store_ops_vec { - store_model - .save(&k, &v) - .expect("InMemoryStore save failed!"); - store_model_json - .save(&k, &v) - .expect("InMemoryJsonStore save failed!"); - store_fs.save(&k, &v).expect("FsStore save failed!"); + let ref mut store_fs = FsStore::new(&temp_dir, StdFiler).expect("FsStore::new failed"); + + for ref op in store_ops_vec { + op.apply(store_model).unwrap(); + op.apply(store_model_json).unwrap(); + op.apply(store_fs).unwrap(); // Models match each other - prop_assert_eq!(store_model.as_map(), store_model_json.as_map()); + check_state(store_model, store_model_json)?; // Models match store_fs - prop_assert_eq!(store_model.as_map(), store_fs.as_map()); - // FIXME: explicit coercion for as_map() - prop_assert_eq!( - store_model_json.as_map() as HashMap, - store_fs.as_map() - ); + check_state(store_model, store_fs)?; + check_state(store_model_json, store_fs)?; } temp_dir.close()?; From a1524d0e6343cb02e0110e38bf23b4358323aa3c Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 02:55:51 +0200 Subject: [PATCH 19/29] feat(rtc_tenclave,kv_store): add alter operation --- rtc_tenclave/src/kv_store/mod.rs | 17 +++++++++++++++++ rtc_tenclave/src/kv_store/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 9eab74c3..3cc37f21 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -27,6 +27,23 @@ pub trait KvStore { /// Delete the saved value for `key`. fn delete(&mut self, key: &str) -> StoreResult<()>; + + /// Alter the value of `key`. + /// + /// This operation is a generalisation of [`Self::load`], [`Self::save`], and [`Self::delete`]. + /// + fn alter(&mut self, key: &str, alter_fn: F) -> StoreResult> + where + F: FnOnce(Option) -> Option, + { + let loaded: Option = self.load(key)?; + let altered: Option = alter_fn(loaded); + match &altered { + None => self.delete(key)?, + Some(value) => self.save(key, value)?, + }; + Ok(altered) + } } mod fs; diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index cd3eae81..97384d9e 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -25,6 +25,9 @@ fn prop_store_ops_match_model() { enum StoreOp { Save { key: String, value: V }, Delete { key: String }, + AlterId { key: String }, + AlterConst { key: String, replacement: Option }, + AlterUpdate { key: String, new_value: V }, } use StoreOp::*; impl StoreOp { @@ -39,6 +42,22 @@ fn prop_store_ops_match_model() { store.delete(key)?; assert_eq!(store.load(key)?, None); } + AlterId { key } => { + let previous = store.load(key)?; + store.alter(key, |loaded| loaded)?; + assert_eq!(store.load(key)?, previous); + } + AlterConst { key, replacement } => { + store.alter(key, |_| replacement.clone())?; + assert_eq!(store.load(key)?.as_ref(), replacement.as_ref()); + } + AlterUpdate { key, new_value } => { + let previous = store.load(key)?; + store.alter(key, |existing: Option| { + existing.map(|_| new_value.clone()) + })?; + assert_eq!(store.load(key)?.as_ref(), previous.map(|_| new_value)); + } }; Ok(()) } @@ -51,6 +70,9 @@ fn prop_store_ops_match_model() { let half_ops = prop_oneof!( (Just("Save"), values.clone().prop_map(Some)), (Just("Delete"), Just(None)), + (Just("AlterId"), Just(None)), + (Just("AlterConst"), proptest::option::of(values.clone())), + (Just("AlterUpdate"), values.clone().prop_map(Some)), ); // This strategy will generate key / value pairs with multiple values per key, @@ -64,6 +86,9 @@ fn prop_store_ops_match_model() { match half_op { ("Save", Some(value)) => Save { key, value }, ("Delete", None) => Delete { key }, + ("AlterId", None) => AlterId { key }, + ("AlterConst", replacement) => AlterConst { key, replacement }, + ("AlterUpdate", Some(new_value)) => AlterUpdate { key, new_value }, unexpected => panic!("unexpected: {:?}", unexpected), } }) From 3ae26415ed9377a3f3268f59c325ea811c826c22 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 09:57:38 +0200 Subject: [PATCH 20/29] refactor(rtc_tenclave,kv_store): mark inspect module test-only --- rtc_tenclave/src/kv_store/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 3cc37f21..787ce70e 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -48,6 +48,7 @@ pub trait KvStore { mod fs; mod in_memory; +#[cfg(test)] mod inspect; #[cfg(test)] From d00a56a09ac3c0db684ffcde3985f7e84a688ec6 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 10:50:00 +0200 Subject: [PATCH 21/29] style(rtc_tenclave,kv_store): reorder mod declarations a bit better --- rtc_tenclave/src/kv_store/fs/mod.rs | 8 +++++--- rtc_tenclave/src/kv_store/mod.rs | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index 11915295..ee577444 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -1,10 +1,9 @@ //! Filesystem-based [`KvStore`] implementation -#[cfg(test)] -pub(crate) mod inspect; +pub mod std_filer; + #[cfg(not(test))] pub mod sgx_filer; -pub mod std_filer; // sgx_tstd (v1.1.3) does not support `fs::read_dir`, so limit the following to tests, for now. // @@ -113,3 +112,6 @@ where Ok(()) } } + +#[cfg(test)] +mod inspect; diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 787ce70e..3769648d 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -3,6 +3,9 @@ use std::boxed::Box; use std::error::Error; +mod fs; +mod in_memory; + type StoreResult = Result>; /// A key-value store. @@ -46,8 +49,6 @@ pub trait KvStore { } } -mod fs; -mod in_memory; #[cfg(test)] mod inspect; From b45cb78357c079fce254373ffe1a4199cb7ba9dd Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 11:28:32 +0200 Subject: [PATCH 22/29] =?UTF-8?q?refactor(rtc=5Ftenclave,kv=5Fstore):=20re?= =?UTF-8?q?name=20InspectStore=20as=5Fmap=20=E2=86=92=20to=5Fmap?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- rtc_tenclave/src/kv_store/fs/inspect.rs | 2 +- rtc_tenclave/src/kv_store/inspect.rs | 6 +++--- rtc_tenclave/src/kv_store/tests.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/inspect.rs b/rtc_tenclave/src/kv_store/fs/inspect.rs index 07f3c845..468e866c 100644 --- a/rtc_tenclave/src/kv_store/fs/inspect.rs +++ b/rtc_tenclave/src/kv_store/fs/inspect.rs @@ -20,7 +20,7 @@ impl InspectStore for FsStore where V: Serialize + DeserializeOwned, { - fn as_map(&self) -> HashMap { + fn to_map(&self) -> HashMap { let entries: impl Iterator> = self .root_dir .read_dir() diff --git a/rtc_tenclave/src/kv_store/inspect.rs b/rtc_tenclave/src/kv_store/inspect.rs index aaac2d66..b37ec8dc 100644 --- a/rtc_tenclave/src/kv_store/inspect.rs +++ b/rtc_tenclave/src/kv_store/inspect.rs @@ -13,14 +13,14 @@ use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::KvStore; pub trait InspectStore { - fn as_map(&self) -> HashMap; + fn to_map(&self) -> HashMap; } impl InspectStore for InMemoryStore where V: Clone, { - fn as_map(&self) -> HashMap { + fn to_map(&self) -> HashMap { self.map.clone() } } @@ -29,7 +29,7 @@ impl InspectStore for InMemoryJsonStore where V: Serialize + DeserializeOwned, { - fn as_map(&self) -> HashMap { + fn to_map(&self) -> HashMap { self.map .keys() .map(|k| { diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 97384d9e..3829b0de 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -99,7 +99,7 @@ fn prop_store_ops_match_model() { /// Helper: Check that store state matches model. fn check_state(store1: &impl InspectStore, store2: &impl InspectStore) -> TestCaseResult { - prop_assert_eq!(store1.as_map(), store2.as_map()); + prop_assert_eq!(store1.to_map(), store2.to_map()); Ok(()) } From 0712677808ebfa887c726a75114c64f6808e1190 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 13:23:08 +0200 Subject: [PATCH 23/29] refactor(rtc_tenclave,kv_store): remove Result from FsStore::new This no longer performs IO to ensure that the root directory exists. --- rtc_tenclave/src/kv_store/fs/mod.rs | 9 ++++++--- rtc_tenclave/src/kv_store/tests.rs | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index ee577444..c37a84ab 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -45,11 +45,14 @@ impl FsStore where F: Filer, { - /// Validate that `root_dir` exists as a directory. + /// # Note + /// + /// The caller must ensure that `root` exists as a directory. + /// #[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests - pub fn new(root: impl AsRef, filer: F) -> StoreResult { + pub fn new(root: impl AsRef, filer: F) -> Self { let root_dir = root.as_ref().to_path_buf(); - Ok(FsStore { root_dir, filer }) + FsStore { root_dir, filer } } /// Resolve file name for the value of `key`. diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 3829b0de..94e1964d 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -110,7 +110,7 @@ fn prop_store_ops_match_model() { // Init the store under test let temp_dir = TempDir::new().unwrap(); - let ref mut store_fs = FsStore::new(&temp_dir, StdFiler).expect("FsStore::new failed"); + let ref mut store_fs = FsStore::new(&temp_dir, StdFiler); for ref op in store_ops_vec { op.apply(store_model).unwrap(); From 9fcc17daf8f2a590a17e1f666fbdfe21cd962665 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 14:07:30 +0200 Subject: [PATCH 24/29] refactor(rtc_tenclave,kv_store): use associated Error type for KvStore This replaces the interim StoreResult type. --- rtc_tenclave/src/kv_store/fs/mod.rs | 37 +++++++++++++++++--------- rtc_tenclave/src/kv_store/in_memory.rs | 29 +++++++++++++++----- rtc_tenclave/src/kv_store/mod.rs | 15 ++++------- rtc_tenclave/src/kv_store/tests.rs | 7 +++-- 4 files changed, 57 insertions(+), 31 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index c37a84ab..8ef43007 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -18,7 +18,7 @@ use std::path::{Path, PathBuf}; use serde::de::DeserializeOwned; use serde::Serialize; -use super::{KvStore, StoreResult}; +use super::KvStore; /// Simplified interface for reading and writing files. pub trait Filer { @@ -68,14 +68,15 @@ where format!("x{}", encoded) } + // FIXME: Just use a generic String as the error type, for now. #[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests - pub(crate) fn decode_key(file_name: &str) -> StoreResult { + pub(crate) fn decode_key(file_name: &str) -> Result { let encoded: &str = file_name .strip_prefix("x") .ok_or_else(|| format!("FsStore::decode_key: missing x prefix for {:?}", file_name))?; - // FIXME: Dodgy err.to_string() let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; - String::from_utf8(bytes).map_err(|err| err.into()) + let decoded = String::from_utf8(bytes).map_err(|err| err.to_string())?; + Ok(decoded) // } } @@ -84,32 +85,44 @@ where F: Filer, V: Serialize + DeserializeOwned, { - fn load(&self, key: &str) -> StoreResult> { + // XXX: More explicit handling of serde_json::Error? + type Error = io::Error; + + fn load(&self, key: &str) -> Result, Self::Error> { let value_file_name = self.value_path(key); // Note: Read all the data into memory first, then deserialize, for efficiency. // See the docs for [`serde_json::de::from_reader`], // and https://github.com/serde-rs/json/issues/160 - let loaded: Option> = self - .filer - .get(&value_file_name) - .map_err(|err| format!("FsStore: read from {:?} failed: {}", value_file_name, err))?; + let loaded: Option> = self.filer.get(&value_file_name).map_err(|err| { + // XXX: Annotate err with some basic debugging context, for now. + Self::Error::new( + err.kind(), + format!("FsStore: read from {:?} failed: {}", value_file_name, err), + ) + })?; let value: Option = loaded .map(|serialised: Vec| serde_json::from_slice(serialised.as_slice())) .transpose()?; Ok(value) } - fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { + fn save(&mut self, key: &str, value: &V) -> Result<(), Self::Error> { let value_file_name = self.value_path(key); let serialized: Vec = serde_json::to_vec(&value)?; self.filer .put(&value_file_name, serialized) - .map_err(|err| format!("FsStore: write to {:?} failed: {}", value_file_name, err))?; + .map_err(|err| { + // XXX: Annotate err with some basic debugging context, for now. + Self::Error::new( + err.kind(), + format!("FsStore: write to {:?} failed: {}", value_file_name, err), + ) + })?; Ok(()) } - fn delete(&mut self, key: &str) -> StoreResult<()> { + fn delete(&mut self, key: &str) -> Result<(), Self::Error> { let path = self.value_path(key); self.filer.delete(path)?; Ok(()) diff --git a/rtc_tenclave/src/kv_store/in_memory.rs b/rtc_tenclave/src/kv_store/in_memory.rs index ec596653..93122353 100644 --- a/rtc_tenclave/src/kv_store/in_memory.rs +++ b/rtc_tenclave/src/kv_store/in_memory.rs @@ -5,7 +5,7 @@ use serde::Serialize; use std::collections::HashMap; use std::prelude::v1::*; -use super::{KvStore, StoreResult}; +use super::KvStore; /// In-memory [`KvStore`] using [`HashMap`] #[derive(Default)] @@ -17,16 +17,18 @@ impl KvStore for InMemoryStore where V: Clone, { - fn load(&self, key: &str) -> StoreResult> { + type Error = Never; + + fn load(&self, key: &str) -> Result, Self::Error> { Ok(self.map.get(key).cloned()) } - fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { + fn save(&mut self, key: &str, value: &V) -> Result<(), Self::Error> { self.map.insert(key.to_string(), value.clone()); Ok(()) } - fn delete(&mut self, key: &str) -> StoreResult<()> { + fn delete(&mut self, key: &str) -> Result<(), Self::Error> { self.map.remove(key); Ok(()) } @@ -42,20 +44,33 @@ impl KvStore for InMemoryJsonStore where V: Serialize + DeserializeOwned, { - fn load(&self, key: &str) -> StoreResult> { + type Error = serde_json::Error; + + fn load(&self, key: &str) -> Result, Self::Error> { let loaded: Option<&[u8]> = self.map.get(key).map(|v| v.as_slice()); let deserialized: Option = loaded.map(serde_json::from_slice).transpose()?; Ok(deserialized) } - fn save(&mut self, key: &str, value: &V) -> StoreResult<()> { + fn save(&mut self, key: &str, value: &V) -> Result<(), Self::Error> { let serialized = serde_json::to_vec(&value)?; self.map.insert(key.to_string(), serialized); Ok(()) } - fn delete(&mut self, key: &str) -> StoreResult<()> { + fn delete(&mut self, key: &str) -> Result<(), Self::Error> { self.map.remove(key); Ok(()) } } + +/// TODO: Replace with ! once stabilized. +/// +/// See: +/// +/// * https://doc.rust-lang.org/beta/unstable-book/language-features/never-type.html +/// * https://github.com/rust-lang/rfcs/blob/master/text/1216-bang-type.md +/// * https://github.com/rust-lang/rust/issues/35121 +/// +#[derive(Debug)] +pub enum Never {} diff --git a/rtc_tenclave/src/kv_store/mod.rs b/rtc_tenclave/src/kv_store/mod.rs index 3769648d..6985944e 100644 --- a/rtc_tenclave/src/kv_store/mod.rs +++ b/rtc_tenclave/src/kv_store/mod.rs @@ -1,41 +1,36 @@ //! Simple key-value store abstraction -use std::boxed::Box; -use std::error::Error; - mod fs; mod in_memory; -type StoreResult = Result>; - /// A key-value store. /// /// These methods borrow key and value references, /// to suit cloning / serialising implementations. /// pub trait KvStore { - // TODO: Use associated type for V? + type Error; /// Load the saved value for `key`, if any. /// /// Return [`None`] if `key` has no previous value. /// - fn load(&self, key: &str) -> StoreResult>; + fn load(&self, key: &str) -> Result, Self::Error>; /// Save a new value for `key`. /// /// This will replace any existing value. /// - fn save(&mut self, key: &str, value: &V) -> StoreResult<()>; + fn save(&mut self, key: &str, value: &V) -> Result<(), Self::Error>; /// Delete the saved value for `key`. - fn delete(&mut self, key: &str) -> StoreResult<()>; + fn delete(&mut self, key: &str) -> Result<(), Self::Error>; /// Alter the value of `key`. /// /// This operation is a generalisation of [`Self::load`], [`Self::save`], and [`Self::delete`]. /// - fn alter(&mut self, key: &str, alter_fn: F) -> StoreResult> + fn alter(&mut self, key: &str, alter_fn: F) -> Result, Self::Error> where F: FnOnce(Option) -> Option, { diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 94e1964d..7a79f630 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -12,7 +12,7 @@ use tempfile::TempDir; use super::fs::{std_filer::StdFiler, FsStore}; use super::in_memory::{InMemoryJsonStore, InMemoryStore}; use super::inspect::InspectStore; -use super::{KvStore, StoreResult}; +use super::KvStore; /// Verify that executing a sequence of store operations matches a simple model. #[test] @@ -32,7 +32,10 @@ fn prop_store_ops_match_model() { use StoreOp::*; impl StoreOp { /// Apply operation, and also check some invariants. - fn apply(&self, store: &mut impl KvStore) -> StoreResult<()> { + fn apply(&self, store: &mut S) -> Result<(), S::Error> + where + S: KvStore, + { match self { Save { key, value } => { store.save(key, value)?; From b772c56d82b55d2b03a1a0cb93a8fc890a19d04d Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 14:25:37 +0200 Subject: [PATCH 25/29] refactor(rtc_tenclave): drop use of feature(impl_trait_in_bindings) Leave the signatures in place as comments, for now, as a reading aid. --- rtc_tenclave/src/kv_store/fs/inspect.rs | 4 ++-- rtc_tenclave/src/lib.rs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/inspect.rs b/rtc_tenclave/src/kv_store/fs/inspect.rs index 468e866c..9b3f4237 100644 --- a/rtc_tenclave/src/kv_store/fs/inspect.rs +++ b/rtc_tenclave/src/kv_store/fs/inspect.rs @@ -21,12 +21,12 @@ where V: Serialize + DeserializeOwned, { fn to_map(&self) -> HashMap { - let entries: impl Iterator> = self + let entries /* impl Iterator> */ = self .root_dir .read_dir() .unwrap_or_else(|_| panic!("read_dir {:?} failed", self.root_dir)); - let keys: impl Iterator = entries.map(|entry: io::Result| { + let keys /* impl Iterator */ = entries.map(|entry: io::Result| { let entry: DirEntry = entry.expect("read_dir entry failed"); let file_path: PathBuf = entry.path(); let os_file_name: &OsStr = file_path diff --git a/rtc_tenclave/src/lib.rs b/rtc_tenclave/src/lib.rs index 9c4dddcd..4cfb33f7 100644 --- a/rtc_tenclave/src/lib.rs +++ b/rtc_tenclave/src/lib.rs @@ -6,7 +6,6 @@ #![feature(const_evaluatable_checked)] #![deny(clippy::mem_forget)] #![cfg_attr(not(test), no_std)] -#![feature(impl_trait_in_bindings)] #[cfg(not(test))] #[macro_use] From ff889dd472d5dc83d3da8bde78b9c568ca7749c5 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 14:34:19 +0200 Subject: [PATCH 26/29] style(rtc_tenclave,kv_store): clippy(toplevel_ref_arg) https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg --- rtc_tenclave/src/kv_store/tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rtc_tenclave/src/kv_store/tests.rs b/rtc_tenclave/src/kv_store/tests.rs index 7a79f630..14a4b45f 100644 --- a/rtc_tenclave/src/kv_store/tests.rs +++ b/rtc_tenclave/src/kv_store/tests.rs @@ -108,12 +108,12 @@ fn prop_store_ops_match_model() { fn test(store_ops_vec: Vec) -> TestCaseResult { // Init the models - let ref mut store_model = InMemoryStore::default(); - let ref mut store_model_json = InMemoryJsonStore::default(); + let store_model = &mut InMemoryStore::default(); + let store_model_json = &mut InMemoryJsonStore::default(); // Init the store under test let temp_dir = TempDir::new().unwrap(); - let ref mut store_fs = FsStore::new(&temp_dir, StdFiler); + let store_fs = &mut FsStore::new(&temp_dir, StdFiler); for ref op in store_ops_vec { op.apply(store_model).unwrap(); From 2b1122bd0a0c66a6145235bda8ee71e06d460e89 Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 14:40:46 +0200 Subject: [PATCH 27/29] style(rtc_tenclave,kv_store): clippy(unit_cmp) https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp --- rtc_tenclave/src/kv_store/fs/std_filer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/std_filer.rs b/rtc_tenclave/src/kv_store/fs/std_filer.rs index 92f059d1..587f53fc 100644 --- a/rtc_tenclave/src/kv_store/fs/std_filer.rs +++ b/rtc_tenclave/src/kv_store/fs/std_filer.rs @@ -91,7 +91,7 @@ mod tests { fn delete_missing() { with_temp_path(|path: &Path| { assert!(!path.exists()); - assert_eq!(StdFiler.delete(path).unwrap(), ()); + assert!(StdFiler.delete(path).is_ok()); assert!(!path.exists()); }) } @@ -102,7 +102,7 @@ mod tests { StdFiler.put(path, "spam").unwrap(); assert_eq!(StdFiler.get(path).unwrap().unwrap(), "spam".as_bytes()); assert!(path.exists()); - assert_eq!(StdFiler.delete(path).unwrap(), ()); + assert!(StdFiler.delete(path).is_ok()); assert!(!path.exists()); }) } From ef3313b6e4a8e352975ff30816793d759aa8f88f Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 18:56:15 +0200 Subject: [PATCH 28/29] refactor(rtc_tenclave,kv_store): pull out encode_to_fs_safe / decode_from_fs_safe --- rtc_tenclave/src/kv_store/fs/inspect.rs | 3 +- rtc_tenclave/src/kv_store/fs/mod.rs | 38 ++++++++++++------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/rtc_tenclave/src/kv_store/fs/inspect.rs b/rtc_tenclave/src/kv_store/fs/inspect.rs index 9b3f4237..f49ccb06 100644 --- a/rtc_tenclave/src/kv_store/fs/inspect.rs +++ b/rtc_tenclave/src/kv_store/fs/inspect.rs @@ -9,6 +9,7 @@ use std::iter::Iterator; use serde::de::DeserializeOwned; use serde::Serialize; +use crate::kv_store::fs::decode_from_fs_safe; use crate::kv_store::inspect::InspectStore; use crate::kv_store::KvStore; @@ -33,7 +34,7 @@ where .file_name() .unwrap_or_else(|| panic!("directory entry lacks file_name: {:?}", file_path)); let file_name: &str = os_file_name.to_str().expect("OsStr.to_str failed"); - Self::decode_key(file_name).expect("FsStore::decode_key failed") + decode_from_fs_safe(file_name).expect("decode_from_fs_safe failed") }); keys.map(|k| { diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index 8ef43007..29b8bbe0 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -57,27 +57,9 @@ where /// Resolve file name for the value of `key`. fn value_path(&self, key: &str) -> PathBuf { - let file_name = Self::encode_key(key); + let file_name = encode_to_fs_safe(key); self.root_dir.join(file_name) } - - // Make keys filesystem-safe using hex (conservative, but effective): - - pub(crate) fn encode_key(key: &str) -> String { - let encoded = hex::encode(key); - format!("x{}", encoded) - } - - // FIXME: Just use a generic String as the error type, for now. - #[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests - pub(crate) fn decode_key(file_name: &str) -> Result { - let encoded: &str = file_name - .strip_prefix("x") - .ok_or_else(|| format!("FsStore::decode_key: missing x prefix for {:?}", file_name))?; - let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; - let decoded = String::from_utf8(bytes).map_err(|err| err.to_string())?; - Ok(decoded) // - } } impl KvStore for FsStore @@ -129,5 +111,23 @@ where } } +/// Helper: Make `key` filesystem-safe. +pub(crate) fn encode_to_fs_safe(key: &str) -> String { + let encoded = hex::encode(key); + format!("x{}", encoded) +} + +/// Inverse of [`encode_to_fs_safe`]. +// FIXME: Just use a generic String as the error type, for now. +#[cfg_attr(not(test), allow(dead_code))] // currently only referenced in tests +pub(crate) fn decode_from_fs_safe(file_name: &str) -> Result { + let encoded: &str = file_name + .strip_prefix("x") + .ok_or_else(|| format!("decode_from_fs_safe: missing x prefix for {:?}", file_name))?; + let bytes: Vec = hex::decode(encoded).map_err(|err| err.to_string())?; + let decoded = String::from_utf8(bytes).map_err(|err| err.to_string())?; + Ok(decoded) +} + #[cfg(test)] mod inspect; From 1d53fa405d7d0345eff85d0ee27a7368fd8054fd Mon Sep 17 00:00:00 2001 From: Pi Delport Date: Thu, 20 May 2021 18:57:56 +0200 Subject: [PATCH 29/29] test(rtc_tenclave,kv_store): round-trip encode_to_fs_safe and decode_from_fs_safe --- rtc_tenclave/src/kv_store/fs/mod.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/rtc_tenclave/src/kv_store/fs/mod.rs b/rtc_tenclave/src/kv_store/fs/mod.rs index 29b8bbe0..c199a6f6 100644 --- a/rtc_tenclave/src/kv_store/fs/mod.rs +++ b/rtc_tenclave/src/kv_store/fs/mod.rs @@ -131,3 +131,32 @@ pub(crate) fn decode_from_fs_safe(file_name: &str) -> Result { #[cfg(test)] mod inspect; + +#[cfg(test)] +mod tests { + use proptest::prelude::*; + + use super::decode_from_fs_safe; + use super::encode_to_fs_safe; + + /// [`encode_to_fs_safe`] encodes to filesystem-safe, and [`decode_from_fs_safe`] round-trips. + #[test] + fn prop_fs_safe_roundtrip() { + let test = |key: &String| { + let encoded = &encode_to_fs_safe(key); + assert!( + is_fs_safe(encoded), + "expected filesystem-safe, got encoded = {:?}", + encoded + ); + let decoded = &decode_from_fs_safe(encoded).unwrap(); + assert_eq!(key, decoded); + }; + proptest!(|(key in ".*")| test(&key)); + } + + /// Helper: Very conservative definition of filesystem-safe. + fn is_fs_safe(encoded: &str) -> bool { + !encoded.is_empty() && encoded.chars().all(|c| c.is_ascii_alphanumeric()) + } +}