Skip to content

Commit 51392af

Browse files
committed
Address DanLeh's Comments
1) Move to an explicit `free` call. 2) Only do `Argon2id` 3) Use `tCost = 3` rather than 2 per best practices. 4) Lower iteration count from 50 -> 30 and worst case from 4 -> 3
1 parent 2c71f89 commit 51392af

File tree

4 files changed

+32
-49
lines changed

4 files changed

+32
-49
lines changed

JetStreamDriver.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2063,7 +2063,8 @@ const BENCHMARKS = [
20632063
preload: {
20642064
wasmBinary: "./wasm/argon2/build/argon2.wasm"
20652065
},
2066-
iterations: 50,
2066+
iterations: 30,
2067+
worstCaseCount: 3,
20672068
testGroup: WasmGroup,
20682069
deterministicRandom: true,
20692070
}),

wasm/argon2/benchmark.js

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ let passwordStrings = [
2727
'123456',
2828
'p@assw0rd',
2929
'qwerty',
30+
'letmein',
3031
'汉字漢字',
3132
'كلمة المرور',
3233
'Z7ihQxGE93',
@@ -35,28 +36,41 @@ let passwordStrings = [
3536
];
3637

3738
// Emscripten doesn't have a way to manage your pointers for you so these will free them when the wrapper dies.
38-
const mallocRegistry = new FinalizationRegistry((ptr) => Module._free(ptr));
3939

40+
// If/when https://github.com/tc39/proposal-explicit-resource-management becomes a thing we should use that syntax/symbol instead of a dispose call.
4041
class CString {
4142
constructor(string) {
4243
this.ptr = Module.stringToNewUTF8(string);
4344
this.length = Module._strlen(this.ptr);
44-
mallocRegistry.register(this, this.ptr);
45+
}
46+
47+
dispose() {
48+
Module._free(this.ptr);
4549
}
4650
}
4751

4852
class MallocPtr {
4953
constructor(size) {
5054
this.ptr = Module._malloc(size);
5155
this.size = size;
52-
mallocRegistry.register(this, this.ptr);
56+
}
57+
58+
dispose() {
59+
Module._free(this.ptr);
5360
}
5461
}
5562

56-
const tCost = 2;
63+
const tCost = 3;
5764
const mCost = 1024;
5865
const parallelism = 1;
59-
const argon2NumberOfTypes = 2;
66+
// There are three argon2 types (modes), we test all three. See wasm/argon2/include/argon2.h for the enum:
67+
// /* Argon2 primitive type */
68+
// typedef enum Argon2_type {
69+
// Argon2_d = 0,
70+
// Argon2_i = 1,
71+
// Argon2_id = 2
72+
// } argon2_type;
73+
const argon2Type = 2;
6074
const version = 0x13;
6175
const saltLength = 12;
6276

@@ -68,7 +82,7 @@ class Benchmark {
6882
}
6983

7084
for (let i = 0; i < passwordStrings.length; ++i)
71-
this.hashAndVerify(passwordStrings[i], i % argon2NumberOfTypes);
85+
this.hashAndVerify(passwordStrings[i], argon2Type);
7286
}
7387

7488
randomSalt() {
@@ -82,15 +96,20 @@ class Benchmark {
8296
hashAndVerify(password, argon2Type) {
8397
password = new CString(password);
8498
let salt = this.randomSalt();
85-
this.hashBuffer = new MallocPtr(24);
86-
this.encodedBuffer = new MallocPtr(Module._argon2_encodedlen(tCost, mCost, parallelism, saltLength, this.hashBuffer.size, argon2Type) + 1);
99+
let hashBuffer = new MallocPtr(24);
100+
let encodedBuffer = new MallocPtr(Module._argon2_encodedlen(tCost, mCost, parallelism, salt.size, hashBuffer.size, argon2Type) + 1);
87101

88-
let status = Module._argon2_hash(tCost, mCost, parallelism, password.ptr, password.length, salt.ptr, salt.size, this.hashBuffer.ptr, this.hashBuffer.size, this.encodedBuffer.ptr, this.encodedBuffer.size, argon2Type, version);
102+
let status = Module._argon2_hash(tCost, mCost, parallelism, password.ptr, password.length, salt.ptr, salt.size, hashBuffer.ptr, hashBuffer.size, encodedBuffer.ptr, encodedBuffer.size, argon2Type, version);
89103
if (status !== 0)
90104
throw new Error(`argon2_hash exited with status: ${status} (${Module.UTF8ToString(Module._argon2_error_message(status))})`);
91105

92-
status = Module._argon2_verify(this.encodedBuffer.ptr, password.ptr, password.length, argon2Type);
106+
status = Module._argon2_verify(encodedBuffer.ptr, password.ptr, password.length, argon2Type);
93107
if (status !== 0)
94108
throw new Error(`argon2_verify exited with status: ${status} (${Module.UTF8ToString(Module._argon2_error_message(status))})`);
109+
110+
password.dispose();
111+
salt.dispose();
112+
hashBuffer.dispose();
113+
encodedBuffer.dispose();
95114
}
96115
}

wasm/argon2/build-wasm.sh

Lines changed: 0 additions & 37 deletions
This file was deleted.

wasm/argon2/build.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ echo "Built on $(date -u '+%Y-%m-%dT%H:%M:%SZ')" | tee "$BUILD_LOG"
1111
echo "Toolchain versions" | tee -a "$BUILD_LOG"
1212
emcc --version | head -n1 | tee -a "$BUILD_LOG"
1313

14-
# FIXME: Redownload the source if argon2 ever has source updates. At the time of writing it was last changed 5 years ago so this is probably not a high priority.
14+
# FIXME: Redownload the source (from https://github.com/P-H-C/phc-winner-argon2) if argon2 ever has source updates. At the time of writing it was last changed 5 years ago so this is probably not a high priority.
1515
SOURCES=(
1616
src/blake2/blake2b.c
1717

0 commit comments

Comments
 (0)