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

Feature decoding ssh key #208

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hchakroun
Copy link

@hchakroun hchakroun commented Dec 26, 2021

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Hassan CHAKROUN and others added 2 commits November 15, 2021 13:10
@jamesrobson-secondmind
Copy link

I'm not sure this fix should go in as-is, the decode from base64 is only needed if you put the ssh into vault as a base64 encoded string. That's not how I've done it and I'm sure I'm not alone in this.
This could be useful, but I think it would need to be available as an option.

@hchakroun
Copy link
Author

I just thought it would be good to store the key as a base64 string to avoid dealing with newlines and to not send the certificate as is to Vault And I think if the plugin follows the same approach it'd be fine, because the Vault certificate credential is expecting a base64 string from Vault at:

keyStore.load(new ByteArrayInputStream(Base64.getDecoder().decode(unwrap(base64KeyStore))), toCharArray(getPassword()));
which is not the case for SSH Certificate

Take a look to this link for a more complex example of storing an SSL Certificate: https://discuss.hashicorp.com/t/store-ssl-certificates-in-vault/30180/6

@jetersen
Copy link
Member

Would be best to check whether the string was base64 encoded or not.

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.

None yet

3 participants