From 5a871f6095a5aee88336a5128bd0e0f1477474a6 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Wed, 28 Aug 2024 08:23:48 +0200
Subject: [PATCH] [SEC] Ensure propagation of API scopes for Conan and
 Container authentication

- The Conan and Container packages use a different type of
authentication. It first authenticates via the regular way (api tokens
or user:password, handled via `auth.Basic`) and then generates a JWT
token that is used by the package software (such as Docker) to do the
action they wanted to do. This JWT token didn't properly propagate the
API scopes that the token was generated for, and thus could lead to a
'scope escalation' within the Conan and Container packages, read
access to write access.
- Store the API scope in the JWT token, so it can be propagated on
subsequent calls that uses that JWT token.
- Integration test added.
- Resolves #5128
---
 release-notes/5149.md                         |  1 +
 routers/api/packages/conan/auth.go            |  8 +-
 routers/api/packages/conan/conan.go           |  6 +-
 routers/api/packages/container/auth.go        |  8 +-
 routers/api/packages/container/container.go   |  6 +-
 services/packages/auth.go                     | 17 ++--
 tests/integration/api_packages_conan_test.go  | 79 ++++++++++++++++++-
 .../api_packages_container_test.go            | 38 +++++++++
 8 files changed, 151 insertions(+), 12 deletions(-)
 create mode 100644 release-notes/5149.md

diff --git a/release-notes/5149.md b/release-notes/5149.md
new file mode 100644
index 0000000000..1f508d282c
--- /dev/null
+++ b/release-notes/5149.md
@@ -0,0 +1 @@
+The scope of application tokens is not verified when writing containers or Conan packages. This is of no consequence when the user associated with the application token does not have write access to packages. If the user has write access to packages, such a token can be used to write containers and Conan packages.
diff --git a/routers/api/packages/conan/auth.go b/routers/api/packages/conan/auth.go
index 521fa12372..e2e1901b08 100644
--- a/routers/api/packages/conan/auth.go
+++ b/routers/api/packages/conan/auth.go
@@ -22,7 +22,7 @@ func (a *Auth) Name() string {
 
 // Verify extracts the user from the Bearer token
 func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
-	uid, err := packages.ParseAuthorizationToken(req)
+	uid, scope, err := packages.ParseAuthorizationToken(req)
 	if err != nil {
 		log.Trace("ParseAuthorizationToken: %v", err)
 		return nil, err
@@ -32,6 +32,12 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 		return nil, nil
 	}
 
+	// Propagate scope of the authorization token.
+	if scope != "" {
+		store.GetData()["IsApiToken"] = true
+		store.GetData()["ApiTokenScope"] = scope
+	}
+
 	u, err := user_model.GetUserByID(req.Context(), uid)
 	if err != nil {
 		log.Error("GetUserByID:  %v", err)
diff --git a/routers/api/packages/conan/conan.go b/routers/api/packages/conan/conan.go
index 07ea3eda34..e07907a8b1 100644
--- a/routers/api/packages/conan/conan.go
+++ b/routers/api/packages/conan/conan.go
@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	packages_model "code.gitea.io/gitea/models/packages"
 	conan_model "code.gitea.io/gitea/models/packages/conan"
@@ -117,7 +118,10 @@ func Authenticate(ctx *context.Context) {
 		return
 	}
 
-	token, err := packages_service.CreateAuthorizationToken(ctx.Doer)
+	// If there's an API scope, ensure it propagates.
+	scope, _ := ctx.Data.GetData()["ApiTokenScope"].(auth_model.AccessTokenScope)
+
+	token, err := packages_service.CreateAuthorizationToken(ctx.Doer, scope)
 	if err != nil {
 		apiError(ctx, http.StatusInternalServerError, err)
 		return
diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go
index 1c7afa95ff..a8b3ec117a 100644
--- a/routers/api/packages/container/auth.go
+++ b/routers/api/packages/container/auth.go
@@ -23,7 +23,7 @@ func (a *Auth) Name() string {
 // Verify extracts the user from the Bearer token
 // If it's an anonymous session a ghost user is returned
 func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
-	uid, err := packages.ParseAuthorizationToken(req)
+	uid, scope, err := packages.ParseAuthorizationToken(req)
 	if err != nil {
 		log.Trace("ParseAuthorizationToken: %v", err)
 		return nil, err
@@ -33,6 +33,12 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 		return nil, nil
 	}
 
+	// Propagate scope of the authorization token.
+	if scope != "" {
+		store.GetData()["IsApiToken"] = true
+		store.GetData()["ApiTokenScope"] = scope
+	}
+
 	u, err := user_model.GetPossibleUserByID(req.Context(), uid)
 	if err != nil {
 		log.Error("GetPossibleUserByID:  %v", err)
diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index 2cb16daebc..f376e7bc59 100644
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -14,6 +14,7 @@ import (
 	"strconv"
 	"strings"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	packages_model "code.gitea.io/gitea/models/packages"
 	container_model "code.gitea.io/gitea/models/packages/container"
 	user_model "code.gitea.io/gitea/models/user"
@@ -154,7 +155,10 @@ func Authenticate(ctx *context.Context) {
 		u = user_model.NewGhostUser()
 	}
 
-	token, err := packages_service.CreateAuthorizationToken(u)
+	// If there's an API scope, ensure it propagates.
+	scope, _ := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
+
+	token, err := packages_service.CreateAuthorizationToken(u, scope)
 	if err != nil {
 		apiError(ctx, http.StatusInternalServerError, err)
 		return
diff --git a/services/packages/auth.go b/services/packages/auth.go
index 8263c28bed..c5bf5af532 100644
--- a/services/packages/auth.go
+++ b/services/packages/auth.go
@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -19,9 +20,10 @@ import (
 type packageClaims struct {
 	jwt.RegisteredClaims
 	UserID int64
+	Scope  auth_model.AccessTokenScope
 }
 
-func CreateAuthorizationToken(u *user_model.User) (string, error) {
+func CreateAuthorizationToken(u *user_model.User, scope auth_model.AccessTokenScope) (string, error) {
 	now := time.Now()
 
 	claims := packageClaims{
@@ -30,6 +32,7 @@ func CreateAuthorizationToken(u *user_model.User) (string, error) {
 			NotBefore: jwt.NewNumericDate(now),
 		},
 		UserID: u.ID,
+		Scope:  scope,
 	}
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
 
@@ -41,16 +44,16 @@ func CreateAuthorizationToken(u *user_model.User) (string, error) {
 	return tokenString, nil
 }
 
-func ParseAuthorizationToken(req *http.Request) (int64, error) {
+func ParseAuthorizationToken(req *http.Request) (int64, auth_model.AccessTokenScope, error) {
 	h := req.Header.Get("Authorization")
 	if h == "" {
-		return 0, nil
+		return 0, "", nil
 	}
 
 	parts := strings.SplitN(h, " ", 2)
 	if len(parts) != 2 {
 		log.Error("split token failed: %s", h)
-		return 0, fmt.Errorf("split token failed")
+		return 0, "", fmt.Errorf("split token failed")
 	}
 
 	token, err := jwt.ParseWithClaims(parts[1], &packageClaims{}, func(t *jwt.Token) (any, error) {
@@ -60,13 +63,13 @@ func ParseAuthorizationToken(req *http.Request) (int64, error) {
 		return setting.GetGeneralTokenSigningSecret(), nil
 	})
 	if err != nil {
-		return 0, err
+		return 0, "", err
 	}
 
 	c, ok := token.Claims.(*packageClaims)
 	if !token.Valid || !ok {
-		return 0, fmt.Errorf("invalid token claim")
+		return 0, "", fmt.Errorf("invalid token claim")
 	}
 
-	return c.UserID, nil
+	return c.UserID, c.Scope, nil
 }
diff --git a/tests/integration/api_packages_conan_test.go b/tests/integration/api_packages_conan_test.go
index 003867441b..9d8f435068 100644
--- a/tests/integration/api_packages_conan_test.go
+++ b/tests/integration/api_packages_conan_test.go
@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/packages"
 	conan_model "code.gitea.io/gitea/models/packages/conan"
@@ -224,6 +225,45 @@ func TestPackageConan(t *testing.T) {
 			assert.Equal(t, "revisions", resp.Header().Get("X-Conan-Server-Capabilities"))
 		})
 
+		t.Run("Token Scope Authentication", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			session := loginUser(t, user.Name)
+
+			testCase := func(t *testing.T, scope auth_model.AccessTokenScope, expectedStatusCode int) {
+				t.Helper()
+
+				token := getTokenForLoggedInUser(t, session, scope)
+
+				req := NewRequest(t, "GET", fmt.Sprintf("%s/v1/users/authenticate", url)).
+					AddTokenAuth(token)
+				resp := MakeRequest(t, req, http.StatusOK)
+
+				body := resp.Body.String()
+				assert.NotEmpty(t, body)
+
+				recipeURL := fmt.Sprintf("%s/v1/conans/%s/%s/%s/%s", url, "TestScope", version1, "testing", channel1)
+
+				req = NewRequestWithJSON(t, "POST", fmt.Sprintf("%s/upload_urls", recipeURL), map[string]int64{
+					conanfileName: 64,
+					"removed.txt": 0,
+				}).AddTokenAuth(token)
+				MakeRequest(t, req, expectedStatusCode)
+			}
+
+			t.Run("Read permission", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+
+				testCase(t, auth_model.AccessTokenScopeReadPackage, http.StatusUnauthorized)
+			})
+
+			t.Run("Write permission", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+
+				testCase(t, auth_model.AccessTokenScopeWritePackage, http.StatusOK)
+			})
+		})
+
 		token := ""
 
 		t.Run("Authenticate", func(t *testing.T) {
@@ -481,6 +521,43 @@ func TestPackageConan(t *testing.T) {
 
 		token := ""
 
+		t.Run("Token Scope Authentication", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			session := loginUser(t, user.Name)
+
+			testCase := func(t *testing.T, scope auth_model.AccessTokenScope, expectedStatusCode int) {
+				t.Helper()
+
+				token := getTokenForLoggedInUser(t, session, scope)
+
+				req := NewRequest(t, "GET", fmt.Sprintf("%s/v2/users/authenticate", url)).
+					AddTokenAuth(token)
+				resp := MakeRequest(t, req, http.StatusOK)
+
+				body := resp.Body.String()
+				assert.NotEmpty(t, body)
+
+				recipeURL := fmt.Sprintf("%s/v2/conans/%s/%s/%s/%s/revisions/%s", url, "TestScope", version1, "testing", channel1, revision1)
+
+				req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/files/%s", recipeURL, conanfileName), strings.NewReader("Doesn't need to be valid")).
+					AddTokenAuth("Bearer " + body)
+				MakeRequest(t, req, expectedStatusCode)
+			}
+
+			t.Run("Read permission", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+
+				testCase(t, auth_model.AccessTokenScopeReadPackage, http.StatusUnauthorized)
+			})
+
+			t.Run("Write permission", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+
+				testCase(t, auth_model.AccessTokenScopeWritePackage, http.StatusCreated)
+			})
+		})
+
 		t.Run("Authenticate", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 
@@ -512,7 +589,7 @@ func TestPackageConan(t *testing.T) {
 
 				pvs, err := packages.GetVersionsByPackageType(db.DefaultContext, user.ID, packages.TypeConan)
 				require.NoError(t, err)
-				assert.Len(t, pvs, 2)
+				assert.Len(t, pvs, 3)
 			})
 		})
 
diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go
index 1197408830..3c28f45660 100644
--- a/tests/integration/api_packages_container_test.go
+++ b/tests/integration/api_packages_container_test.go
@@ -78,6 +78,7 @@ func TestPackageContainer(t *testing.T) {
 	indexManifestContent := `{"schemaVersion":2,"mediaType":"` + oci.MediaTypeImageIndex + `","manifests":[{"mediaType":"application/vnd.docker.distribution.manifest.v2+json","digest":"` + manifestDigest + `","platform":{"os":"linux","architecture":"arm","variant":"v7"}},{"mediaType":"` + oci.MediaTypeImageManifest + `","digest":"` + untaggedManifestDigest + `","platform":{"os":"linux","architecture":"arm64","variant":"v8"}}]}`
 
 	anonymousToken := ""
+	readUserToken := ""
 	userToken := ""
 
 	t.Run("Authenticate", func(t *testing.T) {
@@ -140,6 +141,30 @@ func TestPackageContainer(t *testing.T) {
 			req = NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL)).
 				AddTokenAuth(userToken)
 			MakeRequest(t, req, http.StatusOK)
+
+			// Token that should enforce the read scope.
+			t.Run("Read scope", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+
+				session := loginUser(t, user.Name)
+				token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage)
+
+				req := NewRequest(t, "GET", fmt.Sprintf("%sv2/token", setting.AppURL))
+				req.SetBasicAuth(user.Name, token)
+
+				resp := MakeRequest(t, req, http.StatusOK)
+
+				tokenResponse := &TokenResponse{}
+				DecodeJSON(t, resp, &tokenResponse)
+
+				assert.NotEmpty(t, tokenResponse.Token)
+
+				readUserToken = fmt.Sprintf("Bearer %s", tokenResponse.Token)
+
+				req = NewRequest(t, "GET", fmt.Sprintf("%sv2", setting.AppURL)).
+					AddTokenAuth(readUserToken)
+				MakeRequest(t, req, http.StatusOK)
+			})
 		})
 	})
 
@@ -163,6 +188,10 @@ func TestPackageContainer(t *testing.T) {
 					AddTokenAuth(anonymousToken)
 				MakeRequest(t, req, http.StatusUnauthorized)
 
+				req = NewRequest(t, "POST", fmt.Sprintf("%s/blobs/uploads", url)).
+					AddTokenAuth(readUserToken)
+				MakeRequest(t, req, http.StatusUnauthorized)
+
 				req = NewRequestWithBody(t, "POST", fmt.Sprintf("%s/blobs/uploads?digest=%s", url, unknownDigest), bytes.NewReader(blobContent)).
 					AddTokenAuth(userToken)
 				MakeRequest(t, req, http.StatusBadRequest)
@@ -318,6 +347,11 @@ func TestPackageContainer(t *testing.T) {
 							SetHeader("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
 						MakeRequest(t, req, http.StatusUnauthorized)
 
+						req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/%s", url, tag), strings.NewReader(manifestContent)).
+							AddTokenAuth(readUserToken).
+							SetHeader("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
+						MakeRequest(t, req, http.StatusUnauthorized)
+
 						req = NewRequestWithBody(t, "PUT", fmt.Sprintf("%s/manifests/%s", url, tag), strings.NewReader(manifestContent)).
 							AddTokenAuth(userToken).
 							SetHeader("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
@@ -521,6 +555,10 @@ func TestPackageContainer(t *testing.T) {
 				req = NewRequest(t, "HEAD", fmt.Sprintf("%s/blobs/%s", url, blobDigest)).
 					AddTokenAuth(anonymousToken)
 				MakeRequest(t, req, http.StatusOK)
+
+				req = NewRequest(t, "HEAD", fmt.Sprintf("%s/blobs/%s", url, blobDigest)).
+					AddTokenAuth(readUserToken)
+				MakeRequest(t, req, http.StatusOK)
 			})
 
 			t.Run("GetBlob", func(t *testing.T) {