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  `CryptoRandomString` gives  possible states. - Add a prefix, such that code scanners can easily grep these in source code. - 32 Bytes + prefix

Changes

4 changed files (+19/-28)

• 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])
• 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,
• 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)
• 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