zeppelin icon indicating copy to clipboard operation
zeppelin copied to clipboard

[NO-ISSUE] Implement Origin check for terminal interpreter WebSocket connections

Open tbonelee opened this issue 1 year ago • 2 comments

What is this PR for?

This PR adds an Origin check to ensure that WebSocket connections are initiated from trusted sources only. By validating the Origin header in the initial WebSocket handshake, we can prevent unauthorized or malicious websites from establishing WebSocket connections with our server.

Changes:

  • Added server-side validation of the Origin header during WebSocket connection requests.

Other security enhancements may be needed and can be handled in future iterations.

What type of PR is it?

Improvement

Todos

  • [ ] - Task

How should this be tested?

  • Strongly recommended: add automated unit tests for any new or changed behavior
  • Outline any manual steps to test the PR here.

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

tbonelee avatar Sep 10 '24 14:09 tbonelee

I guess this feature may break some deployments, for example, Zepplein was deployed behind a Gateway, which simply forward the websocket traffic without changing origin.

I believe the origin restriction is part of HTTP protocol standard and a well-known feature, does Jetty/Tomcat or other Web server have a built-in implementation? I haven't gone through the Web service code, does the HTTP 1.0 methods(I mean other GET/POST/DELETE methods exposed by Zeppelin) has the same origin restriction? Do you think we should have a switch for this feature?

pan3793 avatar Sep 19 '24 16:09 pan3793

@pan3793

I guess this feature may break some deployments, for example, Zepplein was deployed behind a Gateway, which simply forward the websocket traffic without changing origin.

This part is a bit unclear to me. If the origin that the browser use to request WebSocket connection (the origin of the iframe src URL) is forwarded by the Gateway without modification, wouldn't the request come through with the intended origin, allowing it to pass the origin check without issues? Could you clarify which specific case you had in mind?

I believe the origin restriction is part of the HTTP protocol standard and a well-known feature. Does Jetty/Tomcat or other web servers have a built-in implementation? I haven’t gone through the web service code—do the HTTP 1.0 methods (like the GET/POST/DELETE methods exposed by Zeppelin) have the same origin restriction? Do you think we should have a switch for this feature?

Regarding this, AFAIK, the WebSocket specification does not force SOP. Also, the Java API for WebSocket states that the default server configuration for origin check is to fail the handshake if the hostname cannot be verified (I understand this as applying to cases like when there is no origin header).

That's why I understood that the NotebookServer added its own checkOrigin() method in the WebSocket server.

tbonelee avatar Sep 20 '24 13:09 tbonelee

I pushed this commit into branch-0.12 as well.

jongyoul avatar Nov 02 '24 12:11 jongyoul