diff --git a/solara/server/flask.py b/solara/server/flask.py index 420d861ac..5e884680c 100644 --- a/solara/server/flask.py +++ b/solara/server/flask.py @@ -141,7 +141,7 @@ def kernels_connection(ws: simple_websocket.Server, kernel_id: str, name: str): logger.info("Solara kernel requested for session_id=%s kernel_id=%s", session_id, kernel_id) if session_id is None: logger.warning("no session cookie") - session_id = "session-id-cookie-unavailable:" + str(uuid4()) + session_id = kernel_context.SESSION_ID_COOKIE_UNAVAILABLE_PREFIX + str(uuid4()) ws_wrapper = WebsocketWrapper(ws) headers_dict: Dict[str, List[str]] = {} for k, v in request.headers.__iter__(): @@ -255,12 +255,12 @@ def read_root(path): samesite = "lax" secure = False - # we want samesite, so we can set a cookie when embedded in an iframe, such as on huggingface + # we want samesite=none, so the cookie is also set when embedded in a cross-site iframe, such as on huggingface # however, samesite=none requires Secure https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite - # when hosted on the localhost domain we can always set the Secure flag - # to allow samesite https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies + # Over plain http on localhost we deliberately do NOT set Secure, since Safari does not accept a Secure cookie + # from http://localhost (see the starlette server for details). o = urlparse(request.base_url) - if request.headers.get("x-forwarded-proto", "http") == "https" or o.hostname == "localhost": + if request.headers.get("x-forwarded-proto", "http") == "https" or o.scheme == "https": samesite = "none" secure = True diff --git a/solara/server/kernel_context.py b/solara/server/kernel_context.py index 3b9a06427..f7a058974 100644 --- a/solara/server/kernel_context.py +++ b/solara/server/kernel_context.py @@ -37,6 +37,10 @@ WebSocket = Any logger = logging.getLogger("solara.server.app") +# session id prefix the servers use when the browser did not send the session cookie (cookies blocked, +# or a Secure cookie the browser refuses on plain http, as Safari does on http://localhost) +SESSION_ID_COOKIE_UNAVAILABLE_PREFIX = "session-id-cookie-unavailable:" + class Local(threading.local): kernel_context_stack: Optional[List[Optional["VirtualKernelContext"]]] = None @@ -931,11 +935,22 @@ def initialize_virtual_kernel(session_id: str, kernel_id: str, websocket: websoc assert context is not None # redact both session ids: they are bearer credentials for takeover once persistence is on. # The client message must NOT echo them back either (it reaches an attacker's browser). - logger.critical( - "Session id mismatch when reusing kernel (hack attempt?): %s != %s", - redact_id(context.session_id), - redact_id(session_id), - ) + if context.session_id.startswith(SESSION_ID_COOKIE_UNAVAILABLE_PREFIX) or session_id.startswith(SESSION_ID_COOKIE_UNAVAILABLE_PREFIX): + # not a hack attempt: without the cookie a reconnect or popout cannot prove it owns the kernel + logger.warning( + "Session id mismatch when reusing kernel %s: the browser did not send the session cookie " + "(cookies blocked, or the browser refused the cookie), so a reconnect or popout cannot be matched to the session that " + "created the kernel: %s != %s", + redact_id(kernel_id), + redact_id(context.session_id), + redact_id(session_id), + ) + else: + logger.critical( + "Session id mismatch when reusing kernel (hack attempt?): %s != %s", + redact_id(context.session_id), + redact_id(session_id), + ) websocket.send_text("Session id mismatch when reusing kernel (hack attempt?)") # to avoid very fast reconnects (we are in a thread anyway) time.sleep(0.5) diff --git a/solara/server/starlette.py b/solara/server/starlette.py index ec3dd5dbc..bb855c186 100644 --- a/solara/server/starlette.py +++ b/solara/server/starlette.py @@ -357,7 +357,7 @@ async def _kernel_connection(ws: starlette.websockets.WebSocket): if not session_id: logger.warning("no session cookie") - session_id = "session-id-cookie-unavailable:" + str(uuid4()) + session_id = kernel_context.SESSION_ID_COOKIE_UNAVAILABLE_PREFIX + str(uuid4()) # we use the jupyter session_id query parameter as the key/id # for a page scope. page_id = ws.query_params["session_id"] @@ -610,11 +610,14 @@ async def root(request: Request, fullpath: str = ""): samesite = "lax" secure = False httponly = settings.session.http_only - # we want samesite, so we can set a cookie when embedded in an iframe, such as on huggingface + # we want samesite=none, so the cookie is also set when embedded in a cross-site iframe, such as on huggingface # however, samesite=none requires Secure https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite - # when hosted on the localhost domain we can always set the Secure flag - # to allow samesite https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies - if request.scope["scheme"] == "https" or request.headers.get("x-forwarded-proto", "http") == "https" or request.base_url.hostname == "localhost": + # Over plain http on localhost we deliberately do NOT set Secure: Chrome and Firefox accept a Secure cookie + # from http://localhost, but Safari does not (https://bugs.webkit.org/show_bug.cgi?id=218980), so Safari would + # never send the session cookie back. Every websocket then gets a fresh session id, and reconnects and popouts + # (ipypopout) fail with "Session id mismatch". A samesite=lax cookie is still sent to an iframe when the + # embedding page is on localhost as well (the port does not matter for samesite). + if request.scope["scheme"] == "https" or request.headers.get("x-forwarded-proto", "http") == "https": samesite = "none" secure = True elif request.base_url.hostname != "localhost": diff --git a/solara/website/pages/documentation/advanced/content/10-howto/40-embed.md b/solara/website/pages/documentation/advanced/content/10-howto/40-embed.md index 9e35dd742..26907c681 100644 --- a/solara/website/pages/documentation/advanced/content/10-howto/40-embed.md +++ b/solara/website/pages/documentation/advanced/content/10-howto/40-embed.md @@ -41,7 +41,7 @@ If you do not see your app, you can open the browser developer tools in your bro ### Security considerations -Solara uses a cookie to implement sessions. To support setting cookies in an iframe, we set the session cookie using `Secure`, and `SameSite=Strict`. See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies) for more details. This means that we can only support iframes via https or localhost. Note that proxy servers can tell +Solara uses a cookie to implement sessions. To support setting cookies in an iframe on a different site, we set the session cookie using `Secure`, and `SameSite=None` when served over https. See [MDN](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies) for more details. This means that we can only support cross-site iframes via https. Over plain http (e.g. while developing on localhost) the cookie is set with `SameSite=Lax`, which is still sent to an iframe when the embedding page is on the same site (such as another port on localhost). Note that proxy servers can tell solara-server that the connection is secure by forwarding the `X-Forwarded-Proto` header, see [our self hosted deployment documentation for more information](https://solara.dev/documentation/getting_started/deploying/self-hosted). diff --git a/tests/integration/server_test.py b/tests/integration/server_test.py index 22e4b1ea4..9c4ae708e 100644 --- a/tests/integration/server_test.py +++ b/tests/integration/server_test.py @@ -220,6 +220,26 @@ def test_kernel_asyncio(browser: playwright.sync_api.Browser, solara_server, sol solara.server.settings.kernel.threaded = threaded +def _session_cookie_attributes(response: requests.Response) -> str: + # the page sets only the session cookie, so the raw header is that cookie with its attributes + header = response.headers["set-cookie"].lower() + assert header.startswith(solara.server.server.COOKIE_KEY_SESSION_ID + "="), header + return header + + +def test_session_cookie_attributes(solara_server, solara_app, extra_include_path): + with extra_include_path(HERE), solara_app("server_test:ClickButton"): + # Over plain http on localhost the cookie must not be Secure: Safari refuses a Secure cookie from + # http://localhost, after which reconnects and popouts fail with "Session id mismatch". + cookie = _session_cookie_attributes(requests.get(solara_server.base_url)) + assert "secure" not in cookie + assert "samesite=lax" in cookie + # behind an https proxy we want a cross-site iframe to work, which requires Secure + SameSite=None + cookie = _session_cookie_attributes(requests.get(solara_server.base_url, headers={"x-forwarded-proto": "https"})) + assert "secure" in cookie + assert "samesite=none" in cookie + + def test_cdn_secure(solara_server, solara_app, extra_include_path): cdn_url = solara_server.base_url + "/_solara/cdn" assert solara.settings.assets.proxy diff --git a/tests/unit/lifecycle_test.py b/tests/unit/lifecycle_test.py index b8aa54b9d..05e00af98 100644 --- a/tests/unit/lifecycle_test.py +++ b/tests/unit/lifecycle_test.py @@ -50,6 +50,29 @@ def test_kernel_max_per_session(monkeypatch): ctx.close() +def test_session_mismatch_without_cookie_is_not_logged_as_hack_attempt(caplog, monkeypatch): + # a browser that does not send the session cookie gets a fresh session id per websocket, so a + # reconnect or popout cannot match the kernel's session: refuse, but explain instead of crying wolf + monkeypatch.setattr(kernel_context.time, "sleep", lambda s: None) + prefix = kernel_context.SESSION_ID_COOKIE_UNAVAILABLE_PREFIX + kernel_context.initialize_virtual_kernel(prefix + "a", "kern-no-cookie", Mock()) + kernel_context.initialize_virtual_kernel("real-session", "kern-with-cookie", Mock()) + try: + with caplog.at_level("WARNING", logger="solara.server.app"): + with pytest.raises(ValueError, match="Session id mismatch"): + kernel_context.initialize_virtual_kernel(prefix + "b", "kern-no-cookie", Mock()) + assert [r.levelname for r in caplog.records if "mismatch" in r.getMessage()] == ["WARNING"] + assert "did not send the session cookie" in caplog.text + caplog.clear() + # two different real cookies for the same kernel is still a potential hack attempt + with pytest.raises(ValueError, match="Session id mismatch"): + kernel_context.initialize_virtual_kernel("other-session", "kern-with-cookie", Mock()) + assert [r.levelname for r in caplog.records if "mismatch" in r.getMessage()] == ["CRITICAL"] + finally: + for ctx in list(kernel_context.contexts.values()): + ctx.close() + + @pytest.mark.skipif(on_windows, reason="This test is flaky on Windows") async def test_kernel_lifecycle_reconnect_simple(short_cull_timeout): # a reconnect should be possible within the reconnect window