Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ksf config #127

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ serde = { version = "1.0.169", features = ["derive"] }
serde-wasm-bindgen = "0.6.5"
tsify = { version = "0.4.5", features = ["js"] }
js-sys = "0.3.67"
generic-array = "0.14"

[dev-dependencies]
wasm-bindgen-test = "0.3.40"
Expand Down
87 changes: 87 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,93 @@ pnpm example:client-with-password-reset:dev

## Advanced usage

### Key Stretching

The password input is passed through a key stretching function before being used in the OPRF. The key stretching function is [`argon2id`](https://www.rfc-editor.org/rfc/rfc9106.html). The [OPAQUE protocol](https://www.ietf.org/archive/id/draft-irtf-cfrg-opaque-17.html) defines recommended parameters, but depending on the application these parameters can be adjusted using param `keyStretching` in the `opaque.client.startRegistration` and `opaque.client.startLogin` functions.

Available options are:

#### Recommended (default)

This is the default in case the option is omitted. It is based on the recommendation in the [Configurations section in the OPAQUE protocol](https://www.ietf.org/archive/id/draft-irtf-cfrg-opaque-17.html#name-configurations) with the exception that the memory is set to (2^21)-1 instead of (2^21) since we noticed (2^21) caused it to crash when running the registration in a browser environment.

Parameters:

- Memory: 2^21-1
- Iterations: 1
- Parallelism: 4

```ts
{
keyStretching: "recommended";
}
```

**Note**: The recommended configuration is the most secure but also the slowest. `client.finishRegistration` and `client.finishLogin` each take around 13 seconds to complete on a MacBook Pro M1, 2020, 16 GB Memory.

#### Memory constrained

This option is based on the recommendation for memory-constrained environments in the [Argon2 RFC](https://www.rfc-editor.org/rfc/rfc9106.html#section-4-6.2).

Parameters:

- Memory: 2^16
- Iterations: 3
- Parallelism: 1

```ts
{
keyStretching: "memory-constrained";
}
```

**Note**: This configuration is faster, but less secure. `client.finishRegistration` and `client.finishLogin` each take around 1 seconds to complete on a MacBook Pro M1, 2020, 16 GB Memory.

#### Custom

You can also provide custom parameters for the key stretching function. The parameters are passed directly to the `argon2id` function. In case you provide an invalid configuration, the function will throw an error.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the function will throw an error.

Now that a custom Ksf is possible to specify, it would be great if there was a way for confirming that the Ksf is valid before attempting to use it. I'm sure it would greatly improve the workflow and usage of developers that want to allow some sort of configuration for Ksf parameters in their applications. Its easier to check then use, rather than use, catch error, and recover

This should be fairly easy to do as well. You could just expose build_argon2_ksf, or just create a more friendly named wrapper function that calls it if necessary.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would users be able to choose the parameters or only choose from a set of options? Especially how would this look like in a login scenario since the server would need to return the parameters. Anything other than a hardcoded preset sounds problematic afaik. And if full dynamic is not supported then developers can test the settings upfront without adding and maintaining an API. Does this make sense or do I miss something?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I was thinking like how Bitwarden allows users to edit the kdf parameters used for their account. If they were using this package, they could have an option for recommended, memory constrained, and custom. If custom was chosen, the app would probably want to first confirm that the parameters are valid before allowing users to save them and attempting the re registration process. The app would just send the parameters to their server, which would check them, and then return ok or not. It would make error handling a lot easier since you know that the parameters are what's wrong instead of something else failing with the registration process, and it would be a better experience for the user.

My thought was this would be completely optional though. You don't have to first confirm the parameters before using them, only if you wanted to. We'll still confirm them when using them.

Especially how would this look like in a login scenario since the server would need to return the parameters

It wouldn't alter the current login scenario at all. This would be completely optional, and really only useful if developers allowed their users to edit parameters.

Does that make sense?


```ts
const memory = 65536;
const iterations = 3;
const parallelism = 1;
{
keyStretching: {
"argon2id-custom": {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for keyStretchingFunctionConfig to take a custom config object directly instead of nesting the custom config inside argon2id-custom? If its an object, then we already know that its a custom config. This would be more convenient for users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought not, since it depends on tsify https://crates.io/crates/tsify-next. But just realized I could try with a manual type overwrite. Any ideas if this could work?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm. Yea I'm not seeing anything that stands out, or finding anything while searching. It isn't that necessary and was more of a convenience thing

memory,
iterations,
parallelism,
},
};
}
```

#### Example usage

**Registration**

```ts
// client
opaque.client.finishRegistration({
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});
```

**Login**

```ts
// client
opaque.client.finishLogin({
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});
```

### ExportKey

After the initial registration flow as well as ever login flow, the client has access to a private key only available to the client. This is the `exportKey`. The key is not available to the server and it is stable. Meaning if you log in multiple times your `exportKey` will stay the same.
Expand Down
4 changes: 4 additions & 0 deletions examples/client-simple-vite/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async function register(userIdentifier, password) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/register/finish`, {
Expand Down Expand Up @@ -94,6 +95,7 @@ async function login(userIdentifier, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (!loginResult) {
Expand Down Expand Up @@ -194,6 +196,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
password,
clientRegistrationState,
registrationResponse,
keyStretching: "memory-constrained",
});

console.log({
Expand Down Expand Up @@ -230,6 +233,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (loginResult == null) {
Expand Down
4 changes: 4 additions & 0 deletions examples/client-simple-webpack/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async function register(userIdentifier, password) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/register/finish`, {
Expand Down Expand Up @@ -94,6 +95,7 @@ async function login(userIdentifier, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (!loginResult) {
Expand Down Expand Up @@ -194,6 +196,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
password,
clientRegistrationState,
registrationResponse,
keyStretching: "memory-constrained",
});

console.log({
Expand Down Expand Up @@ -230,6 +233,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (loginResult == null) {
Expand Down
5 changes: 5 additions & 0 deletions examples/client-with-password-reset/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ async function register(userIdentifier, password) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/register/finish`, {
Expand Down Expand Up @@ -115,6 +116,7 @@ async function login(userIdentifier, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (!loginResult) {
Expand Down Expand Up @@ -213,6 +215,7 @@ async function handleSubmitPasswordResetConfirm(e) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/password-reset/confirm-finish`, {
Expand Down Expand Up @@ -326,6 +329,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
password,
clientRegistrationState,
registrationResponse,
keyStretching: "memory-constrained",
});

console.log({
Expand Down Expand Up @@ -362,6 +366,7 @@ function runFullServerClientFlow(serverSetup, username, password) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (loginResult == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export async function register(userIdentifier: string, password: string) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/api/register/finish`, {
Expand All @@ -63,6 +64,7 @@ export async function login(userIdentifier: string, password: string) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});
console.log({ loginResult });
if (!loginResult) {
Expand Down Expand Up @@ -108,6 +110,7 @@ export async function registerRecovery({
clientRegistrationState,
registrationResponse,
password: recoveryKey,
keyStretching: "memory-constrained",
});

const recoveryLockbox = createRecoveryLockbox({
Expand Down Expand Up @@ -139,6 +142,7 @@ export async function loginRecovery(userIdentifier: string, password: string) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});
console.log({ loginResult });
if (!loginResult) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function runFullServerClientFlow(
password,
clientRegistrationState,
registrationResponse,
keyStretching: "memory-constrained",
});

console.log({
Expand Down Expand Up @@ -86,6 +87,7 @@ function runFullServerClientFlow(
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});

if (loginResult == null) {
Expand Down
26 changes: 26 additions & 0 deletions examples/fullstack-simple-nextjs/app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ async function register(userIdentifier: string, password: string) {
clientRegistrationState,
registrationResponse,
password,
keyStretching: "memory-constrained",
});

const res = await request("POST", `/api/register/finish`, {
Expand All @@ -61,6 +62,7 @@ async function login(userIdentifier: string, password: string) {
clientLoginState,
loginResponse,
password,
keyStretching: "memory-constrained",
});
console.log({ loginResult });
if (!loginResult) {
Expand Down Expand Up @@ -235,6 +237,7 @@ function runFullServerClientFlow(
console.log();
console.log("client.finishRegistration");
console.log("------------------------");
const t1 = performance.now();
const {
registrationRecord,
exportKey: clientRegExportKey,
Expand All @@ -243,7 +246,18 @@ function runFullServerClientFlow(
password,
clientRegistrationState,
registrationResponse,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a comment here (and slightly below) explaining why this commented out code is useful, or just remove it if it isn't.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense! will remove it

// keyStretching: "recommended",
// keyStretching: "memory-constrained",
// keyStretching: {
// "argon2id-custom": {
// iterations: 1,
// memory: 65536,
// parallelism: 4,
// },
// },
});
const t2 = performance.now();
console.log("Time taken: ", t2 - t1);

console.log({
clientRegExportKey,
Expand Down Expand Up @@ -275,11 +289,23 @@ function runFullServerClientFlow(
console.log();
console.log("client.finishLogin");
console.log("-----------------");
const t3 = performance.now();
const loginResult = opaque.client.finishLogin({
clientLoginState,
loginResponse,
password,
// keyStretching: "recommended",
// keyStretching: "memory-constrained",
// keyStretching: {
// "argon2id-custom": {
// iterations: 1,
// memory: 65536,
// parallelism: 4,
// },
// },
});
const t4 = performance.now();
console.log("Time taken: ", t4 - t3);

if (loginResult == null) {
console.log("loginResult is NULL; login failed");
Expand Down
Loading
Loading