From 6f8caad35cd9d25c6e4bbfa42df4905b69ff2718 Mon Sep 17 00:00:00 2001 From: Tommaso Bailetti Date: Thu, 10 Sep 2026 17:06:04 +0200 Subject: [PATCH] feat: added controller attribution --- packages/ns-api-server/README.md | 30 ++++ .../ns-api-server/files/src/methods/auth.go | 11 ++ .../files/src/middleware/middleware.go | 99 +++++++++--- .../files/src/middleware/middleware_test.go | 3 +- .../files/src/middleware/on_behalf_of_test.go | 147 ++++++++++++++++++ .../ns-api-server/files/src/models/auth.go | 1 + packages/ns-api-server/files/src/sudo/sudo.go | 2 + 7 files changed, 269 insertions(+), 24 deletions(-) create mode 100644 packages/ns-api-server/files/src/middleware/on_behalf_of_test.go diff --git a/packages/ns-api-server/README.md b/packages/ns-api-server/README.md index cf0e4fede..444596054 100644 --- a/packages/ns-api-server/README.md +++ b/packages/ns-api-server/README.md @@ -4,6 +4,36 @@ NS API server, see [source code](https://github.com/NethServer/nethsecurity-api) The server is configured to listen on `127.0.0.1:8090`. +## Controller attribution + +Units managed by a controller are accessed with a single machine account, created by +`ns-plug` as the `rpcd.controller` UCI section with a random username. Without further +information, every action performed from the controller would be logged under that name. + +To report the real operator, `POST /login` accepts an optional `on_behalf_of` field: + +```json +{ "username": "", "password": "...", "on_behalf_of": "alice" } +``` + +The field is accepted only when the authenticating user is the account named by +`uci get rpcd.controller.username`; for any other user it is silently ignored. It is also +ignored when empty, longer than 64 characters, or containing control characters. + +When accepted, the value is stored in the `on_behalf_of` JWT claim and reported on the +`[AUTH]` log lines, right before the client IP: + +``` +[INFO][AUTH] authentication success for user 3f2a1b0c9d8e on behalf of alice from 10.0.0.5 +[INFO][AUTH] authorization success for user 3f2a1b0c9d8e on behalf of alice. POST /api/ubus/call {...} +``` + +The token identity (`id` claim) remains the machine account. The delegated identity is used +for logging only: it is not passed to the `/usr/libexec/rpcd/ns.*` handlers. + +Older units ignore the field, and older controllers do not send it: in both cases the login +succeeds and the logs report the machine account alone. + ## Rate limiting The server applies a generous global per-client-IP rate limit as a coarse safety net across diff --git a/packages/ns-api-server/files/src/methods/auth.go b/packages/ns-api-server/files/src/methods/auth.go index ed0935aba..d3ec0896d 100644 --- a/packages/ns-api-server/files/src/methods/auth.go +++ b/packages/ns-api-server/files/src/methods/auth.go @@ -36,6 +36,17 @@ import ( "github.com/NethServer/nethsecurity-api/utils" ) +// GetControllerUsername returns the rpcd account used by the controller. +func GetControllerUsername() string { + out, err := exec.Command("/sbin/uci", "-q", "get", "rpcd.controller.username").Output() + + if err != nil { + return "" + } + + return strings.TrimSpace(string(out)) +} + func CheckAuthentication(username string, password string) error { // define login object login := models.UserLogin{ diff --git a/packages/ns-api-server/files/src/middleware/middleware.go b/packages/ns-api-server/files/src/middleware/middleware.go index b221d6f66..5d0c8ce32 100644 --- a/packages/ns-api-server/files/src/middleware/middleware.go +++ b/packages/ns-api-server/files/src/middleware/middleware.go @@ -30,13 +30,58 @@ import ( ) type login struct { - Username string `form:"username" json:"username" binding:"required"` - Password string `form:"password" json:"password" binding:"required"` + Username string `form:"username" json:"username" binding:"required"` + Password string `form:"password" json:"password" binding:"required"` + OnBehalfOf string `form:"on_behalf_of" json:"on_behalf_of"` } var jwtMiddleware *jwt.GinJWTMiddleware var identityKey = "id" +const onBehalfOfKey = "on_behalf_of" +const onBehalfOfMaxLen = 64 + +// package variable so tests can stub the uci read +var getControllerUsername = methods.GetControllerUsername + +// checkOnBehalfOf return the user accessing the machine from the controller +// only in case it's the controller username to pass this info +func checkOnBehalfOf(onBehalfOf string, username string) string { + onBehalfOf = strings.TrimSpace(onBehalfOf) + if onBehalfOf == "" || len(onBehalfOf) > onBehalfOfMaxLen { + return "" + } + + // control characters would let a crafted name forge extra log lines + if strings.ContainsFunc(onBehalfOf, func(r rune) bool { return r < 0x20 || r == 0x7f }) { + return "" + } + + if onBehalfOf == username { + return "" + } + + controller := getControllerUsername() + if controller == "" || controller != username { + return "" + } + + return onBehalfOf +} + +// logSuffixOnBehalfOf logs which controller user is accessing the machine +func logSuffixOnBehalfOf(onBehalfOf string) string { + if onBehalfOf == "" { + return "" + } + return " on behalf of " + utils.SanitizeForLog(onBehalfOf) +} + +func claimsSuffixOnBehalfOf(claims jwt.MapClaims) string { + onBehalfOf, _ := claims[onBehalfOfKey].(string) + return logSuffixOnBehalfOf(onBehalfOf) +} + func InstanceJWT() *jwt.GinJWTMiddleware { if jwtMiddleware == nil { jwtMiddleware := InitJWT() @@ -74,12 +119,15 @@ func InitJWT() *jwt.GinJWTMiddleware { return nil, jwt.ErrFailedAuthentication } + onBehalfOf := checkOnBehalfOf(loginVals.OnBehalfOf, username) + // login ok action - logs.Logs.Println("[INFO][AUTH] authentication success for user " + utils.SanitizeForLog(username) + " from " + c.ClientIP()) + logs.Logs.Println("[INFO][AUTH] authentication success for user " + utils.SanitizeForLog(username) + logSuffixOnBehalfOf(onBehalfOf) + " from " + c.ClientIP()) // return user auth model return &models.UserAuthorizations{ - Username: username, + Username: username, + OnBehalfOf: onBehalfOf, }, nil }, @@ -89,22 +137,24 @@ func InitJWT() *jwt.GinJWTMiddleware { // check if user require 2fa status, _ := methods.GetUserStatus(user.Username) - if user.SudoRequested { - // create claims map - return jwt.MapClaims{ - identityKey: user.Username, - "role": "", - "actions": []string{}, - "2fa": status == "1", - "sudo": time.Now().Unix(), - } - } - return jwt.MapClaims{ + // create claims map + claims := jwt.MapClaims{ identityKey: user.Username, "role": "", "actions": []string{}, "2fa": status == "1", } + + if user.SudoRequested { + claims["sudo"] = time.Now().Unix() + } + + // only when set, so tokens of regular logins are unchanged + if user.OnBehalfOf != "" { + claims[onBehalfOfKey] = user.OnBehalfOf + } + + return claims } // return claims map @@ -114,11 +164,14 @@ func InitJWT() *jwt.GinJWTMiddleware { // handle identity and extract claims claims := jwt.ExtractClaims(c) + onBehalfOf, _ := claims[onBehalfOfKey].(string) + // create user object user := &models.UserAuthorizations{ - Username: claims[identityKey].(string), - Role: "admin", - Actions: nil, + Username: claims[identityKey].(string), + Role: "admin", + Actions: nil, + OnBehalfOf: onBehalfOf, } // return user @@ -136,7 +189,7 @@ func InitJWT() *jwt.GinJWTMiddleware { // check if token exists if !methods.CheckTokenValidation(claims["id"].(string), token.Raw) { // write logs - logs.Logs.Println("[INFO][AUTH] authorization failed for user " + utils.SanitizeForLog(claims["id"].(string)) + ". " + reqMethod + " " + reqURI) + logs.Logs.Println("[INFO][AUTH] authorization failed for user " + utils.SanitizeForLog(claims["id"].(string)) + claimsSuffixOnBehalfOf(claims) + ". " + reqMethod + " " + reqURI) // not authorized return false @@ -176,7 +229,7 @@ func InitJWT() *jwt.GinJWTMiddleware { reqBody = jsonB } - logs.Logs.Println("[INFO][AUTH] authorization success for user " + utils.SanitizeForLog(claims["id"].(string)) + ". " + reqMethod + " " + reqURI + " " + utils.SanitizeForLog(reqBody)) + logs.Logs.Println("[INFO][AUTH] authorization success for user " + utils.SanitizeForLog(claims["id"].(string)) + claimsSuffixOnBehalfOf(claims) + ". " + reqMethod + " " + reqURI + " " + utils.SanitizeForLog(reqBody)) // authorized return true @@ -192,7 +245,7 @@ func InitJWT() *jwt.GinJWTMiddleware { } // write logs - logs.Logs.Println("[INFO][AUTH] login response success for user " + utils.SanitizeForLog(claims["id"].(string))) + logs.Logs.Println("[INFO][AUTH] login response success for user " + utils.SanitizeForLog(claims["id"].(string)) + claimsSuffixOnBehalfOf(claims)) // return 200 OK c.JSON(200, gin.H{"code": 200, "expire": t, "token": token}) @@ -206,7 +259,7 @@ func InitJWT() *jwt.GinJWTMiddleware { methods.SetTokenValidation(claims["id"].(string), token) // write logs - logs.Logs.Println("[INFO][AUTH] refresh response success for user " + utils.SanitizeForLog(claims["id"].(string))) + logs.Logs.Println("[INFO][AUTH] refresh response success for user " + utils.SanitizeForLog(claims["id"].(string)) + claimsSuffixOnBehalfOf(claims)) // return 200 OK c.JSON(200, gin.H{"code": 200, "expire": t, "token": token}) @@ -220,7 +273,7 @@ func InitJWT() *jwt.GinJWTMiddleware { methods.DelTokenValidation(claims["id"].(string), tokenObj.Raw) // write logs - logs.Logs.Println("[INFO][AUTH] logout response success for user " + utils.SanitizeForLog(claims["id"].(string))) + logs.Logs.Println("[INFO][AUTH] logout response success for user " + utils.SanitizeForLog(claims["id"].(string)) + claimsSuffixOnBehalfOf(claims)) // reutrn 200 OK c.JSON(200, gin.H{"code": 200}) diff --git a/packages/ns-api-server/files/src/middleware/middleware_test.go b/packages/ns-api-server/files/src/middleware/middleware_test.go index 32983c5bd..1b08b95fb 100644 --- a/packages/ns-api-server/files/src/middleware/middleware_test.go +++ b/packages/ns-api-server/files/src/middleware/middleware_test.go @@ -64,7 +64,8 @@ func TestLoginLogInjection(t *testing.T) { r := gin.New() r.POST("/login", InstanceJWT().LoginHandler) - body, _ := json.Marshal(map[string]string{"username": username, "password": "y"}) + // on_behalf_of is attacker controlled too on this path + body, _ := json.Marshal(map[string]string{"username": username, "password": "y", "on_behalf_of": "evil from " + victimIP}) req := httptest.NewRequest(http.MethodPost, "/login", bytes.NewReader(body)) req.Header.Set("Content-Type", "application/json") req.RemoteAddr = clientIP + ":1234" diff --git a/packages/ns-api-server/files/src/middleware/on_behalf_of_test.go b/packages/ns-api-server/files/src/middleware/on_behalf_of_test.go new file mode 100644 index 000000000..e31c8f20a --- /dev/null +++ b/packages/ns-api-server/files/src/middleware/on_behalf_of_test.go @@ -0,0 +1,147 @@ +/* + * Copyright (C) 2026 Nethesis S.r.l. + * http://www.nethesis.it - info@nethesis.it + * + * SPDX-License-Identifier: GPL-2.0-only + */ + +package middleware + +import ( + "bytes" + "log" + "net/http/httptest" + "strings" + "testing" + "time" + + jwt "github.com/appleboy/gin-jwt/v2" + "github.com/gin-gonic/gin" + + "github.com/NethServer/nethsecurity-api/configuration" + "github.com/NethServer/nethsecurity-api/logs" + "github.com/NethServer/nethsecurity-api/models" +) + +const controllerUser = "3f2a1b0c9d8e7f6a5b4c3d2e" + +func stubControllerUsername(t *testing.T, username string) { + t.Helper() + original := getControllerUsername + getControllerUsername = func() string { return username } + t.Cleanup(func() { getControllerUsername = original }) +} + +// TestCheckOnBehalfOf: only the controller machine account can act on behalf of +// somebody else, and only with a sane value. +func TestCheckOnBehalfOf(t *testing.T) { + cases := []struct { + name string + controller string + username string + onBehalfOf string + expected string + }{ + {"controller delegates", controllerUser, controllerUser, "alice", "alice"}, + {"value is trimmed", controllerUser, controllerUser, " alice ", "alice"}, + {"another user cannot delegate", controllerUser, "root", "alice", ""}, + {"unregistered unit", "", "root", "alice", ""}, + {"no delegation requested", controllerUser, controllerUser, "", ""}, + {"delegation to itself", controllerUser, controllerUser, controllerUser, ""}, + {"value too long", controllerUser, controllerUser, strings.Repeat("a", onBehalfOfMaxLen+1), ""}, + {"forged log line", controllerUser, controllerUser, "alice\nauthentication failed for user evil from 8.8.8.8", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stubControllerUsername(t, tc.controller) + + got := checkOnBehalfOf(tc.onBehalfOf, tc.username) + if got != tc.expected { + t.Fatalf("expected %q, got %q", tc.expected, got) + } + }) + } +} + +// TestOnBehalfOfClaim: the claim is stored only when set, so tokens of regular +// logins are unchanged. +func TestOnBehalfOfClaim(t *testing.T) { + cases := map[string]string{ + "delegated login": "alice", + "regular login": "", + } + + for name, onBehalfOf := range cases { + t.Run(name, func(t *testing.T) { + configuration.Config.SecretJWT = "test-secret" + gin.SetMode(gin.TestMode) + jwtMiddleware = nil + + token, _, err := InstanceJWT().TokenGenerator(&models.UserAuthorizations{ + Username: controllerUser, + OnBehalfOf: onBehalfOf, + }) + if err != nil { + t.Fatalf("cannot generate token: %v", err) + } + + parsed, err := InstanceJWT().ParseTokenString(token) + if err != nil { + t.Fatalf("cannot parse token: %v", err) + } + claims := jwt.ExtractClaimsFromToken(parsed) + + if claims["id"] != controllerUser { + t.Fatalf("expected identity %q, got %q", controllerUser, claims["id"]) + } + + value, present := claims[onBehalfOfKey] + if onBehalfOf == "" { + if present { + t.Fatalf("expected no %s claim, got %q", onBehalfOfKey, value) + } + return + } + if value != onBehalfOf { + t.Fatalf("expected %s claim %q, got %q", onBehalfOfKey, onBehalfOf, value) + } + }) + } +} + +// TestOnBehalfOfLogSuffix: the operator is reported next to the machine +// account, and the client IP stays the last IPv4 so banIP bans the right source. +func TestOnBehalfOfLogSuffix(t *testing.T) { + const clientIP = "192.168.1.10" + + configuration.Config.SecretJWT = "test-secret" + configuration.Config.TokensDir = t.TempDir() + configuration.Config.SecretsDir = t.TempDir() + gin.SetMode(gin.TestMode) + jwtMiddleware = nil + + var buf bytes.Buffer + logs.Logs = log.New(&buf, "", 0) + + token, _, err := InstanceJWT().TokenGenerator(&models.UserAuthorizations{ + Username: controllerUser, + OnBehalfOf: "alice", + }) + if err != nil { + t.Fatalf("cannot generate token: %v", err) + } + + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + InstanceJWT().LoginResponse(c, 200, token, time.Now()) + + line := strings.TrimSpace(buf.String()) + if !strings.Contains(line, "for user "+controllerUser+" on behalf of alice") { + t.Fatalf("expected the operator next to the machine account, got %q", line) + } + + authLine := "[INFO][AUTH] authentication success for user " + controllerUser + logSuffixOnBehalfOf("alice") + " from " + clientIP + if lastIPv4(authLine) != clientIP { + t.Fatalf("expected client IP %s as source on line %q", clientIP, authLine) + } +} diff --git a/packages/ns-api-server/files/src/models/auth.go b/packages/ns-api-server/files/src/models/auth.go index e8cf64fef..6861f24af 100644 --- a/packages/ns-api-server/files/src/models/auth.go +++ b/packages/ns-api-server/files/src/models/auth.go @@ -14,6 +14,7 @@ type UserAuthorizations struct { Role string `json:"role" structs:"role"` Actions []string `json:"actions" structs:"actions"` SudoRequested bool `json:"sudo_requested" structs:"sudo_requested"` + OnBehalfOf string `json:"on_behalf_of" structs:"on_behalf_of"` } type OTPJson struct { diff --git a/packages/ns-api-server/files/src/sudo/sudo.go b/packages/ns-api-server/files/src/sudo/sudo.go index a506f411c..24ae5a40e 100644 --- a/packages/ns-api-server/files/src/sudo/sudo.go +++ b/packages/ns-api-server/files/src/sudo/sudo.go @@ -37,6 +37,7 @@ func EnableSudo(c *gin.Context) { claims := jwt.ExtractClaims(c) // Get username and 2FA status from claims username := claims["id"].(string) + onBehalfOf, _ := claims["on_behalf_of"].(string) // Check if password sent is valid var jsonRequest struct { @@ -73,6 +74,7 @@ func EnableSudo(c *gin.Context) { token, _, err := middleware.InstanceJWT().TokenGenerator(&models.UserAuthorizations{ Username: username, SudoRequested: true, + OnBehalfOf: onBehalfOf, }) if err != nil { c.JSON(http.StatusInternalServerError, structs.Map(response.StatusInternalServerError{