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

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

Merged
merged 2 commits into from
Jul 5, 2017
Merged

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

merged 2 commits into from
Jul 5, 2017

Conversation

pid1
Copy link
Contributor

@pid1 pid1 commented Jun 1, 2017

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

@pid1
Copy link
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
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
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
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
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
Contributor Author

pid1 commented Jul 6, 2017

Happy to help!

@ColinHebert
Copy link
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
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