Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare pkg keys rotation #365

Merged
merged 3 commits into from Nov 15, 2016

Conversation

Projects
None yet
3 participants
@olivielpeau
Copy link
Member

olivielpeau commented Sep 22, 2016

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.

@@ -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"

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Sep 22, 2016

Author Member

@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?

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Sep 22, 2016

Author Member

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

This comment has been minimized.

Copy link
@yannmh

yannmh Sep 29, 2016

Member

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

👍

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 5, 2016

Author Member

thanks, updated to DATADOG_RPM_KEY_E09422B3.public

action :install
end

# Download new RPM key

This comment has been minimized.

Copy link
@truthbk

truthbk Sep 22, 2016

Member

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"

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Sep 23, 2016

Author Member

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?

This comment has been minimized.

Copy link
@truthbk
@yannmh

This comment has been minimized.

Copy link
Member

yannmh commented Sep 23, 2016

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

yannmh approved these changes Sep 23, 2016

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

This comment has been minimized.

Copy link
@yannmh

yannmh Sep 23, 2016

Member

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

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Sep 23, 2016

Author Member

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.

This comment has been minimized.

Copy link
@yannmh

yannmh Sep 23, 2016

Member

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.

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Sep 26, 2016

Author Member

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 olivielpeau force-pushed the olivielpeau/trust-new-keys branch 3 times, most recently from f6dc628 to 4d75c5e Sep 26, 2016

@olivielpeau

This comment has been minimized.

Copy link
Member Author

olivielpeau commented Sep 26, 2016

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

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

@olivielpeau olivielpeau added this to the 2.7.0 milestone Sep 27, 2016

[apt][repository] Trust new apt key
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.

@olivielpeau olivielpeau force-pushed the olivielpeau/trust-new-keys branch from 4d75c5e to 8ecc933 Oct 5, 2016

olivielpeau added some commits Sep 22, 2016

[yum][repository] Import new RPM 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.
[ci] Set Ohai log level to :warn
Otherwise `rake spec` can be spammed by debug-level log messages

@olivielpeau olivielpeau force-pushed the olivielpeau/trust-new-keys branch from 8ecc933 to 028aa79 Oct 5, 2016

@olivielpeau olivielpeau merged commit f53a49e into master Nov 15, 2016

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@olivielpeau olivielpeau deleted the olivielpeau/trust-new-keys branch Nov 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.