Java verifier generation issue

Issue #3 resolved
Bernard Wittwer created an issue

Hello!

I have an issue with the HexHashedVerifierGenerator.

Under certain circumstances (still under investigations) the generated verifier is invalid and cannot be used for further login. Reseting the password (same user, same password) will generate a good verifier and fix the login.

See two examples here : https://bitbucket.org/bwittwer/thinbus-srp-js/branch/issue3-tests

We identified such cases by having automated testing scripts creating a lot of new users.

Do you have any clue?

Thanks, Best regards, Bernard.

Comments (8)

  1. simon repo owner

    Hey Bernard,

    Thanks for the code to reproduce. I will run it and debug it as a priority.

    Simon

  2. simon repo owner

    Hi Bernard,

    I posted comments to the commit on your branch that the failure of those test seem to be faults with the tests; such as wrong email and password in the js test at the top of the file. So unfortunately the test don't look to isolate the bug you are experiencing.

    Simon

  3. simon repo owner

    Hi Bernard,

    I found the bug and I have committed a fix to the master branch. Basically the J BigInteger library drops leading zeros whereas the JS will generate fixed length strings with leading zeros. So the JS can compute an 'x' hex string such as such as "0aaa" but the J standard library code will implicitly convert to "aaa" when moving to hex strings. The mismatch in 'x' which is baked into the 'v' is why a given salt+username+password wont work. You have a 1/16 chance that salt+username+password will cause a leading 0 such that the J and JS will not compute the same 'x' value.

    Elsewhere the JS drops the leading zero so for consistency with Nimbus J code so I have changed the JS 'x' code to drop the leading zeros. I suspect that deploying this fix could break 1/16 users who have generated a verifier on the browser; as their old verifier was generated using the "with zero" logic but after the fix is deployed when they try to login their browser client code will compute 'x' using the "without zero" logic and the mismatch would cause an authentication failure. They will need to reset their password/verifier using the new code.

    Can try the fix on the master branch and if it tests out all okay I will deploy a release at the weekend.

    Simon

  4. simon repo owner

    Hi Bernard,

    I have created a branch 1.2.x that has the fix done the other way around; there is a method on the java hex generator generateVerifierWithLeftPadZeros which creates the verifier using 'x' logic which does not drop the left zeros to match the JS 'x' logic. Using that method the TestIssue3.java test passes without changing the JS logic. That change is far more invasive changing a lot of methods and so I have marked the method as deprecated.

    If you could kindly test both the 1.2.x branch code which builds version 1.2.2-SNAPSHOT and the master branch which has 1.3.0-SNAPSHOT? Testing 1.3.0-SNAPSHOT requires deploying new JS libraries then the problem should go away. Testing 1.2.2-SNAPSHOT needs to invoke the new java method as demonstrated in the TestIssue3.js file on that branch.

    If both ways of fixing the problem work then you have a choice which way to go. Deploying 1.2.2 should have no risk of locking any existing users out of your system. Deploying 1.3.0 risks locking out users. That could be coded around by tracking when users registered on your system (before or after your 1.3.0 deployment) then show then alternative JS to login with either 1.2.1 JS or 1.3.0 JS depending on which release they recreated their verifier.

    If there are any future security related fixes I will patch them into the 1.2.x branch. If there are any future features that are not security related I may not put them on that branch; but you are of course welcome to keep it up to date by submitting PRs. To be honest I would expect that the library is mature and functional so there are unlikely to be a large number of changes in the future.

    Simon

  5. Bernard Wittwer reporter

    Hi Simon,

    Thanks a lot for this very quick and efficent troubleshooting. I also apologize for the poor quality of the provided unit tests I mentionned, I should have explained in more details prior to raise this issue.

    I was OoO today and I won't have the access to test the fixes before Monday. For sure I will test both methods but I guess (to be confirmed) that we should use the version 1.3.0, it will be cleaner and a long-term solution, maybe by taking the risk of breaking some users (a force reset password mechanism could be put in place). Anyway I will perform the tests on Monday and keep you informed on the strategy we will use.

    Thanks again for your help, it is really appreciated.

    Cheers, Bernard.

  6. simon repo owner

    You are welcome. I have just pushed release 1.3.0 to maven central it should be showing up by Monday. Given that the only new code on 1.2.x branch is deprecated I won't release that unless you need it as it would only be of interest to folks who are generating verifiers using Java such as emailing out temporary passwords.

  7. Bernard Wittwer reporter

    Hello, I used version 1.3.0 and ran some load tests. Creating and login thousands of users using the java client: I had no more issues. Same from Javascript is it also ok. You can close this issue. Most likely I will not use the version 1.2.2, but if needed I will submit PRs Thanks, Bernard.

  8. Log in to comment