Remove unused custom-alphabet feature of random string generator

Fix random string generator

Random string generator should return error if it fails to read random data via crypto/rand

models/migrations/migrations.go

@@ -457,8 +457,12 @@ func generateOrgRandsAndSalt(x *xorm.Engine) (err error) {
  }
 
  for _, org := range orgs {
- org.Rands = base.GetRandomString(10)
- org.Salt = base.GetRandomString(10)
+ if org.Rands, err = base.GetRandomString(10); err != nil {
+ return err
+ }
+ if org.Salt, err = base.GetRandomString(10); err != nil {
+ return err
+ }
  if _, err = sess.Id(org.ID).Update(org); err != nil {
  return err
  }

models/org.go

@@ -109,8 +109,12 @@ func CreateOrganization(org, owner *User) (err error) {
  }
 
  org.LowerName = strings.ToLower(org.Name)
- org.Rands = GetUserSalt()
- org.Salt = GetUserSalt()
+ if org.Rands, err = GetUserSalt(); err != nil {
+ return err
+ }
+ if org.Salt, err = GetUserSalt(); err != nil {
+ return err
+ }
  org.UseCustomAvatar = true
  org.MaxRepoCreation = -1
  org.NumTeams = 1

models/user.go

@@ -531,7 +531,7 @@ func IsUserExist(uid int64, name string) (bool, error) {
 }
 
 // GetUserSalt returns a ramdom user salt token.
-func GetUserSalt() string {
+func GetUserSalt() (string, error) {
  return base.GetRandomString(10)
 }
 
@@ -603,8 +603,12 @@ func CreateUser(u *User) (err error) {
  u.LowerName = strings.ToLower(u.Name)
  u.AvatarEmail = u.Email
  u.Avatar = base.HashEmail(u.AvatarEmail)
- u.Rands = GetUserSalt()
- u.Salt = GetUserSalt()
+ if u.Rands, err = GetUserSalt(); err != nil {
+ return err
+ }
+ if u.Salt, err = GetUserSalt(); err != nil {
+ return err
+ }
  u.EncodePasswd()
  u.MaxRepoCreation = -1
 

models/user_mail.go

@@ -116,7 +116,9 @@ func (email *EmailAddress) Activate() error {
  if err != nil {
  return err
  }
- user.Rands = GetUserSalt()
+ if user.Rands, err = GetUserSalt(); err != nil {
+ return err
+ }
 
  sess := x.NewSession()
  defer sessionRelease(sess)

modules/base/tool.go

@@ -15,6 +15,7 @@ import (
  "hash"
  "html/template"
  "math"
+ "math/big"
  "net/http"
  "strconv"
  "strings"
@@ -83,18 +84,31 @@ func BasicAuthEncode(username, password string) string {
 }
 
 // GetRandomString generate random string by specify chars.
-func GetRandomString(n int, alphabets ...byte) string {
+func GetRandomString(n int) (string, error) {
  const alphanum = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
- var bytes = make([]byte, n)
- rand.Read(bytes)
- for i, b := range bytes {
- if len(alphabets) == 0 {
- bytes[i] = alphanum[b%byte(len(alphanum))]
- } else {
- bytes[i] = alphabets[b%byte(len(alphabets))]
+
+ buffer := make([]byte, n)
+ max := big.NewInt(int64(len(alphanum)))
+
+ for i := 0; i < n; i++ {
+ index, err := randomInt(max)
+ if err != nil {
+ return "", err
  }
+
+ buffer[i] = alphanum[index]
  }
- return string(bytes)
+
+ return string(buffer), nil
+}
+
+func randomInt(max *big.Int) (int, error) {
+ rand, err := rand.Int(rand.Reader, max)
+ if err != nil {
+ return 0, err
+ }
+
+ return int(rand.Int64()), nil
 }
 
 // PBKDF2 https://code.google.com/p/go/source/browse/pbkdf2/pbkdf2.go?repo=crypto

routers/admin/users.go

@@ -197,7 +197,11 @@ func EditUserPost(ctx *context.Context, form auth.AdminEditUserForm) {
 
  if len(form.Password) > 0 {
  u.Passwd = form.Password
- u.Salt = models.GetUserSalt()
+ var err error
+ if u.Salt, err = models.GetUserSalt(); err != nil {
+ ctx.Handle(500, "UpdateUser", err)
+ return
+ }
  u.EncodePasswd()
  }
 

routers/api/v1/admin/user.go

@@ -87,7 +87,11 @@ func EditUser(ctx *context.APIContext, form api.EditUserOption) {
 
  if len(form.Password) > 0 {
  u.Passwd = form.Password
- u.Salt = models.GetUserSalt()
+ var err error
+ if u.Salt, err = models.GetUserSalt(); err != nil {
+ ctx.Error(500, "UpdateUser", err)
+ return
+ }
  u.EncodePasswd()
  }
 

routers/install.go

@@ -279,7 +279,14 @@ func InstallPost(ctx *context.Context, form auth.InstallForm) {
  cfg.Section("log").Key("ROOT_PATH").SetValue(form.LogRootPath)
 
  cfg.Section("security").Key("INSTALL_LOCK").SetValue("true")
- cfg.Section("security").Key("SECRET_KEY").SetValue(base.GetRandomString(15))
+
+ var secretKey string
+ secretKey, err := base.GetRandomString(10)
+ if err != nil {
+ ctx.RenderWithErr(ctx.Tr("install.secret_key_failed", err), tplInstall, &form)
+ return
+ }
+ cfg.Section("security").Key("SECRET_KEY").SetValue(secretKey)
 
  err := os.MkdirAll(filepath.Dir(setting.CustomConf), os.ModePerm)
  if err != nil {

routers/user/auth.go

@@ -289,7 +289,11 @@ func Activate(ctx *context.Context) {
  // Verify code.
  if user := models.VerifyUserActiveCode(code); user != nil {
  user.IsActive = true
- user.Rands = models.GetUserSalt()
+ var err error
+ if user.Rands, err = models.GetUserSalt(); err != nil {
+ ctx.Handle(500, "UpdateUser", err)
+ return
+ }
  if err := models.UpdateUser(user); err != nil {
  if models.IsErrUserNotExist(err) {
  ctx.Error(404)
@@ -428,8 +432,15 @@ func ResetPasswdPost(ctx *context.Context) {
  }
 
  u.Passwd = passwd
- u.Rands = models.GetUserSalt()
- u.Salt = models.GetUserSalt()
+ var err error
+ if u.Rands, err = models.GetUserSalt(); err != nil {
+ ctx.Handle(500, "UpdateUser", err)
+ return
+ }
+ if u.Salt, err = models.GetUserSalt(); err != nil {
+ ctx.Handle(500, "UpdateUser", err)
+ return
+ }
  u.EncodePasswd()
  if err := models.UpdateUser(u); err != nil {
  ctx.Handle(500, "UpdateUser", err)

routers/user/setting.go

@@ -197,7 +197,11 @@ func SettingsPasswordPost(ctx *context.Context, form auth.ChangePasswordForm) {
  ctx.Flash.Error(ctx.Tr("form.password_not_match"))
  } else {
  ctx.User.Passwd = form.Password
- ctx.User.Salt = models.GetUserSalt()
+ var err error
+ if ctx.User.Salt, err = models.GetUserSalt(); err != nil {
+ ctx.Handle(500, "UpdateUser", err)
+ return
+ }
  ctx.User.EncodePasswd()
  if err := models.UpdateUser(ctx.User); err != nil {
  ctx.Handle(500, "UpdateUser", err)