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

SLING-8029 Retrieve gpg key automatically if it is missing in keyring #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ghenzler
Copy link

@rombert Please have a look if this makes sense for you (tested on OSX but should work on linux as well)

@rombert
Copy link
Contributor

rombert commented Oct 17, 2018

@ghenzler - should we actually do this? I am not sure since key import is a sensitive operation and we should not trust just any key.

@ghenzler
Copy link
Author

ghenzler commented Oct 17, 2018

@rombert Just read through [1] - I think it should be fine (although it would be better if we all had cross-signed keys, at least I haven't done that).

Updated the PR to use --auto-key-retrieve and to log the gpg error if there is one (needed to introduce the check for maven-metadata.xml for that reason).

If we decide to not automatically retrieve the key we can just take out that option, now that the error is logged for the case there is one, it is better self-guiding than before (that was my main problem, I got the message signature "BAD" message when really just the key was missing).

[1] https://unix.stackexchange.com/a/356107

@kwin
Copy link
Member

kwin commented Oct 18, 2018

If I understand correctly, we should trust if the public keys are also listed in https://people.apache.org/keys/group/sling.asc, as that one contains the trusted list of public keys (as those require ASF credentials to add there). For more details see http://sling.apache.org/documentation/development/release-management.html#appendix-a-create-and-add-your-key-to-peopleapacheorg. Is it possible to validate against this list or directly import the public keys from there (as that is a trusted source)?

@rombert
Copy link
Contributor

rombert commented Oct 19, 2018

The worst-case scenario I'm thinking of is the following:

  1. PMC member "Alice" goes on vacation.
  2. Malicious actor "Charlie" creates a GPG key and pushes it to the keyserver, using Alice's email
  3. Charlie breaks into Alice's Nexus account and uploads a malicious release
  4. Charlie forges an email coming from Alice and starts a vote on the dev list with the malicious release
  5. PMC members vote +1 on the release and the key is automatically accepted

Granted, it's a pretty convoluted scenario but it only needs one weakness - the Nexus account credentials from a PMC member. Not automatically importing GPG keys would add a second layer.

It might be that I'm overthinking this and that this is not a really big issue :-)

But I fully agree that at least displaying the error message from GPG would be a great improvement.

@kwin
Copy link
Member

kwin commented Oct 19, 2018

For exactly this reason we only trust keys within https://people.apache.org/keys/group/sling.asc, right? Can we just import those?

@klcodanr
Copy link
Contributor

That scenario could happen now if someone lost control of their ASF account. It would just have a manual step to update the key.

I would think though that the script should display a warning, display some explanation on how to update the key and have the user to perform the update.

Ideally if it could display some sort of helpful information on whether the key is expired, revoked, etc that would be good to help determine if the key update can be trusted.

@ghenzler
Copy link
Author

If we do not use --auto-key-retrieve people will have to use --recv-keys, but I'm not sure that really everyone would cross-check with the fingerprint in [1]... we could automate that by going back to a solution similar to 2a986a9 (without --auto-key-retrieve), but with automatically cross-checking the fingerprint of the received key with [1] (using curl to get [1] and find the downloaded key's fingerprint in that result and then only running gpg --verify if valid)

[1] https://people.apache.org/keys/group/sling.asc

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.

4 participants