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.

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

x/crypto/pkcs12: incorrect results when using pbkdf with SHA265 as hash function #22163

Open
levinalex opened this issue Oct 6, 2017 · 1 comment

Comments

@levinalex
Copy link

commented Oct 6, 2017

This is for Go 1.9

I've found the (unexported) pbkdf function from x/crypto/pkcs12 gives incorrect results when using a hash function that's not 20 bytes long.

I'm working on some code that needs PKCS12 key derivation, and could not make behavior match results I get from existing implementations of my algorithm using bouncycastle* (Java) and forge* (javascript)

I'm using SHA256 as the hash function (Block size 64, Size 32) and pbkdf seemed to truncate internal state A compared to other implementations.

This is due to this code:

	A := make([]byte, c*20)
	var IjBuf []byte
	for i := 0; i < c; i++ {
		//        A.  Set A2=H^r(D||I). (i.e., the r-th hash of D||1,
		//            H(H(H(... H(D||I))))
		Ai := hash(append(D, I...))
		for j := 1; j < r; j++ {
			Ai = hash(Ai)
		}
		copy(A[i*20:], Ai[:])
// ...

where does the 20 come from? I believe this should be block size (u), and indeed my test cases pass when I change it to that. (this is "Step 7" in the RFC)

Testcase:

func TestThatPBKDFHandlesSha256Correctly(t *testing.T) {
	sha256Sum := func(in []byte) []byte {
		sum := sha256.Sum256(in)
		return sum[:]
	}
	salt, _ := bmpString("salt")
	password, _ := bmpString("sesame")

	key := pbkdf(sha256Sum, 32, 64, salt, password, 256, 1, 32)

	if expected := []byte("\x19\xd0\x05\x62\x66\x37\x4c\xdc\x82\x6f\x65\x90\x98\x94\xab\x3d\x05\xea\x8d\x75\x81\x0c\xca\x23\x19\x43\xd7\x3f\x1c\x8b\xcd\xa1"); bytes.Compare(key, expected) != 0 {
		t.Fatalf("expected key '%x', but found '%x'", expected, key)
	}
}

@gopherbot gopherbot added this to the Unreleased milestone Oct 6, 2017

@mdp

This comment has been minimized.

Copy link

commented Sep 1, 2018

I think this library is just implementing the older PBKDF1, which is limited to 160 bits.

Unfortunately, from what I can tell, it looks like this part of x/crypto is not actively being developed. There seems to be some backstory on the development - #14125 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.