From 4dde8ca2e2e478326c0fd39820a4d709cce918a7 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 13 Apr 2021 08:56:11 -0700 Subject: [PATCH 1/9] Allow SSH authentication via GSS. This attempt to implement GSS as requested in #946. I've tried to also document the other environment variable, though I couldn't find where or how they are supposed to be used. I'm also currently trying to find a deployment that could use GSS to test this, but haven't so far. I'm assuming that if GSS is enabled then it takes priority over username/password. --- docs/source/config-options.md | 10 +++++++ .../services/processproxies/processproxy.py | 30 +++++++++++++++---- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/docs/source/config-options.md b/docs/source/config-options.md index 3f99a1219..3fd59abc9 100644 --- a/docs/source/config-options.md +++ b/docs/source/config-options.md @@ -451,6 +451,16 @@ The following environment variables can be used to influence functionality and a The port number used for ssh operations for installations choosing to configure the ssh server on a port other than the default 22. + EG_REMOTE_PWD=None + The password to use to ssh to remote hosts + + EG_REMOTE_USER=None + The username to use when connecting to remote hosts (default to `getpass.getuser()` + when not set). + + EG_REMOTE_GSS_SSH=None + Use gss instead of EG_REMOTE_USER and EG_REMOTE_PWD to connect to remote host via SSH. + EG_YARN_CERT_BUNDLE= The path to a .pem or any other custom truststore used as a CA bundle in yarn-api-client. ``` diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index 06c44a13e..15a3f1b3a 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -18,6 +18,7 @@ import subprocess import sys import time +import warnings from asyncio import Event, TimeoutError from calendar import timegm @@ -586,18 +587,35 @@ def _get_ssh_client(self, host): global remote_user global remote_pwd if remote_user is None: - remote_user = os.getenv('EG_REMOTE_USER', getpass.getuser()) - remote_pwd = os.getenv('EG_REMOTE_PWD') # this should use password-less ssh + use_gss = os.getenv("EG_REMOTE_GSS_SSH", None) + remote_pwd = os.getenv("EG_REMOTE_PWD") # this should use password-less ssh + remote_user = os.getenv("EG_REMOTE_USER", getpass.getuser()) + + if use_gss and (remote_pwd or remote_user): + warnings.warn( + "Both `EG_REMOTE_GSS_SSH` and one of `EG_REMOTE_PWD` or `EG_REMOTE_USER` is set. " + "Those options are mutually exclusive, you configuration may be incorrect. " + "EG_REMOTE_GSS_SSH will take priority." + ) try: ssh = paramiko.SSHClient() ssh.load_system_host_keys() - ssh.set_missing_host_key_policy(paramiko.RejectPolicy()) host_ip = gethostbyname(host) - if remote_pwd: - ssh.connect(host_ip, port=ssh_port, username=remote_user, password=remote_pwd) + if use_gss: + ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) + ssh.connect(host_ip, port=ssh_port, gss_auth=True) else: - ssh.connect(host_ip, port=ssh_port, username=remote_user) + ssh.set_missing_host_key_policy(paramiko.RejectPolicy()) + if remote_pwd: + ssh.connect( + host_ip, + port=ssh_port, + username=remote_user, + password=remote_pwd, + ) + else: + ssh.connect(host_ip, port=ssh_port, username=remote_user) except Exception as e: http_status_code = 500 current_host = gethostbyname(gethostname()) From 5db26f6b2003a230bc7e511ea4da92fd3c7c812c Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 21 Apr 2021 07:55:51 -0700 Subject: [PATCH 2/9] Fix UnboundLocal Error. --- enterprise_gateway/services/processproxies/processproxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index 15a3f1b3a..fb6199df7 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -586,8 +586,8 @@ def _get_ssh_client(self, host): global remote_user global remote_pwd + use_gss = os.getenv("EG_REMOTE_GSS_SSH", None) if remote_user is None: - use_gss = os.getenv("EG_REMOTE_GSS_SSH", None) remote_pwd = os.getenv("EG_REMOTE_PWD") # this should use password-less ssh remote_user = os.getenv("EG_REMOTE_USER", getpass.getuser()) From 5d78b77af9a5dc91689d0aa0087b1de0bbfecab0 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 21 Apr 2021 13:22:47 -0700 Subject: [PATCH 3/9] Remove use of globals, and add debug logs. The globals used to be loaded at module level, which i not the case anymore; move them to instance values. --- .../services/processproxies/processproxy.py | 71 ++++++++++++------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index fb6199df7..d9c475b2c 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -63,13 +63,6 @@ # Number of seconds in 100 years as the max keep-alive interval value. max_keep_alive_interval = 100 * 365 * 24 * 60 * 60 -# These envs are not documented and should default to current user and None, respectively. These -# exist just in case we find them necessary in some configurations (where the service user -# must be different). However, tests show that that configuration doesn't work - so there -# might be more to do. At any rate, we'll use these variables for now. -remote_user = None -remote_pwd = None - # Allow users to specify local ips (regular expressions can be used) that should not be included # when determining the response address. For example, on systems with many network interfaces, # some may have their IPs appear the local interfaces list (e.g., docker's 172.17.0.* is an example) @@ -399,6 +392,8 @@ def __init__(self, kernel_manager, proxy_config): self.ip = None self.pid = 0 self.pgid = 0 + self.remote_user = None + self.remote_pwd = None @abc.abstractmethod async def launch_process(self, kernel_cmd, **kwargs): @@ -584,14 +579,14 @@ def _get_ssh_client(self, host): """ ssh = None - global remote_user - global remote_pwd use_gss = os.getenv("EG_REMOTE_GSS_SSH", None) - if remote_user is None: - remote_pwd = os.getenv("EG_REMOTE_PWD") # this should use password-less ssh - remote_user = os.getenv("EG_REMOTE_USER", getpass.getuser()) + if self.remote_user is None: + self.remote_pwd = os.getenv( + "EG_REMOTE_PWD" + ) # this should use password-less ssh + self.remote_user = os.getenv("EG_REMOTE_USER", getpass.getuser()) - if use_gss and (remote_pwd or remote_user): + if use_gss and (self.remote_pwd or self.remote_user): warnings.warn( "Both `EG_REMOTE_GSS_SSH` and one of `EG_REMOTE_PWD` or `EG_REMOTE_USER` is set. " "Those options are mutually exclusive, you configuration may be incorrect. " @@ -603,29 +598,39 @@ def _get_ssh_client(self, host): ssh.load_system_host_keys() host_ip = gethostbyname(host) if use_gss: + self.log.debug("Connecting to remote host via GSS.") ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) ssh.connect(host_ip, port=ssh_port, gss_auth=True) else: ssh.set_missing_host_key_policy(paramiko.RejectPolicy()) - if remote_pwd: + if self.remote_pwd: + self.log.debug( + "Connecting to remote host with username and password." + ) ssh.connect( host_ip, port=ssh_port, - username=remote_user, - password=remote_pwd, + username=self.remote_user, + password=self.remote_pwd, ) else: - ssh.connect(host_ip, port=ssh_port, username=remote_user) + self.log.debug("Connecting to remote host with ssh key.") + ssh.connect(host_ip, port=ssh_port, username=self.remote_user) except Exception as e: http_status_code = 500 current_host = gethostbyname(gethostname()) - error_message = "Exception '{}' occurred when creating a SSHClient at {} connecting " \ - "to '{}:{}' with user '{}', message='{}'.".\ - format(type(e).__name__, current_host, host, ssh_port, remote_user, e) + error_message = ( + "Exception '{}' occurred when creating a SSHClient at {} connecting " + "to '{}:{}' with user '{}', message='{}'.".format( + type(e).__name__, current_host, host, ssh_port, self.remote_user, e + ) + ) if e is paramiko.SSHException or paramiko.AuthenticationException: http_status_code = 403 error_message_prefix = "Failed to authenticate SSHClient with password" - error_message = error_message_prefix + (" provided" if remote_pwd else "-less SSH") + error_message = error_message_prefix + ( + " provided" if self.remote_pwd else "-less SSH" + ) self.log_and_raise(http_status_code=http_status_code, reason=error_message) return ssh @@ -1284,12 +1289,23 @@ def _send_listener_request(self, request, shutdown_socket=False): sock.shutdown(SHUT_WR) except Exception as e2: if isinstance(e2, OSError) and e2.errno == errno.ENOTCONN: - pass # Listener is not connected. This is probably a follow-on to ECONNREFUSED on connect + # Listener is not connected. This is probably a follow-on to ECONNREFUSED on connect + self.log.debug( + "ERROR: OSError(ENOTCONN) raised on socket shutdown, " + "listener likely not connected. Cannot send {request}", + request=request, + ) else: self.log.warning("Exception occurred attempting to shutdown communication socket to {}:{} " "for KernelID '{}' (ignored): {}".format(self.comm_ip, self.comm_port, self.kernel_id, str(e2))) sock.close() + else: + self.log.debug( + "Invalid comm port, not sending request '{}' to comm_port '{}'.", + request, + self.comm_port, + ) def send_signal(self, signum): """ @@ -1303,17 +1319,20 @@ def send_signal(self, signum): # using anything other than the socket-based signal (via signal_addr) will not work. if self.comm_port > 0: - signal_request = dict() - signal_request['signum'] = signum try: - self._send_listener_request(signal_request) + self._send_listener_request({"signum": signum}) if signum > 0: # Polling (signum == 0) is too frequent self.log.debug("Signal ({}) sent via gateway communication port.".format(signum)) return None except Exception as e: - if isinstance(e, OSError) and e.errno == errno.ECONNREFUSED: # Return False since there's no process. + if ( + isinstance(e, OSError) and e.errno == errno.ECONNREFUSED + ): # Return False since there's no process. + self.log.debug( + "ERROR: ECONNREFUSED, no process listening, cannot send signal." + ) return False self.log.warning("An unexpected exception occurred sending signal ({}) for KernelID '{}': {}" From 612a22646c302a36addc8482c507d746693491eb Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Wed, 21 Apr 2021 14:23:42 -0700 Subject: [PATCH 4/9] doc fixes --- enterprise_gateway/services/processproxies/processproxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index d9c475b2c..e509c1b48 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -349,7 +349,7 @@ def __init__(self, kernel_manager, proxy_config): kernel_manager : RemoteKernelManager The kernel manager instance tied to this process proxy. This drives the process proxy method calls. - proxy_config: dict + proxy_config : dict The dictionary of per-kernel config settings. If none are specified, this will be an empty dict. """ self.kernel_manager = kernel_manager From 39b208c1a4347ff2a8d64aadb7a7e64df0297250 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Sat, 24 Apr 2021 10:36:53 -0700 Subject: [PATCH 5/9] Move extracting and testing env varaible to __init__ --- .../services/processproxies/processproxy.py | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index e509c1b48..f9fd32d2e 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -392,8 +392,20 @@ def __init__(self, kernel_manager, proxy_config): self.ip = None self.pid = 0 self.pgid = 0 - self.remote_user = None - self.remote_pwd = None + _remote_user = os.getenv("EG_REMOTE_USER") + self.remote_pwd = os.getenv("EG_REMOTE_PWD") + self.use_gss = os.getenv("EG_REMOTE_GSS_SSH") + if self.use_gss: + if self.remote_pwd or _remote_user: + warnings.warn( + "Both `EG_REMOTE_GSS_SSH` and one of `EG_REMOTE_PWD` or " + "`EG_REMOTE_USER` is set. " + "Those options are mutually exclusive, you configuration may be incorrect. " + "EG_REMOTE_GSS_SSH will take priority." + ) + self.remote_user = None + else: + self.remote_user = _remote_user if _remote_user else getpass.getuser() @abc.abstractmethod async def launch_process(self, kernel_cmd, **kwargs): @@ -579,25 +591,11 @@ def _get_ssh_client(self, host): """ ssh = None - use_gss = os.getenv("EG_REMOTE_GSS_SSH", None) - if self.remote_user is None: - self.remote_pwd = os.getenv( - "EG_REMOTE_PWD" - ) # this should use password-less ssh - self.remote_user = os.getenv("EG_REMOTE_USER", getpass.getuser()) - - if use_gss and (self.remote_pwd or self.remote_user): - warnings.warn( - "Both `EG_REMOTE_GSS_SSH` and one of `EG_REMOTE_PWD` or `EG_REMOTE_USER` is set. " - "Those options are mutually exclusive, you configuration may be incorrect. " - "EG_REMOTE_GSS_SSH will take priority." - ) - try: ssh = paramiko.SSHClient() ssh.load_system_host_keys() host_ip = gethostbyname(host) - if use_gss: + if self.use_gss: self.log.debug("Connecting to remote host via GSS.") ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) ssh.connect(host_ip, port=ssh_port, gss_auth=True) @@ -631,6 +629,9 @@ def _get_ssh_client(self, host): error_message = error_message_prefix + ( " provided" if self.remote_pwd else "-less SSH" ) + error_message = error_message + "and EG_REMOTE_GSS_SSH={}".format( + self.use_gss + ) self.log_and_raise(http_status_code=http_status_code, reason=error_message) return ssh From e8daac65b46280c6e5bc08bea363ba74ac512a0a Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 4 May 2021 18:51:31 -0700 Subject: [PATCH 6/9] Add docs and be stricter on bool variable. In SSH tunneling section add information about GSS and Kerberos. I realised that this is bool env, and that we likely want to be stricter on which values are allowed, we don't want "0" to mean enable. Thus only allow "True", "False", empty string and unset (case insensitive) as possible values. Store the raw variable and its boolean interpretation separately for error messages. --- docs/source/config-options.md | 4 +++- docs/source/getting-started-security.md | 8 ++++++++ .../services/processproxies/processproxy.py | 14 +++++++++++--- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/source/config-options.md b/docs/source/config-options.md index 3fd59abc9..f1902c4be 100644 --- a/docs/source/config-options.md +++ b/docs/source/config-options.md @@ -339,7 +339,7 @@ RemoteMappingKernelManager(AsyncMappingKernelManager) options ``` -### Addtional supported environment variables +### Additional supported environment variables The following environment variables can be used to influence functionality and are not tied to command-line options: ```text EG_DEFAULT_KERNEL_SERVICE_ACCOUNT_NAME=default @@ -460,6 +460,8 @@ The following environment variables can be used to influence functionality and a EG_REMOTE_GSS_SSH=None Use gss instead of EG_REMOTE_USER and EG_REMOTE_PWD to connect to remote host via SSH. + Case insensitive. 'True' to enable, 'False', '' or unset to disable. + Any other value will error. EG_YARN_CERT_BUNDLE= The path to a .pem or any other custom truststore used as a CA bundle in yarn-api-client. diff --git a/docs/source/getting-started-security.md b/docs/source/getting-started-security.md index 8d2af9b38..e82db6bae 100644 --- a/docs/source/getting-started-security.md +++ b/docs/source/getting-started-security.md @@ -126,6 +126,14 @@ Please perform necessary steps to validate all hosts before enabling SSH tunneli * SSH to each node cluster and accept the host key properly * Configure SSH to disable `StrictHostKeyChecking` +### Using Generic Security Service (Kerberos) + +Jupyter Enterprise Gateway have support for SSH connection using GSS (for example Kerberos), this allow deployment +without use of ssh key, the `EG_REMOTE_GSS_SSH` environment variable can be used to control this behavior. + +See [list of additional supported environment variables](config-options.html#additional-supported-environment-variables). + + ### Securing Enterprise Gateway Server #### Using SSL for encrypted communication diff --git a/enterprise_gateway/services/processproxies/processproxy.py b/enterprise_gateway/services/processproxies/processproxy.py index f9fd32d2e..7034b9313 100644 --- a/enterprise_gateway/services/processproxies/processproxy.py +++ b/enterprise_gateway/services/processproxies/processproxy.py @@ -394,7 +394,13 @@ def __init__(self, kernel_manager, proxy_config): self.pgid = 0 _remote_user = os.getenv("EG_REMOTE_USER") self.remote_pwd = os.getenv("EG_REMOTE_PWD") - self.use_gss = os.getenv("EG_REMOTE_GSS_SSH") + self._use_gss_raw = os.getenv("EG_REMOTE_GSS_SSH", "False") + if self._use_gss_raw.lower() not in ("", "true", "false"): + raise ValueError( + "Invalid Value for EG_REMOTE_GSS_SSH expected one of " + '"", "True", "False", got {!r}'.format(self._use_gss_raw) + ) + self.use_gss = self._use_gss_raw == "true" if self.use_gss: if self.remote_pwd or _remote_user: warnings.warn( @@ -629,8 +635,10 @@ def _get_ssh_client(self, host): error_message = error_message_prefix + ( " provided" if self.remote_pwd else "-less SSH" ) - error_message = error_message + "and EG_REMOTE_GSS_SSH={}".format( - self.use_gss + error_message = ( + error_message + "and EG_REMOTE_GSS_SSH={!r} ({})".format( + self._use_gss_raw, self.use_gss + ) ) self.log_and_raise(http_status_code=http_status_code, reason=error_message) From ff769da163f034f9e050ebed179bf3bee3bee2a2 Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 11 May 2021 08:30:12 -0700 Subject: [PATCH 7/9] Apply suggestions from code review Co-authored-by: Kevin Bates --- docs/source/getting-started-security.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/getting-started-security.md b/docs/source/getting-started-security.md index e82db6bae..5d6a963cc 100644 --- a/docs/source/getting-started-security.md +++ b/docs/source/getting-started-security.md @@ -128,7 +128,7 @@ Please perform necessary steps to validate all hosts before enabling SSH tunneli ### Using Generic Security Service (Kerberos) -Jupyter Enterprise Gateway have support for SSH connection using GSS (for example Kerberos), this allow deployment +Jupyter Enterprise Gateway has support for SSH connections using GSS (for example Kerberos), which enables its deployment without use of ssh key, the `EG_REMOTE_GSS_SSH` environment variable can be used to control this behavior. See [list of additional supported environment variables](config-options.html#additional-supported-environment-variables). From ec69048cc8e5b9423ce159480dfae6907c62e60c Mon Sep 17 00:00:00 2001 From: Matthias Bussonnier Date: Tue, 11 May 2021 08:30:23 -0700 Subject: [PATCH 8/9] Update docs/source/getting-started-security.md Co-authored-by: Kevin Bates --- docs/source/getting-started-security.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/getting-started-security.md b/docs/source/getting-started-security.md index 5d6a963cc..b50c101ae 100644 --- a/docs/source/getting-started-security.md +++ b/docs/source/getting-started-security.md @@ -129,7 +129,7 @@ Please perform necessary steps to validate all hosts before enabling SSH tunneli ### Using Generic Security Service (Kerberos) Jupyter Enterprise Gateway has support for SSH connections using GSS (for example Kerberos), which enables its deployment -without use of ssh key, the `EG_REMOTE_GSS_SSH` environment variable can be used to control this behavior. +without the use of an ssh key. The `EG_REMOTE_GSS_SSH` environment variable can be used to control this behavior. See [list of additional supported environment variables](config-options.html#additional-supported-environment-variables). From 5abe75dcde11a91e9d56f9027644205fb667d522 Mon Sep 17 00:00:00 2001 From: Alan Chin Date: Fri, 21 May 2021 12:09:35 -0700 Subject: [PATCH 9/9] Update docs/source/config-options.md --- docs/source/config-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/config-options.md b/docs/source/config-options.md index f1902c4be..7c0717944 100644 --- a/docs/source/config-options.md +++ b/docs/source/config-options.md @@ -458,7 +458,7 @@ The following environment variables can be used to influence functionality and a The username to use when connecting to remote hosts (default to `getpass.getuser()` when not set). - EG_REMOTE_GSS_SSH=None + EG_REMOTE_GSS_SSH=False Use gss instead of EG_REMOTE_USER and EG_REMOTE_PWD to connect to remote host via SSH. Case insensitive. 'True' to enable, 'False', '' or unset to disable. Any other value will error.