Skip to content

Commit

Permalink
Use CryptoRandomBytes instead of CryptoRandomString (go-gitea#18439)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
Gusted committed Feb 4, 2022
1 parent 88939a5 commit aa23f47
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 28 deletions.
2 changes: 1 addition & 1 deletion integrations/api_oauth2_apps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
22 changes: 18 additions & 4 deletions models/auth/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 0 additions & 12 deletions modules/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 0 additions & 11 deletions modules/secret/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit aa23f47

Please sign in to comment.