617193 - Change JPAKE password hashing

Status: VERIFIED FIXED

(Cloud Services Graveyard :: Firefox Home, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Due to the way JPAKE was implemented in NSS, we have to make a small change to how we derive the "secret" from the PIN. Right now we do this:

 secret = 1 + (SHA256(pin) % (q-1))

I believe this is implemented in your

 static BIGNUM* HashPassword(NSString* password, BIGNUM* q)

function in JPAKEParty.m The bad news is that we have to change this. The good news is that it'll get much simpler, namely:

 secret = pin

Which should translate into something like

 BIGNUM* secret = BN_new();
 BN_bin2bn(password, 8, secret);
 return secret;

for your code. Please confirm. :)

Comment 1

[Philipp von Weitershausen [:philikon]]

CCing Brian to make sure he's in the loop. It might even make sense to have him look over the patch to ensure interop.

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Committed: http://hg.mozilla.org/services/fx-home/rev/a68cc23ea556

Comment 3

[Tony Chung [:tchung]]

can you provide testcases for this?  Will they differ than your comment in https://bugzilla.mozilla.org/show_bug.cgi?id=617153#c0?   Eventually, we'll need to talk again about having unit tests for all your changes.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

We actually have unit tests for JPAKE! I just made sure they are all enabled.

The best test for QA to sign off on this is to simply do a JPAKE exchnge between Firefox and Home.

Comment 5

[Tony Chung [:tchung]]

(In reply to comment #4)
> We actually have unit tests for JPAKE! I just made sure they are all enabled.
> 
Oh nice.  i would love to get some exposure to that.  are they running somewhere in buildbot or some other continuous integration system?

> The best test for QA to sign off on this is to simply do a JPAKE exchnge
> between Firefox and Home.
Sounds easy enough.

Comment 6

[Tony Chung [:tchung]]

Verified fix on 1.1b6, 12/13 build.