gin.BasicAuth timing attack not properly fixed

[jerbob92]

Description

In #2226 and #2609 a fix was discussed and made to prevent timing attacks on the basic auth logic of Gin. However, this was only partly fixed. The decision was made to use subtle.ConstantTimeCompare, however, that has quite a big flaw that it immediately returns when the length of the 2 strings is not equal, which still allows it to be used for timing attacks to guess how long the password should be.

There are basically 2 ways to fix this:

1. Hash both strings so that they are of equal length
2. Pad the shortest string so that they are of equal length

In a personal project I decided to go for option 2, which looks like this:

// ConstantTimeCompareString forces both strings to the same length,
// because subtle.ConstantTimeCompare will return 0 early if the strings are
// not of equal length, which could leak the required string length.
func ConstantTimeCompareString(x, y string) bool {
    maxLength := len(x)
    if len(y) > maxLength {
        maxLength = len(y)
    }

    x = fmt.Sprintf("%*s", maxLength, x)
    y = fmt.Sprintf("%*s", maxLength, y)

    if subtle.ConstantTimeCompare([]byte(x), []byte(y)) == 1 {
        return true
    }

    return false
}

[jerbob92]

I'm just wondering now if the padding loop in Sprintf also might allow for a timing attack. It's probably less vulnerable than subtle.ConstantTimeCompare, but it's still looping for one of the strings to make it equal length.

Maybe hashing is the better way after all?

[icy4ever]

is it a better suggestion to golang crypto package method ConstantTimeCompare?

[jerbob92]

There have been various discussions about that, for example here: https://github.com/golang/go/issues/18936 It doesn't really look like they want to change it.