rewrite browser-test.c / encode-once.js for latest API

[rhelmer]

I was just taking a look at browser-test.c and encode-once.js, I took a quick stab at rewriting the latter:

/*
 * Copyright (c) 2018, Henry Corrigan-Gibbs
 * 
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 */

ChromeUtils.import('resource://gre/modules/Services.jsm');

let [publicKeyA, publicKeyB, batchID, param1, param2, param3] = arguments;

Services.prefs.setStringPref('prio.publicKeyA', publicKeyA);
Services.prefs.setStringPref('prio.publicKeyB', publicKeyB);

async function test() {
  let params =  {
    'browserIsUserDefault': Boolean(param1),
    'safeModeUsage': Boolean(param2),
    'startupCrashDetected': Boolean(param3)
  };

  let result = await PrioEncoder.encode(batchID, params);
}

test().then(console.log("finished"));

This still needs to return the correct output, but I was doing a quick test run with is and seeing:

Hit MOZ_CRASH(NSS_Shutdown failed) at /Users/rhelmer/src/mozilla-central/xpcom/build/XPCOMInit.cpp:1044

I suspect that this has to do with how NSS gets init'd in the xpcshell environment, as I am seeing no problems when running equivalent code in the browser.

[rhelmer]

Ah, here's where it's crashing:

https://searchfox.org/mozilla-central/rev/aff5d4ad5d7fb2919d267cbc23b1d87ae3cf0110/xpcom/build/XPCOMInit.cpp#1036-1049

This assert is being hit because I am using a debug build, I will follow up with NSS folks to see if we're missing something here...

[rhelmer]

Hm so ignoring the NSS shutdown thing for the moment, I think this should be equivalent to the previous encode-once.js but it doesn't seem to pass browser-test:

/*
 * Copyright (c) 2018, Henry Corrigan-Gibbs
 * 
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 */

ChromeUtils.import('resource://gre/modules/Services.jsm');

let [publicKeyA, publicKeyB, batchID, param1, param2, param3] = arguments;

Services.prefs.setStringPref('prio.publicKeyA', publicKeyA);
Services.prefs.setStringPref('prio.publicKeyB', publicKeyB);

async function test() {
  let params =  {
    'browserIsUserDefault': Boolean(param1),
    'safeModeUsage': Boolean(param2),
    'startupCrashDetected': Boolean(param3)
  };

  try {
    let result = await PrioEncoder.encode(batchID, params);

    let textEncoder = new TextEncoder();
    const toHexString = bytes =>
      bytes.reduce((str, byte) => str + byte.toString(16).padStart(2, '0'), '');

    console.log(toHexString(textEncoder.encode(result.a)) + ',$' + toHexString(textEncoder.encode(result.b)));
    console.log('');
  } catch(e) {
    console.log('Failure.', e);
    console.log(v);
  }
}

test().then();

[franziskuskiefer]

Ah, here's where it's crashing:

rand_clear, which is shutting down the instance of NSS you use is probably not called on time. That's why I mentioned that you could use PSM to get an instance of NSS (then clean-up is guaranteed on xpcom shutdown) [1]. That would require some changes here but would be the easiest way to get an instance of NSS in Firefox that's handled properly.

[1] https://searchfox.org/mozilla-central/rev/aff5d4ad5d7fb2919d267cbc23b1d87ae3cf0110/security/manager/ssl/nsNSSComponent.cpp#88

[henrycg]

@rhelmer Which build of the browser are you using for testing? I can build it and try to debug the script.

[rhelmer]

@henrycg this is the latest changeset on bug 1421501, should be able to pull it with:
hg pull -r c3124c62ccc4bae4bcab1e895c89c27c457048e4 https://reviewboard-hg.mozilla.org/gecko

I didn't dig into it much last night, I have a debug build which does produce a lot more output on stdout though, I'll try with an opt build too.

[henrycg]

A couple of things:

• The line with bytes.reduce should read:
  bytes.reduce((str, byte) => str + byte.toString(16).padStart(2, '0'), '') + ',';
  so that there is a comma after every pair of hex digits. (The browser-test tool expects the strings in this format.)
• I suspect that there is something going wrong with the conversion of bytes to hex strings. The strings that PrioEncoder.encode produces should be random strings, but the strings I see in the output have way more c characters than I would expect in a random string.

[henrycg]

This version of encode_once.js seems to work for me. (Forgive me for the poor JS skills.) It does seem like TextEncoder wasn't doing quite the right thing...

/*
 * Copyright (c) 2018, Henry Corrigan-Gibbs
 * 
 * This Source Code Form is subject to the terms of the Mozilla Public
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/.
 */

ChromeUtils.import('resource://gre/modules/Services.jsm');

let [publicKeyA, publicKeyB, batchID, param1, param2, param3] = arguments;

Services.prefs.setStringPref('prio.publicKeyA', publicKeyA);
Services.prefs.setStringPref('prio.publicKeyB', publicKeyB);

async function test() {
  let params =  {
    'browserIsUserDefault': Number(param1),
    'safeModeUsage': Number(param2),
    'startupCrashDetected': Number(param3)
  };

  try {
    let result = await PrioEncoder.encode(batchID, params);

    function toArr(bStr) {
      var len = bStr.length;
      var u8_array = new Uint8Array(len);
      for (var i = 0; i < len; i++) {
        u8_array[i] = bStr.charCodeAt(i);
      }
      return u8_array;
    }
    const toHexString = bytes =>
      bytes.reduce((str, byte) => str + byte.toString(16).padStart(2, '0') + ',', '');

    console.log(toHexString(toArr(result.a)) + '$' + toHexString(toArr(result.b)));
    console.log('');
  } catch(e) {
    console.log('Failure.', e);
    console.log(v);
  }
}

test().then();

[rhelmer]

Ah too bad about TextEncoder, I'll have to dig into that more... I should have got the tests running before I promoted it :)

Thanks!

[rhelmer]

@henrycg hm ok well that produces output like this if I run encode-once.js directly:

rhelmer-23718:browser-test rhelmer$ /Users/rhelmer/src/mozilla-central/obj-x86_64-apple-darwin17.6.0/dist/bin/xpcshell encode-once.js 17E0D22E4078E23232315B850FED3CB4D6FDBEC93DCA54C5A1EF7FC439B47F70 B51C55E86F041067C346DAF8C45546DE951ABB119C0D77F24AFE869054D9EC72 282475249 1 0 0
console.log: "18,d0,3e,d1,a0,e2,21,cc,e5,59,ea,da,98,d8,4d,d4,bd,59,36,78,f3,c7,b1,d9,e6,7d,16,92,7e,d8,93,17,53,d9,2d,95,fd,a6,7c,7a,72,33,d0,9f,b3,d3,04,0f,20,2d,0f,ce,fb,62,37,bf,d4,e2,50,73,e9,67,8c,34,df,df,81,5e,5d,46,35,eb,c7,b8,93,d2,3f,30,3e,94,3a,3c,65,f7,06,33,0f,80,90,36,90,a0,2f,29,8f,f8,34,58,10,96,79,dc,d3,d7,46,a7,55,ea,cf,62,d0,5f,4a,8a,83,5c,3e,22,88,9d,c7,f8,5f,bf,81,3a,81,a3,6d,da,68,32,15,d8,d5,86,e0,27,27,d8,99,cd,23,dc,50,a4,8d,49,fd,16,df,52,66,00,f7,70,95,25,d3,13,48,3f,06,c3,c5,8b,e4,12,97,b0,7d,3d,b1,71,de,0a,2c,ad,81,01,2e,b0,2b,25,d9,67,55,db,52,2e,a5,33,7d,e5,73,f8,da,93,3b,9a,a2,31,72,5a,e8,1a,3a,0d,12,64,16,c5,d8,d9,03,bd,8d,0e,d9,88,70,ae,2e,0f,f6,f7,1e,0a,f7,$1d,9d,81,d9,48,f9,e2,15,ef,e6,a0,d0,1b,84,32,39,04,1b,a4,eb,39,a5,3b,2e,c2,2c,6f,46,14,60,30,6c,17,ef,46,37,34,fe,12,18,a7,20,4e,47,0c,be,d5,1e,b0,f0,7c,2f,a7,23,ba,4d,15,bb,23,af,97,1f,e9,91,fc,62,8b,75,52,7b,30,4e,14,2a,ae,33,59,9d,71,1e,44,31,60,d4,3f,19,83,c3,f6,14,a3,61,bd,84,d5,91,02,fd,19,95,cc,83,cd,1b,67,55,49,31,e2,4f,4a,72,ff,2d,9f,40,cf,28,d8,97,86,0f,42,f6,9f,2c,04,8c,47,9b,71,e8,d4,86,f7,a9,8c,5c,eb,b1,cf,4b,c8,02,f7,65,ea,47,bb,8e,45,80,a4,75,de,c9,2f,ac,13,84,"
console.log: ""

If I run ./browser-test though I get:

Warning: unexpected failure.

I see in the code for browser-test that it prints this if if (rv != SECSuccess) in cleanup, can you suggest a way to get some more info out of this to debug?

[henrycg]

PR #8 adds some debugging printfs. If you're on Linux, it might be fixed by PR #8 as well.

[rhelmer]

Ah thanks was looking at this one first :P will go look at that.

[henrycg]

How difficult is it to write xpcshell tests? One idea would be to extend PrioEncoder.cpp to expose the following two testing methods
PrioEncoder.generateKeyPair() -> (public_key, secret_key)
and
PrioEncoder.testDecrypt(secretA, secretB, batchID, expectedData, forServerA, forServerB) -> 0/1.
The C++ code in PrioEncoder.cpp could implement the stuff that's now in libprio/browser-test and then we could have these run using the standard xpcshell framework (instead of this hacked up browser-test thing).

[rhelmer]

Totally agree that we should have this test in the mozilla-central tree, this is something I can do... was hoping we could limp along a bit more since you already put the work into the browser-test thing :)

[rhelmer]

(adding these sorts of test-only functions is pretty common from what I see poking around the WebIDL, they tend to be behind prefs so they aren't generally available outside of the test framework).

I'll give it a shot once I have a moment.

[henrycg]

Cool. I'm happy to help out with the C++ code that will link up to libprio.