Skip to content

Commit

Permalink
fix!(argon2): respect the salt provided in hash options (#899)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brooooooklyn authored Oct 4, 2024
1 parent fea946f commit 52c4553
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 73 deletions.
2 changes: 1 addition & 1 deletion depracted/deno-lint/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"typanion": "^3.14.0"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"@types/webpack": "^5.28.5"
},
"funding": {
Expand Down
18 changes: 0 additions & 18 deletions packages/argon2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,24 +27,6 @@ It has a simple design aimed at the highest memory filling rate and effective us
Hybrid that mixes Argon2i and Argon2d passes.
Uses the Argon2i approach for the first half pass over memory and Argon2d approach for subsequent passes. This effectively places it in the “middle” between the other two: it doesn’t provide as good TMTO/GPU cracking resistance as Argon2d, nor as good of side-channel resistance as Argon2i, but overall provides the most well-rounded approach to both classes of attacks.

## Support matrix

| | node12 | node14 | node16 | node18 |
| ------------------- | ------ | ------ | ------ | ------ |
| Windows x64 |||||
| Windows x32 |||||
| Windows arm64 |||||
| macOS x64 |||||
| macOS arm64(m chip) |||||
| Linux x64 gnu |||||
| Linux x64 musl |||||
| Linux arm gnu |||||
| Linux arm64 gnu |||||
| Linux arm64 musl |||||
| Android arm64 |||||
| Android armv7 |||||
| FreeBSD x64 |||||

# Benchmarks

See [benchmark/](benchmark/argon2.js).
Expand Down
26 changes: 25 additions & 1 deletion packages/argon2/__test__/argon2.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { randomBytes } from 'crypto'
import { randomBytes } from 'node:crypto'

import test from 'ava'

Expand Down Expand Up @@ -43,6 +43,30 @@ test('should be able to hash string', async (t) => {
)
})

test('should be able to hash string with a defined salt', async (t) => {
await t.notThrowsAsync(() =>
hash('whatever', {
salt: randomBytes(32),
}),
)
await t.notThrowsAsync(() =>
hash('whatever', {
secret: randomBytes(32),
salt: randomBytes(32),
}),
)

const salt = randomBytes(32)
t.is(
await hash('whatever', {
salt,
}),
await hash('whatever', {
salt,
}),
)
})

test('should be able to hashRaw string with a defined salt', async (t) => {
await t.notThrowsAsync(() => hash('whatever'))
await t.notThrowsAsync(() =>
Expand Down
64 changes: 30 additions & 34 deletions packages/argon2/benchmark/argon2.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,38 @@
const { cpus } = require('os')
import { cpus } from 'node:os'

const nodeArgon2 = require('argon2')
const { Suite } = require('benchmark')
const chalk = require('chalk')
import nodeArgon2 from 'argon2'
import { Bench } from 'tinybench'

const { hash, verify, Algorithm } = require('../index')
import { hash, verify, Algorithm } from '../index.js'

const PASSWORD = '$v=19$m=4096,t=3,p=1$fyLYvmzgpBjDTP6QSypj3g$pb1Q3Urv1amxuFft0rGwKfEuZPhURRDV7TJqcBnwlGo'
const CORES = cpus().length

const suite = new Suite('Hash with all cores')

suite
.add(
'@node-rs/argon',
async (deferred) => {
await hash(PASSWORD, {
algorithm: Algorithm.Argon2id,
parallelism: CORES,
})
deferred.resolve()
},
{ defer: true },
)
.add(
'node-argon',
async (deferred) => {
await nodeArgon2.hash(PASSWORD, { type: nodeArgon2.argon2id, parallelism: CORES })
deferred.resolve()
},
{
defer: true,
},
)
.on('cycle', function (event) {
console.info(String(event.target))
const HASHED = await hash(PASSWORD, {
algorithm: Algorithm.Argon2id,
parallelism: CORES,
})

const bench = new Bench('Hash with all cores')

bench
.add('@node-rs/argon hash', async () => {
await hash(PASSWORD, {
algorithm: Algorithm.Argon2id,
parallelism: CORES,
})
})
.add('node-argon hash', async () => {
await nodeArgon2.hash(PASSWORD, { type: nodeArgon2.argon2id, parallelism: CORES })
})
.on('complete', function () {
console.info(`${this.name} bench suite: Fastest is ${chalk.green(this.filter('fastest').map('name'))}`)
.add('@node-rs/argon verify', async () => {
console.assert(await verify(HASHED, PASSWORD))
})
.run()
.add('node-argon verify', async () => {
console.assert(await nodeArgon2.verify(HASHED, PASSWORD))
})

await bench.warmup()
await bench.run()

console.table(bench.table())
3 changes: 3 additions & 0 deletions packages/argon2/benchmark/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "module"
}
5 changes: 3 additions & 2 deletions packages/argon2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@
"version": "napi version && git add npm"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"argon2": "^0.41.0",
"cross-env": "^7.0.3"
"cross-env": "^7.0.3",
"tinybench": "^2.9.0"
}
}
21 changes: 16 additions & 5 deletions packages/argon2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,18 @@ impl Task for HashTask {
type JsValue = String;

fn compute(&mut self) -> Result<Self::Output> {
let salt = SaltString::generate(&mut OsRng);
let salt = if let Some(salt) = &self.options.salt {
SaltString::encode_b64(salt)
.map_err(|err| Error::new(Status::InvalidArg, format!("{err}")))?
} else {
SaltString::generate(&mut OsRng)
};
let hasher = self
.options
.to_argon()
.map_err(|err| Error::new(Status::InvalidArg, format!("{err}")))?;

let hasher = self.options.to_argon();
hasher
.map_err(|err| Error::new(Status::InvalidArg, format!("{err}")))?
.hash_password(self.password.as_slice(), &salt)
.map_err(|err| Error::new(Status::GenericFailure, format!("{err}")))
.map(|h| h.to_string())
Expand Down Expand Up @@ -263,8 +270,12 @@ impl Task for VerifyTask {
type JsValue = bool;

fn compute(&mut self) -> Result<Self::Output> {
let parsed_hash = argon2::PasswordHash::new(self.hashed.as_str())
.map_err(|err| Error::new(Status::InvalidArg, format!("{err}")))?;
let parsed_hash = argon2::PasswordHash::new(self.hashed.as_str()).map_err(|err| {
Error::new(
Status::InvalidArg,
format!("Invalid hashed password: {err}"),
)
})?;
let argon2 = self.options.to_argon();
Ok(
argon2
Expand Down
2 changes: 1 addition & 1 deletion packages/bcrypt/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"url": "https://github.com/napi-rs/node-rs/issues"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"@types/bcrypt": "^5.0.2",
"bcrypt": "^5.1.1",
"bcryptjs": "^2.4.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/crc32/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"url": "https://github.com/napi-rs/node-rs/issues"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"@types/crc": "^3.8.3",
"buffer": "^6.0.3",
"crc": "^4.3.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/jieba/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"url": "https://github.com/napi-rs/node-rs/issues"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"nodejieba": "^3.0.0"
},
"funding": {
Expand Down
2 changes: 1 addition & 1 deletion packages/jsonwebtoken/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
"node": ">= 10"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"@types/jsonwebtoken": "^9.0.6",
"jsonwebtoken": "^9.0.2"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/xxhash/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"url": "https://github.com/napi-rs/node-rs/issues"
},
"devDependencies": {
"@napi-rs/cli": "^3.0.0-alpha.54",
"@napi-rs/cli": "^3.0.0-alpha.63",
"@types/xxhashjs": "^0.2.4",
"webpack": "^5.92.1",
"xxhash": "^0.3.0",
Expand Down
22 changes: 15 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ __metadata:
languageName: node
linkType: hard

"@napi-rs/cli@npm:^3.0.0-alpha.54, @napi-rs/cli@npm:^3.0.0-alpha.63":
"@napi-rs/cli@npm:^3.0.0-alpha.63":
version: 3.0.0-alpha.63
resolution: "@napi-rs/cli@npm:3.0.0-alpha.63"
dependencies:
Expand Down Expand Up @@ -1024,17 +1024,18 @@ __metadata:
version: 0.0.0-use.local
resolution: "@node-rs/argon2@workspace:packages/argon2"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
argon2: "npm:^0.41.0"
cross-env: "npm:^7.0.3"
tinybench: "npm:^2.9.0"
languageName: unknown
linkType: soft

"@node-rs/bcrypt@workspace:packages/bcrypt":
version: 0.0.0-use.local
resolution: "@node-rs/bcrypt@workspace:packages/bcrypt"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
"@types/bcrypt": "npm:^5.0.2"
bcrypt: "npm:^5.1.1"
bcryptjs: "npm:^2.4.3"
Expand All @@ -1046,7 +1047,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@node-rs/crc32@workspace:packages/crc32"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
"@types/crc": "npm:^3.8.3"
buffer: "npm:^6.0.3"
crc: "npm:^4.3.2"
Expand All @@ -1066,7 +1067,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@node-rs/jieba@workspace:packages/jieba"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
nodejieba: "npm:^3.0.0"
languageName: unknown
linkType: soft
Expand All @@ -1075,7 +1076,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@node-rs/jsonwebtoken@workspace:packages/jsonwebtoken"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
"@types/jsonwebtoken": "npm:^9.0.6"
jsonwebtoken: "npm:^9.0.2"
languageName: unknown
Expand All @@ -1085,7 +1086,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@node-rs/xxhash@workspace:packages/xxhash"
dependencies:
"@napi-rs/cli": "npm:^3.0.0-alpha.54"
"@napi-rs/cli": "npm:^3.0.0-alpha.63"
"@types/xxhashjs": "npm:^0.2.4"
webpack: "npm:^5.92.1"
xxhash: "npm:^0.3.0"
Expand Down Expand Up @@ -8246,6 +8247,13 @@ __metadata:
languageName: node
linkType: hard

"tinybench@npm:^2.9.0":
version: 2.9.0
resolution: "tinybench@npm:2.9.0"
checksum: 10c0/c3500b0f60d2eb8db65250afe750b66d51623057ee88720b7f064894a6cb7eb93360ca824a60a31ab16dab30c7b1f06efe0795b352e37914a9d4bad86386a20c
languageName: node
linkType: hard

"tmp@npm:^0.0.33":
version: 0.0.33
resolution: "tmp@npm:0.0.33"
Expand Down

0 comments on commit 52c4553

Please sign in to comment.