Skip to content

Commit d2fa8e9

Browse files
GustedStelios Malathouras
Gusted
authored and
Stelios Malathouras
committed
Use CryptoRandomBytes instead of CryptoRandomString (go-gitea#18439)
- Switch to use `CryptoRandomBytes` instead of `CryptoRandomString`, OAuth's secrets are copied pasted and don't need to avoid dubious characters etc. - `CryptoRandomBytes` gives ![2^256 = 1.15 * 10^77](https://render.githubusercontent.com/render/math?math=2^256%20=%201.15%20\cdot%2010^77) `CryptoRandomString` gives ![62^44 = 7.33 * 10^78](https://render.githubusercontent.com/render/math?math=62^44%20=%207.33%20\cdot%2010^78) possible states. - Add a prefix, such that code scanners can easily grep these in source code. - 32 Bytes + prefix
1 parent 4a6f4d9 commit d2fa8e9

File tree

4 files changed

+19
-28
lines changed

4 files changed

+19
-28
lines changed

integrations/api_oauth2_apps_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func testAPICreateOAuth2Application(t *testing.T) {
4343
DecodeJSON(t, resp, &createdApp)
4444

4545
assert.EqualValues(t, appBody.Name, createdApp.Name)
46-
assert.Len(t, createdApp.ClientSecret, 44)
46+
assert.Len(t, createdApp.ClientSecret, 56)
4747
assert.Len(t, createdApp.ClientID, 36)
4848
assert.NotEmpty(t, createdApp.Created)
4949
assert.EqualValues(t, appBody.RedirectURIs[0], createdApp.RedirectURIs[0])

models/auth/oauth2.go

+18-4
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ package auth
66

77
import (
88
"crypto/sha256"
9+
"encoding/base32"
910
"encoding/base64"
1011
"fmt"
1112
"net/url"
1213
"strings"
1314

1415
"code.gitea.io/gitea/models/db"
15-
"code.gitea.io/gitea/modules/secret"
1616
"code.gitea.io/gitea/modules/timeutil"
1717
"code.gitea.io/gitea/modules/util"
1818

@@ -57,12 +57,22 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool {
5757
return util.IsStringInSlice(redirectURI, app.RedirectURIs, true)
5858
}
5959

60+
// Base32 characters, but lowercased.
61+
const lowerBase32Chars = "abcdefghijklmnopqrstuvwxyz234567"
62+
63+
// base32 encoder that uses lowered characters without padding.
64+
var base32Lower = base32.NewEncoding(lowerBase32Chars).WithPadding(base32.NoPadding)
65+
6066
// GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database
6167
func (app *OAuth2Application) GenerateClientSecret() (string, error) {
62-
clientSecret, err := secret.New()
68+
rBytes, err := util.CryptoRandomBytes(32)
6369
if err != nil {
6470
return "", err
6571
}
72+
// Add a prefix to the base32, this is in order to make it easier
73+
// for code scanners to grab sensitive tokens.
74+
clientSecret := "gto_" + base32Lower.EncodeToString(rBytes)
75+
6676
hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost)
6777
if err != nil {
6878
return "", err
@@ -394,10 +404,14 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(redirectURI, codeChalleng
394404
}
395405

396406
func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) {
397-
var codeSecret string
398-
if codeSecret, err = secret.New(); err != nil {
407+
rBytes, err := util.CryptoRandomBytes(32)
408+
if err != nil {
399409
return &OAuth2AuthorizationCode{}, err
400410
}
411+
// Add a prefix to the base32, this is in order to make it easier
412+
// for code scanners to grab sensitive tokens.
413+
codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)
414+
401415
code = &OAuth2AuthorizationCode{
402416
Grant: grant,
403417
GrantID: grant.ID,

modules/secret/secret.go

-12
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,8 @@ import (
1313
"encoding/hex"
1414
"errors"
1515
"io"
16-
17-
"code.gitea.io/gitea/modules/util"
1816
)
1917

20-
// New creates a new secret
21-
func New() (string, error) {
22-
return NewWithLength(44)
23-
}
24-
25-
// NewWithLength creates a new secret for a given length
26-
func NewWithLength(length int64) (string, error) {
27-
return util.CryptoRandomString(length)
28-
}
29-
3018
// AesEncrypt encrypts text and given key with AES.
3119
func AesEncrypt(key, text []byte) ([]byte, error) {
3220
block, err := aes.NewCipher(key)

modules/secret/secret_test.go

-11
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,6 @@ import (
1010
"github.com/stretchr/testify/assert"
1111
)
1212

13-
func TestNew(t *testing.T) {
14-
result, err := New()
15-
assert.NoError(t, err)
16-
assert.True(t, len(result) == 44)
17-
18-
result2, err := New()
19-
assert.NoError(t, err)
20-
// check if secrets
21-
assert.NotEqual(t, result, result2)
22-
}
23-
2413
func TestEncryptDecrypt(t *testing.T) {
2514
var hex string
2615
var str string

0 commit comments

Comments
 (0)