Make rpm_key aware of multiple keys in a keyfile #75251
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Google makes a RPM key file containing multiple public keys publicly avaiable which is great for making sure ansible's rpm_key module can handle multiple keys being present in a single file.
Multiple keys can be concatenated into a single file however. This aims to make the module check for multiple key fingerprints beng present in a single file. This also allows for a list of fingerprints to be passed to module arguments for verification. If there's any mismatch, then the module throws an error.
ansibot
added
affects_2.12
bug
samdoran
added
the
ci_verified
samdoran
reviewed
Jul 15, 2021
| - name: Issue 50615 - Verify key fingerprints. This should fail due to multiple key signatures | ||
| rpm_key: | ||
| # TODO: This needs to be rehosted on ansible CI infra | ||
| key: https://dl.google.com/linux/linux_signing_key.pub |
There was a problem hiding this comment.
Suggested change
| key: https://dl.google.com/linux/linux_signing_key.pub | |
| key: https://ansible-ci-files.s3.amazonaws.com/test/integration/targets/rpm_key/linux_signing_key.pub |
samdoran
reviewed
Jul 15, 2021
| - name: Issue 50615 - Add Google GPG keys to system | ||
| rpm_key: | ||
| # TODO: This needs to be rehosted on ansible CI infra | ||
| key: https://dl.google.com/linux/linux_signing_key.pub |
There was a problem hiding this comment.
Suggested change
| key: https://dl.google.com/linux/linux_signing_key.pub | |
| key: https://ansible-ci-files.s3.amazonaws.com/test/integration/targets/rpm_key/linux_signing_key.pub |
nitzmahone
added
P3
sivel
requested changes
Jul 15, 2021
| return True | ||
| for keyid in keyids: | ||
| if keyid in line.split(':')[4]: | ||
| matching_keys += 1 |
There was a problem hiding this comment.
We're concerned about the potential edge cases where this may return a false positive. I think we'd be more comfortable, if we actually tracked the matching keys, and compared them against the actual content of keyids instead of asserting the lengths are equal.
There was a problem hiding this comment.
Good point! I'll update this so it does an actual comparison.
| - name: Issue 50615 - Verify key fingerprints. This should fail due to multiple key signatures | ||
| rpm_key: | ||
| # TODO: This needs to be rehosted on ansible CI infra | ||
| key: https://dl.google.com/linux/linux_signing_key.pub |
There was a problem hiding this comment.
We'll need to get these uploaded to our S3 bucket. cc @samdoran
| elif ret.startswith('0X'): | ||
| normalized_keyids.append(ret[2:]) | ||
| else: | ||
| normalized_keyids.append(ret) |
There was a problem hiding this comment.
Can we maybe keep normalize_keyid and create a normalize_keyids that calls the singular version. That can make unit testing of this function in the future easier.
ansibot
added
the
stale_ci
ansibot
added
core_review
samdoran
added
the
ci_verified
ansibot
added
the
stale_ci
|
I let this go stale; I'll redo it and resubmit in a new PR. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Fixes #50615.
Make
rpm_keymodule use lists for key ids since a single key file can contain multiple keys.I also made the fingerprint option accept lists of fingerprints since they correspond with key ids. All fingerprints in a keyfile should be present when the fingerprints option is set for task success.
ISSUE TYPE
COMPONENT NAME
rpm_key
ADDITIONAL INFORMATION
Example:
The rpm_key module only checked for the presence of the first key in a file when importing keys from files. This meant that in cases where a single file contained multiple keys it was possible for later keys to not be imported if the first key in a key file was already installed. It could also mean that multiple keys could be installed even when only expecting a single key in a file.