Role of the key store (discussion)

[dewyatt]

I wanted to have a quick discussion on what the role of the key store should be as the lines seem a bit blurry (to me) right now.

I guess my main question revolves around the passphrase parameters in things like:

bool
rnp_key_store_write_to_mem(rnp_t *          rnp,
                           rnp_key_store_t *key_store,
                           const uint8_t *  passphrase,
                           const unsigned   armour,
                           pgp_memory_t *   memory)

So if we had 1000 unencrypted keys in this key store, it would use the same passphrase to encrypt all of them, which is a bit odd. This issue is mostly avoided currently because our key generation involves immediately writing the generated key to disk (when the generated key is the only decrypted key in the store).

This is basically inherited from netpgp, where saving a keyring involved pgp_write_xfer_*key (which we still do in master, though it's not correct). This is also one of the reasons we don't currently support writing out a keyring that contains an unencrypted (loaded) key.

My initial thoughts are that we should be able to:

• Load an entire keyring of encrypted+unencrypted (s2k_usage==0) keys in to memory (not providing any passphrases) and just temporarily decrypt keys when needed (we already do this).
• Write this entire keyring out, unchanged. (full round-trip)
• We should provide an API to explicitly set/remove/change a passphrase for a particular key.

I did tackle the first 2 for .gpg/.kbx in my branch here

But it seems worth discussing whether the key store should deal with passphrases and key encryption/decryption. I personally lean towards no at the moment.

(Also I haven't spent time looking at the private-keys-v1.d/s-expr format so I don't know if that adds restrictions, etc.)

[catap]

@dewyatt Well...

rnp_key_store_write_to_mem use passphrase to save unencrypted key from keystore. So, if keystore has 2 keys and one of them unencrypted, this function will use this passphrase to encrypt only unencrypted key.

Another keys will save as is. So, I agree that I should add somenotes how it works and change the API because now it might confused.

So, I think we only need the API to set/change/remove passphrase to specified key. So, as result we will remove passphrase option from rnp_key_store_write_to_mem.

[ronaldtse]

I think we should look at the key store as a secure key storage structure for "keys", this includes public and private (asymmetric / symmetric) keys.

At the most general level a key store interface should be able to:

• read keys from / write keys into a given key store
• modify existing keys inside the key store
• import keys into / export keys from the key store
• maybe support groupings of keys ("multiple keychains")

A key store could take any of the following forms (even each might only be able to support certain features):

• OpenPGP storage like GnuPG keyring / private-keys-v1.d / KBX
• macOS keychain
• SSH keychain
• Database-backed collection of keys
• and of course, a key store of a custom format

I'm just going to list out some thoughts here and see anyone agrees.

A good key store should

• Support a large number of keys (and keychains if supported)
• Protect its keys (and keychains if it supports multiple keychains) with encryption and passphrase(s)
• Allows locating keys through filtering (attributes) and searching (textual matches)

Private keys should:

• be stored securely with optional individual passphrases
• be supported regardless the algorithm generated and algorithm-specific attributes stored alongside

[ni4]

I also came over this issue with key passwords recently. My opinion is that:

• key store just reads/searches/modifies/writes key collection, like @ronaldtse described in details
• key encryption/decryption are the separate operations which are handled by key store or other part of the library. It could be just two calls decrypt_key(key, password), encrypt_key(key, password). So during the key generation new key is plain and not encrypted. Then key generation function should call encrypt_key(), which will fill the blob, and only then keystorage_save() should be called.