Closed
Bug 1004279
Opened 11 years ago
Closed 10 years ago
create in-tree CA pinning preload list
Categories
(Release Engineering :: General, defect, P2)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mmc, Assigned: coop)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
text/plain
|
Details |
Bug 1002696 requires changes to command-line arguments of http://mxr.mozilla.org/mozilla-central/source/security/manager/tools/getHSTSPreloadList.js in order to generate CA pinsets.
I can't find its invocation in the repo anywhere, so this is reminder to request the relevant changes in the build scripts when that happens.
Comment 1•11 years ago
|
||
Reporter | ||
Comment 2•11 years ago
|
||
Thanks philor. After discussing with Camilo and David we decided that generating pinsets should be a separate script from HSTS, because of various differences in their constraints. This script is now reviewed in bug 1002696 and has the following invocation:
if (arguments.length != 3) {
throw "Usage: genHPKPins.js <absolute path to PreloadedHPKPins.json> " +
"<absolute path to default-ee.der> " +
"<absolute path to StaticHPKPins.h>";
}
PreloadedHPKPins.json: security/manager/tools/PreloadedHPKPins.json
default-ee.der: security/manager/ssl/tests/unit/tlsserver/default-ee.der
StaticHPKPins.h: security/manager/boot/src/StaticHPKPins.h
What are the next steps?
Thanks,
Monica
Summary: update in-tree HSTS preload list to generate CA pinsets as well → create in-tree CA pinning preload list
Reporter | ||
Comment 3•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Attachment #8425776 -
Attachment is obsolete: true
Reporter | ||
Comment 5•10 years ago
|
||
Comment on attachment 8425818 [details] [diff] [review]
Update HPKP preload list in-tree (r=)
Review of attachment 8425818 [details] [diff] [review]:
-----------------------------------------------------------------
Hi Chris,
I think I've taken this change as far as I can go. Can you help me find an owner for this bug who can actually run and test this change on non-dry-run mode? Here's where I get to, without appropriate creds.
INFO: HPKP preload lists differ, updating hg.
hg -R hpkp pull
abort: repository hpkp not found!
Thanks,
Monica
Attachment #8425818 -
Flags: review?(catlee)
Comment 6•10 years ago
|
||
Comment on attachment 8425818 [details] [diff] [review]
Update HPKP preload list in-tree (r=)
Review of attachment 8425818 [details] [diff] [review]:
-----------------------------------------------------------------
This doesn't seem like a credentials issue...Can you attach the full log for debugging?
Comment 7•10 years ago
|
||
coop, you're more familiar with this script, any ideas?
Flags: needinfo?(coop)
Reporter | ||
Comment 8•10 years ago
|
||
Comment on attachment 8425818 [details] [diff] [review]
Update HPKP preload list in-tree (r=)
Review of attachment 8425818 [details] [diff] [review]:
-----------------------------------------------------------------
::: scripts/hpkp/update_hpkp_preload_list.sh
@@ +25,5 @@
> CLOSED_TREE=false
> DONTBUILD=false
> APPROVAL=false
> HG_SSH_USER='ffxbld'
> HG_SSH_KEY='~cltbld/.ssh/ffxbld_dsa'
I don't have these ssh keys, nor do I want them :)
Reporter | ||
Comment 9•10 years ago
|
||
Also there is probably a bootstrapping issue with creating repodir hpkp, which doesn't exist.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #5)
> INFO: HPKP preload lists differ, updating hg.
> hg -R hpkp pull
> abort: repository hpkp not found!
The fact that it goes from the info line to the pull indicates that there is a directory already present with the name 'hpkp.' The script naively assumes this is an hg clone:
https://hg.mozilla.org/build/tools/file/7ec82453de56/scripts/hsts/update_hsts_preload_list.sh#l174
Monica: Does that 'hpkp' dir already exist in your working dir? The script only creates it as part of the clone process, so I'm not sure how else it would be there.
Flags: needinfo?(coop) → needinfo?(mmc)
Reporter | ||
Comment 11•10 years ago
|
||
Hi catlee,
Here's the output of running the script from with cwd tools/script. There is an hpkp directory in the cwd.
Thanks,
Monica
Flags: needinfo?(mmc)
Reporter | ||
Comment 12•10 years ago
|
||
And of course, I was running the script from the wrong directory. It needs to be run from the cwd, not 1-level up. Please review.
We would like this script to be run similar to the hsts script it was copied from (every week on Nightly and Aurora), because it updates an expiration time in the output file.
Reporter | ||
Updated•10 years ago
|
Attachment #8426353 -
Flags: review?(coop)
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #11)
> Created attachment 8426353 [details]
> output of sh -x hpkp/update_hpkp_preload_list.sh
>
> Here's the output of running the script from with cwd tools/script. There is
> an hpkp directory in the cwd.
It sounds like you already found your problem based on comment #12, and the rest of the output looks akin to the HSTS update script running.
re: attachment #8425818 [details] [diff] [review] - since we're performing multiple updates of similar files via similar means, I would rather see the single script adapted to do multiple updates sequentially, rather than incur the overhead of downloading the artifacts and cloning the source in two separate scripts.
Feel free to switch the review to me if you go that route. ;)
Assignee | ||
Updated•10 years ago
|
Attachment #8426353 -
Flags: review?(coop)
Reporter | ||
Comment 14•10 years ago
|
||
Comment on attachment 8425818 [details] [diff] [review]
Update HPKP preload list in-tree (r=)
Review of attachment 8425818 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, coop! To be honest, I feel uncomfortable modifying these scripts because I know nothing about how they are run. I would feel a lot better if someone who actually worked in releng could modify this. Is that something that someone on your team could take over?
Thanks,
Monica
Attachment #8425818 -
Flags: review?(catlee) → review?(coop)
Assignee | ||
Updated•10 years ago
|
Attachment #8425818 -
Flags: review?(coop)
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #14)
> Thanks, coop! To be honest, I feel uncomfortable modifying these scripts
> because I know nothing about how they are run. I would feel a lot better if
> someone who actually worked in releng could modify this. Is that something
> that someone on your team could take over?
I'll take a stab at a unified script next week.
Monica: what's your timeframe for getting this deployed? Are there any any hard dates I should be aware of?
Flags: needinfo?(mmc)
Reporter | ||
Comment 16•10 years ago
|
||
(In reply to Chris Cooper [:coop] from comment #15)
> (In reply to [:mmc] Monica Chew (please use needinfo) from comment #14)
> > Thanks, coop! To be honest, I feel uncomfortable modifying these scripts
> > because I know nothing about how they are run. I would feel a lot better if
> > someone who actually worked in releng could modify this. Is that something
> > that someone on your team could take over?
>
> I'll take a stab at a unified script next week.
>
> Monica: what's your timeframe for getting this deployed? Are there any any
> hard dates I should be aware of?
Thanks, that's a load off my back :) Pinning has landed in FF 32 (currently nightly). It would be great to get this script integrated into the build process before the next merge. If it doesn't, then we will most likely have to request some manual uplifts to Aurora to manage the expiration time of the output. That's not a big deal but is also not ideal.
Btw, if you choose to unify this script with the one for HSTS: the JS generators take different inputs, produce different outputs, and check different things, so it would be good to separate their error files.
Thanks,
Monica
Flags: needinfo?(mmc)
Assignee | ||
Comment 17•10 years ago
|
||
(In reply to [:mmc] Monica Chew (please use needinfo) from comment #16)
> Btw, if you choose to unify this script with the one for HSTS: the JS
> generators take different inputs, produce different outputs, and check
> different things, so it would be good to separate their error files.
Understood. It's more about minimizing the overhead for downloads and cloning. I'll make sure things are separate.
Reporter | ||
Comment 18•10 years ago
|
||
Hi coop,
It's coming up on merge date. Is there any chance that this can be integrated before then? If not, we need to figure out what the best expiration date is before the output of this script hits aurora and requires uplifts.
Thanks,
Monica
Assignee | ||
Comment 19•10 years ago
|
||
Working on this today.
Assignee: mmc → coop
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
Assignee | ||
Comment 20•10 years ago
|
||
Monica: I have a working script based on your patch, but I am unclear about the JSON file, PreloadedHPKPins.json.
In my tests so far, a new JSON file isn't created. You also don't try to diff the new and old JSON files in your patch, and yet you *do* try to copy the JSON into the source tree for updating.
Is it possible that the JSON file *could* be updated as part of this process, or is it purely an input file?
Flags: needinfo?(mmc)
Reporter | ||
Comment 21•10 years ago
|
||
Hi coop,
PreloadedHPKPins.json is input-only, it should not be modified as part of the process. Sorry the initial script was wrong and tried to copy it back.
Only the output file StaticHPKPins.h and error file should be diffed and updated in the tree.
Thanks,
Monica
Flags: needinfo?(mmc)
Assignee | ||
Comment 22•10 years ago
|
||
Script has landed over in bug 869051. Working on the buildbot changes to support it now.
Assignee | ||
Comment 23•10 years ago
|
||
Buildbot changes have landed in https://bugzilla.mozilla.org/show_bug.cgi?id=869051#c16 and https://bugzilla.mozilla.org/show_bug.cgi?id=869051#c17.
This will run for the first time on a scheduled basis this coming weekend. Leaving this bug open until we verify that happens as expected.
Reporter | ||
Comment 24•10 years ago
|
||
Does't seem like this worked according to hg log on security/manager/boot/src/StaticHPKPins.{errors,h}.
Flags: needinfo?(coop)
Assignee | ||
Comment 25•10 years ago
|
||
Monica: I'm trying to track down the log of the build over the weekend so see what went wrong. Will report back once I find it.
Flags: needinfo?(coop)
Assignee | ||
Comment 26•10 years ago
|
||
(In reply to Chris Cooper [:coop] from comment #25)
> Monica: I'm trying to track down the log of the build over the weekend so
> see what went wrong. Will report back once I find it.
I landed the changes last Wednesday, but the changes didn't actually get deployed until yesterday. I had a reasonable expectation that they would have deployed before the weekend, but they didn't. Automated deployments will help us here.
Because of the delay, I've triggered a run by hand against mozilla-central so we don't end up waiting until the weekend to find out something else is wrong. Will report back with results.
Assignee | ||
Comment 27•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 28•10 years ago
|
||
Thanks very much, coop :)
Updated•7 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•