movetokube / postgres-operator

Postgres operator for Kubernetes
MIT License
167 stars 59 forks source link

Add secure string generator for Password generator #149

Closed kanor1306 closed 7 months ago

kanor1306 commented 9 months ago

Hi,

I have had some occurrences where a service in different environments, newly bootstrapped, was getting the same password for both environments (dev and staging). I am not very proficient in Go, but I assume it has to do with the usage of the math random function without a seed. In any case I have been reading and it seems to be recommended the usage of crypto/rand for password generation (and also the usage of Seed is deprecated)

As I said I am not an expert Go programmer, but I have tried to add a new function following the same structure of the current one but with the usage of the crypto/rand library. I think it likely that this is not a totally secure implementation, as while investigating how to do this I have seen implementations that go in a lot more detail, but I thought that this was a good enough step in this direction.

One thing to note is that apparently the password generation can fail if the secure number generation has issues, so now when creating the password a new error has to be handled. I have chose for re queuing the instance, as it is done in other errors, though I have to say I am not certain if this would be a recoverable error.

Resources that I looked into for this implementation: https://pkg.go.dev/crypto/rand https://github.com/golang/go/issues/56319 https://stackoverflow.com/a/32351471 https://stackoverflow.com/questions/22892120/how-to-generate-a-random-string-of-a-fixed-length-in-go https://gist.github.com/denisbrodbeck/635a644089868a51eccd6ae22b2eb800

hitman99 commented 7 months ago

@kanor1306 Tests are failing, could you check it out please?

kanor1306 commented 7 months ago

@kanor1306 Tests are failing, could you check it out please?

Will check it out tomorrow @hitman99, I thought that I had run the tests, but obviously I messed something up

kanor1306 commented 7 months ago

Should be fine now @hitman99, somehow I missed an import