Skip to content

Commit 511298e

Browse files
wxiaoguangwxiaoguang
authored
Use general token signing secret (#29205) (#29325)
Backport #29205 (including #29172) Use a clearly defined "signing secret" for token signing.
1 parent 7ea2ffa commit 511298e

File tree

13 files changed

+130
-70
lines changed

13 files changed

+130
-70
lines changed

‎cmd/generate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func runGenerateInternalToken(c *cli.Context) error {
7070
}
7171

7272
func runGenerateLfsJwtSecret(c *cli.Context) error {
73-
_, jwtSecretBase64, err := generate.NewJwtSecretBase64()
73+
_, jwtSecretBase64, err := generate.NewJwtSecretWithBase64()
7474
if err != nil {
7575
return err
7676
}

‎modules/base/tool.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func CreateTimeLimitCode(data string, minutes int, startInf any) string {
129129

130130
// create sha1 encode string
131131
sh := sha1.New()
132-
_, _ = sh.Write([]byte(fmt.Sprintf("%s%s%s%s%d", data, setting.SecretKey, startStr, endStr, minutes)))
132+
_, _ = sh.Write([]byte(fmt.Sprintf("%s%s%s%s%d", data, hex.EncodeToString(setting.GetGeneralTokenSigningSecret()), startStr, endStr, minutes)))
133133
encoded := hex.EncodeToString(sh.Sum(nil))
134134

135135
code := fmt.Sprintf("%s%06d%s", startStr, minutes, encoded)

‎modules/context/context.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package context
66

77
import (
88
"context"
9+
"encoding/hex"
910
"html"
1011
"html/template"
1112
"io"
@@ -134,7 +135,7 @@ func NewWebContext(base *Base, render Render, session session.Store) *Context {
134135
func Contexter() func(next http.Handler) http.Handler {
135136
rnd := templates.HTMLRenderer()
136137
csrfOpts := CsrfOptions{
137-
Secret: setting.SecretKey,
138+
Secret: hex.EncodeToString(setting.GetGeneralTokenSigningSecret()),
138139
Cookie: setting.CSRFCookieName,
139140
SetCookie: true,
140141
Secure: setting.SessionConfig.Secure,

‎modules/generate/generate.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package generate
77
import (
88
"crypto/rand"
99
"encoding/base64"
10+
"fmt"
1011
"io"
1112
"time"
1213

@@ -38,19 +39,24 @@ func NewInternalToken() (string, error) {
3839
return internalToken, nil
3940
}
4041

41-
// NewJwtSecret generates a new value intended to be used for JWT secrets.
42-
func NewJwtSecret() ([]byte, error) {
43-
bytes := make([]byte, 32)
44-
_, err := io.ReadFull(rand.Reader, bytes)
45-
if err != nil {
42+
const defaultJwtSecretLen = 32
43+
44+
// DecodeJwtSecretBase64 decodes a base64 encoded jwt secret into bytes, and check its length
45+
func DecodeJwtSecretBase64(src string) ([]byte, error) {
46+
encoding := base64.RawURLEncoding
47+
decoded := make([]byte, encoding.DecodedLen(len(src))+3)
48+
if n, err := encoding.Decode(decoded, []byte(src)); err != nil {
4649
return nil, err
50+
} else if n != defaultJwtSecretLen {
51+
return nil, fmt.Errorf("invalid base64 decoded length: %d, expects: %d", n, defaultJwtSecretLen)
4752
}
48-
return bytes, nil
53+
return decoded[:defaultJwtSecretLen], nil
4954
}
5055

51-
// NewJwtSecretBase64 generates a new base64 encoded value intended to be used for JWT secrets.
52-
func NewJwtSecretBase64() ([]byte, string, error) {
53-
bytes, err := NewJwtSecret()
56+
// NewJwtSecretWithBase64 generates a jwt secret with its base64 encoded value intended to be used for saving into config file
57+
func NewJwtSecretWithBase64() ([]byte, string, error) {
58+
bytes := make([]byte, defaultJwtSecretLen)
59+
_, err := io.ReadFull(rand.Reader, bytes)
5460
if err != nil {
5561
return nil, "", err
5662
}

‎modules/generate/generate_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package generate
5+
6+
import (
7+
"encoding/base64"
8+
"strings"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestDecodeJwtSecretBase64(t *testing.T) {
15+
_, err := DecodeJwtSecretBase64("abcd")
16+
assert.ErrorContains(t, err, "invalid base64 decoded length")
17+
_, err = DecodeJwtSecretBase64(strings.Repeat("a", 64))
18+
assert.ErrorContains(t, err, "invalid base64 decoded length")
19+
20+
str32 := strings.Repeat("x", 32)
21+
encoded32 := base64.RawURLEncoding.EncodeToString([]byte(str32))
22+
decoded32, err := DecodeJwtSecretBase64(encoded32)
23+
assert.NoError(t, err)
24+
assert.Equal(t, str32, string(decoded32))
25+
}
26+
27+
func TestNewJwtSecretWithBase64(t *testing.T) {
28+
secret, encoded, err := NewJwtSecretWithBase64()
29+
assert.NoError(t, err)
30+
assert.Len(t, secret, 32)
31+
decoded, err := DecodeJwtSecretBase64(encoded)
32+
assert.NoError(t, err)
33+
assert.Equal(t, secret, decoded)
34+
}

‎modules/setting/lfs.go

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,19 @@
44
package setting
55

66
import (
7-
"encoding/base64"
87
"fmt"
98
"time"
109

1110
"code.gitea.io/gitea/modules/generate"
12-
"code.gitea.io/gitea/modules/util"
1311
)
1412

1513
// LFS represents the configuration for Git LFS
1614
var LFS = struct {
17-
StartServer bool `ini:"LFS_START_SERVER"`
18-
JWTSecretBase64 string `ini:"LFS_JWT_SECRET"`
19-
JWTSecretBytes []byte `ini:"-"`
20-
HTTPAuthExpiry time.Duration `ini:"LFS_HTTP_AUTH_EXPIRY"`
21-
MaxFileSize int64 `ini:"LFS_MAX_FILE_SIZE"`
22-
LocksPagingNum int `ini:"LFS_LOCKS_PAGING_NUM"`
15+
StartServer bool `ini:"LFS_START_SERVER"`
16+
JWTSecretBytes []byte `ini:"-"`
17+
HTTPAuthExpiry time.Duration `ini:"LFS_HTTP_AUTH_EXPIRY"`
18+
MaxFileSize int64 `ini:"LFS_MAX_FILE_SIZE"`
19+
LocksPagingNum int `ini:"LFS_LOCKS_PAGING_NUM"`
2320

2421
Storage *Storage
2522
}{}
@@ -61,10 +58,10 @@ func loadLFSFrom(rootCfg ConfigProvider) error {
6158
return nil
6259
}
6360

64-
LFS.JWTSecretBase64 = loadSecret(rootCfg.Section("server"), "LFS_JWT_SECRET_URI", "LFS_JWT_SECRET")
65-
LFS.JWTSecretBytes, err = util.Base64FixedDecode(base64.RawURLEncoding, []byte(LFS.JWTSecretBase64), 32)
61+
jwtSecretBase64 := loadSecret(rootCfg.Section("server"), "LFS_JWT_SECRET_URI", "LFS_JWT_SECRET")
62+
LFS.JWTSecretBytes, err = generate.DecodeJwtSecretBase64(jwtSecretBase64)
6663
if err != nil {
67-
LFS.JWTSecretBytes, LFS.JWTSecretBase64, err = generate.NewJwtSecretBase64()
64+
LFS.JWTSecretBytes, jwtSecretBase64, err = generate.NewJwtSecretWithBase64()
6865
if err != nil {
6966
return fmt.Errorf("error generating JWT Secret for custom config: %v", err)
7067
}
@@ -74,8 +71,8 @@ func loadLFSFrom(rootCfg ConfigProvider) error {
7471
if err != nil {
7572
return fmt.Errorf("error saving JWT Secret for custom config: %v", err)
7673
}
77-
rootCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64)
78-
saveCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(LFS.JWTSecretBase64)
74+
rootCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(jwtSecretBase64)
75+
saveCfg.Section("server").Key("LFS_JWT_SECRET").SetValue(jwtSecretBase64)
7976
if err := saveCfg.Save(); err != nil {
8077
return fmt.Errorf("error saving JWT Secret for custom config: %v", err)
8178
}

‎modules/setting/oauth2.go

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@
44
package setting
55

66
import (
7-
"encoding/base64"
87
"math"
98
"path/filepath"
9+
"sync/atomic"
1010

1111
"code.gitea.io/gitea/modules/generate"
1212
"code.gitea.io/gitea/modules/log"
13-
"code.gitea.io/gitea/modules/util"
1413
)
1514

1615
// OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data
@@ -98,7 +97,6 @@ var OAuth2 = struct {
9897
RefreshTokenExpirationTime int64
9998
InvalidateRefreshTokens bool
10099
JWTSigningAlgorithm string `ini:"JWT_SIGNING_ALGORITHM"`
101-
JWTSecretBase64 string `ini:"JWT_SECRET"`
102100
JWTSigningPrivateKeyFile string `ini:"JWT_SIGNING_PRIVATE_KEY_FILE"`
103101
MaxTokenLength int
104102
DefaultApplications []string
@@ -123,29 +121,50 @@ func loadOAuth2From(rootCfg ConfigProvider) {
123121
return
124122
}
125123

126-
OAuth2.JWTSecretBase64 = loadSecret(rootCfg.Section("oauth2"), "JWT_SECRET_URI", "JWT_SECRET")
124+
jwtSecretBase64 := loadSecret(rootCfg.Section("oauth2"), "JWT_SECRET_URI", "JWT_SECRET")
127125

128126
if !filepath.IsAbs(OAuth2.JWTSigningPrivateKeyFile) {
129127
OAuth2.JWTSigningPrivateKeyFile = filepath.Join(AppDataPath, OAuth2.JWTSigningPrivateKeyFile)
130128
}
131129

132130
if InstallLock {
133-
if _, err := util.Base64FixedDecode(base64.RawURLEncoding, []byte(OAuth2.JWTSecretBase64), 32); err != nil {
134-
key, err := generate.NewJwtSecret()
131+
jwtSecretBytes, err := generate.DecodeJwtSecretBase64(jwtSecretBase64)
132+
if err != nil {
133+
jwtSecretBytes, jwtSecretBase64, err = generate.NewJwtSecretWithBase64()
135134
if err != nil {
136135
log.Fatal("error generating JWT secret: %v", err)
137136
}
138-
139-
OAuth2.JWTSecretBase64 = base64.RawURLEncoding.EncodeToString(key)
140137
saveCfg, err := rootCfg.PrepareSaving()
141138
if err != nil {
142139
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
143140
}
144-
rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(OAuth2.JWTSecretBase64)
145-
saveCfg.Section("oauth2").Key("JWT_SECRET").SetValue(OAuth2.JWTSecretBase64)
141+
rootCfg.Section("oauth2").Key("JWT_SECRET").SetValue(jwtSecretBase64)
142+
saveCfg.Section("oauth2").Key("JWT_SECRET").SetValue(jwtSecretBase64)
146143
if err := saveCfg.Save(); err != nil {
147144
log.Fatal("save oauth2.JWT_SECRET failed: %v", err)
148145
}
149146
}
147+
generalSigningSecret.Store(&jwtSecretBytes)
148+
}
149+
}
150+
151+
// generalSigningSecret is used as container for a []byte value
152+
// instead of an additional mutex, we use CompareAndSwap func to change the value thread save
153+
var generalSigningSecret atomic.Pointer[[]byte]
154+
155+
func GetGeneralTokenSigningSecret() []byte {
156+
old := generalSigningSecret.Load()
157+
if old == nil || len(*old) == 0 {
158+
jwtSecret, _, err := generate.NewJwtSecretWithBase64()
159+
if err != nil {
160+
log.Fatal("Unable to generate general JWT secret: %s", err.Error())
161+
}
162+
if generalSigningSecret.CompareAndSwap(old, &jwtSecret) {
163+
// FIXME: in main branch, the signing token should be refactored (eg: one unique for LFS/OAuth2/etc ...)
164+
log.Warn("OAuth2 is not enabled, unable to use a persistent signing secret, a new one is generated, which is not persistent between restarts and cluster nodes")
165+
return jwtSecret
166+
}
167+
return *generalSigningSecret.Load()
150168
}
169+
return *old
151170
}

‎modules/setting/oauth2_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package setting
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/modules/generate"
10+
"code.gitea.io/gitea/modules/test"
11+
12+
"github.com/stretchr/testify/assert"
13+
)
14+
15+
func TestGetGeneralSigningSecret(t *testing.T) {
16+
// when there is no general signing secret, it should be generated, and keep the same value
17+
assert.Nil(t, generalSigningSecret.Load())
18+
s1 := GetGeneralTokenSigningSecret()
19+
assert.NotNil(t, s1)
20+
s2 := GetGeneralTokenSigningSecret()
21+
assert.Equal(t, s1, s2)
22+
23+
// the config value should always override any pre-generated value
24+
cfg, _ := NewConfigProviderFromData(`
25+
[oauth2]
26+
JWT_SECRET = BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
27+
`)
28+
defer test.MockVariableValue(&InstallLock, true)()
29+
loadOAuth2From(cfg)
30+
actual := GetGeneralTokenSigningSecret()
31+
expected, _ := generate.DecodeJwtSecretBase64("BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB")
32+
assert.Len(t, actual, 32)
33+
assert.EqualValues(t, expected, actual)
34+
}

‎modules/util/util.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package util
66
import (
77
"bytes"
88
"crypto/rand"
9-
"encoding/base64"
109
"fmt"
1110
"math/big"
1211
"strconv"
@@ -246,13 +245,3 @@ func ToFloat64(number any) (float64, error) {
246245
func ToPointer[T any](val T) *T {
247246
return &val
248247
}
249-
250-
func Base64FixedDecode(encoding *base64.Encoding, src []byte, length int) ([]byte, error) {
251-
decoded := make([]byte, encoding.DecodedLen(len(src))+3)
252-
if n, err := encoding.Decode(decoded, src); err != nil {
253-
return nil, err
254-
} else if n != length {
255-
return nil, fmt.Errorf("invalid base64 decoded length: %d, expects: %d", n, length)
256-
}
257-
return decoded[:length], nil
258-
}

‎modules/util/util_test.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package util
55

66
import (
7-
"encoding/base64"
87
"regexp"
98
"strings"
109
"testing"
@@ -234,16 +233,3 @@ func TestToPointer(t *testing.T) {
234233
val123 := 123
235234
assert.False(t, &val123 == ToPointer(val123))
236235
}
237-
238-
func TestBase64FixedDecode(t *testing.T) {
239-
_, err := Base64FixedDecode(base64.RawURLEncoding, []byte("abcd"), 32)
240-
assert.ErrorContains(t, err, "invalid base64 decoded length")
241-
_, err = Base64FixedDecode(base64.RawURLEncoding, []byte(strings.Repeat("a", 64)), 32)
242-
assert.ErrorContains(t, err, "invalid base64 decoded length")
243-
244-
str32 := strings.Repeat("x", 32)
245-
encoded32 := base64.RawURLEncoding.EncodeToString([]byte(str32))
246-
decoded32, err := Base64FixedDecode(base64.RawURLEncoding, []byte(encoded32), 32)
247-
assert.NoError(t, err)
248-
assert.Equal(t, str32, string(decoded32))
249-
}

‎routers/install/install.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ func SubmitInstall(ctx *context.Context) {
407407
cfg.Section("server").Key("LFS_START_SERVER").SetValue("true")
408408
cfg.Section("lfs").Key("PATH").SetValue(form.LFSRootPath)
409409
var lfsJwtSecret string
410-
if _, lfsJwtSecret, err = generate.NewJwtSecretBase64(); err != nil {
410+
if _, lfsJwtSecret, err = generate.NewJwtSecretWithBase64(); err != nil {
411411
ctx.RenderWithErr(ctx.Tr("install.lfs_jwt_secret_failed", err), tplInstall, &form)
412412
return
413413
}

‎services/auth/source/oauth2/jwtsigningkey.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func InitSigningKey() error {
300300
case "HS384":
301301
fallthrough
302302
case "HS512":
303-
key, err = loadSymmetricKey()
303+
key = setting.GetGeneralTokenSigningSecret()
304304
case "RS256":
305305
fallthrough
306306
case "RS384":
@@ -333,12 +333,6 @@ func InitSigningKey() error {
333333
return nil
334334
}
335335

336-
// loadSymmetricKey checks if the configured secret is valid.
337-
// If it is not valid, it will return an error.
338-
func loadSymmetricKey() (any, error) {
339-
return util.Base64FixedDecode(base64.RawURLEncoding, []byte(setting.OAuth2.JWTSecretBase64), 32)
340-
}
341-
342336
// loadOrCreateAsymmetricKey checks if the configured private key exists.
343337
// If it does not exist a new random key gets generated and saved on the configured path.
344338
func loadOrCreateAsymmetricKey() (any, error) {

‎services/packages/auth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func CreateAuthorizationToken(u *user_model.User) (string, error) {
3333
}
3434
token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
3535

36-
tokenString, err := token.SignedString([]byte(setting.SecretKey))
36+
tokenString, err := token.SignedString(setting.GetGeneralTokenSigningSecret())
3737
if err != nil {
3838
return "", err
3939
}
@@ -57,7 +57,7 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) {
5757
if _, ok := t.Method.(*jwt.SigningMethodHMAC); !ok {
5858
return nil, fmt.Errorf("unexpected signing method: %v", t.Header["alg"])
5959
}
60-
return []byte(setting.SecretKey), nil
60+
return setting.GetGeneralTokenSigningSecret(), nil
6161
})
6262
if err != nil {
6363
return 0, err

0 commit comments

Comments
 (0)