chore: shorten provisioner key #14017
Conversation
7fc8554 to
0f992d6
Compare
coderd/httpmw/provisionerdaemon.go
Outdated
| if err != nil { | ||
| handleOptional(http.StatusUnauthorized, codersdk.Response{ | ||
| handleOptional(http.StatusBadGateway, codersdk.Response{ |
There was a problem hiding this comment.
http.StatusBadGateway? This should be unauthorized, no?
There was a problem hiding this comment.
This error has a detail that specifies the length must be 64, and I thought it felt more like a 400 in that regard.
There was a problem hiding this comment.
omg I meant StatusBadRequest...
| @@ -15,40 +13,31 @@ import ( | |||
| ) | |||
|
|
|||
| func New(organizationID uuid.UUID, name string, tags map[string]string) (database.InsertProvisionerKeyParams, string, error) { | |||
| id := uuid.New() | |||
| secret, err := cryptorand.HexString(64) | |||
| secret, err := cryptorand.String(64) | |||
There was a problem hiding this comment.
64 is probably longer than we need if we are using alpha (lower & upper) + numbers. When we using hex strings, technically they are case insensitive, so you only get 16 options vs the now 62 (26 lower, 26 upper, 10 numeric)
You only need ~43 characters to match the same search space as 64 hex characters. We only use 22 characters for our api keys (I bet we copied someone else here).
It does not really matter, but if we are shortening our keys, we can at least shorten them to 43 and keep the same level of security as 64 hex characters. Which is super common.
16^64 = 1.157920e+77
62^43 = 1.182613e+77
log(16^64)/log(62) = 42.9948 <-- basically 43
Thoughts on going even shorter? 64 is such an excessive level of security.
There was a problem hiding this comment.
Of course, I was leaning on better safe than sorry but let's go short - 32 sound good?
There was a problem hiding this comment.
32 is ~190 bits of entropy. 43 gets us to 256, which is common in symmetric key encryption.
At the end of the day, 32 vs 43 feels completely arbitrary and probably sufficient, but 43 feels like "random" than "32" in the fact it lines up with 256bit. 🤷♂️
I don't think 43 looks too bad. That's just my 2c.
43 characters long
RrMe58fCjqyf7A8uRivKq3hQDcC3y5N5rnUWSRz5QA
1KH5y6hAUAJjymG8Xrjia4NY3yz3crenkmuEkT4T3N
3TEiuE7aDvUZyG1TqAtiikKR5rW6WkmG2RuEjCLXi3
z2B0ZRpbhZibWFfdDjRbb6fQzqy7baCBq2JQFKjDeA
32
VB3StT1Uj0CvP6LX2iMzX3eDACKPEBzF
jaDjJSPNLygpKLUELrUU5m5fuNbTfvV7
3QvMTSuRLwn7x0hbX4YJQKf2GMcL3TA5
G6LkDZdw1rumSZuLyMLGUYi15hvR38jA
64
qySKB6Zi6XLmahApVJLM5GMYP9TGy6BEyCYUC7KbVH0AE5h07W0eWg2Cqmjw7gCF
pSngALfmYcVmMETAAcntnp2bzZkRC90pYRa2fC5MmvHMuQHZ7hwg5eGMnWfnbCUu
t5gFjyTD3iy8J9emR6B7TSCwCYrcwJGLQXBw4zPMM0AS9E89DMXy4bK4V4FtJ1qw
v4C06YWqALYBpMScm7AuVYwzun0iEb5iCpQyg2fEWgfreNL2W6itRfzeS1cic1XY
537TcRdc3McvbTKtN4uZRfRp68hrnYJieHHtJp4d9tNWbFFxvrECv0umzig7UUk9
This shortens the provisioner key to a random 32 character string. We no longer include the ID prefixed in the token and simply look up by the hash of the input.