str4d/rage

UX: New decrypt API seems hard to use

Roguelazer opened this issue · 3 comments

What were you trying to do

Upgrade an internal application that uses the age library from 0.4 to 0.5

What happened

I have a daemon that loads a bunch of keys at startup and does many decryption operations using them. This was pretty straightforward in 0.4, but I'm kind of at a loss for how to do it ergonomically in 0.5. What I have now looks basically like this

struct Keychain {
    secret_keys: Vec<age::x25519::Identity>
}

fn box_key(key: &age::x25519::Identity) -> Box<dyn age::Identity> {
    Box::new(key.clone())
}

impl Keychain {
    fn decrypt_message(&self, message: &[u8]) {
        let armoror = age::armor::ArmoredReader::new(message);
        let decryptor = age::Decryptor::new(armoror)?;
        let decryptor = match decryptor {
            age::Decryptor::Passphrase(_) => return Err(DecryptionError::InvalidEncryptedKeyType),
            age::Decryptor::Recipients(r) => r,
        };
        let identities = self
            .secret_keys
            .iter()
            .map(box_key)
            .collect::<Vec<Box<dyn age::Identity>>>();
        let mut reader = decryptor.decrypt(identities.into_iter())?;
        // ... snip ...
    }
}

It feels wrong that I have to clone the keys on every operation, but Decryptor takes an iterable of Box<dyn age::Identity> (instead of &Box<dyn age::Identity>), and you can't clone a boxed trait objects, so I don't really see any other way?

Would it be possible to change the Decryptor::decrypt method to take identities by reference?

str4d commented

Hmm, I'm not sure why I opted to go with Box<dyn Identity> as the iterator item type (when migrating from 0.4's Identity struct to 0.5's Identity trait), instead of &'a dyn Identity. It was probably for parity with Encryptor::with_recipients (which takes and stores Vec<Box<dyn Recipient>>).

Switching to &'a dyn Identity does make using a Vec<Box<Identity>> more complex in the default single-use case, but enabling the multi-file decryption case for identities does make sense given that all the APIs in the Identity trait only take the identity by &self. This would also better matches the 0.4 API that took a slice of identities.

Could you give #213 a try and see how it works for you? I'll need to test it myself against my other age crate uses to check it doesn't break any of them (though I don't see how it would).

That branch definitely simplifies things! I changed my keyring to store Vec<Box<dyn age::Identity>> (and to turn everything into trait objects at startup) and now I can call the decryptor with self.secret_keys.iter().map(|k| &**k). Still a little perlish but at least I'm not cloning the entire keychain on every decrypt operation.

str4d commented

I don't currently see a way to improve this further, so I'll go with this for the next release.