Refactor internal routers (partial backport, auth token const time comparing) (#32473) (#32479)

Partially backport #32473. LFS related changes are not in 1.22, so skip
them.

1. Ignore non-existing repos during migrations
2. Improve ReadBatchLine's comment
3. Use `X-Gitea-Internal-Auth` header for internal API calls and make
the comparing constant time (it wasn't a serous problem because in a
real world it's nearly impossible to timing-attack the token, but indeed
security related and good to fix and backport)
4. Fix route mock nil check
This commit is contained in:
wxiaoguang 2024-11-13 10:26:37 +08:00 committed by GitHub
parent 26437a03b0
commit ef339713c2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 28 additions and 15 deletions

View File

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
giturl "code.gitea.io/gitea/modules/git/url" giturl "code.gitea.io/gitea/modules/git/url"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
"xorm.io/xorm" "xorm.io/xorm"
) )
@ -163,7 +164,9 @@ func migratePushMirrors(x *xorm.Engine) error {
func getRemoteAddress(ownerName, repoName, remoteName string) (string, error) { func getRemoteAddress(ownerName, repoName, remoteName string) (string, error) {
repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git") repoPath := filepath.Join(setting.RepoRootPath, strings.ToLower(ownerName), strings.ToLower(repoName)+".git")
if exist, _ := util.IsExist(repoPath); !exist {
return "", nil
}
remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName) remoteURL, err := git.GetRemoteAddress(context.Background(), repoPath, remoteName)
if err != nil { if err != nil {
return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err) return "", fmt.Errorf("get remote %s's address of %s/%s failed: %v", remoteName, ownerName, repoName, err)

View File

@ -146,9 +146,8 @@ func catFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi
} }
// ReadBatchLine reads the header line from cat-file --batch // ReadBatchLine reads the header line from cat-file --batch
// We expect: // We expect: <oid> SP <type> SP <size> LF
// <sha> SP <type> SP <size> LF // then leaving the rest of the stream "<contents> LF" to be read
// sha is a hex encoded here
func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err error) { func ReadBatchLine(rd *bufio.Reader) (sha []byte, typ string, size int64, err error) {
typ, err = rd.ReadString('\n') typ, err = rd.ReadString('\n')
if err != nil { if err != nil {

View File

@ -43,7 +43,7 @@ Ensure you are running in the correct environment or set the correct configurati
req := httplib.NewRequest(url, method). req := httplib.NewRequest(url, method).
SetContext(ctx). SetContext(ctx).
Header("X-Real-IP", getClientIP()). Header("X-Real-IP", getClientIP()).
Header("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken)). Header("X-Gitea-Internal-Auth", fmt.Sprintf("Bearer %s", setting.InternalToken)).
SetTLSClientConfig(&tls.Config{ SetTLSClientConfig(&tls.Config{
InsecureSkipVerify: true, InsecureSkipVerify: true,
ServerName: setting.Domain, ServerName: setting.Domain,

View File

@ -5,6 +5,7 @@ package web
import ( import (
"net/http" "net/http"
"reflect"
"strings" "strings"
"code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/modules/web/middleware"
@ -80,15 +81,23 @@ func (r *Route) getPattern(pattern string) string {
return strings.TrimSuffix(newPattern, "/") return strings.TrimSuffix(newPattern, "/")
} }
func isNilOrFuncNil(v any) bool {
if v == nil {
return true
}
r := reflect.ValueOf(v)
return r.Kind() == reflect.Func && r.IsNil()
}
func (r *Route) wrapMiddlewareAndHandler(h []any) ([]func(http.Handler) http.Handler, http.HandlerFunc) { func (r *Route) wrapMiddlewareAndHandler(h []any) ([]func(http.Handler) http.Handler, http.HandlerFunc) {
handlerProviders := make([]func(http.Handler) http.Handler, 0, len(r.curMiddlewares)+len(h)+1) handlerProviders := make([]func(http.Handler) http.Handler, 0, len(r.curMiddlewares)+len(h)+1)
for _, m := range r.curMiddlewares { for _, m := range r.curMiddlewares {
if m != nil { if !isNilOrFuncNil(m) {
handlerProviders = append(handlerProviders, toHandlerProvider(m)) handlerProviders = append(handlerProviders, toHandlerProvider(m))
} }
} }
for _, m := range h { for _, m := range h {
if h != nil { if !isNilOrFuncNil(m) {
handlerProviders = append(handlerProviders, toHandlerProvider(m)) handlerProviders = append(handlerProviders, toHandlerProvider(m))
} }
} }

View File

@ -5,6 +5,7 @@
package private package private
import ( import (
"crypto/subtle"
"net/http" "net/http"
"strings" "strings"
@ -18,22 +19,23 @@ import (
chi_middleware "github.com/go-chi/chi/v5/middleware" chi_middleware "github.com/go-chi/chi/v5/middleware"
) )
// CheckInternalToken check internal token is set func authInternal(next http.Handler) http.Handler {
func CheckInternalToken(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
tokens := req.Header.Get("Authorization")
fields := strings.SplitN(tokens, " ", 2)
if setting.InternalToken == "" { if setting.InternalToken == "" {
log.Warn(`The INTERNAL_TOKEN setting is missing from the configuration file: %q, internal API can't work.`, setting.CustomConf) log.Warn(`The INTERNAL_TOKEN setting is missing from the configuration file: %q, internal API can't work.`, setting.CustomConf)
http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
return return
} }
if len(fields) != 2 || fields[0] != "Bearer" || fields[1] != setting.InternalToken {
tokens := req.Header.Get("X-Gitea-Internal-Auth") // TODO: use something like JWT or HMAC to avoid passing the token in the clear
after, found := strings.CutPrefix(tokens, "Bearer ")
authSucceeded := found && subtle.ConstantTimeCompare([]byte(after), []byte(setting.InternalToken)) == 1
if !authSucceeded {
log.Debug("Forbidden attempt to access internal url: Authorization header: %s", tokens) log.Debug("Forbidden attempt to access internal url: Authorization header: %s", tokens)
http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
} else { return
next.ServeHTTP(w, req)
} }
next.ServeHTTP(w, req)
}) })
} }
@ -51,7 +53,7 @@ func bind[T any](_ T) any {
func Routes() *web.Route { func Routes() *web.Route {
r := web.NewRoute() r := web.NewRoute()
r.Use(context.PrivateContexter()) r.Use(context.PrivateContexter())
r.Use(CheckInternalToken) r.Use(authInternal)
// Log the real ip address of the request from SSH is really helpful for diagnosing sometimes. // Log the real ip address of the request from SSH is really helpful for diagnosing sometimes.
// Since internal API will be sent only from Gitea sub commands and it's under control (checked by InternalToken), we can trust the headers. // Since internal API will be sent only from Gitea sub commands and it's under control (checked by InternalToken), we can trust the headers.
r.Use(chi_middleware.RealIP) r.Use(chi_middleware.RealIP)