Closed Bug 1253309 Opened 9 years ago Closed 8 years ago

Signing Linux 64 packages with GPG using the signing worker

Categories

(Release Engineering :: General, defect)

Product:
Release Engineering
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: selenamarie, Assigned: mozilla)

References

Details

Attachments

(5 files, 2 obsolete files)

interdiff
4.67 KB, patch
Details | Diff | Splinter Review
puppetize_signing_scriptworker2.diff
35.55 KB, patch
Callek
: review+
Details | Diff | Splinter Review
bug 1253309 - signing payload links (signingscript==0.4.0).
58 bytes, text/x-review-board-request
Callek
: review+
Details
puppet.diff
2.54 KB, patch
rail
: review+
Details | Diff | Splinter Review
cltsign_not_cltbld.diff
3.58 KB, patch
rail
: review+
Details | Diff | Splinter Review
This is a planning bug to track work related to signing Linux 64 packages with GPG using the signing worker.
We already do similar thing for release tarballs: GPG of a checksum of files. In this case we would need to sign a tarball instead. We use https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/source.yml.tmpl#L79-L127 to define a task. One thing that may be tricky is to sign the task. Signing workers look at task.extra.signing.signature and verify if the task was signed using a known GPG key. Otherwise signing workers reject signing. Example task: https://tools.taskcluster.net/task-inspector/#j41mkl7CRcONmNQjLbNjcw/
Assignee: nobody → aki
Depends on: 1244181
Depends on: 1279890
We have scriptworker, so this doesn't depend on the balrog portion anymore. I'm running on py2 signtool, so this doesn't hard block on py3 signtool.
No longer depends on: 1244181, 1279890
I'm able to sign files through taskcluster -> signing scriptworker at this point. We want to hook everything up as we get the entire nightly graph running, and we want to add the chain of trust so we don't sign invalid files.
* I worked with :garbas to get the signingscript + scriptworker in docker with nix ( https://github.com/escapewindow/docker-nix-signingscript ). * Currently I'm going to just deploy to a micro centos ec2 instance, so I may just use puppet initially, without nix+docker. Working on the puppet portion atm. https://github.com/escapewindow/build-puppet/tree/scriptworker
Created http://puppetagain.pub.build.mozilla.org/data/python/packages-3.5/ and http://puppetagain.pub.build.mozilla.org/data/repos/yum/custom/mozilla-python35/x86_64/ , which I'm pretty sure won't break anything. I think I'm ready to test the puppet changes, but I'm a bit more nervous about those breaking something. Reading up on https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/HowTo/Set_up_a_user_environment .
(In reply to Aki Sasaki [:aki] from comment #5) > I think I'm ready to test the puppet changes, Ah, I can test now, but I still have to add alert-on-ssh. I imagine I'll test first and make sure things are working, and then add that.
Depends on: 1289247
Freshly puppetized signing run: https://tools.taskcluster.net/task-inspector/#CS6ogijpRSiCMT_Zcwt1Qg/ I imagine both scriptworker and signingscript will change in non-trivial ways in the not-distant future, but let's roll out now and make incremental changes as we go. This patch: * adds $signing_scriptworker variables to moco-config.pp * adds signing scriptworkers to moco-nodes.pp, with the name signing-linux-.* * adds python35 support! This is similar to, but different than, py27 support. I think it's different enough that using a separate module makes sense. * adds the various config files for both scriptworker and signingscript. passwords.json.erb is similar to, but different from, the other ones (signingworker, buildbot masters) in that it only points at the dep signing servers, even for nightly and release signing. Once we implement Chain of Trust and are ready to promote to tier 1, we can point at the nightly and release signing servers. Let me know if you have any questions! If you're not the right reviewer, or don't have time to look, let me know and I can ask someone else.
Attachment #8780326 - Flags: review?(bugspam.Callek)
Comment on attachment 8780326 [details] [diff] [review] puppetize_signing_scriptworker.diff Review of attachment 8780326 [details] [diff] [review]: ----------------------------------------------------------------- Overall one blocking issue, and a few nits. Looks good. My only implementation concern is the "log" doesn't show up in the task inspector view, have to click-through. That said I'm not certain thats easily possible for jobs using a different worker, so far from blocking (and isn't a puppet change anyway). ::: manifests/moco-config.pp @@ +395,5 @@ > + # TC signing scriptworkers > + $signing_scriptworker_provisioner_id = "scriptworker-prov-v1" > + $signing_scriptworker_worker_group = "signing-linux-v1" > + $signing_scriptworker_worker_type = "signing-linux-v1" > + $signing_scriptworker_taskcluster_client_id = secret("signing_scriptworker_taskcluster_client_id") I, at least, don't consider client_id to be secret. *however* since everything else in puppet currently does this is probably safest :-) @@ +398,5 @@ > + $signing_scriptworker_worker_type = "signing-linux-v1" > + $signing_scriptworker_taskcluster_client_id = secret("signing_scriptworker_taskcluster_client_id") > + $signing_scriptworker_taskcluster_access_token = secret("signing_scriptworker_taskcluster_access_token") > + $signing_scriptworker_root = "/builds/scriptworker" > + $signing_scriptworker_signingscript_repo = "https://github.com/escapewindow/signingscript" if we're deploying this -- I'd rather it be in either `mozilla/` or `mozilla-releng/` :hwine can help with the process to transfer ownership. I do consider this issue blocking. ::: modules/packages/manifests/mozilla/python35.pp @@ +10,5 @@ > + > + case $::operatingsystem { > + default: { > + # everywhere else, we install from a custom-built package > + $python = '/tools/python35/bin/python3.5' nit: even though this is namespaces to be ::mozilla::python35 it makes sense mentally to me to be $python3 rather than $python @@ +22,5 @@ > + target => "/tools/python35"; > + } -> Anchor['packages::mozilla::python35::end'] > + > + case $::operatingsystem { > + CentOS: { feels strange to have case $::operatingsystem inside a case $::operatingsystem. Maybe pull this out, so all logic is case CentOS: and default: is a fail("Don't know how...") ? (It's not like we have a package for Ubuntu/Windows/Etc yet.) @@ +38,5 @@ > + } > + } > + > + # sanity check > + if (!$python) { nit: if you change the var above, here too ::: modules/packages/manifests/mozilla/python35.spec @@ +13,5 @@ > +Group: mozilla > +License: Python > +URL: http://python.org > +Source0: http://python.org/ftp/python/%{pyver}.%{pyrel}/Python-%{pyver}.%{pyrel}.tgz > +Patch0: python-2.6-fix-cgi.patch Patch0 could use a comment explaining why its here, since this is py35, quick glances may be like "huh why" and not realize its still needed. ::: modules/python35/manifests/user_pip_conf.pp @@ +31,5 @@ > + ensure => directory, > + owner => $user, > + group => $group_; > + "$homedir_/.pip/pip.conf": > + content => template("python35/user-pip-conf.erb"), (note: I don't consider this blocking at present) -- I *think* this would conflict with py27 if we ever installed it alongside py3... Also I'm not sure all that pip magic in our puppet is necessary for py3 given: https://docs.python.org/3/installing/ ""pip is the preferred installer program. Starting with Python 3.4, it is included by default with the Python binary installers."" ::: modules/signing_scriptworker/manifests/init.pp @@ +52,5 @@ > + mode => 600, > + owner => "${users::builder::username}", > + group => "${users::builder::group}", > + content => template("${module_name}/script_config.json.erb"), > + show_diff => false; I don't see any passwords in script_config.json, if there is indeed nothing secret I'd rather omit `show_diff => false` for it. ::: modules/signing_scriptworker/templates/nagios.cfg.erb @@ +1,1 @@ > +command[check_signing_scriptworker]=<%= scope.lookupvar("nrpe::base::plugins_dir") %>/check_procs -c 1:1 -C scriptworker Good call here, of course once this deploys we'll need nagios config for it -- but since you added this I presume you already know that.
Attachment #8780326 - Flags: review?(bugspam.Callek) → review-
Attached patch interdiff — — Splinter Review
Changes I've made to address review comments.
See the interdiff for incremental changes. (In reply to Justin Wood (:Callek) from comment #10) > Overall one blocking issue, and a few nits. Looks good. Thanks! > My only implementation concern is the "log" doesn't show up in the task > inspector view, have to click-through. That said I'm not certain thats > easily possible for jobs using a different worker, so far from blocking (and > isn't a puppet change anyway). This is now bug 1294785 . > ::: manifests/moco-config.pp > @@ +395,5 @@ > > + # TC signing scriptworkers > > + $signing_scriptworker_provisioner_id = "scriptworker-prov-v1" > > + $signing_scriptworker_worker_group = "signing-linux-v1" > > + $signing_scriptworker_worker_type = "signing-linux-v1" > > + $signing_scriptworker_taskcluster_client_id = secret("signing_scriptworker_taskcluster_client_id") > > I, at least, don't consider client_id to be secret. *however* since > everything else in puppet currently does this is probably safest :-) I think it's in there unencrypted. Since the client_id and access_token are tied, it makes sense to me to put them in the same place; as you noted, I followed an existing pattern for that. > @@ +398,5 @@ > > + $signing_scriptworker_worker_type = "signing-linux-v1" > > + $signing_scriptworker_taskcluster_client_id = secret("signing_scriptworker_taskcluster_client_id") > > + $signing_scriptworker_taskcluster_access_token = secret("signing_scriptworker_taskcluster_access_token") > > + $signing_scriptworker_root = "/builds/scriptworker" > > + $signing_scriptworker_signingscript_repo = "https://github.com/escapewindow/signingscript" > > if we're deploying this -- I'd rather it be in either `mozilla/` or > `mozilla-releng/` :hwine can help with the process to transfer ownership. I actually stopped cloning signingscript in puppet; we now pull from a puppet python package. This was left over from before. However, point taken, and agreed. I have moved the RoR to mozilla-releng/signingscript. I had already moved scriptworker and signtool previously. > I do consider this issue blocking. > > ::: modules/packages/manifests/mozilla/python35.pp > @@ +10,5 @@ > > + > > + case $::operatingsystem { > > + default: { > > + # everywhere else, we install from a custom-built package > > + $python = '/tools/python35/bin/python3.5' > > nit: even though this is namespaces to be ::mozilla::python35 it makes > sense mentally to me to be $python3 rather than $python Fixed. > @@ +22,5 @@ > > + target => "/tools/python35"; > > + } -> Anchor['packages::mozilla::python35::end'] > > + > > + case $::operatingsystem { > > + CentOS: { > > feels strange to have case $::operatingsystem inside a case > $::operatingsystem. > > Maybe pull this out, so all logic is case CentOS: and default: is a > fail("Don't know how...") ? (It's not like we have a package for > Ubuntu/Windows/Etc yet.) Done. > @@ +38,5 @@ > > + } > > + } > > + > > + # sanity check > > + if (!$python) { > > nit: if you change the var above, here too Done. > ::: modules/packages/manifests/mozilla/python35.spec > @@ +13,5 @@ > > +Group: mozilla > > +License: Python > > +URL: http://python.org > > +Source0: http://python.org/ftp/python/%{pyver}.%{pyrel}/Python-%{pyver}.%{pyrel}.tgz > > +Patch0: python-2.6-fix-cgi.patch > > Patch0 could use a comment explaining why its here, since this is py35, > quick glances may be like "huh why" and not realize its still needed. Done. > ::: modules/python35/manifests/user_pip_conf.pp > @@ +31,5 @@ > > + ensure => directory, > > + owner => $user, > > + group => $group_; > > + "$homedir_/.pip/pip.conf": > > + content => template("python35/user-pip-conf.erb"), > > (note: I don't consider this blocking at present) -- I *think* this would > conflict with py27 if we ever installed it alongside py3... The two erb files have 0 diffs, so I would guess they wouldn't conflict. Do you know how to reuse the python[2] template inside the python35 package? > Also I'm not sure all that pip magic in our puppet is necessary for py3 > given: > > https://docs.python.org/3/installing/ > > ""pip is the preferred installer program. Starting with Python 3.4, it is > included by default with the Python binary installers."" Pip is there, but pip.conf points at our puppet servers for python packages and prevents pip from pointing at pypi.python.org, so I think it's important to keep. > ::: modules/signing_scriptworker/manifests/init.pp > @@ +52,5 @@ > > + mode => 600, > > + owner => "${users::builder::username}", > > + group => "${users::builder::group}", > > + content => template("${module_name}/script_config.json.erb"), > > + show_diff => false; > > I don't see any passwords in script_config.json, if there is indeed nothing > secret I'd rather omit `show_diff => false` for it. Done. > ::: modules/signing_scriptworker/templates/nagios.cfg.erb > @@ +1,1 @@ > > +command[check_signing_scriptworker]=<%= scope.lookupvar("nrpe::base::plugins_dir") %>/check_procs -c 1:1 -C scriptworker > > Good call here, of course once this deploys we'll need nagios config for it > -- but since you added this I presume you already know that. As noted in IRC, I'm going to do this as a followup step.
Attachment #8780326 - Attachment is obsolete: true
Attachment #8780675 - Flags: review?(bugspam.Callek)
(In reply to Aki Sasaki [:aki] from comment #12) > Created attachment 8780675 [details] [diff] [review] > puppetize_signing_scriptworker2.diff > > ::: modules/python35/manifests/user_pip_conf.pp > > @@ +31,5 @@ > > > + ensure => directory, > > > + owner => $user, > > > + group => $group_; > > > + "$homedir_/.pip/pip.conf": > > > + content => template("python35/user-pip-conf.erb"), > > > > (note: I don't consider this blocking at present) -- I *think* this would > > conflict with py27 if we ever installed it alongside py3... > > The two erb files have 0 diffs, so I would guess they wouldn't conflict. Do > you know how to reuse the python[2] template inside the python35 package? Few Options: * ```template("python/user-pip-conf.erb")``` (Though that could pose an issue with future included vars) * Move the pip config stuff out of python/ and python35/ into a pip-config/ and include it in both py3 and py2 stuff * simply `ln -s ...` the erb's and commit. Noteworthy, since the erb's are the same and generate same output (afaict at a quick glance) this is also not blocking and can be factored out to another bug.
Comment on attachment 8780674 [details] [diff] [review] interdiff Review of attachment 8780674 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/signing_scriptworker/manifests/init.pp @@ +53,4 @@ > owner => "${users::builder::username}", > group => "${users::builder::group}", > content => template("${module_name}/script_config.json.erb"), > + show_diff => true; show_diff => true is default, but doesn't hurt.
Comment on attachment 8780675 [details] [diff] [review] puppetize_signing_scriptworker2.diff Review of attachment 8780675 [details] [diff] [review]: ----------------------------------------------------------------- Based on interdiff
Attachment #8780675 - Flags: review?(bugspam.Callek) → review+
Rolled out again to signing-linux-1... I nuked /build/scriptworker and redeployed. Re-ran the signing task: https://tools.taskcluster.net/task-inspector/#YIeXSc-6RCagXt_utUHIUw/
Comment on attachment 8780791 [details] bug 1253309 - bump signingscript==0.3.0. https://reviewboard.mozilla.org/r/71422/#review68950
Attachment #8780791 - Flags: review?(bugspam.Callek) → review+
Do we need live-logging for script/signing workers, or just the final logs?
(In reply to Chris AtLee [:catlee] from comment #22) > Do we need live-logging for script/signing workers, or just the final logs? That's bug 1294785. IMO, single-binary gpg signing happens so quickly that live logs are of limited benefit, but certainly if we start hitting retries with backoffs or start running longer jobs in scriptworker they'll be of more benefit. So I'm thinking it's a non-blocker nice-to-have that we can bump in priority if we want.
Depends on: 1295196
Attachment #8780791 - Attachment is obsolete: true
Comment on attachment 8781330 [details] bug 1253309 - signing payload links (signingscript==0.4.0). https://reviewboard.mozilla.org/r/71746/#review69484 future-stamp for r+ handed to :aki for version bumps here from me.
Attachment #8781330 - Flags: review?(bugspam.Callek) → review+
https://hg.mozilla.org/build/puppet/rev/cb4186cc88a3d74e1f0824863c76066c1e205f1e bug 1253309 - signing payload links (signingscript==0.4.0). r=callek
Attached patch puppet.diff — — Splinter Review
* only allow the shortlist of ssh users to the signing scriptworkers * chmod /builds/scriptworker 700 worked in my env.
Attachment #8783152 - Flags: review?(rail)
Attachment #8783152 - Flags: review?(rail) → review+
https://hg.mozilla.org/build/puppet/rev/d0a15602f5b637d020e7e5133d63c932173a0c0e bug 1253309 - stricter permissions on signing scriptworkers. r=rail
Switches the user from cltbld to cltsign. Since cltbld has lots users in .ssh/authorized_keys and cltsign has none, this helps lock down who can access the signing scriptworkers. When testing, I had to: * first run puppet once to create the cltsign user * reboot because cltbld was logged in * re-run puppet, noticed scriptworker wasn't running. many files in /builds/scriptworker/* were owned by 500 instead of cltsign * rm -rf /builds/scriptworker; reran puppet. that worked. I think we'll be able to simplify by * run puppet once * rm -rf /builds/scriptworker * reboot * rerun puppet
Attachment #8784484 - Flags: review?(rail)
Attachment #8784484 - Flags: review?(rail) → review+
We're switched over to cltsign instead of cltbld. signing-linux-3 ran this test task just fine: https://tools.taskcluster.net/task-inspector/#VdKQHUb5SPK1t8xY9-y1vA/
Aki: is there any update here? Are we waiting/blocked on the CoT work?
Flags: needinfo?(aki)
We should be done; I'm waiting to make sure Kim's graph works and lands, with the signing bits. Want me to close this out?
Flags: needinfo?(aki)
(In reply to Aki Sasaki [:aki] from comment #36) > We should be done; I'm waiting to make sure Kim's graph works and lands, > with the signing bits. > Want me to close this out? WFM.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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: