Use `CryptoRandomBytes` instead of `CryptoRandomString` (#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

diff --git a/integrations/api_oauth2_apps_test.go b/integrations/api_oauth2_apps_test.go
index e51549568a6a0b3d1c6198654b8227e6335ec6f9..2c08338b25b9298e680c40957bf34a060e15b7d7 100644 (file)
--- a/integrations/api_oauth2_apps_test.go
+++ b/integrations/api_oauth2_apps_test.go
@@ -43,7 +43,7 @@ func testAPICreateOAuth2Application(t *testing.T) {
        DecodeJSON(t, resp, &createdApp)
 
        assert.EqualValues(t, appBody.Name, createdApp.Name)
-       assert.Len(t, createdApp.ClientSecret, 44)
+       assert.Len(t, createdApp.ClientSecret, 56)
        assert.Len(t, createdApp.ClientID, 36)
        assert.NotEmpty(t, createdApp.Created)
        assert.EqualValues(t, appBody.RedirectURIs[0], createdApp.RedirectURIs[0])

diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go
index e7030fce28981975715de2df41ae1f9bcd78cfbc..2341e0862025f3187a92b0dee2dfc5b3e389d7ce 100644 (file)
--- a/models/auth/oauth2.go
+++ b/models/auth/oauth2.go
@@ -6,13 +6,13 @@ package auth
 
 import (
        "crypto/sha256"
+       "encoding/base32"
        "encoding/base64"
        "fmt"
        "net/url"
        "strings"
 
        "code.gitea.io/gitea/models/db"
-       "code.gitea.io/gitea/modules/secret"
        "code.gitea.io/gitea/modules/timeutil"
        "code.gitea.io/gitea/modules/util"
 
@@ -57,12 +57,22 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool {
        return util.IsStringInSlice(redirectURI, app.RedirectURIs, true)
 }
 
+// Base32 characters, but lowercased.
+const lowerBase32Chars = "abcdefghijklmnopqrstuvwxyz234567"
+
+// base32 encoder that uses lowered characters without padding.
+var base32Lower = base32.NewEncoding(lowerBase32Chars).WithPadding(base32.NoPadding)
+
 // GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database
 func (app *OAuth2Application) GenerateClientSecret() (string, error) {
-       clientSecret, err := secret.New()
+       rBytes, err := util.CryptoRandomBytes(32)
        if err != nil {
                return "", err
        }
+       // Add a prefix to the base32, this is in order to make it easier
+       // for code scanners to grab sensitive tokens.
+       clientSecret := "gto_" + base32Lower.EncodeToString(rBytes)
+
        hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost)
        if err != nil {
                return "", err
@@ -394,10 +404,14 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(redirectURI, codeChalleng
 }
 
 func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) {
-       var codeSecret string
-       if codeSecret, err = secret.New(); err != nil {
+       rBytes, err := util.CryptoRandomBytes(32)
+       if err != nil {
                return &OAuth2AuthorizationCode{}, err
        }
+       // Add a prefix to the base32, this is in order to make it easier
+       // for code scanners to grab sensitive tokens.
+       codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)
+
        code = &OAuth2AuthorizationCode{
                Grant:               grant,
                GrantID:             grant.ID,

diff --git a/modules/secret/secret.go b/modules/secret/secret.go
index 6b410f238197bdbf8d11f481eeaeefd98711c1a4..e7edc7a95e528e3c5b96942742c73c340f084f7e 100644 (file)
--- a/modules/secret/secret.go
+++ b/modules/secret/secret.go
@@ -13,20 +13,8 @@ import (
        "encoding/hex"
        "errors"
        "io"
-
-       "code.gitea.io/gitea/modules/util"
 )
 
-// New creates a new secret
-func New() (string, error) {
-       return NewWithLength(44)
-}
-
-// NewWithLength creates a new secret for a given length
-func NewWithLength(length int64) (string, error) {
-       return util.CryptoRandomString(length)
-}
-
 // AesEncrypt encrypts text and given key with AES.
 func AesEncrypt(key, text []byte) ([]byte, error) {
        block, err := aes.NewCipher(key)

diff --git a/modules/secret/secret_test.go b/modules/secret/secret_test.go
index f3a88eecc825e1ae5471cbc21ba6d6ed4a38bb52..b1c99d851363c06ea35feb4f544f16df09543ac6 100644 (file)
--- a/modules/secret/secret_test.go
+++ b/modules/secret/secret_test.go
@@ -10,17 +10,6 @@ import (
        "github.com/stretchr/testify/assert"
 )
 
-func TestNew(t *testing.T) {
-       result, err := New()
-       assert.NoError(t, err)
-       assert.True(t, len(result) == 44)
-
-       result2, err := New()
-       assert.NoError(t, err)
-       // check if secrets
-       assert.NotEqual(t, result, result2)
-}
-
 func TestEncryptDecrypt(t *testing.T) {
        var hex string
        var str string