feat(user-auth): initial implementation #1

jdvlio · 2023-01-13T11:11:17Z

Initial implementation of the user authentication enclave

user-auth/README.md

tshepang · 2023-01-24T11:43:59Z

user-auth/README.md

+|   |-- Enclave.lds
+|   |-- Makefile
+|   |-- Xargo.toml
+|   +-- rust-toolchain


same here, to avoid duplicating this info

This is a draft only. The final version will look different.

tshepang · 2023-01-24T11:49:39Z

General note: copyright blurbs are not needed at the top of the source files, because LICENSE (at the repo root) should cover it. See Rust project (rust-lang/rust) as an example.

tshepang · 2023-01-26T09:28:04Z

I see that we actually have a mix of licenses, something to be clear about (what is the criteria?).

As a sidenote, it looks like one can create AGPL software that depends (via static or dynamic linking) on Apache 2 software.

jdvlio · 2023-01-26T10:19:19Z

General note: copyright blurbs are not needed at the top of the source files, because LICENSE (at the repo root) should cover it. See Rust project (rust-lang/rust) as an example.

In this case yes but this is not true in general. Free software licenses, even permissive ones. virtually always require that one preserve any copyright notices. See for example section 4 of the Apache-2.0 license.

tshepang · 2023-01-26T10:45:50Z

ah, good to know

user-auth/enclave/src/lib.rs

user-auth/enclave/Cargo.toml

user-auth/enclave/src/lib.rs

tshepang · 2023-02-15T10:38:36Z

user-auth/enclave/src/lib.rs

@@ -0,0 +1,161 @@
+#![no_std]


perhaps redundant, since std will result in build errors anyways

unexcellent thought... ignore me

user-auth/app/src/ecall.rs

user-auth/app/src/lib.rs

user-auth/app/src/ecall.rs

tshepang · 2023-02-16T11:33:37Z

user-auth/enclave/src/lib.rs

+fn new_hash_context<'a>(pepper: &'a [u8]) -> Result<Argon2<'a>, argon2::Error> {
+    Argon2::<'a>::new_with_secret(


lifetime could be elided

tshepang · 2023-02-16T14:35:26Z

user-auth/enclave/src/lib.rs

+    let hash_ctx = new_hash_context(pepper.as_ref());
+
+    match hash_ctx {
+        Err(_) => sgx_status_t::SGX_ERROR_UNEXPECTED,


would be kool to display the error value (perhaps with eprintln! OCALL), of if it really is not useful, use an if let Ok(ctx) to reduce right-ward drift (and perhaps improve readability)

jdvlio · 2023-02-17T08:40:33Z

user-auth/enclave/src/lib.rs

+// Convert the Argon2 memory cost from MiB to KiB
+const fn m_cost_calc(mebibytes: u32) -> {
+    mebibytes * 1024
+}


Just do this in a constant called M_COST_KIB and remove M_COST_MIB. Much simpler.

jdvlio · 2023-02-17T08:41:28Z

user-auth/enclave/src/lib.rs

+/// dynammically calculated!
+// XXX: Certain assumptions are made here that relate to the hash context. See
+// the doc comment above.
+pub const HASH_STRING_LENGTH: usize = 97;


Link the PHC specification

tshepang · 2023-03-07T06:04:13Z

common/Cargo.toml

@@ -3,4 +3,5 @@
 [workspace]
 members = [
 	"sealing",
+	"user-auth"


Suggested change

"user-auth"

"user-auth",

make diffs more pleasant to read :)

tshepang · 2023-03-07T06:06:56Z

common/user-auth/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "user-auth-common"
+version = "0.1.0"


nit: I like leaving this at 0.0.0 until we actually make releases, where it actually matters

tshepang · 2023-03-07T06:08:01Z

common/user-auth/Cargo.toml

+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]


nit: noise :)

Suggested change

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

tshepang · 2023-03-07T06:08:54Z

common/user-auth/target/.rustc_info.json

@@ -0,0 +1 @@
+{"rustc_fingerprint":14295617405981936619,"outputs":{"15697416045686424142":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n","stderr":""},"10376369925670944939":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n/home/jean-pierre/.rustup/toolchains/nightly-2022-10-22-x86_64-unknown-linux-gnu\ndebug_assertions\npanic=\"unwind\"\nproc_macro\ntarget_abi=\"\"\ntarget_arch=\"x86_64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"unix\"\ntarget_feature=\"fxsr\"\ntarget_feature=\"llvm14-builtins-abi\"\ntarget_feature=\"sse\"\ntarget_feature=\"sse2\"\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_has_atomic_equal_alignment=\"16\"\ntarget_has_atomic_equal_alignment=\"32\"\ntarget_has_atomic_equal_alignment=\"64\"\ntarget_has_atomic_equal_alignment=\"8\"\ntarget_has_atomic_equal_alignment=\"ptr\"\ntarget_has_atomic_load_store=\"16\"\ntarget_has_atomic_load_store=\"32\"\ntarget_has_atomic_load_store=\"64\"\ntarget_has_atomic_load_store=\"8\"\ntarget_has_atomic_load_store=\"ptr\"\ntarget_os=\"linux\"\ntarget_pointer_width=\"64\"\ntarget_thread_local\ntarget_vendor=\"unknown\"\nunix\n","stderr":""},"4614504638168534921":{"success":true,"status":"","code":0,"stdout":"rustc 1.66.0-nightly (5c8bff74b 2022-10-21)\nbinary: rustc\ncommit-hash: 5c8bff74bc1c52bef0c79f3689bb227f51f3e82d\ncommit-date: 2022-10-21\nhost: x86_64-unknown-linux-gnu\nrelease: 1.66.0-nightly\nLLVM version: 15.0.2\n","stderr":""}},"successes":{}}


hm, somehow .gitignore was ignored :)

tshepang · 2023-03-07T06:11:12Z

user-auth/app/Cargo.toml

+password-hash = { version = "0.4", features = ["std"] }
+argon2 = { version = "0.4", features = ["rand"] }
+
+[patch.'https://github.com/apache/teaclave-sgx-sdk.git']


unused patch

tshepang · 2023-03-07T06:19:13Z

user-auth/app/src/ecall.rs

+    match verify_status {
+        VerifyPasswordStatus::InvalidPassword => Ok(VerifyPasswordStatus::InvalidPassword),
+        VerifyPasswordStatus::PasswordVerified => Ok(VerifyPasswordStatus::PasswordVerified),
+    }


Suggested change

match verify_status {

VerifyPasswordStatus::InvalidPassword => Ok(VerifyPasswordStatus::InvalidPassword),

VerifyPasswordStatus::PasswordVerified => Ok(VerifyPasswordStatus::PasswordVerified),

}

Ok(verify_status)

tshepang · 2023-03-07T06:19:55Z

user-auth/app/src/lib.rs

+    user_auth_sgx,
+    InvalidHashString,
+    PyException,
+    "supplied hash is not a valid PHC string"


Suggested change

"supplied hash is not a valid PHC string"

"supplied hash is not a valid PHC string",

tshepang · 2023-03-07T06:22:09Z

user-auth/enclave/Enclave.config.xml

+  <ProdID>0</ProdID>
+  <ISVSVN>0</ISVSVN>
+  <StackMaxSize>0x40000</StackMaxSize>
+  <HeapMaxSize>0x1600000</HeapMaxSize>


would be kool to explan here why this was increased

tshepang · 2023-03-07T06:23:08Z

user-auth/enclave/src/lib.rs

@@ -0,0 +1,161 @@
+#![no_std]


unexcellent thought... ignore me

tshepang · 2023-03-07T06:28:00Z

user-auth/enclave/src/lib.rs

+                unsafe { *verify_status = VerifyPasswordStatus::InvalidPassword };
+                sgx_status_t::SGX_SUCCESS
+            }
+            Err(_) => sgx_status_t::SGX_ERROR_UNEXPECTED,


I would at least do eprintln! ocall, or debugging this may get unpleasant, requiring a re-compile. We can also do a TODO note, so we don't forget to look at it again when it gets to production.

jdvlio requested review from sonasi and tshepang January 13, 2023 11:11

jdvlio force-pushed the feat-initial-user-auth-enclave branch 6 times, most recently from be2452a to 906fb31 Compare January 17, 2023 11:15

jdvlio force-pushed the feat-initial-user-auth-enclave branch 8 times, most recently from 473784e to 45b9074 Compare January 24, 2023 10:34

tshepang suggested changes Jan 24, 2023

View reviewed changes

jdvlio force-pushed the feat-initial-user-auth-enclave branch from 45b9074 to e084035 Compare January 26, 2023 08:01

jdvlio force-pushed the feat-initial-user-auth-enclave branch from e084035 to 391c07d Compare January 26, 2023 10:06

jdvlio force-pushed the feat-initial-user-auth-enclave branch from 391c07d to fb2d3b6 Compare January 26, 2023 10:19

jdvlio requested a review from tshepang January 26, 2023 10:35

jdvlio force-pushed the feat-initial-user-auth-enclave branch from fb2d3b6 to 15d4de7 Compare January 26, 2023 10:37

jdvlio force-pushed the feat-initial-user-auth-enclave branch 3 times, most recently from fd94cf6 to 26bd60a Compare January 26, 2023 13:16

tshepang mentioned this pull request Jan 26, 2023

docs: record some setup steps #2

Closed

jdvlio force-pushed the feat-initial-user-auth-enclave branch 5 times, most recently from f23ded4 to 259933f Compare January 31, 2023 09:47

jdvlio changed the base branch from main to doc-clarify-repo-purpose January 31, 2023 09:48

jdvlio requested review from IscoRuta98 and tshepang January 31, 2023 09:58

jdvlio force-pushed the feat-initial-user-auth-enclave branch from 259933f to 4c7fd4d Compare February 2, 2023 09:42

tshepang reviewed Feb 3, 2023

View reviewed changes

user-auth/enclave/src/lib.rs Outdated Show resolved Hide resolved

jdvlio force-pushed the feat-initial-user-auth-enclave branch from 4c7fd4d to 5261174 Compare February 8, 2023 07:41

Base automatically changed from doc-clarify-repo-purpose to main February 11, 2023 00:18

tshepang marked this pull request as ready for review February 14, 2023 00:24

sonasi requested a review from tshepang February 14, 2023 13:17

tshepang reviewed Feb 15, 2023

View reviewed changes

user-auth/app/src/ecall.rs Outdated Show resolved Hide resolved

tshepang reviewed Feb 15, 2023

View reviewed changes

user-auth/app/src/ecall.rs Outdated Show resolved Hide resolved

tshepang reviewed Feb 16, 2023

View reviewed changes

jdvlio commented Feb 17, 2023

View reviewed changes

jdvlio added 4 commits March 6, 2023 17:43

feat(user-auth): initial implementation

6141311

bump(user-auth): sgx-keys revision`

1abf2b4

chore: simplify Argon2 parameters

07bd131

chore(user-auth): factor out common code

fea6380

jdvlio force-pushed the feat-initial-user-auth-enclave branch from 5261174 to fea6380 Compare March 6, 2023 15:46

jdvlio added 3 commits March 6, 2023 18:01

chore(common): add user-auth to manifest

eb4f750

refactor(user-auth): correct constant naming

fafea8f

chore(user-auth): lifetime can be elided

b4e5420

tshepang suggested changes Mar 7, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(user-auth): initial implementation #1

feat(user-auth): initial implementation #1

jdvlio commented Jan 13, 2023

tshepang Jan 24, 2023

jdvlio Jan 26, 2023

tshepang commented Jan 24, 2023

tshepang commented Jan 26, 2023

jdvlio commented Jan 26, 2023 •

edited

Loading

tshepang commented Jan 26, 2023

tshepang Feb 15, 2023

tshepang Mar 7, 2023

tshepang Feb 16, 2023

tshepang Feb 16, 2023

jdvlio Feb 17, 2023

jdvlio Feb 17, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

tshepang Mar 7, 2023

		fn new_hash_context<'a>(pepper: &'a [u8]) -> Result<Argon2<'a>, argon2::Error> {
		Argon2::<'a>::new_with_secret(


		# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

		[dependencies]

		@@ -0,0 +1 @@
		{"rustc_fingerprint":14295617405981936619,"outputs":{"15697416045686424142":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n","stderr":""},"10376369925670944939":{"success":true,"status":"","code":0,"stdout":"___\nlib___.rlib\nlib___.so\nlib___.so\nlib___.a\nlib___.so\n/home/jean-pierre/.rustup/toolchains/nightly-2022-10-22-x86_64-unknown-linux-gnu\ndebug_assertions\npanic=\"unwind\"\nproc_macro\ntarget_abi=\"\"\ntarget_arch=\"x86_64\"\ntarget_endian=\"little\"\ntarget_env=\"gnu\"\ntarget_family=\"unix\"\ntarget_feature=\"fxsr\"\ntarget_feature=\"llvm14-builtins-abi\"\ntarget_feature=\"sse\"\ntarget_feature=\"sse2\"\ntarget_has_atomic=\"16\"\ntarget_has_atomic=\"32\"\ntarget_has_atomic=\"64\"\ntarget_has_atomic=\"8\"\ntarget_has_atomic=\"ptr\"\ntarget_has_atomic_equal_alignment=\"16\"\ntarget_has_atomic_equal_alignment=\"32\"\ntarget_has_atomic_equal_alignment=\"64\"\ntarget_has_atomic_equal_alignment=\"8\"\ntarget_has_atomic_equal_alignment=\"ptr\"\ntarget_has_atomic_load_store=\"16\"\ntarget_has_atomic_load_store=\"32\"\ntarget_has_atomic_load_store=\"64\"\ntarget_has_atomic_load_store=\"8\"\ntarget_has_atomic_load_store=\"ptr\"\ntarget_os=\"linux\"\ntarget_pointer_width=\"64\"\ntarget_thread_local\ntarget_vendor=\"unknown\"\nunix\n","stderr":""},"4614504638168534921":{"success":true,"status":"","code":0,"stdout":"rustc 1.66.0-nightly (5c8bff74b 2022-10-21)\nbinary: rustc\ncommit-hash: 5c8bff74bc1c52bef0c79f3689bb227f51f3e82d\ncommit-date: 2022-10-21\nhost: x86_64-unknown-linux-gnu\nrelease: 1.66.0-nightly\nLLVM version: 15.0.2\n","stderr":""}},"successes":{}}

	"supplied hash is not a valid PHC string"
	"supplied hash is not a valid PHC string",

feat(user-auth): initial implementation #1

Are you sure you want to change the base?

feat(user-auth): initial implementation #1

Conversation

jdvlio commented Jan 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tshepang commented Jan 24, 2023

tshepang commented Jan 26, 2023

jdvlio commented Jan 26, 2023 • edited Loading

tshepang commented Jan 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdvlio commented Jan 26, 2023 •

edited

Loading