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

Jump to bottom

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

Closed
levinalex opened this issue Oct 6, 2017 · 3 comments
Closed

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

levinalex opened this issue Oct 6, 2017 · 3 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Unreleased

Comments

@levinalex
Copy link

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
Copy link

mdp 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)

@FiloSottile
Copy link
Contributor

Is this behavior observable from the public x/crypto/pkcs12 API? Does it lead to Decode failing for valid inputs?

@FiloSottile FiloSottile added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 7, 2020
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@golang golang locked and limited conversation to collaborators Aug 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@levinalex @mdp @FiloSottile @gopherbot