-
Notifications
You must be signed in to change notification settings - Fork 139
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix hash-related method signatures #531
base: master
Are you sure you want to change the base?
Conversation
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.
Code LGTM!
Indeed it's a breaking change, i ran "make install" on master branch, switched to the fixed branch and rebuilt the API and got the following error:
[HUSKYCI][*] poc-golang-gosec -> https://github.com/globocom/huskyCI.git
[HUSKYCI][ERROR] Sending request to huskyCI: Unauthorized Husky-Token ZjMxYjU3YTUtMjkwMS00ZjliLWFlYzgtMWVmMDNmODE5ZTk0OnllSVAyMEdsNDNPcW9XazdBaFhlZ1ZMdkZUdGpaVl9tWkxHbGxuNmhGbXc9
make: *** [Makefile:155: run-client] Error 1
So it also affects client access tokens to the API.
Maybe we can think of a script to be executed on build and regenerate all tokens/hashes?
Also got this log (manually debugging :D)
huskyCI_API | 2021/03/23 03:26:06 {"version":"1.1","host":"af2ed42488eb","short_message":" [Hash value from random data is different]","full_message":" [Hash value from random data is different]","timestamp":1616469966,"level":6,"action":"testToken","app":"undefined","file":"/go/src/github.com/globocom/huskyCI/api/log/log.go","info":"VALIDATE","line":31,"tags":"undefined"}
That error is returned from the ValidateRandomData (api/token/token.go line 83), as expected on the hash part of the token it fails. 🐼
@@ -1,4 +1,4 @@ | |||
FROM golang:1.15 | |||
FROM golang:1.16 |
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.
Not sure if we need to fix on this specific version, could just allow for latest image.
Hi guys, I've been testing this fix in a fresh new installation and it works fine. |
...to prevent hash functions that do not produce unique values
Description
Closes #530.
Proposed Changes
Since pbkdf2 works with a
func() Hash.hash
type, returning such type atGetValidHashFunction
inapi/auth/authmongo.go
should simplify the calls topbkdf2.Key
. Our hash functions passed topbkdf2.Key
should not produce unique values anymore (#526), and we can upgrade our Dockerfile to Go 1.16.Testing
Test locally and check that the
panic: crypto/hmac: hash generation function does not produce unique values
is not called anymore.You might want to make more checks. I believe that the stored hashes for users passwords in a production system should not be valid anymore, since apparently we were not using the pbkdf2 library correctly.