-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enable SSL on forwarded requests #169
base: main
Are you sure you want to change the base?
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,3 +1,5 @@ | ||||||
import ssl | ||||||
|
||||||
from .handlers import setup_handlers, SuperviseAndProxyHandler | ||||||
from .config import ServerProxy, make_handlers, get_entrypoint_server_processes, make_server_process | ||||||
from notebook.utils import url_path_join as ujoin | ||||||
|
@@ -28,8 +30,19 @@ def load_jupyter_server_extension(nbapp): | |||||
server_handlers = make_handlers(base_url, server_proccesses) | ||||||
nbapp.web_app.add_handlers('.*', server_handlers) | ||||||
|
||||||
# Configure SSL support | ||||||
ssl_options = None | ||||||
if serverproxy.https: | ||||||
ssl_context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH, cafile=serverproxy.cafile) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The default value of
|
||||||
if serverproxy.certfile or serverproxy.keyfile: | ||||||
ssl_context.load_cert_chain(serverproxy.certfile, serverproxy.keyfile or None) | ||||||
else: | ||||||
ssl_context.load_default_certs() | ||||||
ssl_context.check_hostname = serverproxy.check_hostname | ||||||
ssl_options = ssl_context | ||||||
|
||||||
# Set up default handler | ||||||
setup_handlers(nbapp.web_app, serverproxy.host_whitelist) | ||||||
setup_handlers(nbapp.web_app, serverproxy.host_whitelist, ssl_options) | ||||||
|
||||||
launcher_entries = [] | ||||||
icons = {} | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ | |||||
Traitlets based configuration for jupyter_server_proxy | ||||||
""" | ||||||
from notebook.utils import url_path_join as ujoin | ||||||
from traitlets import Dict, List, Union, default | ||||||
from traitlets import Bool, Dict, List, Unicode, Union, default | ||||||
from traitlets.config import Configurable | ||||||
from .handlers import SuperviseAndProxyHandler, AddSlashHandler | ||||||
import pkg_resources | ||||||
|
@@ -203,3 +203,51 @@ def host_whitelist(handler, host): | |||||
@default("host_whitelist") | ||||||
def _host_whitelist_default(self): | ||||||
return ["localhost", "127.0.0.1"] | ||||||
|
||||||
keyfile = Unicode( | ||||||
"", | ||||||
help=""" | ||||||
Path to optional SSL key. | ||||||
|
||||||
Use with `https=True` and `certfile`. | ||||||
""", | ||||||
config=True | ||||||
) | ||||||
|
||||||
certfile = Unicode( | ||||||
"", | ||||||
help=""" | ||||||
Path to optional SSL cert. | ||||||
|
||||||
Use with `https=True` and `keyfile`. | ||||||
""", | ||||||
config=True | ||||||
) | ||||||
|
||||||
cafile = Unicode( | ||||||
"", | ||||||
help=""" | ||||||
Path to optional CA file. | ||||||
|
||||||
Use with `https=True`. | ||||||
""", | ||||||
config=True | ||||||
) | ||||||
|
||||||
https = Bool( | ||||||
False, | ||||||
help=""" | ||||||
Whether to use SSL for forwarded client requests. | ||||||
|
||||||
If this is set to `True` then you should provide a path to an SSL key, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
cert, and CA. Use this if the proxied service expects to service | ||||||
requests over SSL. | ||||||
""", | ||||||
config=True | ||||||
) | ||||||
Comment on lines
+207
to
+247
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to get all these SSL informations from mybinder (which uses SSL)? |
||||||
|
||||||
check_hostname = Bool( | ||||||
False, | ||||||
help="Whether to check hostname.", | ||||||
config=True | ||||||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,7 @@ def __init__(self, *args, **kwargs): | |
self.absolute_url = kwargs.pop('absolute_url', False) | ||
self.host_whitelist = kwargs.pop('host_whitelist', ['localhost', '127.0.0.1']) | ||
self.subprotocols = None | ||
self.ssl_options = kwargs.pop('ssl_options', None) | ||
super().__init__(*args, **kwargs) | ||
|
||
# Support all the methods that tornado does by default except for GET which | ||
|
@@ -165,7 +166,8 @@ def _build_proxy_request(self, host, port, proxied_path, body): | |
|
||
headers = self.proxy_request_headers() | ||
|
||
client_uri = self.get_client_uri('http', host, port, proxied_path) | ||
protocol = 'http' if self.ssl_options is None else 'https' | ||
client_uri = self.get_client_uri(protocol, host, port, proxied_path) | ||
# Some applications check X-Forwarded-Context and X-ProxyContextPath | ||
# headers to see if and where they are being proxied from. | ||
if not self.absolute_url: | ||
|
@@ -276,7 +278,8 @@ async def proxy_open(self, host, port, proxied_path=''): | |
if not proxied_path.startswith('/'): | ||
proxied_path = '/' + proxied_path | ||
|
||
client_uri = self.get_client_uri('ws', host, port, proxied_path) | ||
protocol = 'ws' if self.ssl_options is None else 'wss' | ||
client_uri = self.get_client_uri(protocol, host, port, proxied_path) | ||
headers = self.request.headers | ||
current_loop = ioloop.IOLoop.current() | ||
ws_connected = current_loop.asyncio_loop.create_future() | ||
|
@@ -307,7 +310,8 @@ def ping_cb(data): | |
async def start_websocket_connection(): | ||
self.log.info('Trying to establish websocket connection to {}'.format(client_uri)) | ||
self._record_activity() | ||
request = httpclient.HTTPRequest(url=client_uri, headers=headers) | ||
request = httpclient.HTTPRequest(url=client_uri, headers=headers, | ||
ssl_options=self.ssl_options) | ||
self.ws = await pingable_ws_connect(request=request, | ||
on_message_callback=message_cb, on_ping_callback=ping_cb, | ||
subprotocols=self.subprotocols) | ||
|
@@ -330,7 +334,11 @@ def proxy_request_headers(self): | |
def proxy_request_options(self): | ||
'''A dictionary of options to be used when constructing | ||
a tornado.httpclient.HTTPRequest instance for the proxy request.''' | ||
<<<<<<< HEAD | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rcthomas this might be your culprit, pesky git 😄 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh bother, thought I got them all... |
||
return dict(follow_redirects=False, connect_timeout=250.0, request_timeout=300.0) | ||
======= | ||
return dict(follow_redirects=False, ssl_options=self.ssl_options) | ||
>>>>>>> Enable SSL on forwarded requests | ||
|
||
def check_xsrf_cookie(self): | ||
''' | ||
|
@@ -556,17 +564,21 @@ def options(self, path): | |
return self.proxy(self.port, path) | ||
|
||
|
||
def setup_handlers(web_app, host_whitelist): | ||
def setup_handlers(web_app, host_whitelist, ssl_options): | ||
host_pattern = '.*$' | ||
web_app.add_handlers('.*', [ | ||
(url_path_join(web_app.settings['base_url'], r'/proxy/(.*):(\d+)(.*)'), | ||
RemoteProxyHandler, {'absolute_url': False, 'host_whitelist': host_whitelist}), | ||
RemoteProxyHandler, {'absolute_url': False, 'host_whitelist': host_whitelist, | ||
'ssl_options': ssl_options}), | ||
(url_path_join(web_app.settings['base_url'], r'/proxy/absolute/(.*):(\d+)(.*)'), | ||
RemoteProxyHandler, {'absolute_url': True, 'host_whitelist': host_whitelist}), | ||
RemoteProxyHandler, {'absolute_url': True, 'host_whitelist': host_whitelist, | ||
'ssl_options': ssl_options}), | ||
(url_path_join(web_app.settings['base_url'], r'/proxy/(\d+)(.*)'), | ||
LocalProxyHandler, {'absolute_url': False}), | ||
LocalProxyHandler, {'absolute_url': False, | ||
'ssl_options': ssl_options}), | ||
(url_path_join(web_app.settings['base_url'], r'/proxy/absolute/(\d+)(.*)'), | ||
LocalProxyHandler, {'absolute_url': True}), | ||
LocalProxyHandler, {'absolute_url': True, | ||
'ssl_options': ssl_options}), | ||
]) | ||
|
||
# vim: set et ts=4 sw=4: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we setup the context like this, do we also get all the "normal" CAs or only the one in the
cafile
?"Normal" CAs in this case would be those that your OS trusts.