mirror of
https://github.com/ClusterCockpit/cc-backend
synced 2026-06-06 03:37:29 +02:00
Fix issues after security audit
Entire-Checkpoint: bc18358a9343
This commit is contained in:
18
CLAUDE.md
18
CLAUDE.md
@@ -314,6 +314,24 @@ job,function=stop_job event="{\"jobId\":123,\"cluster\":\"test\",\"startTime\":1
|
|||||||
- Messages are logged; no responses are sent back to publishers
|
- Messages are logged; no responses are sent back to publishers
|
||||||
- If NATS client is unavailable, API subscriptions are skipped (logged as warning)
|
- If NATS client is unavailable, API subscriptions are skipped (logged as warning)
|
||||||
|
|
||||||
|
### Security Considerations
|
||||||
|
|
||||||
|
**The NATS API has no application-layer authentication or authorization.** Unlike
|
||||||
|
the REST endpoints (which require a JWT with `RoleAPI`), the subscribers process
|
||||||
|
any message delivered on the configured subjects. Anyone with publish rights to
|
||||||
|
those subjects on the broker can:
|
||||||
|
|
||||||
|
- Insert arbitrary jobs (potentially attributed to other users)
|
||||||
|
- Mark running jobs as stopped, triggering archive/finalization
|
||||||
|
- Overwrite node state and health metadata for any cluster
|
||||||
|
|
||||||
|
Operators MUST restrict publish ACLs at the NATS broker (per-account or
|
||||||
|
per-subject permissions) so that only trusted producers — e.g. the scheduler
|
||||||
|
integration on a known host or service account — can publish to the configured
|
||||||
|
`subject-job-event` and `subject-node-state` subjects. A shared, unrestricted
|
||||||
|
NATS broker is not a safe deployment topology for this API. A startup warning
|
||||||
|
is logged when these subscriptions are enabled.
|
||||||
|
|
||||||
## Development Guidelines
|
## Development Guidelines
|
||||||
|
|
||||||
### Performance
|
### Performance
|
||||||
|
|||||||
@@ -129,11 +129,19 @@ func (s *Server) init() error {
|
|||||||
s.router.Use(middleware.Compress(5))
|
s.router.Use(middleware.Compress(5))
|
||||||
s.router.Use(middleware.Recoverer)
|
s.router.Use(middleware.Recoverer)
|
||||||
s.router.Use(cors.Handler(cors.Options{
|
s.router.Use(cors.Handler(cors.Options{
|
||||||
AllowCredentials: true,
|
AllowCredentials: false,
|
||||||
AllowedHeaders: []string{"X-Requested-With", "Content-Type", "Authorization", "Origin"},
|
AllowedHeaders: []string{"X-Requested-With", "Content-Type", "Authorization", "Origin"},
|
||||||
AllowedMethods: []string{"GET", "POST", "HEAD", "OPTIONS"},
|
AllowedMethods: []string{"GET", "POST", "HEAD", "OPTIONS"},
|
||||||
AllowedOrigins: []string{"*"},
|
AllowedOrigins: []string{"*"},
|
||||||
}))
|
}))
|
||||||
|
s.router.Use(func(next http.Handler) http.Handler {
|
||||||
|
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.TLS != nil || r.Header.Get("X-Forwarded-Proto") == "https" {
|
||||||
|
rw.Header().Set("Strict-Transport-Security", "max-age=31536000; includeSubDomains")
|
||||||
|
}
|
||||||
|
next.ServeHTTP(rw, r)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
s.restAPIHandle = api.New()
|
s.restAPIHandle = api.New()
|
||||||
|
|
||||||
|
|||||||
@@ -170,6 +170,7 @@ func setup(t *testing.T) *api.RestAPI {
|
|||||||
|
|
||||||
archiver.Start(repository.GetJobRepository(), context.Background())
|
archiver.Start(repository.GetJobRepository(), context.Background())
|
||||||
|
|
||||||
|
t.Setenv("SESSION_KEY", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
|
||||||
if cfg := ccconf.GetPackageConfig("auth"); cfg != nil {
|
if cfg := ccconf.GetPackageConfig("auth"); cfg != nil {
|
||||||
auth.Init(&cfg)
|
auth.Init(&cfg)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -151,7 +151,10 @@ func (api *NatsAPI) StartSubscriptions() error {
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
cclog.Info("NATS API subscriptions started")
|
cclog.Warnf("NATS API subscriptions started on subjects %q and %q — these are UNAUTHENTICATED: "+
|
||||||
|
"anyone with publish rights on the broker can start/stop jobs and update node state. "+
|
||||||
|
"Restrict publish ACLs on the NATS broker to trusted producers only.",
|
||||||
|
s.SubjectJobEvent, s.SubjectNodeState)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -156,6 +156,7 @@ func setupNatsTest(t *testing.T) *NatsAPI {
|
|||||||
|
|
||||||
archiver.Start(repository.GetJobRepository(), context.Background())
|
archiver.Start(repository.GetJobRepository(), context.Background())
|
||||||
|
|
||||||
|
t.Setenv("SESSION_KEY", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=")
|
||||||
if cfg := ccconf.GetPackageConfig("auth"); cfg != nil {
|
if cfg := ccconf.GetPackageConfig("auth"); cfg != nil {
|
||||||
auth.Init(&cfg)
|
auth.Init(&cfg)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -9,7 +9,6 @@ package auth
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"crypto/rand"
|
|
||||||
"database/sql"
|
"database/sql"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
@@ -17,6 +16,7 @@ import (
|
|||||||
"fmt"
|
"fmt"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
@@ -187,19 +187,15 @@ func Init(authCfg *json.RawMessage) {
|
|||||||
|
|
||||||
sessKey := os.Getenv("SESSION_KEY")
|
sessKey := os.Getenv("SESSION_KEY")
|
||||||
if sessKey == "" {
|
if sessKey == "" {
|
||||||
cclog.Warn("environment variable 'SESSION_KEY' not set (will use non-persistent random key)")
|
cclog.Fatal("environment variable 'SESSION_KEY' not set: refusing to start with an ephemeral session key. " +
|
||||||
bytes := make([]byte, 32)
|
"Set SESSION_KEY in .env (base64-encoded 32 random bytes); a random key would invalidate all sessions on every restart " +
|
||||||
if _, err := rand.Read(bytes); err != nil {
|
"and prevent sessions from validating across replicas.")
|
||||||
cclog.Fatal("Error while initializing authentication -> failed to generate random bytes for session key")
|
|
||||||
}
|
}
|
||||||
authInstance.sessionStore = sessions.NewCookieStore(bytes)
|
keyBytes, err := base64.StdEncoding.DecodeString(sessKey)
|
||||||
} else {
|
|
||||||
bytes, err := base64.StdEncoding.DecodeString(sessKey)
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
cclog.Fatal("Error while initializing authentication -> decoding session key failed")
|
cclog.Fatal("Error while initializing authentication -> decoding session key failed")
|
||||||
}
|
}
|
||||||
authInstance.sessionStore = sessions.NewCookieStore(bytes)
|
authInstance.sessionStore = sessions.NewCookieStore(keyBytes)
|
||||||
}
|
|
||||||
|
|
||||||
if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err == nil {
|
if d, err := time.ParseDuration(config.Keys.SessionMaxAge); err == nil {
|
||||||
authInstance.SessionMaxAge = d
|
authInstance.SessionMaxAge = d
|
||||||
@@ -325,6 +321,7 @@ func (auth *Authentication) SaveSession(rw http.ResponseWriter, r *http.Request,
|
|||||||
session.Options.Secure = false
|
session.Options.Secure = false
|
||||||
}
|
}
|
||||||
session.Options.SameSite = http.SameSiteLaxMode
|
session.Options.SameSite = http.SameSiteLaxMode
|
||||||
|
session.Options.HttpOnly = true
|
||||||
session.Values["username"] = user.Username
|
session.Values["username"] = user.Username
|
||||||
session.Values["projects"] = user.Projects
|
session.Values["projects"] = user.Projects
|
||||||
session.Values["roles"] = user.Roles
|
session.Values["roles"] = user.Roles
|
||||||
@@ -388,10 +385,13 @@ func (auth *Authentication) Login(
|
|||||||
cclog.Infof("login successfull: user: %#v (roles: %v, projects: %v)", user.Username, user.Roles, user.Projects)
|
cclog.Infof("login successfull: user: %#v (roles: %v, projects: %v)", user.Username, user.Roles, user.Projects)
|
||||||
ctx := context.WithValue(r.Context(), repository.ContextUserKey, user)
|
ctx := context.WithValue(r.Context(), repository.ContextUserKey, user)
|
||||||
|
|
||||||
if r.FormValue("redirect") != "" {
|
if redirect := r.FormValue("redirect"); redirect != "" {
|
||||||
http.RedirectHandler(r.FormValue("redirect"), http.StatusFound).ServeHTTP(rw, r.WithContext(ctx))
|
if u, perr := url.Parse(redirect); perr == nil && u.Scheme == "" && u.Host == "" {
|
||||||
|
http.RedirectHandler(redirect, http.StatusFound).ServeHTTP(rw, r.WithContext(ctx))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
cclog.Warnf("login redirect rejected (not a relative path): %q", redirect)
|
||||||
|
}
|
||||||
|
|
||||||
http.RedirectHandler("/", http.StatusFound).ServeHTTP(rw, r.WithContext(ctx))
|
http.RedirectHandler("/", http.StatusFound).ServeHTTP(rw, r.WithContext(ctx))
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
"crypto/ed25519"
|
"crypto/ed25519"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"os"
|
"os"
|
||||||
|
|
||||||
@@ -119,22 +120,26 @@ func (ja *JWTCookieSessionAuthenticator) Login(
|
|||||||
rawtoken = jwtCookie.Value
|
rawtoken = jwtCookie.Value
|
||||||
}
|
}
|
||||||
|
|
||||||
token, err := jwt.Parse(rawtoken, func(t *jwt.Token) (any, error) {
|
parser := jwt.NewParser(jwt.WithValidMethods([]string{jwt.SigningMethodEdDSA.Alg()}))
|
||||||
if t.Method != jwt.SigningMethodEdDSA {
|
|
||||||
return nil, errors.New("only Ed25519/EdDSA supported")
|
unverified, _, perr := parser.ParseUnverified(rawtoken, jwt.MapClaims{})
|
||||||
|
if perr != nil {
|
||||||
|
cclog.Warn("JWT cookie session: error while parsing token")
|
||||||
|
return nil, perr
|
||||||
|
}
|
||||||
|
issuer, _ := unverified.Claims.(jwt.MapClaims)["iss"].(string)
|
||||||
|
|
||||||
|
var key any
|
||||||
|
switch issuer {
|
||||||
|
case jc.TrustedIssuer:
|
||||||
|
key = ja.publicKeyCrossLogin
|
||||||
|
case "":
|
||||||
|
key = ja.publicKey
|
||||||
|
default:
|
||||||
|
return nil, fmt.Errorf("untrusted JWT issuer: %q", issuer)
|
||||||
}
|
}
|
||||||
|
|
||||||
unvalidatedIssuer, success := t.Claims.(jwt.MapClaims)["iss"].(string)
|
token, err := parser.Parse(rawtoken, func(*jwt.Token) (any, error) { return key, nil })
|
||||||
if success && unvalidatedIssuer == jc.TrustedIssuer {
|
|
||||||
// The (unvalidated) issuer seems to be the expected one,
|
|
||||||
// use public cross login key from config
|
|
||||||
return ja.publicKeyCrossLogin, nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// No cross login key configured or issuer not expected
|
|
||||||
// Try own key
|
|
||||||
return ja.publicKey, nil
|
|
||||||
})
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
cclog.Warn("JWT cookie session: error while parsing token")
|
cclog.Warn("JWT cookie session: error while parsing token")
|
||||||
return nil, err
|
return nil, err
|
||||||
|
|||||||
Reference in New Issue
Block a user