Skip to content
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.

Sign up for GitHub

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

Jump to bottom

x/crypto/argon2: non-repeatable hashes #23245

Closed
renatoathaydes opened this issue Dec 26, 2017 · 7 comments
Closed

x/crypto/argon2: non-repeatable hashes #23245

renatoathaydes opened this issue Dec 26, 2017 · 7 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@renatoathaydes
Copy link

What version of Go are you using (go version)?

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/renato/go"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build622935985=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

I added this test to the test suite of the argon2 crypto:

func TestReproducibleKeys(t *testing.T) {
	password := []byte("password")
	salt := []byte("choosing random salts is hard")
	mvs := []uint32{32, 64, 128, 256, 512, 1024, 10 * 1024, 32 * 1024, 64 * 1024}

	for _, m := range mvs {
		h1 := Key(password, salt, 4, m, 4, 32)
		h2 := Key(password, salt, 4, m, 4, 32)
		require.Equal(t, h1, h2, "Failed with m=%d", m)
	}
}

What did you expect to see?

Tests pass.

What did you see instead?

Tests fail at around m=128 or m=256.

Test run example:

--- FAIL: TestReproducibleKeys (0.01s)
        Error Trace:    argon2_test.go:125
	Error:      	Not equal: 
	            	expected: []byte{0xaa, 0x47, 0xc9, 0x3f, 0x7c, 0x82, 0x60, 0x1b, 0x3c, 0xfa, 0x2, 0xd, 0x85, 0x10, 0x6b, 0x5, 0x10, 0x1e, 0xe3, 0x39, 0x6e, 0xe9, 0x13, 0xca, 0x7, 0x8d, 0x64, 0xdf, 0xc, 0xcb, 0x74, 0x30}
	            	actual: []byte{0xcc, 0xaa, 0x3a, 0xe6, 0x3d, 0x70, 0x67, 0x10, 0xdd, 0xe2, 0xd7, 0xaf, 0xf9, 0x9f, 0x1b, 0xd8, 0x88, 0x99, 0xe3, 0x49, 0x3d, 0x86, 0x7e, 0xa8, 0xc0, 0x5d, 0x52, 0x5a, 0xc9, 0x52, 0xde, 0xd1}
	            	
	            	Diff:
	            	--- Expected
	            	+++ Actual
	            	@@ -1,4 +1,4 @@
	            	 ([]uint8) (len=32) {
	            	- 00000000  aa 47 c9 3f 7c 82 60 1b  3c fa 02 0d 85 10 6b 05  |.G.?|.`.<.....k.|
	            	- 00000010  10 1e e3 39 6e e9 13 ca  07 8d 64 df 0c cb 74 30  |...9n.....d...t0|
	            	+ 00000000  cc aa 3a e6 3d 70 67 10  dd e2 d7 af f9 9f 1b d8  |..:.=pg.........|
	            	+ 00000010  88 99 e3 49 3d 86 7e a8  c0 5d 52 5a c9 52 de d1  |...I=.~..]RZ.R..|
	            	 }
	Messages:   	Failed with m=512
FAIL
exit status 1
FAIL	github.com/golang/crypto/argon2	0.011s
@dgryski
Copy link
Contributor

dgryski commented Dec 26, 2017

Dup of #23200 ?

@bradfitz bradfitz changed the title crypto/argon2: non-repeatable hashes x/crypto/argon2: non-repeatable hashes Dec 27, 2017
@gopherbot gopherbot added this to the Unreleased milestone Dec 27, 2017
@bradfitz
Copy link
Contributor

/cc @aead

@aead
Copy link
Contributor

aead commented Dec 28, 2017

Yes, this is a dup of #23200 - The fix is https://golang.org/cl/85055.

@renatoathaydes
Copy link
Author

When does the fix get pushed to GitHub?

@aead
Copy link
Contributor

aead commented Dec 28, 2017

@renatoathaydes
Cannot tell when the review is done - however you can use this implementation until the CL is merged.

@renatoathaydes
Copy link
Author

@aead thanks! There's a few other implementations as well (or bindings to the C version):

https://github.com/xebia/argon2-go
https://github.com/lhecker/argon2

But I was hoping to rely on the official version. Will use your suggestion for now.

@aead
Copy link
Contributor

aead commented Jan 1, 2018

@renatoathaydes Fix is merged.

@golang golang locked and limited conversation to collaborators Jan 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@bradfitz @dgryski @renatoathaydes @gopherbot @aead