Add Origin Checking. #4845

Merged
merged 9 commits into from Jan 31, 2014

Conversation

Projects
None yet
5 participants
@rgbkrk
Member

rgbkrk commented Jan 21, 2014

This verifies that requests originate from the same host that a notebook is run on.

IPython/html/base/zmqhandlers.py
+
+ # Check to see that origin matches host directly, including ports
+ if origin != host:
+ self.log.critical("Cross Origin WebSocket Attempt.", exc_info=True)

This comment has been minimized.

@minrk

minrk Jan 21, 2014

Member

Probably don't need exc_info=True, since there is no exception

@minrk

minrk Jan 21, 2014

Member

Probably don't need exc_info=True, since there is no exception

IPython/html/base/zmqhandlers.py
@@ -17,6 +17,11 @@
#-----------------------------------------------------------------------------
+try:
+ from urllib.parse import urlparse

This comment has been minimized.

@fperez

fperez Jan 21, 2014

Member

I'd add a comment here indicating this is the py3 codepath. That way we'll know in the future and when we drop py2 compatibility, we can remove the py2 path.

@fperez

fperez Jan 21, 2014

Member

I'd add a comment here indicating this is the py3 codepath. That way we'll know in the future and when we drop py2 compatibility, we can remove the py2 path.

IPython/html/base/zmqhandlers.py
+ origin = parsed_origin.netloc
+
+ # Check to see that origin matches host directly, including ports
+ if origin != host:

This comment has been minimized.

@minrk

minrk Jan 21, 2014

Member

just need to check that Host and Origin are both affected in the same way by proxies / tunnels

@minrk

minrk Jan 21, 2014

Member

just need to check that Host and Origin are both affected in the same way by proxies / tunnels

IPython/html/base/zmqhandlers.py
+ def check_origin(self):
+ """Check origin from headers."""
+ origin_header = self.request.headers["Origin"]
+ host = self.request.headers["Host"]

This comment has been minimized.

@minrk

minrk Jan 22, 2014

Member

Probably use get, in the unlikely event these are undefined.

@minrk

minrk Jan 22, 2014

Member

Probably use get, in the unlikely event these are undefined.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jan 22, 2014

Member

You mentioned moving the check to on_open in authenticated handler, did you still want to do that?

Member

minrk commented Jan 22, 2014

You mentioned moving the check to on_open in authenticated handler, did you still want to do that?

@rgbkrk

This comment has been minimized.

Show comment
Hide comment
@rgbkrk

rgbkrk Jan 23, 2014

Member

Yeah, I'll override open.

Member

rgbkrk commented Jan 23, 2014

Yeah, I'll override open.

@ghost ghost assigned ellisonbg Jan 23, 2014

@damianavila

This comment has been minimized.

Show comment
Hide comment
@damianavila

damianavila Jan 25, 2014

Contributor

AFAIK, LGTM 😉

Contributor

damianavila commented Jan 25, 2014

AFAIK, LGTM 😉

minrk added a commit that referenced this pull request Jan 31, 2014

Merge pull request #4845 from rgbkrk/origin_host
Add Origin checking for websockets.

@minrk minrk merged commit e5b669c into ipython:master Jan 31, 2014

1 check passed

default The Travis CI build passed
Details

pankajp added a commit to pankajp/ipython that referenced this pull request Feb 19, 2014

@rgbkrk rgbkrk deleted the rgbkrk:origin_host branch May 8, 2014

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #4845 from rgbkrk/origin_host
Add Origin checking for websockets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment