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

HashiCorp Vault feature #613

Closed
wants to merge 9 commits into from

Conversation

soleinik-figment
Copy link

@soleinik-figment soleinik-figment commented Sep 8, 2022

In brief

This change adds support for HashiCorp Vault, Transit Engine.

Currently there are few signing providers available - HSMs, SaaS and softsign. This change will add support for HashiCorp Vault - an identity-based secrets and encryption management system

What's the usecase?

Vault provides Encryption as a Service (EaaS) to enable security teams to fortify data during transit and at rest. So even if an intrusion occurs, your data is encrypted and the attacker would never get a hold of the raw data. In addition, Vault provides networked access, software based general purpose secure storage for any sensitive information, such as keys, password or certificates.

@mkaczanowski
Copy link
Contributor

great feature, looking forward to see this PR reviewed and merged

Copy link

@cloudnull cloudnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic addition. Well done.

@tony-iqlusion
Copy link
Member

It looks like this is pulling in an old version of hyper as a transitive dependency of reqwest, which is breaking the security audit.

It also looks like there's a PR open to update reqwest which has been open since July 2021 with no response from the maintainer: ChrisMacNaughton/vault-rs#57

Is the hashicorp_vault crate actually maintained?

@soleinik-figment
Copy link
Author

let's see if I get PR merged

ChrisMacNaughton/vault-rs#59

@soleinik-figment
Copy link
Author

"hashicorp_vault" dependency is replaced with "ureq" - A simple, safe HTTP client.

@tony-iqlusion
Copy link
Member

@soleinik-figment thanks, that eliminates the concern with the unmaintained dependency.

I'll try to look at this in-depth soon.

Cargo.toml Outdated Show resolved Hide resolved
@soleinik-figment
Copy link
Author

@tony-iqlusion is there anything that I can help with to get this PR merged?

@helder-moreira
Copy link

@soleinik-figment would be great to have TLS support for production systems.

@soleinik
Copy link

@helder-moreira configuring api_endpoint configuration property with https
api_endpoint= "https://127.0.0.1:8200"
would enable TLS comm to Vault

@helder-moreira
Copy link

@helder-moreira configuring api_endpoint configuration property with https api_endpoint= "https://127.0.0.1:8200" would enable TLS comm to Vault

But a CA certificate is required. I have done it here.

#[derive(Clone, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
/// Hashicorp Vault signer configuration
pub struct HashiCorpConfig {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to let the access_token to be read from the file, for instance:

pub enum AuthConfig {

@tony-iqlusion
Copy link
Member

@soleinik-figment are you still interested in landing this, and if so, can you rebase?

@tony-iqlusion tony-iqlusion changed the title HashiCorp feature HashiCorp Vault feature Oct 25, 2023
@mkaczanowski
Copy link
Contributor

(if no one is interested I am willing to rebase this) I think this is great feature

@soleinik
Copy link

Go ahead Mateusz... I wont be able to work on this

@helder-moreira
Copy link

I also think this is a great feature. I tried to do it myself but I don't know Rust enough. This was actually my first contact with Rust.

I have a variation of this branch here if it helps somehow. Its the one we are using. It adds the option to specify a CA certificate, and replaces access_token with token_file to specify a path to a token instead of the token itself. Also, the upload command does not use tmkms.toml, and relies on env vars VAULT_TOKEN, VAULT_CACERT and VAULT_ADDR like the vault cli (its faster to quickly upload a key).

There are two additional things that I think would be a plus:

  • Support VAULT_SKIP_VERIFY to not verify certificate.
  • Re-connect when connection to Vault drops. Currently it just hangs and stops signing.

@mkaczanowski let me know if I can be of any help.

@mkaczanowski
Copy link
Contributor

okay, I'll rebase and add the feature to read access token from disk later this week

@mkaczanowski
Copy link
Contributor

thanks @helder-moreira I will definitely take a look

@mkaczanowski
Copy link
Contributor

(fyi) I have a rebased, cleaned up branch with integration tests here:
https://github.com/mkaczanowski/tmkms/tree/hashicorp-feature-tmkms

(still WIP, as I am testing it out to address the connection issues)

@tony-iqlusion
Copy link
Member

Closing in favor of #840

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants