-
Notifications
You must be signed in to change notification settings - Fork 461
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
apt::keyring doesn't validate key id #1175
Comments
How would you acquire the key ID to check against? Typically that is from the same source as the key itself, so I'm not sure this would enhance security very much. Such a check should be optional, at least. About supporting imports from keyservers, since |
The optional check would be completely fine, just give the user the choice to choose if the want to check.
👍 makes sense, I guess its pretty hard to check the IDs with pure puppet.
often the IDs are mentioned in the docs of the repo, for example elk: https://www.elastic.co/guide/en/elasticsearch/reference/current/deb.html#deb-key |
Right, I'm just wondering if the extra verification would be serving a useful purpose. Since those docs and the key are from the same place, do you really gain security by providing the key ID to puppet and having puppet use GPG to check the ID? If someone can compromise the key, wouldn't they also be able to update key ID in the docs? |
if someone gained access to the reposerver/keyserver after I added the key and id to our puppet the key doesn't get updated with the infected key automatically. In my opinion a second factor is better than just blindly update the key automatically by puppet. |
with the new method
apt::keyring
we can just pass an url which gets downloaded and stored to file directly without any checks.In the old world of apt-key we could pass an id of a pub key and only if that id matches the key gets imported: https://github.com/puppetlabs/puppetlabs-apt/blob/main/lib/puppet/provider/apt_key/apt_key.rb#L182
In terms of security, it would be nice to:
The text was updated successfully, but these errors were encountered: