Use base32 for 2FA scratch token (#18384) * Use base32 for 2FA scratch token * rename Secure* to Crypto*, add comments

Changes

11 changed files (+41/-37)

• models/auth/twofactor.go
  • @@ -8,6 +8,7 @@ import (
    "crypto/md5" "crypto/sha256" "crypto/subtle" + "encoding/base32" "encoding/base64" "fmt"
  • @@ -58,11 +59,14 @@ func init() {
    // GenerateScratchToken recreates the scratch token the user is using. func (t *TwoFactor) GenerateScratchToken() (string, error) { - token, err := util.RandomString(8) + tokenBytes, err := util.CryptoRandomBytes(6) if err != nil { return "", err } - t.ScratchSalt, _ = util.RandomString(10) + // these chars are specially chosen, avoid ambiguous chars like `0`, `O`, `1`, `I`. + const base32Chars = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" + token := base32.NewEncoding(base32Chars).WithPadding(base32.NoPadding).EncodeToString(tokenBytes) + t.ScratchSalt, _ = util.CryptoRandomString(10) t.ScratchHash = HashToken(token, t.ScratchSalt) return token, nil }
• models/migrations/v71.go
  • @@ -53,7 +53,7 @@ func addScratchHash(x *xorm.Engine) error {
    for _, tfa := range tfas { // generate salt - salt, err := util.RandomString(10) + salt, err := util.CryptoRandomString(10) if err != nil { return err }
• models/migrations/v85.go
  • @@ -65,7 +65,7 @@ func hashAppToken(x *xorm.Engine) error {
    for _, token := range tokens { // generate salt - salt, err := util.RandomString(10) + salt, err := util.CryptoRandomString(10) if err != nil { return err }
• models/token.go
  • @@ -62,7 +62,7 @@ func init() {
    // NewAccessToken creates new access token. func NewAccessToken(t *AccessToken) error { - salt, err := util.RandomString(10) + salt, err := util.CryptoRandomString(10) if err != nil { return err }
• models/user/user.go
  • @@ -533,7 +533,7 @@ const SaltByteLength = 16
    // GetUserSalt returns a random user salt token. func GetUserSalt() (string, error) { - rBytes, err := util.RandomBytes(SaltByteLength) + rBytes, err := util.CryptoRandomBytes(SaltByteLength) if err != nil { return "", err }
• modules/generate/generate.go
  • @@ -60,7 +60,7 @@ func NewJwtSecretBase64() (string, error) {
    // NewSecretKey generate a new value intended to be used by SECRET_KEY. func NewSecretKey() (string, error) { - secretKey, err := util.RandomString(64) + secretKey, err := util.CryptoRandomString(64) if err != nil { return "", err }
• modules/secret/secret.go
  • @@ -24,7 +24,7 @@ func New() (string, error) {
    // NewWithLength creates a new secret for a given length func NewWithLength(length int64) (string, error) { - return util.RandomString(length) + return util.CryptoRandomString(length) } // AesEncrypt encrypts text and given key with AES.
• modules/util/util.go
  • @@ -137,8 +137,8 @@ func MergeInto(dict map[string]interface{}, values ...interface{}) (map[string]i
    return dict, nil } - // RandomInt returns a random integer between 0 and limit, inclusive - func RandomInt(limit int64) (int64, error) { + // CryptoRandomInt returns a crypto random integer between 0 and limit, inclusive + func CryptoRandomInt(limit int64) (int64, error) { rInt, err := rand.Int(rand.Reader, big.NewInt(limit)) if err != nil { return 0, err
  • @@ -146,27 +146,27 @@ func RandomInt(limit int64) (int64, error) {
    return rInt.Int64(), nil } - const letters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" + const alphanumericalChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" - // RandomString generates a random alphanumerical string - func RandomString(length int64) (string, error) { - bytes := make([]byte, length) - limit := int64(len(letters)) - for i := range bytes { - num, err := RandomInt(limit) + // CryptoRandomString generates a crypto random alphanumerical string, each byte is generated by [0,61] range + func CryptoRandomString(length int64) (string, error) { + buf := make([]byte, length) + limit := int64(len(alphanumericalChars)) + for i := range buf { + num, err := CryptoRandomInt(limit) if err != nil { return "", err } - bytes[i] = letters[num] + buf[i] = alphanumericalChars[num] } - return string(bytes), nil + return string(buf), nil } - // RandomBytes generates `length` bytes - // This differs from RandomString, as RandomString is limits each byte to have - // a maximum value of 63 instead of 255(max byte size) - func RandomBytes(length int64) ([]byte, error) { - bytes := make([]byte, length) - _, err := rand.Read(bytes) - return bytes, err + // CryptoRandomBytes generates `length` crypto bytes + // This differs from CryptoRandomString, as each byte in CryptoRandomString is generated by [0,61] range + // This function generates totally random bytes, each byte is generated by [0,255] range + func CryptoRandomBytes(length int64) ([]byte, error) { + buf := make([]byte, length) + _, err := rand.Read(buf) + return buf, err }
• modules/util/util_test.go
  • @@ -120,20 +120,20 @@ func Test_NormalizeEOL(t *testing.T) {
    } func Test_RandomInt(t *testing.T) { - int, err := RandomInt(255) + int, err := CryptoRandomInt(255) assert.True(t, int >= 0) assert.True(t, int <= 255) assert.NoError(t, err) } func Test_RandomString(t *testing.T) { - str1, err := RandomString(32) + str1, err := CryptoRandomString(32) assert.NoError(t, err) matches, err := regexp.MatchString(`^[a-zA-Z0-9]{32}$`, str1) assert.NoError(t, err) assert.True(t, matches) - str2, err := RandomString(32) + str2, err := CryptoRandomString(32) assert.NoError(t, err) matches, err = regexp.MatchString(`^[a-zA-Z0-9]{32}$`, str1) assert.NoError(t, err)
  • @@ -141,13 +141,13 @@ func Test_RandomString(t *testing.T) {
    assert.NotEqual(t, str1, str2) - str3, err := RandomString(256) + str3, err := CryptoRandomString(256) assert.NoError(t, err) matches, err = regexp.MatchString(`^[a-zA-Z0-9]{256}$`, str3) assert.NoError(t, err) assert.True(t, matches) - str4, err := RandomString(256) + str4, err := CryptoRandomString(256) assert.NoError(t, err) matches, err = regexp.MatchString(`^[a-zA-Z0-9]{256}$`, str4) assert.NoError(t, err)
  • @@ -157,18 +157,18 @@ func Test_RandomString(t *testing.T) {
    } func Test_RandomBytes(t *testing.T) { - bytes1, err := RandomBytes(32) + bytes1, err := CryptoRandomBytes(32) assert.NoError(t, err) - bytes2, err := RandomBytes(32) + bytes2, err := CryptoRandomBytes(32) assert.NoError(t, err) assert.NotEqual(t, bytes1, bytes2) - bytes3, err := RandomBytes(256) + bytes3, err := CryptoRandomBytes(256) assert.NoError(t, err) - bytes4, err := RandomBytes(256) + bytes4, err := CryptoRandomBytes(256) assert.NoError(t, err) assert.NotEqual(t, bytes3, bytes4)
• routers/web/auth/openid.go
  • @@ -416,7 +416,7 @@ func RegisterOpenIDPost(ctx *context.Context) {
    if length < 256 { length = 256 } - password, err := util.RandomString(int64(length)) + password, err := util.CryptoRandomString(int64(length)) if err != nil { ctx.RenderWithErr(err.Error(), tplSignUpOID, form) return
• routers/web/repo/setting.go
  • @@ -337,7 +337,7 @@ func SettingsPost(ctx *context.Context) {
    return } - remoteSuffix, err := util.RandomString(10) + remoteSuffix, err := util.CryptoRandomString(10) if err != nil { ctx.ServerError("RandomString", err) return