- Notifications
You must be signed in to change notification settings - Fork924
chore: shorten provisioner key#14017
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
http.StatusBadGateway? This should be unauthorized, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
omg I meantStatusBadRequest...
| @@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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 use22 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+7762^43 = 1.182613e+77log(16^64)/log(62) = 42.9948 <-- basically 43Thoughts on going even shorter? 64 is such an excessive level of security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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
RrMe58fCjqyf7A8uRivKq3hQDcC3y5N5rnUWSRz5QA1KH5y6hAUAJjymG8Xrjia4NY3yz3crenkmuEkT4T3N3TEiuE7aDvUZyG1TqAtiikKR5rW6WkmG2RuEjCLXi3z2B0ZRpbhZibWFfdDjRbb6fQzqy7baCBq2JQFKjDeA32
VB3StT1Uj0CvP6LX2iMzX3eDACKPEBzFjaDjJSPNLygpKLUELrUU5m5fuNbTfvV73QvMTSuRLwn7x0hbX4YJQKf2GMcL3TA5G6LkDZdw1rumSZuLyMLGUYi15hvR38jA64
qySKB6Zi6XLmahApVJLM5GMYP9TGy6BEyCYUC7KbVH0AE5h07W0eWg2Cqmjw7gCFpSngALfmYcVmMETAAcntnp2bzZkRC90pYRa2fC5MmvHMuQHZ7hwg5eGMnWfnbCUut5gFjyTD3iy8J9emR6B7TSCwCYrcwJGLQXBw4zPMM0AS9E89DMXy4bK4V4FtJ1qwv4C06YWqALYBpMScm7AuVYwzun0iEb5iCpQyg2fEWgfreNL2W6itRfzeS1cic1XY537TcRdc3McvbTKtN4uZRfRp68hrnYJieHHtJp4d9tNWbFFxvrECv0umzig7UUk9There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
43 it is 😄
6c2336b intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.