I think dropping import-clean from the default keyserver options is the right way to go. It is not clear what additional benefit import-clean provides given that we are already using self-sigs-only. And the idea of non-additive behavior to the local keyring when pulling from a keyserver is a deeply surprising change for multiple users i've talked to.

I've just pushed rGbe99eec2b105eb5f8e3759147ae351dcc40560ad to the dkg-fix-T4628 branch, which should address this.

You are partly right. I missed that we also do clean the original keyblock while updating a key. The code is

strip_self_sig (imported_keyblock)
clean_key (imported_keyblock)
....
if (is_new_key)
     clean_key (imported_keyblock)
     insert_key (imported_keyblock)
else
    clean_key (imported_keyblock)
    merge_key (imported into original_keyblock)
    clean_key (original_keyblock)  [1]
    update_key (original_keyblock)
ready

only [1] here is wrong. The fix is thus not to drop IMPORT_CLEAN but drop it only at [1] so that we keep the benefits of IMPORT_CLEAN on the imported_keyblock.
We need to figure out the intention of the given options. For example: SELF_SIGS_ONLY and IMPORT_CLEAN could be interpreted
as not wanting to do [1].

that pseudocode is strange to me -- it looks like you have (two) duplicate calls to clean_key (imported_keyblock) (though maybe i just don't know what .... means in this pseudocode).

I can see this regression characterized (rightly) as a potentially-severe data loss bug. it also has implications on signature validity checks.

So i'm a little surprised at the "Normal" priority. Until there is an approved upstream resolution for this, i'll be patching out the "import-clean" default entirely to avoid both of these problems.

Data loss happens in cases like this:

• Alice has her own certificate
• Bob sends Alice an e-mail with his third-party certification over Alice's cert
• Alice imports it, even though she don't have a copy of Bob's OpenPGP certificate
• Alice deletes Bob's e-mail message because it contains nothing she needs any longer
• Alice refreshes her own certificate from the keyservers

Alice no longer has a copy of Bob's certification over her certificate.

The signature verification problem looks like this:

• Alice has Bob's certificate
• Alice gets an e-mail from Bob at time T, signed by Bob (which Alice can verify because the certificate -- including Bob's user ID -- is valid at time T)
• Alice refreshes Bob's certificate from the keyserver network, which provides a new self-sig at time T+1
• Alice goes back to her mail user agent, and tries to verify the e-mail signature again, but Bob's "cleaned" certificate has no valid self-sig covering time T (when the message was sent).

The once-valid message signature is now marked as dubious because of the date mismatch between the signature and the certificate's user ID validity window.

i've merged a variant of rGbe99eec2b105eb5f8e3759147ae351dcc40560ad into the GnuPG packaging in debian unstable as of version 2.2.17-3 to avoid the risks of data loss and signature verification failures. I'll revert it if i see the concern addressed upstream.

In the meantime, perhaps we want to apply the same change at least on STABLE-BRANCH-2-2 to make it clear that the current behavior is understood to be a bug.

The code has comments why we do a first clean_key on the imported keyblock.

Right it is a regression, but this is on purpose due to the keyserver problems. I even released an RC for comments and nobody complained about is. Keyservers break your keys anyway so removing stuff from a local key might actually be an advantage.

I'm aware of you releasing an RC for comments, and i apologize for not catching this particular case earlier. As you know from T4607, i was even advocating for it. i didn't understand the full implications of the "import-then-clean" approach at the time, and was thinking it would only apply to the incoming material, not the stored material.

(both you and i understand the option space and the use cases for GnuPG about as well as anyone does, afaict, and the fact that neither of us caught this before it was released says something about the ecosystem cost that we bear based on the complexity of the tooling here ☹)

Anyway, "might be an advantage" doesn't make up for "might destroy data in an unrecoverable way", which is why i've removed it in 2.2.17-3. If the user decides they want to run "clean" themselves manually, they can do that like they always have. It should not be on by default in a way that affects local keyring storage based on interactions with the outside world.