can not set a user password to an md5 string, because it will be re-hashed

[mtesch-um]

This used to work, but has been broken by always hashing the password string before sending it to CREATE USER.

There are now two PRs to fix this - I did the second one because I forgot there was already one:

#90

#96

[hoxu]

This was broken in 004a620. I added this comment on PR #90:

PASSWORD { 'password' | 'md5hash' | 'sha256hash' | DISABLE }

Is there a reason why the password string is not simply passed as-is to CREATE USER, and let Redshift sort out whether it's a plaintext or hashed password?

Commit 004a620 certainly does not provide any explanation for why the MD5 hashing in the provider code side was added. @sworisbreathing what was the reasoning?

[hoxu]

Commit 004a620 went into the v0.2.0 release, PR #7.

v0.1.1...v0.2.0

That PR only quotes the CREATE USER documentation:

As a more secure alternative to passing the CREATE USER password parameter as clear text, you can specify an MD5 hash of a string that includes the password and user name.

The "more secure" part applies when comparing, for example, a SQL migration file with a plaintext password versus one with a hashed version of the same.

I don't see any security gained from doing the hashing in the provider execution - the connection is encrypted unless you explicitly disable that. In fact, the current MD5 hashing may be inferior to whatever (stronger) hashing Redshift does now or in the future.

I propose the MD5-hashing in 004a620 to be reverted, and instead the plain text password string passed as-is to Redshift. Redshift then either hashes the password using whatever is the current default hashing algorithm, or validates an already hashed password and stores it. Simpler provider code, and no need to maintain an implementation containing a separate, hardcoded, eventually outdated, list of supported hash algorithms with inferior validation logic.

@sworisbreathing would you accept a PR reverting that MD5-hashing and passing the password as-is to Redshift, as before?

[sworisbreathing]

@hoxu it probably should be configurable rather than forcing any of the options. I think I made the original PR to go with md5 because I was concerned about a) passwords being written in plaintext to query logs, and b) plaintext passwords being stored in general.

I probably would have used sha256 instead of md5 had I been aware that redshift supported it at the time. I was never a big fan of md5 but it seemed better than nothing.

Unfortunately I can't accept any PRs. I'm not a maintainer on this project, only a contributor like yourself.

[mtesch-um]

I would guess the reason to hash is to keep the unhashed password from passing over the wire unencrypted - probably a minor concern for most users, since the connection should be SSL anyhow.

I probably would have used sha256 instead of md5 had I been aware that redshift

I dug into using sha256 at one point - if I'm remembering correctly, sha256 is only accepted for CREATE USER and not for ALTER USER.

[hoxu]

We really need the ability to pass hashed passwords, or else we need to use something else than this provider.

Can someone with commit rights (@rg00d, @szemek, @winglot?) chime in and comment on the proposed solutions?

1. I proposed that the hashing code is reverted, and the passwords are passed to Redshift as-is. This will actually improve security, as the passwords no longer need to be plaintext in the Terraform sources. It will also support all the supported hash algortihms, as Redshift will figure out whether the password needs to be still hashed or already is.
2. PRs support md5 password in terraform code #90 & dont re-hash an already hashed password #96 are both attempts at hard-coding support for MD5 hashes in the provider code. These are currently missing SHA256 support, and will need to be updated in the future if more hash algorithms are supported in Redshift.

I may be able to work on a simple PR, but let's agree on what's the correct way to fix this first.

[mtesch-um]

fwiw, my vote is for solution 1.

[hoxu]

PR #103 created with proposed solution 1. I hope someone with write access can review it.