Skip to content

Use full key ID when adding GPG keys on Ubuntu#329

Merged
truthbk merged 2 commits intoDataDog:masterfrom
pid1:master
Jul 5, 2017
Merged

Use full key ID when adding GPG keys on Ubuntu#329
truthbk merged 2 commits intoDataDog:masterfrom
pid1:master

Conversation

@pid1
Copy link
Copy Markdown
Contributor

@pid1 pid1 commented Jun 1, 2017

To decrease the chances of a collision attack, use the full key fingerprint.

@pid1
Copy link
Copy Markdown
Contributor Author

pid1 commented Jun 1, 2017

Looks like this failed because of the check on the GPG fingerprint changing, which is entirely valid. Happy to see that included. I assume we can just have someone on the team verify the fingerprint before merging as a workaround, and update the Travis check.

@pid1
Copy link
Copy Markdown
Contributor Author

pid1 commented Jun 1, 2017

Ah, I found that defined in

. If desired, I can add a fix for that to the PR as well.

@truthbk
Copy link
Copy Markdown
Member

truthbk commented Jun 16, 2017

@pid1 I'd be happy to make this change - you're absolutely right, the short key ID has been proven vulnerable, and the change should be transparent for users that have the right keys installed. Let's get the tests fixed before merging this in :)

Thank you!

@truthbk truthbk added this to the 1.11.0 milestone Jun 16, 2017
@pid1
Copy link
Copy Markdown
Contributor Author

pid1 commented Jun 22, 2017

@truthbk Travis checks are fixed; we should be good to go, but if there is anything else that needs updating let me know.

Copy link
Copy Markdown
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this @pid1 - looks ready to go. I appreciate the effort! 👍

@truthbk truthbk merged commit 143154d into DataDog:master Jul 5, 2017
@pid1
Copy link
Copy Markdown
Contributor Author

pid1 commented Jul 6, 2017

Happy to help!

@ColinHebert
Copy link
Copy Markdown
Contributor

Hey @truthbk, @pid1, this change broke the install_key behaviour. It used to rely on apt-key list using the short ID to detect whether the command needed to be executed. Now because we're using the fingerprint, the unless condition is never met, which means that the exec is run no matter what each time.

@pid1
Copy link
Copy Markdown
Contributor Author

pid1 commented Oct 24, 2017

@ColinHebert Thanks for the heads up. I'll send up a new PR to fix that behavior ASAP.

cegeka-jenkins pushed a commit to cegeka/puppet-datadog_agent that referenced this pull request Jan 31, 2018
* Use the full key fingerprint

* Fix failing Travis check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants