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

Move installed gpg keys to the currently configured storage #3347

Open
ffesti opened this issue Sep 30, 2024 · 16 comments · May be fixed by #3474
Open

Move installed gpg keys to the currently configured storage #3347

ffesti opened this issue Sep 30, 2024 · 16 comments · May be fixed by #3474
Assignees
Labels
crypto Signatures, keys, hashes and their verification RFE

Comments

@ffesti
Copy link
Contributor

ffesti commented Sep 30, 2024

Pratically we only really support gpg-pubkey packages as storage for pgp keys. If we support more backends in earnest we need a way to move keys from one storage backend to another to prevent users needing to re-import the keys.

This should probably be done completely automatically to make transition from one default to another easy and seamless. Alternatively would could offer a way to export the keys to a script(let) could export them, switch the default and then re- import the keys. While adding support to export keys may seem useful (although may not really needed if stored on disk anyway) doing the transition within rpm itself if probably less hassle and saver.

@ffesti ffesti added RFE crypto Signatures, keys, hashes and their verification labels Sep 30, 2024
@pmatilai
Copy link
Member

Right, we could have a keystore "rebuild" mode akin to how rpmdb backend can be changed by rebuilding it.

I was pretty much thinking of just letting users reimport the keys though - a distro user will only see 4.x -> 6.x and the associated keystore change when upgrading their distro, and at least in Fedora each version has their own key so lots of unused cruft accumulates over time. My home server has keys from eight different Fedora versions imported.

@pmatilai
Copy link
Member

Such a rebuild mode serves as a cleanup operation too - just like with the rpmdb. And just like --rebuilddb I think the keystore rebuild should

  • drop any keys it cannot load (which in this context is rather obvious), with a warning
  • be able to convert from one keystore to another
  • ...including the original format for a simple "rebuild" behavior

@ffesti ffesti self-assigned this Nov 8, 2024
@ffesti ffesti moved this from Backlog to In Progress in RPM Nov 8, 2024
@ffesti
Copy link
Contributor Author

ffesti commented Nov 8, 2024

Hmm, the question is on how to do that in a save way. E.g. for the database backend there is no clean slate to start from. We ofc could remove all gpgpubkey packages and then add the loaded keys back. But that leaves things very vulnerable inbetween.

The file based backends can at least save stuff elsewhere by manipulating %_keyringpath.

Hmm, with the key store in the rpmdb directory does that survive an rpmdb --rebuild?

@pmatilai
Copy link
Member

pmatilai commented Nov 8, 2024

Yes, gpg-pubkeys do survive rpmdb --rebuild currently. But they don't get reimported so they stay in the short id mode.

@pmatilai
Copy link
Member

pmatilai commented Nov 8, 2024

For the db, I guess you could basically create an empty rpmdb in an alternative path where you put them temporarily.

@ffesti
Copy link
Contributor Author

ffesti commented Nov 8, 2024

Should have worded this better: Do the fs backend keys survive a rpmdb --rebuild?

@pmatilai
Copy link
Member

pmatilai commented Nov 8, 2024

Oh, that. No idea.

@ffesti
Copy link
Contributor Author

ffesti commented Nov 11, 2024

AC:

  • Read keys from all supported back ends
  • For file based back ends
    • create a new instance in temporary directory
    • add all keys
    • move new instance to proper place
    • delete all old key directories
    • delete all pgp-pubkey packages
  • For rpmdb backend,
    • Add all keys to the rpmdb replacing existing ones
    • Remove all gpg-pubkey packages not just added
    • delete all old key directories

@pmatilai
Copy link
Member

pmatilai commented Nov 13, 2024

That AC proposal neglets to mention just how exactly this behavior is triggered, which is rather critical piece of information. Are new APIs and cli switches being added - which ones?

Stuff like "move new instance to proper place" is an uninteresting implementation detail, what we're really interested is the semantics that are also testable. For example, "add all keys" sounds like all keys get moved no matter what, but that's not what should happen, we should weed out any no longer legit keys in the process - in other words, those that fail to import. So it's more like "attempt to reimport all previously imported keys" than just "add all". And as a part of that reimport, any short keyids get converted to fingerprints - which is what we want. And on the semantic level I dont' think we should or need to differentiate between rpmdb and other backends, from rpmkeys perspective they should all behave the same.

So I think the requirements are more like

  • a way to invoke keystore rebuild from the command line and API (and describe how it'll look like)
  • supports moving keys between all keystores, including itself (eg rpmdb -> rpmdb)
  • old invalid (expired, weak crypto, whatnot) keys are discarded in the process with a warning message
  • old short keyid based keys are converted to fingerprint based ones

ffesti added a commit to ffesti/rpm that referenced this issue Nov 28, 2024
This rebuild the public key storage. If _keyring is set to a new value
keys from old storage are moved there and the old stores are deleted.
Keys are stored in the latest format. Keys no longer accepted are
dropped.

Resolves: rpm-software-management#3347
@ffesti ffesti linked a pull request Nov 28, 2024 that will close this issue
@pmatilai
Copy link
Member

pmatilai commented Dec 2, 2024

From the PR:

Technically we will always a rpmdb - so there can be gpg-pubkey packages in there no matter what you configured in the macros file

Nothing can put gpg-pubkeys into the rpmdb if it's not configured. We shouldn't look there if not told to do so.

Also an update of rpm that is changing the keystore might pull in a new keys right away - ending up with keys in two different keystores.

Any change in the default will only take place in the next transaction. So you only have the previous, and the new.
This is something that only happens in a distro upgrade really.

I do agree we want to be a bit more conservative with just adding random keys from somewhere. But should at least look for other stores and report them. May be require --force or something to actually use them.

I just fail to see a scenario where we would legitimately end up with keys in multiple places.

I'm starting there should simply be an explicit --from <type> argument for converting from one keystore to another, and no conversions ever happen without it. For databases we probe, but that isn't without its problems, you can actually lose data there if the stars align just so.

It's really up to the distros to arrange for the keystore conversion if they choose to change it. The default will only change in a distro upgrade, and that's where you'd put such a conversion too. That's exactly how we migrated away from BDB, and this is quite similar.

@ffesti
Copy link
Contributor Author

ffesti commented Dec 5, 2024

Well, just installing the rpm-6.0 for testing will get your already installed keys lost.

Thinking a bit more about this we should probably have better discoverability for this situation. We could check for keys all back ends and give a warning if there are keys outside of the configured backend.

That would allow people to actually give the right --from argument. May be rpmkeys --list should actually show these lost keys.

@pmatilai
Copy link
Member

pmatilai commented Dec 5, 2024

Well, just installing the rpm-6.0 for testing will get your already installed keys lost.

That's assuming the default is changed (it's not yet), and the provider of that package hasn't considered this. And even then they're not actually lost, just not used, and still neatly in exactly one place. And pretty much the worst that will happen is dnf etc reimporting them.

The main user of the keystore conversion is distro makers, who know perfectly well what to convert and when. That's the primary use-case to consider. It'll need to work and be understandable for the individual tinkered too of course, but having your keys suddenly scattered all over the place is not something that just happens.

For 6.0, we could just assume rpmdb keystore and get it 99% right... gets more complicated after that.

@pmatilai
Copy link
Member

pmatilai commented Dec 5, 2024

Thinking a bit more about this we should probably have better discoverability for this situation. We could check for keys all back ends and give a warning if there are keys outside of the configured backend.

This can follow the rpmdb conversion model: if currently configured keystore doesn't seem to exist (or is empty), then go look for others, indicate that and stop there.

@pmatilai
Copy link
Member

pmatilai commented Dec 5, 2024

But yup, what this really needs is the practical view of:

  • how is a distro supposed to convert their users to a new keystore?
  • how does an advanced user convert from one keystore to another?

@ffesti
Copy link
Contributor Author

ffesti commented Dec 13, 2024

OK,

  • When loading the keyring check if other backends do contain keys and issue a warning
  • rpmkeys --rebuild gets two additional optional parameters
    • --include_backends that gets one or more backend names
    • --drop_backends that will just delete other backends instead of erroring out if the rebuild would drop keys

Distros would then just do rpmkeys --rebuild --include_backends rpmdb

Advanced users could just do rpmkeys --rebuild --drop_backends to get rid of the old cruft and start fresh or use the command above.

ffesti added a commit to ffesti/rpm that referenced this issue Dec 13, 2024
This rebuildis the public key storage. If _keyring is set to a new value
and there are keys left in an old store this fails with an error message.

The keys can either be included by using one or more instances of
--include_backend or dropped with --drop_backends.

Keys are stored in the latest format. Keys no longer accepted are
dropped.

Resolves: rpm-software-management#3347
ffesti added a commit to ffesti/rpm that referenced this issue Dec 13, 2024
This rebuildis the public key storage. If _keyring is set to a new value
and there are keys left in an old store this fails with an error message.

The keys can either be included by using one or more instances of
--include_backend or dropped with --drop_backends.

Keys are stored in the latest format. Keys no longer accepted are
dropped.

Resolves: rpm-software-management#3347
@pmatilai
Copy link
Member

pmatilai commented Dec 17, 2024

The idea was to agree on the design first, and only once that agreement is there, follow up with the (updated) implemenation. This is going the backwards route again, and being painful because of that.

Your suggestion + updated PR is still based around the notion that there would be data in multiple different keystores, but that's just not a legit use-case AFAICS and I don't see anything proving the contrary here. Somebody doing multiple imports with random different keyring definitions is not a use-case we care about. Moving between different keystores is not an "end user" thing, you're supposed to know what you're doing, including where your keys were really. And, the pubkeys aren't precious data you can't afford to lose. The worst case, you reimport them. They're much much less precious than the rpmdb contents.

I really just want to see a 1:1 rebuild operation that supports arbityrary src:dest combinations. The rebuild should follow the same exact semantics whether src and dest differ or not. You shouldn't need a backend-specific rebuild operation either. Don't try to be overly clever about possible theoretical usecases, we only care about practical ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Signatures, keys, hashes and their verification RFE
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants