Closed Bug 1309293 Opened 8 years ago Closed 8 years ago

signing scriptworker chain of trust verification

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozilla, Assigned: mozilla)

References

Details

Attachments

(4 files)

Now that we have signed chain of trust artifacts being uploaded for decision, docker-image, build, and signing tasks, we need to verify them.

To build the task list to verify, we need to implement task.extra.chainOfTrust.inputs, per https://gist.github.com/escapewindow/a6a6944f51d4219d08284ededc65aa30 .  This will land on the project branch 'date' first.

We'll need scriptworker functionality to build the list of tasks, and verify

* the download links
* the chain of trust artifact signatures; json schema
* the important artifact shas
* the docker hub shas are in an allowlist
* the built docker images are built from the tree
* the repo/branch are in an allowlist; revision is not older than MAX_AGE
* the docker worker tasks were non-interactive
* the task definitions of the docker-image, build, and signing tasks are as defined in the decision task full graph (docker-image is special; will have to deal with out-of-graph docker-image tasks)
* more possible verification https://public.etherpad-mozilla.org/p/jonasfj-chain-of-trust-artifact

I'd like to do this in a way where the same functions are usable not only elsewhere in the graph (e.g. scriptworker balrog/funsize/pushapk/beetmover etc.) but also for other projects (servo, addons).

At some point this code will probably be moved into a separate module, but for now I'm planning on putting it in scriptworker.cot
:dustin, https://hg.mozilla.org/try/rev/6303f495a435db5ac63f2acb87393eaa92b2c909 created a valid android nightly task, but in the task, task.extra.chainOfTrust.inputs remained {"docker-image": "<docker-image>"} https://queue.taskcluster.net/v1/task/FEuDTJb5SkGLczq7VlWSqw

I'm guessing I'm missing a transform.  Should I continue along that line, or more like https://hg.mozilla.org/try/rev/dce842b311c57edfee519de8f187eec73fb223bd ?
Flags: needinfo?(dustin)
If you want to use <..>, you need to enclose it in {"task-reference": "..<..>.."}, so that should be
  {"docker-image": {"task-reference": "<docker-image>"}}
Flags: needinfo?(dustin)
Attachment #8800411 - Flags: review?(dustin)
Comment on attachment 8800411 [details] [diff] [review]
build task.extra.chainOfTrust.inputs

Review of attachment 8800411 [details] [diff] [review]:
-----------------------------------------------------------------

::: taskcluster/taskgraph/transforms/build.py
@@ +24,5 @@
>              job['worker']['chainOfTrust'] = True
> +            job.setdefault('extra', {})
> +            job['extra'].setdefault('chainOfTrust', {})
> +            job['extra']['chainOfTrust'].setdefault('inputs', {})
> +            job['extra']['chainOfTrust']['inputs']['docker-image'] = {"task-reference": "<docker-image>"}

This looks good.

As this broadens out to other worker implementations, let's think about making this something handled by task.py.  For example, task_desc['chain-of-trust']['inputs'] can contain custom inputs, and the docker-worker/docker-engine payload builder can add the docker image to those inputs, and set up the chainOfTrust feature.

BTW, when that refactor happens, it should be "chain-of-trust", not "chainOfTrust" -- all of the "description" properties are lower case with dashes.  I just missed that on review :)
Attachment #8800411 - Flags: review?(dustin) → review+
Keywords: leave-open
Depends on: 1311111
Attached patch verbose.diffSplinter Review
The signing scriptworkers appear to have hung again... I'd like to leave them in debug mode so I can possibly catch what's happening.
Attachment #8802772 - Flags: review?(bugspam.Callek)
Attachment #8802772 - Flags: review?(bugspam.Callek) → review+
Attached patch garndt.diffSplinter Review
Add :garndt's pubkey to the list of allowed chain of trust github repo pubkeys.
Attachment #8805609 - Flags: review?(bugspam.Callek)
Comment on attachment 8805609 [details] [diff] [review]
garndt.diff

Review of attachment 8805609 [details] [diff] [review]:
-----------------------------------------------------------------

Stamp=me for aki adding any @m.c keys (prior to tier 1, when the second human looking makes more sense; anyway)
Attachment #8805609 - Flags: review?(bugspam.Callek) → review+
This patch:
* adds a scriptworker crontab entry for gpg homedir creation.  This pulls gpg homedir creation from inside the scriptworker process, into an external cron- and puppet-driven process.  There's a lockfile, and rebuild_gpg_homedirs rebuilds in a tempdir, which should avoid conflicts.
* retriggers gpg homedir creation when we push a new pubkey to puppet
* adds a couple of entries into the scriptworker config.
Attachment #8805739 - Flags: review?(bugspam.Callek)
Comment on attachment 8805739 [details] [diff] [review]
0.9.0 puppet.diff

Review of attachment 8805739 [details] [diff] [review]:
-----------------------------------------------------------------

::: modules/signing_scriptworker/templates/scriptworker.cron.erb
@@ +2,5 @@
> +  # 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/. -%>
> +MAILTO=root
> +*/5 * * * * <%= scope["users::signer::username"] %> <%= scope["signing_scriptworker::settings::root"] %>/bin/rebuild_gpg_homedirs <%= scope["signing_scriptworker::settings::root"] %>/scriptworker.json <%= scope["signing_scriptworker::settings::root"] %>/cot_config.json >/dev/null 2>&1
> +@reboot <%= scope["users::signer::username"] %> <%= scope["signing_scriptworker::settings::root"] %>/bin/create_initial_gpg_homedirs <%= scope["signing_scriptworker::settings::root"] %>/scriptworker.json <%= scope["signing_scriptworker::settings::root"] %>/cot_config.json >/dev/null 2>&1

I'm not sure (offhand) if @reboot has the "system got halted/kernel-crashed" execution issue that I saw in the past on foopies (e.g. that it wasn't always run when there was an issue like that).  On the presumption that we'll know pretty quickly if this wasn't run and needs to be, I don't care enough to block on a different approach.
Attachment #8805739 - Flags: review?(bugspam.Callek) → review+
(In reply to Justin Wood (:Callek) from comment #17)
> Comment on attachment 8805739 [details] [diff] [review]
> 0.9.0 puppet.diff
> 
> Review of attachment 8805739 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: modules/signing_scriptworker/templates/scriptworker.cron.erb
> @@ +2,5 @@
> > +  # 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/. -%>
> > +MAILTO=root
> > +*/5 * * * * <%= scope["users::signer::username"] %> <%= scope["signing_scriptworker::settings::root"] %>/bin/rebuild_gpg_homedirs <%= scope["signing_scriptworker::settings::root"] %>/scriptworker.json <%= scope["signing_scriptworker::settings::root"] %>/cot_config.json >/dev/null 2>&1
> > +@reboot <%= scope["users::signer::username"] %> <%= scope["signing_scriptworker::settings::root"] %>/bin/create_initial_gpg_homedirs <%= scope["signing_scriptworker::settings::root"] %>/scriptworker.json <%= scope["signing_scriptworker::settings::root"] %>/cot_config.json >/dev/null 2>&1
> 
> I'm not sure (offhand) if @reboot has the "system got halted/kernel-crashed"
> execution issue that I saw in the past on foopies (e.g. that it wasn't
> always run when there was an issue like that).  On the presumption that
> we'll know pretty quickly if this wasn't run and needs to be, I don't care
> enough to block on a different approach.

Yeah, you're right.  I don't even think we need it on reboot, really.  Puppet should re-run if we need to, and there are easy ways to manually force it (remove or alter the pubkeys checksum file, remove or alter the git revision file).  I think I'm going to remove the @reboot line.
https://hg.mozilla.org/build/puppet/rev/b50e0836e779d8f99be9a3d1858faa2be9e63a4c
bug 1309293 - fix gpg homedir creation by pulling it out of the scriptworker process. r=callek
Depends on: 1315415
Callek:
a) could you do your l10n stuff here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=caab45343721077834132aea83241742f9c2a231
b) what's the secret?
Flags: needinfo?(bugspam.Callek)
n/m, this was breaking date, so I landed with an r=bustage. I'd still like to learn the trick though.
Flags: needinfo?(bugspam.Callek)
https://github.com/mozilla-releng/scriptworker/pull/25 landed, as did bug 1316702.

This is done for signing scriptworker, and the framework is there for all other instance types.  Resolving and opening new bugs for those.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Summary: scriptworker chain of trust verification → signing scriptworker chain of trust verification
Blocks: 1317789
Component: Tools → General
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: