2019 Q1 Recurity Assessment: LoginState HMAC Issues

https://gitlab.com/gitlab-com/gl-security/engineering/issues/329

Details

The Gitlab/:Geo/:Oauth/:LoginState class generates HMAC values in order to sign a redirect URI (see also chapters 3.1 and 3.3). The derivation of the HMAC key is performed by concatenation of an 8 byte random salt and the secret_key_base:

def generate_hmac
  digest = OpenSSL/:Digest/:SHA256.new
  key = Gitlab/:Application.secrets.secret_key_base + salt
  OpenSSL/:HMAC.hexdigest(digest, key, return_to.to_s)
end

This construction is not advisable as the simple concatenation of the salt and the secret_key_base ease, for instance, brute force attacks on the secret_key_base. Additionally, within the generate_hmac routine, it should be ensured that the salt value is of sufficient length. Otherwise, depending on the calling methods, an empty string might be used as the salt value, leading to secret_key_base being the actual HMAC key.

Reproduction Steps

Observe the code at ee/lib/gitlab/geo/oauth/login_state.rb.

Recommendation

A proper key derivation mechanism, such as the built-in key derivation mechanism of Ruby on Rails, should be utilized.