Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions solara/server/flask.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__():
Expand Down Expand Up @@ -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

Expand Down
25 changes: 20 additions & 5 deletions solara/server/kernel_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 8 additions & 5 deletions solara/server/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down Expand Up @@ -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":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).


Expand Down
20 changes: 20 additions & 0 deletions tests/integration/server_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions tests/unit/lifecycle_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading