From 2d3333ada21aca46cb5850a76ba1ccc1eec3f1a1 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 28 Jan 2022 04:38:59 +0100 Subject: [PATCH 1/8] Use `CryptoRandomBytes` instead of `CryptoRandomString` - Switch to use `CryptoRandomBytes` instead of `CryptoRandomString`, OAuth's secrets are copied pasted and don't need to avoid dubious characters etc. - `CryptoRandomBytes` gives 44^256 = 5.295 * 10^420 `CryptoRandomString` gives 44^62 = 7.83 * 10^101 possible states. --- models/auth/oauth2.go | 5 ++++- modules/secret/secret.go | 12 ------------ modules/secret/secret_test.go | 11 ----------- 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index e7030fce28981..a902fcffce889 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -7,6 +7,7 @@ package auth import ( "crypto/sha256" "encoding/base64" + "encoding/hex" "fmt" "net/url" "strings" @@ -59,10 +60,12 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { // 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(44) if err != nil { return "", err } + clientSecret := hex.EncodeToString(rBytes) + hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost) if err != nil { return "", err diff --git a/modules/secret/secret.go b/modules/secret/secret.go index 6b410f238197b..e7edc7a95e528 100644 --- 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 f3a88eecc825e..b1c99d851363c 100644 --- 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 From dfcc61e08e742a963bfc3bfcd6d618f2529886ca Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 28 Jan 2022 04:56:56 +0100 Subject: [PATCH 2/8] Fix last reference --- models/auth/oauth2.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index a902fcffce889..7b2247b82299b 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -13,7 +13,6 @@ import ( "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" @@ -397,10 +396,12 @@ 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(44) + if err != nil { return &OAuth2AuthorizationCode{}, err } + codeSecret := hex.EncodeToString(rBytes) + code = &OAuth2AuthorizationCode{ Grant: grant, GrantID: grant.ID, From fb6628578f811af1b604e83fc35574d2812cc2e9 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 28 Jan 2022 05:15:32 +0100 Subject: [PATCH 3/8] Use 32 Bytes + prefix --- models/auth/oauth2.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 7b2247b82299b..0fc4d5d89419a 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -59,11 +59,11 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { // GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database func (app *OAuth2Application) GenerateClientSecret() (string, error) { - rBytes, err := util.CryptoRandomBytes(44) + rBytes, err := util.CryptoRandomBytes(32) if err != nil { return "", err } - clientSecret := hex.EncodeToString(rBytes) + clientSecret := "gto_" + hex.EncodeToString(rBytes) hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost) if err != nil { @@ -396,11 +396,11 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(redirectURI, codeChalleng } func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) { - rBytes, err := util.CryptoRandomBytes(44) + rBytes, err := util.CryptoRandomBytes(32) if err != nil { return &OAuth2AuthorizationCode{}, err } - codeSecret := hex.EncodeToString(rBytes) + codeSecret := "gta_" + hex.EncodeToString(rBytes) code = &OAuth2AuthorizationCode{ Grant: grant, From 0c9321ea5c68d22ea709036edf7a0466905a3176 Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 28 Jan 2022 05:20:39 +0100 Subject: [PATCH 4/8] Use 34 bytes instead of 32 --- models/auth/oauth2.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 0fc4d5d89419a..80c3e425b4d2d 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -59,10 +59,12 @@ func (app *OAuth2Application) ContainsRedirectURI(redirectURI string) bool { // GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database func (app *OAuth2Application) GenerateClientSecret() (string, error) { - rBytes, err := util.CryptoRandomBytes(32) + rBytes, err := util.CryptoRandomBytes(34) if err != nil { return "", err } + // Add a prefix to the hexadecimal, this is in order to make it easier + // for code scanners to grab sensitive tokens. clientSecret := "gto_" + hex.EncodeToString(rBytes) hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost) @@ -396,10 +398,12 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(redirectURI, codeChalleng } func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) { - rBytes, err := util.CryptoRandomBytes(32) + rBytes, err := util.CryptoRandomBytes(34) if err != nil { return &OAuth2AuthorizationCode{}, err } + // Add a prefix to the hexadecimal, this is in order to make it easier + // for code scanners to grab sensitive tokens. codeSecret := "gta_" + hex.EncodeToString(rBytes) code = &OAuth2AuthorizationCode{ From 6eb83d20642cfba6c5daa107ab43336620b98d5f Mon Sep 17 00:00:00 2001 From: Gusted Date: Fri, 28 Jan 2022 11:28:09 +0100 Subject: [PATCH 5/8] Use base32 --- models/auth/oauth2.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index 80c3e425b4d2d..c9fed8a6501c0 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -6,8 +6,8 @@ package auth import ( "crypto/sha256" + "encoding/base32" "encoding/base64" - "encoding/hex" "fmt" "net/url" "strings" @@ -57,15 +57,21 @@ 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) { rBytes, err := util.CryptoRandomBytes(34) if err != nil { return "", err } - // Add a prefix to the hexadecimal, this is in order to make it easier + // Add a prefix to the base32, this is in order to make it easier // for code scanners to grab sensitive tokens. - clientSecret := "gto_" + hex.EncodeToString(rBytes) + clientSecret := "gto_" + base32Lower.EncodeToString(rBytes) hashedSecret, err := bcrypt.GenerateFromPassword([]byte(clientSecret), bcrypt.DefaultCost) if err != nil { @@ -402,9 +408,9 @@ func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, if err != nil { return &OAuth2AuthorizationCode{}, err } - // Add a prefix to the hexadecimal, this is in order to make it easier + // Add a prefix to the base32, this is in order to make it easier // for code scanners to grab sensitive tokens. - codeSecret := "gta_" + hex.EncodeToString(rBytes) + codeSecret := "gta_" + base32Lower.EncodeToString(rBytes) code = &OAuth2AuthorizationCode{ Grant: grant, From 65bc8eacbd9816c2b5ac9a5d1c6ee2380e5b7027 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 30 Jan 2022 13:05:49 +0100 Subject: [PATCH 6/8] Use 32 bytes --- models/auth/oauth2.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index c9fed8a6501c0..2341e0862025f 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -65,7 +65,7 @@ var base32Lower = base32.NewEncoding(lowerBase32Chars).WithPadding(base32.NoPadd // GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database func (app *OAuth2Application) GenerateClientSecret() (string, error) { - rBytes, err := util.CryptoRandomBytes(34) + rBytes, err := util.CryptoRandomBytes(32) if err != nil { return "", err } @@ -404,7 +404,7 @@ func (grant *OAuth2Grant) GenerateNewAuthorizationCode(redirectURI, codeChalleng } func (grant *OAuth2Grant) generateNewAuthorizationCode(e db.Engine, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) { - rBytes, err := util.CryptoRandomBytes(34) + rBytes, err := util.CryptoRandomBytes(32) if err != nil { return &OAuth2AuthorizationCode{}, err } From 1b5410e82894fb15ab7dd786feda7389630c5668 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 2 Feb 2022 18:59:21 +0100 Subject: [PATCH 7/8] Adjust tests --- integrations/api_oauth2_apps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_oauth2_apps_test.go b/integrations/api_oauth2_apps_test.go index e51549568a6a0..3f061e22bc340 100644 --- 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, 52) assert.Len(t, createdApp.ClientID, 36) assert.NotEmpty(t, createdApp.Created) assert.EqualValues(t, appBody.RedirectURIs[0], createdApp.RedirectURIs[0]) From ce00d9086ee6278c6fcf121058b25bb06328d1da Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 3 Feb 2022 09:41:46 +0100 Subject: [PATCH 8/8] fix --- integrations/api_oauth2_apps_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/api_oauth2_apps_test.go b/integrations/api_oauth2_apps_test.go index 3f061e22bc340..2c08338b25b92 100644 --- 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, 52) + 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])