-
Notifications
You must be signed in to change notification settings - Fork 1
Differential tests #43
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
Conversation
0d1219e to
1973df4
Compare
* generate target before padded secret * reduce target range by 1
* compute target before secret padding * reduce target modulus by one
- skip field serialization if empty - replace `String` params and keys to `Value` or concrete `Jwk` type - remove `code` from params.
1973df4 to
f004e8f
Compare
a2597f9 to
df9c4a6
Compare
mfkdf2-web/src/api.ts
Outdated
| return value; | ||
| } | ||
|
|
||
| function stringifyFactorParams(value: any): any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: This can definitely be simplified a lot, maybe removed altogether.
stringification is needed due to nested parsing done in differential test. the derived key similarity validation reads the internal state of the key.
| thread_local! { | ||
| static RNG: RefCell<ChaCha20Rng> = RefCell::new(ChaCha20Rng::from_seed(DEFAULT_SEED)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is cursed rust.
Can you use a OnceCell or whatever here to create a static?
This clearly works but I've never seen it done this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnceCell can't work due to no mutable reference in static.
Other options would be a lazylock, but that would require a mutex. I used this to prevent any locks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? I see.
The thread_local! is also bizarre.
Anyway, not a big deal especially if it works because this is really just for differential testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.rs/rand/latest/src/rand/rngs/thread.rs.html#118-127
that's how rust rand do it as well.
|
Let me go review the related PRs |
| ], { id: 'key1' }) | ||
| const setup2Clone = JSON.parse(JSON.stringify(setup2)) | ||
|
|
||
| // purposely modify the setup2Clone to make it similar to the setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooba derived key is modified by hardcoding:
nextparam: can't be same because node's native rsa encrypt uses internal rngext: node's native Jwk struct addsextfor browser support. Not mentioned in RFChmac: can't be same due to above fields.
|
This is looking good! Almost there. Resolved many comments. Left up relevant ones. Let's get all of your documentation and the differential tests nicely displayed on the github pages so we can easily point to this for further security work. Again, awesome job! <3 |
8588c90 to
4beec91
Compare
4beec91 to
7d1e0fe
Compare
|
@Autoparallel this seems good to merge. I've opened another PR for mdbook and documentation detailing steps to setup, reproduce the bindings and differential testing. |
* add mdbook * publish book * add roadmap and fix readme * add toc, fix justfile commands
d12b5fc to
a1b5fda
Compare
Changes needed to test this repository with MFKDF reference implementation.
Other PR/packages
Changes
differential-testfeature flagdet_rngmodule for deterministic rngu32 -> f64MFDKF2FactorPolicyFactor: skip field serialization if emptyStringparams and keys toValueor concreteJwktypecodefrom params, and add uppercase.Policy: deterministic id generation by hashing all factor idsBTreeMapinstead ofHashMapfor ordered keys in factor, outputsNote
nextparam: can't be same because node's native rsa encrypt uses internal rngext: node's native Jwk struct addsextfor browser support. Not mentioned in RFChmac: can't be same due to above fields.