You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "tbonelee (via GitHub)" <gi...@apache.org> on 2024/09/10 14:16:00 UTC

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

tbonelee opened a new pull request, #4823:
URL: https://github.com/apache/zeppelin/pull/4823

   ### 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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [NO-ISSUE] Implement Origin check for terminal interpreter WebSocket connections [zeppelin]

Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul merged PR #4823:
URL: https://github.com/apache/zeppelin/pull/4823


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [NO-ISSUE] Implement Origin check for terminal interpreter WebSocket connections [zeppelin]

Posted by "jongyoul (via GitHub)" <gi...@apache.org>.
jongyoul commented on PR #4823:
URL: https://github.com/apache/zeppelin/pull/4823#issuecomment-2452973218

   I pushed this commit into branch-0.12 as well.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [NO-ISSUE] Implement Origin check for terminal interpreter WebSocket connections [zeppelin]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4823:
URL: https://github.com/apache/zeppelin/pull/4823#issuecomment-2361588274

   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 method has the same origin restriction? Do you think we should have a switch for this feature?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [NO-ISSUE] Implement Origin check for terminal interpreter WebSocket connections [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on PR #4823:
URL: https://github.com/apache/zeppelin/pull/4823#issuecomment-2363766105

   @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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org