Closed Bug 1004279 Opened 10 years ago Closed 10 years ago

create in-tree CA pinning preload list

Categories

(Release Engineering :: General, defect, P2)

Product:
Release Engineering
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: mmc, Assigned: coop)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

Update HPKP preload list in-tree (r=)
8.32 KB, patch
Details | Diff | Splinter Review
output of sh -x hpkp/update_hpkp_preload_list.sh
18.90 KB, 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.
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
Assignee: nobody → mmc
Status: NEW → ASSIGNED
Attachment #8425776 - Attachment is obsolete: true
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 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?
coop, you're more familiar with this script, any ideas?
Flags: needinfo?(coop)
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 :)
Also there is probably a bootstrapping issue with creating repodir hpkp, which doesn't exist.
(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)
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)
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.
Attachment #8426353 - Flags: review?(coop)
(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. ;)
Attachment #8426353 - Flags: review?(coop)
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)
Attachment #8425818 - Flags: review?(coop)
(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)
(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)
(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.
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
Working on this today.
Assignee: mmc → coop
OS: Mac OS X → All
Priority: -- → P2
Hardware: x86 → All
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)
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)
Script has landed over in bug 869051. Working on the buildbot changes to support it now.
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.
Does't seem like this worked according to hg log on security/manager/boot/src/StaticHPKPins.{errors,h}.
Flags: needinfo?(coop)
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)
(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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Thanks very much, coop :)
Depends on: 1035003
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: