Skip to content

bytes/hash: upper 32-bits of Hash result are independent of seed on 32-bit CPUs #34925

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

Closed
mdempsky opened this issue Oct 15, 2019 · 3 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@mdempsky
Copy link
Contributor

Package bytes/hash claims:

// All bits of the Hash result are close to uniformly and
// independently distributed, so can be safely restricted to a range
// using bit masking, shifting, or modular arithmetic.

This is false on 32-bit CPUs:

package main

import "bytes/hash"

func main() {
	for i := 0; i < 10; i++ {
		h := hash.New()
		h.AddString("foo")
		println(h.Hash() >> 32)
	}
}
$ GOARCH=386 go run x.go
2561895305
2561895305
2561895305
2561895305
2561895305
2561895305
2561895305
2561895305
2561895305
2561895305

This is because hash.New() only generates 32 bits of entropy (all in the lower 32-bits of seed), so the "hi" computation in hash.rthash does not involve any entropy.

Mixing lo/hi like the TODO comment says would address this, but the simpler fix would be to generate 64-bits of entropy.

@mdempsky
Copy link
Contributor Author

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 15, 2019
@randall77
Copy link
Contributor

The"uniformly and independently distributed" is intended to apply to changing the key, not the seed. There is no claim of even distribution of bits when changing the seed. Perhaps that could be worded better. For use in a hash table, it is the distribution when varying the key that matters.

But yes, we want to have at least some result dependence on seed, to thwart collision attacks. It need not be as perfect as the key dependence.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/202577 mentions this issue: bytes/hash: initialize all 64 bits of hash seed

@golang golang locked and limited conversation to collaborators Oct 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants