Page MenuHomePhabricator

[shared] bump opaque-ke to 2.1.0-pre.1
ClosedPublic

Authored by varun on Nov 7 2024, 1:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 19, 5:55 AM
Unknown Object (File)
Tue, Dec 17, 3:10 AM
Unknown Object (File)
Sun, Dec 15, 2:01 AM
Unknown Object (File)
Fri, Dec 13, 11:17 PM
Unknown Object (File)
Fri, Dec 13, 11:17 PM
Unknown Object (File)
Fri, Dec 13, 11:17 PM
Unknown Object (File)
Fri, Dec 13, 11:15 PM
Unknown Object (File)
Fri, Dec 13, 11:09 PM
Subscribers

Details

Summary

This version of opaque-ke uses the latest version of voprf. The previous version of opaque-ke was using an older version of voprf that no longer works with the latest version of Rust. Hence, in order to upgrade to the latest version of Rust, we need to first upgrade opaque-ke.

As part of this upgrade, we need a wrapper for Argon2 that uses the default params from argon 0.4. Further explanation provided inline.

Basically copied this code: https://github.com/drey7925/perovskite/commit/2076604cb44ba9c103eb0208efa7449cfb34bb73#diff-400eb1f04580ae4870aa9d4aae3210268771ed3a5237703cc070d87296830d38

This resolves: https://linear.app/comm/issue/ENG-9164/upgrade-opaque-ke-20

Next diff will bump our Rust version from 1.76 to 1.82 (latest stable version)

Test Plan
  1. deployed custom identity image to staging with these changes
  2. checked out master branch
  3. successfully logged in to existing account on staging identity from native, web, keyserver
  4. successfully registered and logged in to staging from native
  5. switched back to feature branch
  6. successfully logged in to existing account on staging identity from native, web, keyserver
  7. successfully registered and logged in to staging from native

Note that I ran cleaninstall after switching branches

Diff Detail

Repository
rCOMM Comm
Branch
opauqe (branched from master)
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

varun requested review of this revision.Nov 7 2024, 1:36 PM

realize there's Rust code so @ashoat isn't a good reviewer

shared/comm-opaque2/src/opaque.rs
23 ↗(On Diff #45697)

These params come from this comment: https://github.com/facebook/opaque-ke/issues/359#issuecomment-2436428572

Basically, argon 0.5 uses different default params than 0.4. we need to use the 0.4 default params for existing users to be able to log in to identity

varun edited the summary of this revision. (Show Details)

get argon2 0.5 from root Cargo.toml

will added inline comments.
shared/comm-opaque2/src/opaque.rs
23 ↗(On Diff #45697)

These params apparently represent the memory cost, time cost, and parallelism degree . Curious why increasing the time and memory costs caused this to break

This revision is now accepted and ready to land.Nov 8 2024, 11:33 AM
shared/comm-opaque2/src/opaque.rs
23 ↗(On Diff #45697)

Changing any of these params will change the generated hash

https://argon2.online/

You can test it here if you’d like

bartek added inline comments.
shared/comm-opaque2/src/opaque.rs
23 ↗(On Diff #45697)

Curious if at some point, to increase security, we'd want to change these defaults and recommend users to reset their password. Bitwarden did a similar thing recently: https://bitwarden.com/help/kdf-algorithms/#low-kdf-iterations

shared/comm-opaque2/src/opaque.rs
23 ↗(On Diff #45697)

Curious if at some point, to increase security, we'd want to change these defaults and recommend users to reset their password. Bitwarden did a similar thing recently: https://bitwarden.com/help/kdf-algorithms/#low-kdf-iterations

maybe, but it would be a considerable amount of work. we'd have to re-register the user with the new params when they next log in and track which users have migrated to the new params. we'd also probably want to upgrade to opaque-ke v3 at the same time.

IMO we shouldn't do this unless there's a real security risk. the opaque-ke maintainers didn't express any concerns with us keeping the same default params as before, so i think it's probably safe?

This revision was automatically updated to reflect the committed changes.