Update an existing GPG key when the same key is added again

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/gitlab:jmrp-gpg-key-update-in-place into master

What does this MR do and why?

A GPG key cannot be changed after it is added: a new version of the same key (a later expiration, a new subkey or user ID, a revocation) fails with "Fingerprint has already been taken", so users remove the key and add it again, which unlinks every signature and checks it again.

Now, when the key added in User settings > GPG keys has the fingerprint of one of the user's keys, Repositories::GpgKeys::UpdateService merges the new version into it, as gpg --import does.

  • Nothing is removed, and an older version cannot undo a revocation or an expiration change.
  • Refused with a message: another key, several keys, a secret key, a key revoked or still expired after the merge, a merged key over 4 MiB. Another user's fingerprint still answers "Fingerprint has already been taken".
  • The same text again answers "This GPG key is already up to date." and writes nothing.
  • When the merge adds something, new subkeys get their rows and InvalidGpgSignatureUpdateWorker runs with its existing rule, as after adding a key.
  • Docs: a new "Update a GPG key" section.

Follow-ups, as stacked MRs on this one, in this order:

  1. The InvalidGpgSignatureUpdater rule, so a verified subkey signature keeps its status.
  2. The owner's email, behind the updated_gpg_key_email flag (rollout issue #636223).
  3. The update_gpg_key audit event.

The REST API comes after them.

Database

No migration. The update runs GpgKey#stored_duplicate (a SELECT on gpg_keys by user_id and fingerprint, after a create failed on uniqueness), lock! (SELECT ... FOR UPDATE by id), save! (UPDATE gpg_keys ... WHERE id), and one INSERT INTO gpg_key_subkeys per new subkey, as the create does.

How I tested it

Ruby 3.3.11 and PostgreSQL 17, without the GDK:

  • bundle exec rspec spec/lib/gitlab/gpg_spec.rb spec/models/gpg_key_spec.rb spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb spec/requests/user_settings/gpg_keys_controller_spec.rb spec/features/profiles/gpg_keys_spec.rb spec/services/repositories/gpg_keys/update_service_spec.rb spec/services/gpg_keys/: 162 examples, 1 failure, the one below.
  • The service, request and model specs with FOSS_ONLY=1: 87 examples, 0 failures.
  • RuboCop, gettext_extractor, markdownlint-cli2 and Vale on the changed files: clean.

I saw the new specs fail on master's code first. invalid_gpg_signature_updater_spec.rb:70 fails on master too (a let_it_be_with_reload record keeps the other example's commit double), and this MR does not touch it.

The new fixtures in spec/support/helpers/gpg_helpers.rb come from the secret keys already there, with GnuPG 2.2.40: --gen-revoke, --quick-set-expire under --faked-system-time, --quick-add-uid, and revkey in --edit-key.

References

Closes #296977

Screenshots or screen recordings

Only the notice changes:

Before After
A new version of your key: "Fingerprint has already been taken" "GPG key updated."
The same key again: "Key has already been taken" "This GPG key is already up to date."

How to set up and validate locally

  1. Make a throwaway key with an email address of your user, and add the output in User settings > GPG keys:

    export GNUPGHOME=$(mktemp -d)
g() { gpg --batch --pinentry-mode loopback --passphrase '' "$@"; }
g --quick-gen-key 'Test User <you@example.com>' ed25519 sign 1y
FPR=$(gpg --list-keys --with-colons | awk -F: '/^fpr/ { print $10; exit }')
gpg --armor --export "$FPR"

  2. Run g --quick-add-key "$FPR" ed25519 sign 1y, then add the new gpg --armor --export "$FPR". GitLab shows GPG key updated. and lists the new subkey.

  3. Add the same export again. GitLab shows This GPG key is already up to date.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by José M. Requena Plens

Merge request reports