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

bcrypt builtin function #14725

Merged
merged 1 commit into from
May 30, 2017
Merged

bcrypt builtin function #14725

merged 1 commit into from
May 30, 2017

Conversation

jasmingacic
Copy link

Feature Proposal

Problem

Users need to provide passwords as strings that need to be encrypted with bcrypt through Terraform builtin functions.

Proposed Solution

  • Add a new bcrypt function to the built-in Terraform interpolation functions.
  • The function would generate hash from password using https://godoc.org/golang.org/x/crypto/bcrypt#GenerateFromPassword
  • bcrypt will have variable number of string arguments to allow users to pass optional parameter cost. Only the first argument would be accepted, If cost parameter is not provided it will default to 10. Return type will be string.

Required Dependencies

The Go library will be used for the built-in interpolation function.
-golang.org/x/crypto/bcrypt/bcrypt.go

Example Usage

Prior HCL with password string:

admin_password_hash = "$2a$15$JhQLJz1I1.O8elxxeGfFMuMPAPC3GxiAnW4AU7G0eEu92T6YVHDiO"

New HCL with the bcrypt function:

With default cost value:

admin_password = "${bcrypt(“1234”)}"

With custom cost value:

admin_password = "${bcrypt(“1234”, 15)}"

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

LGTM.

Just a friendly reminder that we always prefer any vendor changes to be submitted in a separate PR. 😉

Thanks.

@jasmingacic
Copy link
Author

@radeksimko thanks

@jasmingacic jasmingacic merged commit 5d33023 into hashicorp:master May 30, 2017
@jasmingacic
Copy link
Author

jasmingacic commented May 30, 2017

@radeksimko A question regarding what you said about vendor changes. If I didn't add dependencies CI tests would have failed. How should we handle that?

@apparentlymart
Copy link
Contributor

@jasminSPC, I'm not Radek 😀 but usually what we try to do is first make a commit that only changes files under vendor and then separately make one or more commits that use the new vendored code. Both can still be filed as part of the same PR, but this way it's easier to see in the commit log that a vendored library changed, since sometimes vendor changes can have unexpected consequences for other code. (Probably not in this case, though.)

@jasmingacic
Copy link
Author

@apparentlymart gotcha. Vendors first relevant changes later.

@jasmingacic jasmingacic deleted the bcrypt branch June 1, 2017 10:49
@ghost
Copy link

ghost commented Apr 11, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants