Skip to content

Commit

Permalink
Add API Token Cache (#16547)
Browse files Browse the repository at this point in the history
One of the issues holding back performance of the API is the problem of hashing.
Whilst banning BASIC authentication with passwords will help, the API Token scheme
still requires a PBKDF2 hash - which means that heavy API use (using Tokens) can
still cause enormous numbers of hash computations.

A slight solution to this whilst we consider moving to using JWT based tokens and/or
a session orientated solution is to simply cache the successful tokens. This has some
security issues but this should be balanced by the security issues of load from
hashing.

Related #14668

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
  • Loading branch information
zeripath and lunny authored Aug 17, 2021
1 parent 274aeb3 commit e0853d4
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 1 deletion.
4 changes: 4 additions & 0 deletions custom/conf/app.example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,10 @@ INTERNAL_TOKEN=
;;
;; Validate against https://haveibeenpwned.com/Passwords to see if a password has been exposed
;PASSWORD_CHECK_PWN = false
;;
;; Cache successful token hashes. API tokens are stored in the DB as pbkdf2 hashes however, this means that there is a potentially significant hashing load when there are multiple API operations.
;; This cache will store the successfully hashed tokens in a LRU cache as a balance between performance and security.
;SUCCESSFUL_TOKENS_CACHE_SIZE = 20

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
Expand Down
1 change: 1 addition & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ relation to port exhaustion.
- spec - use one or more special characters as ``!"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~``
- off - do not check password complexity
- `PASSWORD_CHECK_PWN`: **false**: Check [HaveIBeenPwned](https://haveibeenpwned.com/Passwords) to see if a password has been exposed.
- `SUCCESSFUL_TOKENS_CACHE_SIZE`: **20**: Cache successful token hashes. API tokens are stored in the DB as pbkdf2 hashes however, this means that there is a potentially significant hashing load when there are multiple API operations. This cache will store the successfully hashed tokens in a LRU cache as a balance between performance and security.

## OpenID (`openid`)

Expand Down
10 changes: 10 additions & 0 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

// Needed for the MySQL driver
_ "github.com/go-sql-driver/mysql"
lru "github.com/hashicorp/golang-lru"
"xorm.io/xorm"
"xorm.io/xorm/names"
"xorm.io/xorm/schemas"
Expand Down Expand Up @@ -234,6 +235,15 @@ func NewEngine(ctx context.Context, migrateFunc func(*xorm.Engine) error) (err e
return fmt.Errorf("sync database struct error: %v", err)
}

if setting.SuccessfulTokensCacheSize > 0 {
successfulAccessTokenCache, err = lru.New(setting.SuccessfulTokensCacheSize)
if err != nil {
return fmt.Errorf("unable to allocate AccessToken cache: %v", err)
}
} else {
successfulAccessTokenCache = nil
}

return nil
}

Expand Down
41 changes: 40 additions & 1 deletion models/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ import (
"code.gitea.io/gitea/modules/util"

gouuid "github.com/google/uuid"
lru "github.com/hashicorp/golang-lru"
)

var successfulAccessTokenCache *lru.Cache

// AccessToken represents a personal access token.
type AccessToken struct {
ID int64 `xorm:"pk autoincr"`
Expand Down Expand Up @@ -52,6 +55,21 @@ func NewAccessToken(t *AccessToken) error {
return err
}

func getAccessTokenIDFromCache(token string) int64 {
if successfulAccessTokenCache == nil {
return 0
}
tInterface, ok := successfulAccessTokenCache.Get(token)
if !ok {
return 0
}
t, ok := tInterface.(int64)
if !ok {
return 0
}
return t
}

// GetAccessTokenBySHA returns access token by given token value
func GetAccessTokenBySHA(token string) (*AccessToken, error) {
if token == "" {
Expand All @@ -66,17 +84,38 @@ func GetAccessTokenBySHA(token string) (*AccessToken, error) {
return nil, ErrAccessTokenNotExist{token}
}
}
var tokens []AccessToken

lastEight := token[len(token)-8:]

if id := getAccessTokenIDFromCache(token); id > 0 {
token := &AccessToken{
TokenLastEight: lastEight,
}
// Re-get the token from the db in case it has been deleted in the intervening period
has, err := x.ID(id).Get(token)
if err != nil {
return nil, err
}
if has {
return token, nil
}
successfulAccessTokenCache.Remove(token)
}

var tokens []AccessToken
err := x.Table(&AccessToken{}).Where("token_last_eight = ?", lastEight).Find(&tokens)
if err != nil {
return nil, err
} else if len(tokens) == 0 {
return nil, ErrAccessTokenNotExist{token}
}

for _, t := range tokens {
tempHash := hashToken(token, t.TokenSalt)
if subtle.ConstantTimeCompare([]byte(t.TokenHash), []byte(tempHash)) == 1 {
if successfulAccessTokenCache != nil {
successfulAccessTokenCache.Add(token, t.ID)
}
return &t, nil
}
}
Expand Down
2 changes: 2 additions & 0 deletions modules/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ var (
PasswordComplexity []string
PasswordHashAlgo string
PasswordCheckPwn bool
SuccessfulTokensCacheSize int

// UI settings
UI = struct {
Expand Down Expand Up @@ -840,6 +841,7 @@ func NewContext() {
PasswordHashAlgo = sec.Key("PASSWORD_HASH_ALGO").MustString("pbkdf2")
CSRFCookieHTTPOnly = sec.Key("CSRF_COOKIE_HTTP_ONLY").MustBool(true)
PasswordCheckPwn = sec.Key("PASSWORD_CHECK_PWN").MustBool(false)
SuccessfulTokensCacheSize = sec.Key("SUCCESSFUL_TOKENS_CACHE_SIZE").MustInt(20)

InternalToken = loadInternalToken(sec)

Expand Down

0 comments on commit e0853d4

Please sign in to comment.