diff --git a/cmd/server_foreground.go b/cmd/server_foreground.go index bd667e528..7ce07bfff 100644 --- a/cmd/server_foreground.go +++ b/cmd/server_foreground.go @@ -859,6 +859,17 @@ func parseAdminEmails(cfg *config.GlobalConfig) []string { return adminEmailList } +// resolveSessionSecret resolves the deployment-wide session secret from the +// --session-secret flag, falling back to the SCION_SERVER_SESSION_SECRET env +// var. The same value backs both the web session cookie store and the hub JWT +// signing keys so that all replicas behind the load balancer agree. +func resolveSessionSecret() string { + if webSessionSecret != "" { + return webSessionSecret + } + return os.Getenv("SCION_SERVER_SESSION_SECRET") +} + // initHubServer creates and configures the Hub server. func initHubServer(ctx context.Context, cfg *config.GlobalConfig, s store.Store, hubEndpoint, devAuthToken string, adminEmailList []string, adminMode bool, maintenanceMessage string, requestLogger, messageLogger *slog.Logger, globalDir string, pluginMgr *scionplugin.Manager, secretBackend secret.SecretBackend) (*hub.Server, error) { hubCfg := hub.ServerConfig{ @@ -929,6 +940,12 @@ func initHubServer(ctx context.Context, cfg *config.GlobalConfig, s store.Store, MaintenanceConfig: resolveMaintenanceConfig(cfg), SecretBackend: secretBackend, GCPProjectID: cfg.Hub.GCPProjectID, + // Derive the agent/user JWT signing keys from the same shared session + // secret the web cookie store uses, so every replica behind the load + // balancer agrees on the signing key regardless of its host-derived + // HubID. Without this, a JWT minted by one replica fails validation on + // another (cross-replica "session_expired" login loop). + SharedSigningSecret: resolveSessionSecret(), } hubSrv, err := hub.New(hubCfg, s) @@ -1123,10 +1140,7 @@ func initWebServer(ctx context.Context, cfg *config.GlobalConfig, hubSrv *hub.Se } // Allow env var overrides for session/OAuth config - sessionSecret := webSessionSecret - if sessionSecret == "" { - sessionSecret = os.Getenv("SCION_SERVER_SESSION_SECRET") - } + sessionSecret := resolveSessionSecret() baseURL := webBaseURL if baseURL == "" { baseURL = os.Getenv("SCION_SERVER_BASE_URL") diff --git a/pkg/hub/web_test.go b/pkg/hub/web_test.go index 2853400da..5dafa97eb 100644 --- a/pkg/hub/web_test.go +++ b/pkg/hub/web_test.go @@ -27,7 +27,6 @@ import ( "time" "github.com/GoogleCloudPlatform/scion/pkg/store" - "github.com/gorilla/securecookie" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -1288,20 +1287,85 @@ func TestSessionStore_CookieConfiguration(t *testing.T) { "HTTP base URL should produce non-secure cookies") } -func TestSessionStore_NoMaxLengthLimit(t *testing.T) { - // The FilesystemStore stores data on disk, not in cookies, so the default - // securecookie 4096-byte limit must be removed. JWT tokens in the session - // regularly exceed that limit after gob+base64 encoding. - ws := newTestWebServer(t, WebServerConfig{}) - for _, codec := range ws.sessionStore.Codecs { - if sc, ok := codec.(*securecookie.SecureCookie); ok { - // Encode a large value — if MaxLength were still 4096 this would fail. - large := make(map[interface{}]interface{}) - large["token"] = string(make([]byte, 8000)) - _, err := securecookie.EncodeMulti("test", large, sc) - assert.NoError(t, err, "session store should allow values larger than 4096 bytes") - } +func TestSessionStore_CrossReplicaRoundTrip(t *testing.T) { + // Behind a load balancer the OAuth login, the provider callback, and every + // follow-up API request can each land on a different replica. With a + // cookie-backed session store, any replica configured with the same + // SESSION_SECRET must be able to read a session cookie minted by another + // replica. This is the regression test for the "state_mismatch" login + // failures (and dropped post-login sessions) caused by the previous + // filesystem-backed, process-local store. + const secret = "test-shared-session-secret-value-1234567890" + + replicaA := newTestWebServer(t, WebServerConfig{SessionSecret: secret}) + replicaB := newTestWebServer(t, WebServerConfig{SessionSecret: secret}) + + // A realistic post-login payload: identity plus access/refresh JWTs, in + // addition to the short-lived OAuth CSRF state. + svc, err := NewUserTokenService(UserTokenConfig{}) + require.NoError(t, err) + access, refresh, _, err := svc.GenerateTokenPair("user_123", "user@example.com", "Test User", "admin", ClientTypeWeb) + require.NoError(t, err) + + // Replica A writes the session and emits the cookie (e.g. during /auth/login + // and the callback that completes login). + reqA := httptest.NewRequest(http.MethodGet, "/auth/login/google", nil) + recA := httptest.NewRecorder() + sessA, err := replicaA.sessionStore.Get(reqA, webSessionName) + require.NoError(t, err) + sessA.Values[sessKeyOAuthState] = "state-token-abc123" + sessA.Values[sessKeyUserID] = "user_123" + sessA.Values[sessKeyUserEmail] = "user@example.com" + sessA.Values[sessKeyHubAccessToken] = access + sessA.Values[sessKeyHubRefreshToken] = refresh + require.NoError(t, sessA.Save(reqA, recA)) + + cookies := recA.Result().Cookies() + require.NotEmpty(t, cookies, "replica A should set a session cookie") + + // Replica B receives the cookie minted by replica A and must decode it. + reqB := httptest.NewRequest(http.MethodGet, "/auth/callback/google", nil) + for _, c := range cookies { + reqB.AddCookie(c) + } + sessB, err := replicaB.sessionStore.Get(reqB, webSessionName) + require.NoError(t, err) + assert.False(t, sessB.IsNew, "replica B must decode the session cookie minted by replica A") + assert.Equal(t, "state-token-abc123", sessB.Values[sessKeyOAuthState], + "OAuth state must survive across replicas (fixes state_mismatch)") + assert.Equal(t, "user_123", sessB.Values[sessKeyUserID]) + assert.Equal(t, access, sessB.Values[sessKeyHubAccessToken], + "post-login access token must survive across replicas") + assert.Equal(t, refresh, sessB.Values[sessKeyHubRefreshToken]) +} + +func TestSessionStore_DifferentSecretCannotDecode(t *testing.T) { + // A replica configured with a different SESSION_SECRET must NOT be able to + // read another replica's session cookie — the cookie is authenticated and + // encrypted with keys derived from the shared secret. + replicaA := newTestWebServer(t, WebServerConfig{SessionSecret: "secret-A-1234567890-abcdefghijklmnop"}) + replicaC := newTestWebServer(t, WebServerConfig{SessionSecret: "secret-C-1234567890-abcdefghijklmnop"}) + + reqA := httptest.NewRequest(http.MethodGet, "/auth/login/google", nil) + recA := httptest.NewRecorder() + sessA, err := replicaA.sessionStore.Get(reqA, webSessionName) + require.NoError(t, err) + sessA.Values[sessKeyOAuthState] = "state-token-abc123" + require.NoError(t, sessA.Save(reqA, recA)) + + reqC := httptest.NewRequest(http.MethodGet, "/auth/callback/google", nil) + for _, c := range recA.Result().Cookies() { + reqC.AddCookie(c) + } + sessC, err := replicaC.sessionStore.Get(reqC, webSessionName) + // A cookie authenticated/encrypted with a different secret fails to decode: + // gorilla returns a decode error together with a fresh, empty session. + // Either way, the state must not leak across mismatched secrets. + if err == nil { + assert.True(t, sessC.IsNew, "session from a mismatched secret should be new/empty") } + assert.Nil(t, sessC.Values[sessKeyOAuthState], + "OAuth state must not decode under a different secret") } func TestSetters(t *testing.T) {