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

Fix logging in rsadecrypt #18333

Merged
merged 1 commit into from
Jun 26, 2018
Merged

Fix logging in rsadecrypt #18333

merged 1 commit into from
Jun 26, 2018

Conversation

ashald
Copy link
Contributor

@ashald ashald commented Jun 26, 2018

rsadecrypt logs private key when cannot base64 decode input string

@apparentlymart
Copy link
Contributor

Thanks for finding and fixing this, @ashald!

We had a bug on master at the time you forked to work on this it seems, which is what caused the test failures here. It's now been fixed, so it should work once we rebase it.

This set of function implementations will be phased out in the next major release, and it looks like this bug was also ported over to the replacement function implementations for the new language implementation:

b, err := base64.StdEncoding.DecodeString(s)
if err != nil {
return cty.UnknownVal(cty.String), fmt.Errorf("Failed to decode input %q: cipher text must be base64-encoded", key)
}

...so we'll need to make sure to get that fixed on the branch before we merge it down to master.

Thanks again, Ashald!

rsadecrypt logs private key when cannot base64 decode input string
@ashald
Copy link
Contributor Author

ashald commented Jun 26, 2018

Pushed rebased version.
Looks like there is nothing I can do about the new version of the function but will keep an eye on it. ;)

@apparentlymart
Copy link
Contributor

Thanks for rebasing! I was actually about to do that myself while merging, but nice to see confirmation that the tests are indeed green now. 😀

@apparentlymart apparentlymart merged commit c811440 into hashicorp:master Jun 26, 2018
@apparentlymart
Copy link
Contributor

I just pushed d12a3d6 to make the same change to the new implementation, along with removing the key from two other error messages in this function. This version will be the one used in the next major release of Terraform.

@ghost
Copy link

ghost commented Apr 3, 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 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants