Improve key storage approach

[emschwartz]

Right now, the Store is responsible for managing secret values like API keys (for authenticating with peers via HTTP or BTP). The Redis store uses encryption and hmac keys derived from a configured seed.

This approach isn't great because:

• It's not very secure (all of they key material is on the same machine)
• The seed needs to be passed in as an environment variable (is that even a good approach?)
• Cannot easily change the seed
• Every store implementation needs to re-implement similar crypto functionality or import some shared code (this issue was inspired by @cjimison asking about the right way to approach this for https://github.com/emschwartz/interledger-rs/issues/52)

Also, the keys should be included in the Account type in such a way that they are never logged. For example, they could have special types that implement Debug and Display but only print <redacted> or something like that.

How should we improve the key storage? If necessary, we can also revisit the use of API keys (rather than more complex auth) if there is some decent solution that's more secure but not impossible to use.

[gakonst]

We could store keys in Vault and access them over an authenticated API whenever they're needed.

Accessing vault data over http: https://learn.hashicorp.com/vault/getting-started/apis Rust candidates for vault client:

• https://github.com/Metaswitch/vault-client
• https://github.com/lawliet89/vault-rs
• https://github.com/chrismacnaughton/vault-rs

[emschwartz]

Vault seems like a good option for this.

In the future we may need to abstract this away to enable embedding parts of interledger.rs as a library in other code (rather than running a full node), but we may be able to do that in a refactor later.

[gakonst]

I will initially investigate how complex vault is to set-up, and if it's good I'll go with that. @emschwartz mentioned it might be overkill for our use-case. If so, I'll explore alternatives.

[emschwartz]

Some other options to consider:

• encrypted file using PGP
• separate abstraction for secret provider that could be linked to different cloud services or hsms

Also, we should consider using https://crates.io/crates/secrecy to ensure the secrets are never logged.

[gakonst]

• separate abstraction for secret provider that could be linked to different cloud services or hsms

Vault should be able to do that?

Also, we should consider using https://crates.io/crates/secrecy to ensure the secrets are never logged.

Agreed. I'll use this issue to track all variables we have which should remain secret.

• encrypted file using PGP

Not sure about this

[gakonst]

https://github.com/cesarb/clear_on_drop maybe useful

[emschwartz]

Vault should be able to do that?

I meant an internal abstraction within interledger.rs (like the Store but separate) in case we want to integrate with non-Vault secret storage layers. How useful that is kind of depends on how complicated Vault is to set up.

[inthecloud247]

By integrating/using a tool like sops (https://github.com/mozilla/sops) we could support encryption with AWS KMS, GCP KMS, Azure Key Vault and PGP.

Intro to sops: https://www.youtube.com/watch?v=V2PRhxphH2w

[gakonst]

There's an open PR for Vault KMS as well: https://github.com/mozilla/sops/pull/507. I am sold on SOPS for the management and deployment of config files.

We probably want to tackle this once @dora-gt wraps up https://github.com/interledger-rs/interledger-rs/pull/236. A good idea might be to have a ops repository in the interledger-rs organization, where we store the encrypted secrets and configuration files for deployment, along with any docker compose files. @inthecloud247 what do you think?

--

I will wrap all secret types that are loaded in memory with Secrecy. The Secret type implements Zeroize, so clear_on_drop shouldn't be needed.

[emschwartz]

Does sops have an HTTP API or way to use it as a library from Rust? If not, how would we actually interact with it?

A good idea might be to have a ops repository in the interledger-rs organization, where we store the encrypted secrets and configuration files for deployment

I don't think this should live in this repository. There may be many deployments of this project so secrets should be kept in other repos

[emschwartz]

@gakonst @inthecloud247 @tarcieri if we are still planning to have the node encrypt/decrypt API tokens and secrets like that, how much value would it add to use something like sops? It seems like passing in the node secret as an env var would be fine.

A more substantial change would be to completely offload the encryption/decryption to an external service and make it so the node doesn't have a decryption key but only an API token or something for the service that is actually going to store the secrets.

[gakonst]

node secret as env var

wonder if that was the cause behind this tweet :D https://twitter.com/bascule/status/1166566607335895041

We will have a SOPS encrypted config file. When launching a node, that config file gets loaded and there's a programmatic call to SOPS to decrypt it and load the data in memory in Secret types, without the decrypted file ever touching the disk.

use std::process::Command;

static PGP_FINGERPRINT: &str = "YOUR_PGP_FINGERPRINT";
static SOPS_CMD: &str = "sops"; // path to sops

use serde_yaml;
use serde::{Serialize, Deserialize};

#[derive(Debug, Serialize, Deserialize)]
struct Config {
    ilp_address: String,
    secret_seed: String, // this will be a secret type for sure
    admin_auth_token: String, // this will be a Secret type for sure
    redis_connection: String,
    settlement_address: String,
    http_address: String,
    btp_address: String,
    default_spsp_account: String,
}

fn main() {
    let output = Command::new(SOPS_CMD)
        .arg("-d") // decrypt flag
        .arg("--pgp") // or KMS flag
        .arg(PGP_FINGERPRINT)
        .arg("alice.yaml") // the encrypted config file you're loading
        .output()
        .expect("failed to execute process");

    let data: Config = serde_yaml::from_slice(&output.stdout).unwrap();

    println!("{:?}", data);
}

What do you think @emschwartz? This approach assumes that the user has configured SOPS properly on their box. I've tested it and it works fine, the decrypted config info is in the data variable, while the file itself remains encrytped on disk.

[emschwartz]

Looking at this code snippet makes me think we may want to allow piping the config file in via stdin or something like that. I'm not a big fan of requiring the use of an external tool like this (and calling a CLI tool from Rust) but wouldn't mind making sure the integration works nicely and recommending its use. That would look something like calling sops ... alice.yaml | cargo run

[gakonst]

I am fine with that. We need to add an option to pass in config files from stdin in https://github.com/interledger-rs/interledger-rs/pull/236.

That said, I'd prefer if we discouraged^Wprohibited users from storing cleartext config files on disk, hence why I suggested that the program would consume an encrypted config file. By piping, we allow lazy operators to have decrypted files on disk. At the very least we should remove environment variables from a possible configuration option.

[emschwartz]

It can be part of #236 or a separate PR.

I'm not convinced that we should disallow configuring it with env vars or unencrypted files. For example, we definitely don't want to bother encrypting the configuration for tests or testnet nodes. I think it's enough to include strong recommendations in the documentation and instructions. Someone can always ignore all of the recommendations and set their system up insecurely (they could also encrypt the file with an insecure password).

[emschwartz]

Also, do you think the way it encrypts the API tokens should be considered part of the node as opposed to part of the store? If so, we should move the encryption/decryption functionality out of the redis store and into the node's code itself.

[gakonst]

ACK let's do it on 236.

The API token encryption and decryption should move out of the specific/cache implementation. This is related to the database layer abstractions from https://github.com/interledger-rs/interledger-rs/issues/195.