Prepare pkg keys rotation #365

olivielpeau · 2016-09-22T15:57:18Z

~~WIP, PR open for discussion. Tests are failing for now, I'll fix them and add new ones once we agree that this is the right approach~~

The idea is to make the pkg managers trust the new keys in addition to the current keys, in the next version of this cookbook (before the new keys are used). In the future, once we switch the APT repository and the RPM packages over to the new keys, we could release yet another version of the cookbook that only trusts the newer keys.

This approach ensures that, when we switch over to the new keys:

for people using a version of the cookbook that includes this PR, the switch will be unnoticeable
for people using an older version of the cookbook, apt pkg installs will fail (no way to work around that), rpm installs won't fail as long as the Agent is pinned to a version that's signed with the older keys.

AFAIK we import the new keys safely (through a keyserver with apt, and with a fingerprint check with rpm).

More details in the commit messages.

olivielpeau · 2016-09-22T15:59:17Z

attributes/default.rb

@@ -91,6 +91,10 @@
 default['datadog']['yumrepo_proxy_password'] = nil
 default['datadog']['windows_agent_url'] = 'https://s3.amazonaws.com/ddagent-windows-stable/'

+# Location of additional rpm gpgkey to import (with signature `e09422b3`). In the future the rpm packages
+# of the Agent will be signed with this key.
+default['datadog']['yumrepo_gpgkey_new'] = "#{yum_protocol}://yum.datadoghq.com/DATADOG_RPM_KEY.public.E09422B3"


@yannmh: the new rpm key currently lives at this URL, I'm not a huge fan of its name though, I'd prefer something like DATADOG_RPM_KEY_E09422B3.public, what do you think?

Also: we can discuss the new attribute name, I dislike new in an attribute name but haven't found a better alternative

I'd prefer something like DATADOG_RPM_KEY_E09422B3.public, what do you think?

👍

thanks, updated to DATADOG_RPM_KEY_E09422B3.public

truthbk · 2016-09-22T21:25:29Z

recipes/repository.rb

+      action :install
+    end
+
+    # Download new RPM key


I don't think you need to download the file... You should be able to "rpm --import #{yum_protocol}://yum.datadoghq.com/DATADOG_RPM_KEY.public.E09422B3"

Actually I did this to make sure that we're trusting the right key (in particular we download the key through plain http on CentOS 5 so I think it's better to check that we are importing the correct key). What do you think?

Makes sense, should've made the connection with: https://github.com/DataDog/chef-datadog/pull/365/files#diff-62b784dc2245888950b940c524931921R65

yannmh · 2016-09-23T15:13:22Z

Installation/upgrades scenario

Datadog Agent signed with the old key

+======================+============+==============+
| Scenario\Cookbook    | < versions | this version |
+----------------------+------------+--------------+
| New install          | ✓          | ✓            |
+----------------------+------------+--------------+
| Upgrade from > 5.8.5 | ✓          | ✓            |
+----------------------+------------+--------------+
| Upgrade from < 5.8.5 | ✓          | ✓            |
+----------------------+------------+--------------+

Datadog Agent signed with the new key

+======================+============+==============+
| Scenario\Cookbook    | < versions | this version |
+----------------------+------------+--------------+
| New install          | ✗          | ✓            |
+----------------------+------------+--------------+
| Upgrade from > 5.8.5 | ✓          | ✓            |
+----------------------+------------+--------------+
| Upgrade from < 5.8.5 | ✗          | ✓            |
+----------------------+------------+--------------+

yannmh · 2016-09-23T15:28:31Z

recipes/repository.rb

+    command 'apt-key adv --recv-keys --keyserver hkp://keyserver.ubuntu.com:80 A2923DFF56EDA6E76E55E492D3A80E30382E94DE'
+    not_if 'apt-key list | grep 382E94DE'
+  end
+


Naive question: can we simply use the apt cookbook twice to do that, i.e.

# Legacy key apt_repository 'datadog' do keyserver 'hkp://keyserver.ubuntu.com:80' key 'C7A7DA52' uri node['datadog']['aptrepo'] distribution node['datadog']['aptrepo_dist'] components ['main'] action :add end # New key apt_repository 'datadog' do keyserver 'hkp://keyserver.ubuntu.com:80' key '382E94DE' uri node['datadog']['aptrepo'] distribution node['datadog']['aptrepo_dist'] components ['main'] action :add end

Hmm we could use the resource twice (as long as we name the 2 differently, otherwise we're cloning the resource, which is a Bad thing to do), but that would define 2 .list files with the same repository, which apt doesn't like (it will spew out warnings because the repo is duplicated).

And it wouldn't look good either. That said I'll look into it a bit more to see if we couldn't make use of the logic that's already implemented in the apt cookbook.

No need to spend too much time on this.

I am just a little worried about the "flow" for trusting the new RPM key. It involves multiple commands, with | and grep. I was wondering if we can be sure that the output will be consistent across all systems using RPM and the different versions of rpm-import.

So I had another look at the apt cookbook, and unfortunately we can't really re-use anything from there, so I think I have to stick to the current approach.

About the rpm key: from what I've tested the rpm --import, rpm -q and gpg --with-fingerprint <keyfile> have very consistent behaviors (I've checked manually on CentOS 5 and 7). I understand your concern though, especially on the grep command which could be very sensitive to the slightest change in gpg's output, I'll see if this could be made a little clearer/more resilient.

olivielpeau · 2016-09-26T17:23:04Z

Added spec and integration tests, @yannmh could you have one last look? 🙇

Also, do you have an opinion about the name of the new RPM key? (both its current URL and the attribute name) If you're fine with them then we're all set

Ease transition to new apt key that will sign our repo in the future. This ensures that the cookbook installs the new key. When we switch to using the new key to sign the `Release` file of the apt repo, this will make sure that the new key is trusted. After the switch, we should make this new key the main key in the `apt_repository` resource and remove this `execute` resource. Optionally, we could make the cookbook delete the old key.

Ease transition to new RPM key that will sign our RPM packages in the future. This will install the new key. The location of the key is defined in an attribute for users that host the keys internally. The signature of the downloaded key is checked to make sure that the key is legitimate. Once packages start being signed with the new key, we should make the cookbook set the correct `gpgkey` on the `yum_repository` resource depending on the version of the Agent that needs installing. Optionally, we could make the cookbook delete the other key.

Otherwise `rake spec` can be spammed by debug-level log messages

olivielpeau added in progress feature needs spec test labels Sep 22, 2016

olivielpeau assigned yannmh Sep 22, 2016

olivielpeau commented Sep 22, 2016

View reviewed changes

truthbk reviewed Sep 22, 2016

View reviewed changes

yannmh approved these changes Sep 23, 2016

View reviewed changes

olivielpeau force-pushed the olivielpeau/trust-new-keys branch 3 times, most recently from f6dc628 to 4d75c5e Compare September 26, 2016 17:14

olivielpeau changed the title ~~[WIP] Prepare pkg keys rotation~~ Prepare pkg keys rotation Sep 26, 2016

olivielpeau removed in progress needs spec test labels Sep 27, 2016

olivielpeau added this to the 2.7.0 milestone Sep 27, 2016

olivielpeau mentioned this pull request Sep 27, 2016

Trust new APT and RPM keys DataDog/ansible-datadog#30

Merged

olivielpeau force-pushed the olivielpeau/trust-new-keys branch from 4d75c5e to 8ecc933 Compare October 5, 2016 16:53

olivielpeau added 2 commits October 5, 2016 13:51

[ci] Set Ohai log level to :warn

028aa79

Otherwise `rake spec` can be spammed by debug-level log messages

olivielpeau force-pushed the olivielpeau/trust-new-keys branch from 8ecc933 to 028aa79 Compare October 5, 2016 17:53

olivielpeau merged commit f53a49e into master Nov 15, 2016

olivielpeau deleted the olivielpeau/trust-new-keys branch November 15, 2016 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare pkg keys rotation #365

Prepare pkg keys rotation #365

olivielpeau commented Sep 22, 2016 •

edited

Loading

olivielpeau Sep 22, 2016

olivielpeau Sep 22, 2016

yannmh Sep 29, 2016

olivielpeau Oct 5, 2016

truthbk Sep 22, 2016 •

edited

Loading

olivielpeau Sep 23, 2016

truthbk Sep 23, 2016

yannmh commented Sep 23, 2016

yannmh Sep 23, 2016

olivielpeau Sep 23, 2016

yannmh Sep 23, 2016

olivielpeau Sep 26, 2016

olivielpeau commented Sep 26, 2016

Prepare pkg keys rotation #365

Prepare pkg keys rotation #365

Conversation

olivielpeau commented Sep 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

truthbk Sep 22, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yannmh commented Sep 23, 2016

Installation/upgrades scenario

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olivielpeau commented Sep 26, 2016

olivielpeau commented Sep 22, 2016 •

edited

Loading

truthbk Sep 22, 2016 •

edited

Loading