loading up the forgejo repo on tangled to test page performance
chore(sec): unify usage of `crypto/rand.Read` (#7453)
- Unify the usage of [`crypto/rand.Read`](https://pkg.go.dev/crypto/rand#Read) to `util.CryptoRandomBytes`.
- Refactor `util.CryptoRandomBytes` to never return an error. It is documented by Go, https://go.dev/issue/66821, to always succeed. So if we still receive a error or if the returned bytes read is not equal to the expected bytes to be read we panic (just to be on the safe side).
- This simplifies a lot of code to no longer care about error handling.
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7453
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
authored by
Gusted
Gusted
and committed by
Earl Warren
53df0bf9
99fc04b7
+61
-163
+1
-4
cmd/generate.go
+1
-4
cmd/generate.go
+1
-6
models/actions/runner.go
+1
-6
models/actions/runner.go
···
168
168
// UpdateSecret updates the hash based on the specified token. It does not
169
169
// ensure that the runner's UUID matches the first 16 bytes of the token.
170
170
func (r *ActionRunner) UpdateSecret(token string) error {
171
-
saltBytes, err := util.CryptoRandomBytes(16)
172
-
if err != nil {
173
-
return fmt.Errorf("CryptoRandomBytes %v", err)
174
-
}
175
-
176
-
salt := hex.EncodeToString(saltBytes)
171
+
salt := hex.EncodeToString(util.CryptoRandomBytes(16))
177
172
178
173
r.Token = token
179
174
r.TokenSalt = salt
+1
-5
models/actions/utils.go
+1
-5
models/actions/utils.go
···
22
22
if err != nil {
23
23
return "", "", "", "", err
24
24
}
25
-
buf, err := util.CryptoRandomBytes(20)
26
-
if err != nil {
27
-
return "", "", "", "", err
28
-
}
29
-
token := hex.EncodeToString(buf)
25
+
token := hex.EncodeToString(util.CryptoRandomBytes(20))
30
26
hash := auth_model.HashToken(token, salt)
31
27
return token, salt, hash, token[len(token)-8:], nil
32
28
}
+1
-5
models/auth/access_token.go
+1
-5
models/auth/access_token.go
···
111
111
if err != nil {
112
112
return err
113
113
}
114
-
token, err := util.CryptoRandomBytes(20)
115
-
if err != nil {
116
-
return err
117
-
}
118
114
t.TokenSalt = salt
119
-
t.Token = hex.EncodeToString(token)
115
+
t.Token = hex.EncodeToString(util.CryptoRandomBytes(20))
120
116
t.TokenHash = HashToken(t.Token, t.TokenSalt)
121
117
t.TokenLastEight = t.Token[len(t.Token)-8:]
122
118
return nil
+1
-4
models/auth/auth_token.go
+1
-4
models/auth/auth_token.go
···
63
63
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp, purpose AuthorizationPurpose) (lookupKey, validator string, err error) {
64
64
// Request 64 random bytes. The first 32 bytes will be used for the lookupKey
65
65
// and the other 32 bytes will be used for the validator.
66
-
rBytes, err := util.CryptoRandomBytes(64)
67
-
if err != nil {
68
-
return "", "", err
69
-
}
66
+
rBytes := util.CryptoRandomBytes(64)
70
67
hexEncoded := hex.EncodeToString(rBytes)
71
68
validator, lookupKey = hexEncoded[64:], hexEncoded[:64]
72
69
+2
-10
models/auth/oauth2.go
+2
-10
models/auth/oauth2.go
···
184
184
185
185
// GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database
186
186
func (app *OAuth2Application) GenerateClientSecret(ctx context.Context) (string, error) {
187
-
rBytes, err := util.CryptoRandomBytes(32)
188
-
if err != nil {
189
-
return "", err
190
-
}
191
187
// Add a prefix to the base32, this is in order to make it easier
192
188
// for code scanners to grab sensitive tokens.
193
-
clientSecret := "gto_" + base32Lower.EncodeToString(rBytes)
189
+
clientSecret := "gto_" + base32Lower.EncodeToString(util.CryptoRandomBytes(32))
194
190
195
191
hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost)
196
192
if err != nil {
···
475
471
476
472
// GenerateNewAuthorizationCode generates a new authorization code for a grant and saves it to the database
477
473
func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) {
478
-
rBytes, err := util.CryptoRandomBytes(32)
479
-
if err != nil {
480
-
return &OAuth2AuthorizationCode{}, err
481
-
}
482
474
// Add a prefix to the base32, this is in order to make it easier
483
475
// for code scanners to grab sensitive tokens.
484
-
codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)
476
+
codeSecret := "gta_" + base32Lower.EncodeToString(util.CryptoRandomBytes(32))
485
477
486
478
code = &OAuth2AuthorizationCode{
487
479
Grant: grant,
+3
-7
models/auth/twofactor.go
+3
-7
models/auth/twofactor.go
···
61
61
}
62
62
63
63
// GenerateScratchToken recreates the scratch token the user is using.
64
-
func (t *TwoFactor) GenerateScratchToken() (string, error) {
65
-
tokenBytes, err := util.CryptoRandomBytes(6)
66
-
if err != nil {
67
-
return "", err
68
-
}
64
+
func (t *TwoFactor) GenerateScratchToken() string {
69
65
// these chars are specially chosen, avoid ambiguous chars like `0`, `O`, `1`, `I`.
70
66
const base32Chars = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
71
-
token := base32.NewEncoding(base32Chars).WithPadding(base32.NoPadding).EncodeToString(tokenBytes)
67
+
token := base32.NewEncoding(base32Chars).WithPadding(base32.NoPadding).EncodeToString(util.CryptoRandomBytes(6))
72
68
t.ScratchSalt, _ = util.CryptoRandomString(10)
73
69
t.ScratchHash = HashToken(token, t.ScratchSalt)
74
-
return token, nil
70
+
return token
75
71
}
76
72
77
73
// HashToken return the hashable salt
+2
-6
models/organization/org.go
+2
-6
models/organization/org.go
···
289
289
}
290
290
291
291
org.LowerName = strings.ToLower(org.Name)
292
-
if org.Rands, err = user_model.GetUserSalt(); err != nil {
293
-
return err
294
-
}
295
-
if org.Salt, err = user_model.GetUserSalt(); err != nil {
296
-
return err
297
-
}
292
+
org.Rands = user_model.GetUserSalt()
293
+
org.Salt = user_model.GetUserSalt()
298
294
org.UseCustomAvatar = true
299
295
org.MaxRepoCreation = -1
300
296
org.NumTeams = 1
+2
-6
models/user/email_address.go
+2
-6
models/user/email_address.go
···
266
266
if err != nil {
267
267
return err
268
268
}
269
-
if user.Rands, err = GetUserSalt(); err != nil {
270
-
return err
271
-
}
269
+
user.Rands = GetUserSalt()
272
270
email.IsActivated = activate
273
271
if _, err := db.GetEngine(ctx).ID(email.ID).Cols("is_activated").Update(email); err != nil {
274
272
return err
···
403
401
// The user's activation state should be synchronized with the primary email
404
402
if user.IsActive != activate {
405
403
user.IsActive = activate
406
-
if user.Rands, err = GetUserSalt(); err != nil {
407
-
return fmt.Errorf("unable to generate salt: %w", err)
408
-
}
404
+
user.Rands = GetUserSalt()
409
405
if err = UpdateUserCols(ctx, user, "is_active", "rands"); err != nil {
410
406
return fmt.Errorf("unable to updateUserCols() for user ID: %d: %w", userID, err)
411
407
}
+4
-12
models/user/user.go
+4
-12
models/user/user.go
···
387
387
return err
388
388
}
389
389
390
-
if u.Salt, err = GetUserSalt(); err != nil {
391
-
return err
392
-
}
390
+
u.Salt = GetUserSalt()
393
391
if u.Passwd, err = hash.Parse(setting.PasswordHashAlgo).Hash(passwd, u.Salt); err != nil {
394
392
return err
395
393
}
···
559
557
const SaltByteLength = 16
560
558
561
559
// GetUserSalt returns a random user salt token.
562
-
func GetUserSalt() (string, error) {
563
-
rBytes, err := util.CryptoRandomBytes(SaltByteLength)
564
-
if err != nil {
565
-
return "", err
566
-
}
560
+
func GetUserSalt() string {
567
561
// Returns a 32 bytes long string.
568
-
return hex.EncodeToString(rBytes), nil
562
+
return hex.EncodeToString(util.CryptoRandomBytes(SaltByteLength))
569
563
}
570
564
571
565
// Note: The set of characters here can safely expand without a breaking change,
···
772
766
773
767
u.LowerName = strings.ToLower(u.Name)
774
768
u.AvatarEmail = u.Email
775
-
if u.Rands, err = GetUserSalt(); err != nil {
776
-
return err
777
-
}
769
+
u.Rands = GetUserSalt()
778
770
if u.Passwd != "" {
779
771
if err = u.SetPassword(u.Passwd); err != nil {
780
772
return err
+5
-19
modules/generate/generate.go
+5
-19
modules/generate/generate.go
···
5
5
package generate
6
6
7
7
import (
8
-
"crypto/rand"
9
8
"encoding/base64"
10
9
"fmt"
11
-
"io"
12
10
"time"
13
11
14
12
"forgejo.org/modules/util"
···
18
16
19
17
// NewInternalToken generate a new value intended to be used by INTERNAL_TOKEN.
20
18
func NewInternalToken() (string, error) {
21
-
secretBytes := make([]byte, 32)
22
-
_, err := io.ReadFull(rand.Reader, secretBytes)
23
-
if err != nil {
24
-
return "", err
25
-
}
26
-
27
-
secretKey := base64.RawURLEncoding.EncodeToString(secretBytes)
19
+
secretKey := base64.RawURLEncoding.EncodeToString(util.CryptoRandomBytes(32))
28
20
29
21
now := time.Now()
30
22
31
-
var internalToken string
32
-
internalToken, err = jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
23
+
internalToken, err := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
33
24
"nbf": now.Unix(),
34
25
}).SignedString([]byte(secretKey))
35
26
if err != nil {
···
54
45
}
55
46
56
47
// NewJwtSecret generates a new base64 encoded value intended to be used for JWT secrets.
57
-
func NewJwtSecret() ([]byte, string, error) {
58
-
bytes := make([]byte, 32)
59
-
_, err := rand.Read(bytes)
60
-
if err != nil {
61
-
return nil, "", err
62
-
}
63
-
64
-
return bytes, base64.RawURLEncoding.EncodeToString(bytes), nil
48
+
func NewJwtSecret() ([]byte, string) {
49
+
bytes := util.CryptoRandomBytes(32)
50
+
return bytes, base64.RawURLEncoding.EncodeToString(bytes)
65
51
}
66
52
67
53
// NewSecretKey generate a new value intended to be used by SECRET_KEY.
+1
-2
modules/generate/generate_test.go
+1
-2
modules/generate/generate_test.go
+3
-5
modules/keying/keying.go
+3
-5
modules/keying/keying.go
···
17
17
18
18
import (
19
19
"crypto/hkdf"
20
-
"crypto/rand"
21
20
"crypto/sha256"
22
21
"encoding/binary"
22
+
23
+
"forgejo.org/modules/util"
23
24
24
25
"golang.org/x/crypto/chacha20poly1305"
25
26
)
···
95
96
}
96
97
97
98
// Generate a random nonce.
98
-
nonce := make([]byte, aeadNonceSize)
99
-
if n, err := rand.Read(nonce); err != nil || n != aeadNonceSize {
100
-
panic(err)
101
-
}
99
+
nonce := util.CryptoRandomBytes(aeadNonceSize)
102
100
103
101
// Returns the ciphertext of this plaintext.
104
102
return e.Seal(nonce, nonce, plaintext, additionalData)
+1
-4
modules/setting/lfs.go
+1
-4
modules/setting/lfs.go
···
80
80
jwtSecretBase64 := loadSecret(rootCfg.Section("server"), "LFS_JWT_SECRET_URI", "LFS_JWT_SECRET")
81
81
LFS.JWTSecretBytes, err = generate.DecodeJwtSecret(jwtSecretBase64)
82
82
if err != nil {
83
-
LFS.JWTSecretBytes, jwtSecretBase64, err = generate.NewJwtSecret()
84
-
if err != nil {
85
-
return fmt.Errorf("error generating JWT Secret for custom config: %v", err)
86
-
}
83
+
LFS.JWTSecretBytes, jwtSecretBase64 = generate.NewJwtSecret()
87
84
88
85
// Save secret
89
86
saveCfg, err := rootCfg.PrepareSaving()
+2
-8
modules/setting/oauth2.go
+2
-8
modules/setting/oauth2.go
···
138
138
if InstallLock {
139
139
jwtSecretBytes, err := generate.DecodeJwtSecret(jwtSecretBase64)
140
140
if err != nil {
141
-
jwtSecretBytes, jwtSecretBase64, err = generate.NewJwtSecret()
142
-
if err != nil {
143
-
log.Fatal("error generating JWT secret: %v", err)
144
-
}
141
+
jwtSecretBytes, jwtSecretBase64 = generate.NewJwtSecret()
145
142
saveCfg, err := rootCfg.PrepareSaving()
146
143
if err != nil {
147
144
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
···
161
158
func GetGeneralTokenSigningSecret() []byte {
162
159
old := generalSigningSecret.Load()
163
160
if old == nil || len(*old) == 0 {
164
-
jwtSecret, _, err := generate.NewJwtSecret()
165
-
if err != nil {
166
-
log.Fatal("Unable to generate general JWT secret: %v", err)
167
-
}
161
+
jwtSecret, _ := generate.NewJwtSecret()
168
162
if generalSigningSecret.CompareAndSwap(old, &jwtSecret) {
169
163
return jwtSecret
170
164
}
+9
-3
modules/util/util.go
+9
-3
modules/util/util.go
···
88
88
// CryptoRandomBytes generates `length` crypto bytes
89
89
// This differs from CryptoRandomString, as each byte in CryptoRandomString is generated by [0,61] range
90
90
// This function generates totally random bytes, each byte is generated by [0,255] range
91
-
func CryptoRandomBytes(length int64) ([]byte, error) {
91
+
func CryptoRandomBytes(length int64) []byte {
92
+
// crypto/rand.Read is documented to never return a error.
93
+
// https://go.dev/issue/66821
92
94
buf := make([]byte, length)
93
-
_, err := rand.Read(buf)
94
-
return buf, err
95
+
n, err := rand.Read(buf)
96
+
if err != nil || n != int(length) {
97
+
panic(err)
98
+
}
99
+
100
+
return buf
95
101
}
96
102
97
103
// ToUpperASCII returns s with all ASCII letters mapped to their upper case.
+8
-10
modules/util/util_test.go
+8
-10
modules/util/util_test.go
···
163
163
}
164
164
165
165
func Test_RandomBytes(t *testing.T) {
166
-
bytes1, err := util.CryptoRandomBytes(32)
167
-
require.NoError(t, err)
166
+
bytes1 := util.CryptoRandomBytes(32)
167
+
bytes2 := util.CryptoRandomBytes(32)
168
168
169
-
bytes2, err := util.CryptoRandomBytes(32)
170
-
require.NoError(t, err)
171
-
169
+
assert.Len(t, bytes1, 32)
170
+
assert.Len(t, bytes2, 32)
172
171
assert.NotEqual(t, bytes1, bytes2)
173
172
174
-
bytes3, err := util.CryptoRandomBytes(256)
175
-
require.NoError(t, err)
173
+
bytes3 := util.CryptoRandomBytes(256)
174
+
bytes4 := util.CryptoRandomBytes(256)
176
175
177
-
bytes4, err := util.CryptoRandomBytes(256)
178
-
require.NoError(t, err)
179
-
176
+
assert.Len(t, bytes3, 256)
177
+
assert.Len(t, bytes4, 256)
180
178
assert.NotEqual(t, bytes3, bytes4)
181
179
}
182
180
+2
-3
options/locale_next/locale_en-US.json
+2
-3
options/locale_next/locale_en-US.json
···
20
20
"error.not_found.title": "Page not found",
21
21
"alert.asset_load_failed": "Failed to load asset files from {path}. Please make sure the asset files can be accessed.",
22
22
"alert.range_error": " must be a number between %[1]s and %[2]s.",
23
-
"install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s",
24
-
"install.lfs_jwt_secret_failed": "Unable to generate a LFS JWT secret: %[1]s"
25
-
}
23
+
"install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s"
24
+
}
+2
-10
routers/install/install.go
+2
-10
routers/install/install.go
···
408
408
if form.LFSRootPath != "" {
409
409
cfg.Section("server").Key("LFS_START_SERVER").SetValue("true")
410
410
cfg.Section("lfs").Key("PATH").SetValue(form.LFSRootPath)
411
-
var lfsJwtSecret string
412
-
if _, lfsJwtSecret, err = generate.NewJwtSecret(); err != nil {
413
-
ctx.RenderWithErr(ctx.Tr("install.lfs_jwt_secret_failed", err), tplInstall, &form)
414
-
return
415
-
}
411
+
_, lfsJwtSecret := generate.NewJwtSecret()
416
412
cfg.Section("server").Key("LFS_JWT_SECRET").SetValue(lfsJwtSecret)
417
413
} else {
418
414
cfg.Section("server").Key("LFS_START_SERVER").SetValue("false")
···
483
479
// FIXME: at the moment, no matter oauth2 is enabled or not, it must generate a "oauth2 JWT_SECRET"
484
480
// see the "loadOAuth2From" in "setting/oauth2.go"
485
481
if !cfg.Section("oauth2").HasKey("JWT_SECRET") && !cfg.Section("oauth2").HasKey("JWT_SECRET_URI") {
486
-
_, jwtSecretBase64, err := generate.NewJwtSecret()
487
-
if err != nil {
488
-
ctx.RenderWithErr(ctx.Tr("install.secret_key_failed", err), tplInstall, &form)
489
-
return
490
-
}
482
+
_, jwtSecretBase64 := generate.NewJwtSecret()
491
483
cfg.Section("oauth2").Key("JWT_SECRET").SetValue(jwtSecretBase64)
492
484
}
493
485
+1
-5
routers/web/auth/2fa.go
+1
-5
routers/web/auth/2fa.go
···
133
133
// Validate the passcode with the stored TOTP secret.
134
134
if twofa.VerifyScratchToken(form.Token) {
135
135
// Invalidate the scratch token.
136
-
_, err = twofa.GenerateScratchToken()
137
-
if err != nil {
138
-
ctx.ServerError("UserSignIn", err)
139
-
return
140
-
}
136
+
twofa.GenerateScratchToken()
141
137
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
142
138
ctx.ServerError("UserSignIn", err)
143
139
return
+1
-5
routers/web/auth/auth.go
+1
-5
routers/web/auth/auth.go
···
784
784
785
785
func handleAccountActivation(ctx *context.Context, user *user_model.User) {
786
786
user.IsActive = true
787
-
var err error
788
-
if user.Rands, err = user_model.GetUserSalt(); err != nil {
789
-
ctx.ServerError("UpdateUser", err)
790
-
return
791
-
}
787
+
user.Rands = user_model.GetUserSalt()
792
788
if err := user_model.UpdateUserCols(ctx, user, "is_active", "rands"); err != nil {
793
789
if user_model.IsErrUserNotExist(err) {
794
790
ctx.NotFound("UpdateUserCols", err)
+2
-6
routers/web/auth/password.go
+2
-6
routers/web/auth/password.go
···
242
242
243
243
if regenerateScratchToken {
244
244
// Invalidate the scratch token.
245
-
_, err := twofa.GenerateScratchToken()
246
-
if err != nil {
247
-
ctx.ServerError("UserSignIn", err)
248
-
return
249
-
}
250
-
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
245
+
twofa.GenerateScratchToken()
246
+
if err := auth.UpdateTwoFactor(ctx, twofa); err != nil {
251
247
ctx.ServerError("UserSignIn", err)
252
248
return
253
249
}
+2
-10
routers/web/user/setting/security/2fa.go
+2
-10
routers/web/user/setting/security/2fa.go
···
40
40
return
41
41
}
42
42
43
-
token, err := t.GenerateScratchToken()
44
-
if err != nil {
45
-
ctx.ServerError("SettingsTwoFactor: Failed to GenerateScratchToken", err)
46
-
return
47
-
}
43
+
token := t.GenerateScratchToken()
48
44
49
45
if err = auth.UpdateTwoFactor(ctx, t); err != nil {
50
46
ctx.ServerError("SettingsTwoFactor: Failed to UpdateTwoFactor", err)
···
220
216
t = &auth.TwoFactor{
221
217
UID: ctx.Doer.ID,
222
218
}
223
-
token, err := t.GenerateScratchToken()
224
-
if err != nil {
225
-
ctx.ServerError("SettingsTwoFactor: Failed to generate scratch token", err)
226
-
return
227
-
}
219
+
token := t.GenerateScratchToken()
228
220
229
221
// Now we have to delete the secrets - because if we fail to insert then it's highly likely that they have already been used
230
222
// If we can detect the unique constraint failure below we can move this to after the NewTwoFactor
+2
-6
tests/e2e/utils_e2e_test.go
+2
-6
tests/e2e/utils_e2e_test.go
···
5
5
6
6
import (
7
7
"context"
8
-
"crypto/rand"
9
8
"encoding/hex"
10
9
"fmt"
11
10
"net"
···
23
22
"forgejo.org/modules/json"
24
23
modules_session "forgejo.org/modules/session"
25
24
"forgejo.org/modules/setting"
25
+
"forgejo.org/modules/util"
26
26
"forgejo.org/tests"
27
27
28
28
"code.forgejo.org/go-chi/session"
···
153
153
require.NoError(t, err)
154
154
155
155
return func(stateFile string, user *user_model.User) {
156
-
buf := make([]byte, opt.IDLength/2)
157
-
_, err = rand.Read(buf)
158
-
require.NoError(t, err)
159
-
160
-
sessionID := hex.EncodeToString(buf)
156
+
sessionID := hex.EncodeToString(util.CryptoRandomBytes(int64(opt.IDLength) / 2))
161
157
162
158
s, err := vsp.Read(sessionID)
163
159
require.NoError(t, err)
+2
-2
tests/integration/api_packages_test.go
+2
-2
tests/integration/api_packages_test.go
···
486
486
defer tests.PrintCurrentTest(t)()
487
487
488
488
// Upload and delete a generic package and upload a container blob
489
-
data, _ := util.CryptoRandomBytes(5)
489
+
data := util.CryptoRandomBytes(5)
490
490
url := fmt.Sprintf("/api/packages/%s/generic/cleanup-test/1.1.1/file.bin", user.Name)
491
491
req := NewRequestWithBody(t, "PUT", url, bytes.NewReader(data)).
492
492
AddBasicAuth(user.Name)
···
496
496
AddBasicAuth(user.Name)
497
497
MakeRequest(t, req, http.StatusNoContent)
498
498
499
-
data, _ = util.CryptoRandomBytes(5)
499
+
data = util.CryptoRandomBytes(5)
500
500
url = fmt.Sprintf("/v2/%s/cleanup-test/blobs/uploads?digest=sha256:%x", user.Name, sha256.Sum256(data))
501
501
req = NewRequestWithBody(t, "POST", url, bytes.NewReader(data)).
502
502
AddBasicAuth(user.Name)