genHPKPStaticpins.js is not hooked up into the build anywhere, so static_pins.h will slowly go out of date, and we will not notice bitrot in the generator file. This script should be integrated into http://mxr.mozilla.org/mozilla-central/source/security/manager/tools/getHSTSPreloadList.js which already takes care of this problem.

We are going to keep these separate but they should share code as much as possible.

Comment on attachment 8416128 [details] [diff] [review] Minimum set of changes to make genHPKPStaticPins.js productionizable ( Review of attachment 8416128 [details] [diff] [review]: ----------------------------------------------------------------- LGTM

Comment on attachment 8416128 [details] [diff] [review] Minimum set of changes to make genHPKPStaticPins.js productionizable ( Review of attachment 8416128 [details] [diff] [review]: ----------------------------------------------------------------- Great - r=me with nits addressed. ::: security/manager/tools/genHPKPStaticPins.js @@ +10,5 @@ > +// [absolute path to]/PreloadedHPKPins.json \ > +// [absolute path to]/default-ee.der \ > +// [absolute path to]/StaticHPKPins.h > + > +if (arguments.length < 3) { nit: arguments.length != 3? @@ +33,5 @@ > " * file, You can obtain one at http://mozilla.org/MPL/2.0/. */\n" + > "\n" + > "/*****************************************************************************/\n" + > "/* This is an automatically generated file. If you're not */\n" + > +"/* PublicKeyPinningService.cpp, you shouldn't be #including it. */\n" + nit: one less space after "#including it." (so the end of the line aligns) @@ +52,5 @@ > "} StaticPinset;\n"; > > +// Command-line arguments > +var gStaticPins = parseJson(arguments[0]); > +var gTestDerFile = arguments[1]; maybe gTestCertFile? @@ +100,5 @@ > } > > // Returns a pair of maps [certNameToSKD, certSDKToName] between cert > // nicknames and digests of the SPKInfo for the mozilla trust store > +function loadNSSCertinfo(testDerFile) { We're missing an opportunity to have a German joke here ("derTestFile" :) @@ +134,5 @@ > return [certNameToSKD, certSDKToName]; > } > > +function parseJson(filename) { > + dump("parsing " + filename + "\n"); nit: not sure this dump is necessary @@ +235,5 @@ > } > > // **************************************************************************** > // This is where the action happens: > +let [certNameToSKD, certSDKToName] = loadNSSCertinfo(gTestDerFile); nit: while you're changing this line, it would be nice to have spaces after/before [/]

Fixed and the tree is closed! > We're missing an opportunity to have a German joke here ("derTestFile" :) True story, I thought for the longest time that all of the crypto authors must be German (or German-philes) because of the naming conventions. D'oh!