RFE: Support for hermetic builds - include! a securestore

[rbtcollins]

trafficstars

It seems like if we're updating git and code using a secret in lockstep, that reading a securestore from disk is pointless - and it makes packing into Docker that little bit harder.

It would be nice to do something like

let store = KeySource::from(include_bytes!('../sstore'));

[mqudsi]

@rbtcollins, thanks for being a securestore user and for filing this issue. I've taken some time to think about this before replying and I don't think I can endorse such functionality in the core securestore library/crate (although I am open to changing my mind).

A few thoughts, in no particular order:

• I'm not sure if I'm understanding the very first thing you said correctly or not, but I want to clarify this: You update secrets and the code using them in git in lock-step, but this doesn't have anything to do with the private key. The private key is generated once, deployed to the production servers, and used client-side (without ever being in git) to update the SecureStore vault. Changes to the vault do not change the key, and you don't need to re-deploy the key each time (unless your instances are ephemeral and any changes to the code result in a new server being spun up altogether).
• My initial instinct was to say "sure, we can add a KeySource::Bytes or similar with the caveat that you can't use it if you redistribute your application to end-users. Yes, many people (not you!) may not realize that this isn't what they want and distribute secrets with the key baked into a client-redistributed app, but that's not necessarily a reason not to do this if we support it with enough literature and make the API scary enough.
• My actual concern is that right now, it's very clear where the security boundary is. ssclient generates a key file and prevents you from accidentally committing it to git (by adding an exclude rule for it). Anything and everything in your project folder can be public except for your keyfile and that's it. But once you bake the key into the application, suddenly you are at risk of the secret being leaked through a thousand side channels. You have CI artifacts, docker images, debug symbols, etc. Anything that causes your binary file to leak will leak the secret along with it (vs with a separate key file you would need to leak the active memory and not just a static file at rest).

My unofficial recommendation for deploying with docker would be to not ship the secrets in the docker image at all but rather map/mount a path containing the secret key from the host to the docker instance. Keep the secret on the production server, don't keep the secret in the docker image itself, and give it access at runtime only. That keeps the docker image and all other CI artifacts safe and protects the integrity of your encrypted secrets better.

If you absolutely insist on deploying the secrets in the exe itself, I obviously cannot stop you. I don't think it makes sense for us to add an API that would itself embed the secret key or even imply that the secret must be embedded to use; but I can probably add a KeySource that can take a std::io::Read instead of a &Path, and then you can use a trait/extension method to load the key like this:

trait SecureStoreEx {
    fn load_with_key_from_buffer(vault_path: &Path, key: &[u8]) -> SecretsManager {
        let key = Cursor::new(key);
        SecretsManager::load(vault_path, KeySource::Read(key))
    }
}

You can already write the same with the current securestore release, but you'd have to round-trip the key to disk so you can get a path to pass to SecretsManager::load() (or you can parse the contents of the keyfile to get a KeySource::Password and use that instead).

Happy to hear your thoughts on this. Thanks again for using securestore!

[rbtcollins]

Thank you for the comprehensive reply.

However, my unfamiliarity with the codebase may have caused confusion. I was not intending to suggest putting the unlock secret into the binary - that should obviously be delivered out of band to permit the use of generic image repositories for binary distribution.

So I hadn't read closely enough I guess - KeySource to me looked like the right thing but I see now its not.

So to start over: it would be great to be able to do two things:

• not have to ship the current version of the vault separately to the binary - even putting them both into the same docker image isn't a great answer, not with WASM / WASI execution environments.
• be able to inject the secret without projecting it to disk (I didn't mention this first time around, and perhaps it should be a separate bug) : projection to disk adds additional attack vectors vs just capturing the variable in the process environ block.

If we look at https://docs.rs/securestore/latest/securestore/struct.SecretsManager.html#method.load it is very file-centric. If vault_path was instead any implementor of Read; or if there was a separate method to populate the in-memory contents that didn't presume on-disk access, that would address what I intended to raise in this bug.

And GenericKeySource being sealed is (AFAICT) the only thing preventing implementing a from-env-variable implementation of the trait being added externally.

[mqudsi]

@rbtcollins ok, I see what you're asking for now and indeed, it's very different than what I had understood from your initial post! This is definitely something we can improve out-of-the-box and possibly also make easier for users to extend.

GenericKeySource is sealed to prevent it from being part of the stable API as it was assumed to be of little benefit for outside types. Clearly I wasn't thinking of all the possible scenarios and your suggested extensions are very sensible; we could probably relax this constraint.

[mqudsi]

@rbtcollins Do you mind taking a look at the current master code and seeing if this works for your purposes? GenericKeySource is now public and unsealed, so it can be implemented on anything that can derive from its own state a password, path, or buffer containing the keys.

~~Sorry this took longer than anticipated but the change to support loading the vault itself from non-pathed sources in a backwards-compatible fashion was actually much more difficult than I had thought it would be and I'm still not sure if I nailed all the lifetime trait bounds correctly - but I've added two tests (see tests/derives.rs) that are currently passing at least. They illustrate how GenericVaultSource may be implemented returning either a borrow or an owned type.~~

Edit: I reverted the GenericVaultSource stuff as it was overly complicated. Including a file is now as straightforward as the following:

fn main() {
    let sman = SecretsManager::load_from(include_bytes!("../secrets.json").as_slice(),
        KeySource::Password(&std::env::var("VAULT_PASSWORD").unwrap())
    ).unwrap();
    assert_eq!(sman.get("foo").unwrap(), "bar");
}

[mqudsi]

@rbtcollins did you ever get a chance to look at the changes? Is there anything else that needs to be done here?

[rbtcollins]

Catching up late - sorry for going radio silent, I did mean to look at it, but I just haven't had time. The description very much seems good htough - thank you for listening!

[mqudsi]

No worries. I'm happy with the addition of this feature in general, but if you needed something more specific please feel free to bring it up here or in a new issue.