This repository has been archived by the owner on Feb 15, 2019. It is now read-only.
use newtypes to make the API safer #3
Comments
warner
added a commit
that referenced
this issue
Jun 3, 2018
This a breaking API change. The next release should bump the minor version number. As discussed in #3 and https://github.com/warner/magic-wormhole.rs/issues/32 , if an application were to accidentally swap the "password" and "identity" arguments (mainly for start_symmetric which only takes two args), the app would appear to work, but would contain a devastating security vulnerability (online brute-force password attack, with precomputation enabled). You might think of newtypes as giving the API named parameters. Instead of: `s = start_symmetric(b"pw", b"appid")` you get: `s = start_symmetric(&Password::new(b"pw"), &Identity::new(b"appid"))` but it protects you (with a compile-time error) against mistakes like: `s = start_symmetric(&Identity::new(b"appid"), &Password::new(b"pw"))` I'd like to find a way to remove requirement to pass a reference (and enable `start_symmetric(Password::new(..)..)`).
|
Fixed in release 0.1.0 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
In https://github.com/warner/magic-wormhole.rs/issues/32 I discovered a bug in which
SPAKE2::start_symmetric()was called with thepasswordandid_sarguments swapped. This compiles and even self-interoperates, because both values had the same slice type (&[u8]). However it introduces a serious security bug: it makes the protocol vulnerable to an online man-in-the-middle attack (since the SPAKE2 blinding factors become constant). The attacker merely has to test one hash per possible password and use the application's Key Confirmation Message as an oracle. For magic-wormhole this would have taken less than a millisecond.I discovered this bug while introducing "newtypes" into magic-wormhole: structs which wrap a single value, used just for typechecking arguments.
We should introduce a
PasswordandIdentitypair of newtypes into the spake2.rs API and use them for these arguments. I think that would make this sort of mistake harder to make for other applications in the future.The text was updated successfully, but these errors were encountered: