Use cryptographically secure RNG for random_string, or don't recommend them for passwords?

[danielcompton]

Affected Resource(s)

• random_string

Description

Reading index:

Unless otherwise stated within the documentation of a specific resource, this provider's results are not sufficiently random for cryptographic use.

But on random_string, it shows a demo using it for generating a password, and the API for random_string seems to be geared around generating passwords.

Is random_string suitable for generating passwords? If so, it could be good to explicitly mention this on the resource docs, or alternatively, mention that this resource is not for generating passwords?

[adrian-rt]

I'm interested in this one too.

[apparentlymart]

Hi @danielcompton!

Agreed that the documentation here is not sufficient, in retrospect. If we change nothing else then we should at least repeat explicitly on the random_string documentation page that it is using a standard (non-crypto) random number generator, and let the user make a decision about whether to use it for passwords.

Looking at the random_string implementation, it seems like it could in principle be updated to use crypto/rand.Int instead of math/rand.Int, and do so without any breaking changes because (thankfully) the seed argument isn't exposed here. Since performance isn't a concern here, I don't see a significant downside to using the crypto RNG here even though it might be overkill for some uses of this resource.

[Sodki]

Another alternative would be to have an argument that enables the use of a crypto random number generator. In this case the defaults don't change, but the user can easily enable it, if he needs to. Same can be used for all other resources in this provider.

[apparentlymart]

That is indeed an option. Since this provider doesn't currently make any promises about what random number generator it is using (and in general probably never will, since that would be constraining to future development) my instinct is that we should feel free to just change this unilaterally and not make it an option at all (since the result will still be a random string as promised, after all) but if we were to find a reason why that weren't true then we could indeed make it optional.

I'd shy away from making this a global option applying to all resources here. Having multiple implementations of each resource would add considerable complexity to this provider and make it harder to maintain and improve over time.

[danielcompton]

I can't think of any reasons where a cryptographically secure random number generator would be an issue here, I can't imagine the performance cost would ever be an issue. For that reason it seems like the safest option would be to make the random backend a cryptographically secure RNG. This would make it harder to misuse Terraform and create passwords or other secret values unsafely. This feels like a similar issue to hashicorp/terraform-provider-aws#3989.

[phekmat]

@apparentlymart I may be missing something, but isn't random_string using crypto/rand.Int at the moment? I see the import at https://github.com/terraform-providers/terraform-provider-random/blob/261db0b7b823936602edccdc8e1ab4bcc59b1a4d/random/resource_string.go#L4

edit: This resource in particular looks like it was covered in #37

[radeksimko]

As mentioned the documentation part was covered in #37 and we also now have a new resource which can be used for generating passwords (random_password), which was added in #52 and released as part of v2.2.0.

[github-actions]

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.