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

crypto/subtle: ConstantTimeCompare doesn't always run in constant time #47001

Closed
nickvellios opened this issue Jun 30, 2021 · 4 comments
Closed

Comments

@nickvellios
Copy link

Type: Potential Exploit
Go v1.17 (Latest)

Problem:

if len(x) != len(y) {
return 0
}

The ConstantTimeCompare function in crypto/subtle/constant_time.go will return in O(1) if the lengths of both function parameters are not equal, and O(n) if the lengths are equal.

Security concern:

bcrypt's CompareHashAndPassword function uses subtle's constant time compare to combat timing attacks. One could discover the length of a password hash which could be an attack vector when paired with another exploit.

Risk level is low, but any unanticipated leak of data is still a leak and this should be addressed.

Potential solution:

If lengths of x and y are not the same, proceed to compare as normal but in this case, always return 0. I'm open to suggestions.

@randall77
Copy link
Contributor

Dup of #31355, closing.

@jmhodges
Copy link
Contributor

jmhodges commented Jul 1, 2021

As the original author of the Go bcrypt lib, let me chime in on why this is not a security issue for bcrypt:

The use of ConstantTimeCompare in bcrypt is only given the hashes of the password and those hashes are always the same length.

See the use here:

https://github.com/golang/crypto/blob/5ff15b29337e062d850872081bcd9f4d784f4c25/bcrypt/bcrypt.go#L111-L113

and the slice creation in Hash here:

https://github.com/golang/crypto/blob/5ff15b29337e062d850872081bcd9f4d784f4c25/bcrypt/bcrypt.go#L234-L235

@jmhodges
Copy link
Contributor

jmhodges commented Jul 1, 2021

(You're not the first person to worry about this! I've gotten this kind of thing wrong before, too. Thanks for making a ticket!)

@nickvellios
Copy link
Author

@jmhodges I appreciate your contributions to this issue and the Go project. Thanks for your input.

@golang golang locked and limited conversation to collaborators Jul 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants