kspearrin commented 7 years ago

This is a pretty odd bug, so bear with me.

We've had isolated reports coming from users for a very long time about not being able to log into our application via the website. Today I was finally able to narrow it down to users on Android devices using the Samsung web browser (based on Chromium with a somewhat significant marketshare on mobile since it comes pre-installed on all Samsung devices as the default browser). For whatever reason, the PBKDF2 function returns a different value in that web browser. Among thousands of users across all web browsers of our application, this is the only web browser that has ever been reported as problematic.

I have recreated the problem with the following test case debugging with Chrome vs Samsung browser on my Nexus 5X device (debugged via attaching to a desktop Chrome dev tools through chrome://inspect).

Test Case

Using forge v0.7.1

var keyBytes = forge.pbkdf2(
    forge.util.encodeUtf8('123456789'), // password
    forge.util.encodeUtf8('hello@bitwarden.com'), // salt
    5000, // iterations
    256 / 8, // key size
    'sha256'); // hash function

Results

keyBytes string returned here is:

Suamsung: "6¸©¢Q¬¬^DûLXkÍüíñÒC> «$Û" or base 64 as "NripolGBFaysXpcRRPtMWGvNmPzt8dKXQz4gqyTbjhg="

Chrome (and all others that we know of): "A6WRÆæ¤Ù«)=Çpk.Ë?Ö;8" or base 64 as "A0GZNhlXUpHGEuak2YurKT0IxwJwax4ukAbLkD/WOzg="

Debugging the forge.pbkdf2 function in both browsers points me to see a difference in computed values occur after looping of the iterations here:

  for(var j = 2; j <= c; ++j) {
    prf.start(null, null);
    prf.update(u_c1);
    u_c = prf.digest().getBytes();
    // F(p, s, c, i)
    xor = forge.util.xorBytes(xor, u_c, hLen);
    u_c1 = u_c;
  }

Samsung:

xor "6¸©¢Q¬¬^DûLXkÍüíñÒC> «$Û" u_c1 "HÉ¿Ð¦~ùÀíØVÁyÝ,°Ûh^AtV :@w)p" u_c "HÉ¿Ð¦~ùÀíØVÁyÝ,°Ûh^AtV :@w)p"

Chrome:

xor "A6WRÆæ¤Ù«)=Çpk.Ë?Ö;8" u_c1 "¸¦%þL^$KÜMæÂ~]H§ ÇAØ¦°7ðûZa²ÚëÉ" u_c "¸¦%þL^$KÜMæÂ~]H§ ÇAØ¦°7ðûZa²ÚëÉ"

Up to that point everything appeared the same to me between the two.

davidlehn commented 7 years ago

Ugh. Unfortunately not really all that odd. pbkdf2 had issues with a webkit optimization bug earlier this year: https://github.com/digitalbazaar/forge/issues/428. They fixed that upstream and some forge hacks made optimizer work either way. I just tried your sample with the samsung browser and see the failures. Looks like it has different output values about every time. I threw together a page to iterate through iteration values (see below). Constant output in chrome (desktop and android) but samsung has ok early values then random values after some point. Sometimes. And seems to crash the tabs if i reload too fast sometimes? I tried the super minimal test from 428 (just value swaps) and it didn't fail so I guess this is a different type of bug that is in the v8 from chrome 51.x that samsung is using. I probably don't have time to track this down right now but it might be possible to reduce the test down to something they could fix and/or forge could hack around. Before we were just doing some nonsense shifting and that fixed things. Need to explore more to figure out where the math is going wrong here. I would guess it only fails after code has been run a while and gets optimized. So tests can perhaps show early loops working and later loops failing.

<html>
  <head>
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script>
    <script src="https://unpkg.com/node-forge@0.7.1/dist/forge.min.js"></script>
    <script type="text/javascript">
$(function() {
function kb(it) {
  forge.options.usePureJavaScript = true;
var keyBytes = forge.pbkdf2(
    forge.util.encodeUtf8('123456789'), // password
    forge.util.encodeUtf8('hello@bitwarden.com'), // salt
    //5000, // iterations
    it, // iterations
    256 / 8, // key size
    'sha256'); // hash function
return forge.util.encode64(keyBytes);
};
var its = [5,50,100,500,1000,2000,3000,5000];
its.forEach(function(it) {
  var val = kb(it);
  console.log(it, val);
  var vh = $('<tr><td>' + it + '</td><td>' + val + '</td></tr>');
  $('#results').append(vh);
});
});
    </script>
  </head>
  <body>
    Results
    <table>
      <thead>
        <tr>
          <th>i</th>
          <th>kb</th>
        </tr>
      </thead>
      <tbody id="results">
      </tbody>
  </body>
</html>

kspearrin commented 7 years ago

When I was testing this I did see that after the first iteration the values were both equal between the two (Chome vs Samsung). But ever setting the breakpoint after the loop things were different.

digitalbazaar / forge

Samsung web browser computes different PBKDF2 than all other browsers #519

Test Case

Results