Skip to content
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

[WIP] LocalProcessProxy Kernels: user impersonate & working_dir #971

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

cceyda
Copy link

@cceyda cceyda commented May 21, 2021

If running kernel gateway as root, launching local python kernels launches them as root!

  • This PR enables user impersonation for LocalProcessProxy kernels. User information is extracted from KERNEL_USERNAME env variable. (Not sure if this is the most secure approach)
  • Also fixes KERNEL_WORKING_DIR not being setup when using a LocalProcessProxy kernel.

Related issue: #789

Please provide feedback~

What is a local kernel?

Example:

{"metadata": {
    "process_proxy": {
      "class_name": "enterprise_gateway.services.processproxies.processproxy.LocalProcessProxy",
                "config":{  }
    }
  },
 "argv": [
  "python3",
  "-m",
  "ipykernel_launcher",
  "-f",
  "{connection_file}"
 ],
 "display_name": "Python 3 (Local)",
 "language": "python"
}

Why run gateway as root?

So I can launch containers/kernels on behalf of users.

My setup:
JupyterHub (managed by root) -> Jupyterlab (user jupyterlab instance spawned by jupyterhub) -> Gateway Kernels (managed by root)

  • Style check code

  • Better error messages

if username:
uid=getpwnam(username).pw_uid
guid=grp.getgrnam(username).gr_gid
os.chown(self.connection_file,uid,guid)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to change ownership of connection files. Because connection files are created by the gateway process (root) and when the kernel is launched as a user Permission denied happens. Alternative approaches are welcome~

@kevin-bates
Copy link
Member

Hi @cceyda - thanks for opening this pull request.

I guess I'd like to better understand your need for using local kernels in a hub environment. The primary use-case of using local kernels with EG is for parity with Kernel Gateway environments in which each user spawns their own instance of the gateway. You mention using containerized kernels so I'd like to understand why you're not leveraging either of the container solutions that use Kubernetes or Docker-related process proxy implementations. Could you elaborate a bit more about the scenario you're working within?

I think you should also be setting the impersonation_enabled trait, either via the env (EG_IMPERSONATION_ENABLED), config EnterpriseGatewayApp.impersonation_enabled or command line --EnterpriseGatewayApp.impersonation_enabled. When enabled, a warning message (we could make it fatal if necessary) is issued when the gateway user (root in this case) is not also in the list of unauthorized_users so as to prevent kernels from being launched as the gateway user. (root is in the list of unauthorized users by default, so this should get issued once the impersonation flag is set.)

Regarding the changes, I think it's great that working-dir is getting applied to the LocalProcessProxy. However, regarding the setuid, I'd rather not introduce a dependency on jupyterhub, so we'd need a different means of doing that - of which I'm sure there are many.

First, though, I want to make sure using the LocalProcessProxy is the right approach for this, especially if these kernels will be running in their own containers.

Thank you.

@cceyda
Copy link
Author

cceyda commented May 21, 2021

EG is for parity with Kernel Gateway environments in which each user spawns their own instance of the gateway.

I would rather avoid each user having to spin their own kernel gateway.

When Jupyterhub & gateway is run by an admin (let's say root):
Users can just login to jupyterhub and be welcomed with a jupyterlab instance with preset kernels (defined in /usr/share/jupyter/kernels). Users can also add their own kernels, if they choose to, by putting it in ~/.local/share/jupyter/kernel...) The current user based kernelspec filtering works really nicely in this scenario.

This also gives users the flexibility to choose between using dockerized kernels or conda kernels or spark or just the local kernel. I think conda kernels also get launched using LocalProcessProxy.

I'd like to understand why you're not leveraging either of the container solutions that use Kubernetes or Docker-related process proxy implementations.

Honestly for small setups (with 1-2servers 3-6 users) managing Kubernetes is not really easy (or necessary).
Not every user using the server (through jupyterhub) knows docker or wants to use it (Dockerized kernels come with their own hurdles of mounting volumes etc)
Some people really love their conda.
some don't care and just want the main python kernel.

I think you should also be setting the impersonation_enabled trait.

yes~ partially my reason for naming it user impersonate. I think we should do this impersonation only if EG_IMPERSONATION_ENABLED=True. I think user authorization check is already done by _enforce_authorization in BaseProcessProxyABC in launch_process. which LocalProcessProxy inherits and calls super in launch_process. I will recheck just in case. (authorized list is extracted from the kernel config)

I'd rather not introduce a dependency on jupyterhub

Yep~ That was my lazy way of testing if this implementation worked. I will replace it with something standalone later.

@@ -571,8 +573,17 @@ def write_connection_file(self):
self.hb_port = ports[3]
self.control_port = ports[4]

return super(RemoteKernelManager, self).write_connection_file()
cf = super(RemoteKernelManager, self).write_connection_file()
username=self.user_overrides.get('KERNEL_USERNAME',None)
Copy link
Author

@cceyda cceyda May 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to get the username at this point? like:
username = KernelSessionManager.get_kernel_username(**kwargs)

It would be the best to write_connection_file to the correct user rather than chown-ing it later. But idk https://github.com/jupyter/jupyter_client/blob/98faaf91d00b4fbf17b2fbf0b08b1fd23adc27bf/jupyter_client/connect.py#L42

user=kwargs["env"].get("KERNEL_USERNAME",None)
self.log.info(f"KARGS {kwargs}")

if user:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can check here if EG_IMPERSONATION_ENABLED & user. (The current EG behavior ignores the KERNEL_USERNAME)

btw,I will also remove unnecessary logging, it was to help me understand how EG works.

@@ -147,7 +149,7 @@ async def start_kernel(self, *args, **kwargs):
username = KernelSessionManager.get_kernel_username(**kwargs)
self.log.debug("RemoteMappingKernelManager.start_kernel: {kernel_name}, kernel_username: {username}".
format(kernel_name=kwargs['kernel_name'], username=username))

self.connection_dir=os.path.join(os.path.expanduser('~'+username),".local/share/jupyter/runtime/")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.connection_dir is normally determined by JUPYTER_RUNTIME_DIR but if this is not in user Paths in which to search for connection files it breaks so I did another hacky update. https://github.com/jupyter/jupyter_client/blob/98faaf91d00b4fbf17b2fbf0b08b1fd23adc27bf/jupyter_client/connect.py#L191

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to call jupyter_core.paths.get_home_dir() to, presumably get /root, then replace that portion of jupyter_runtime_dir() with the target user's home directory? Otherwise, this isn't producing the correct path relative to the platform.

We should also ensure the user's home directory exists, since KERNEL_USERNAME is not necessarily reflective of a user. (I suppose setuid will enforce that constraint though.)

How will these kinds of files be cleaned up?

self.connection_dir is on the multi kernel manager which spans all kernel manager instances - so this isn't going to work. I'm not sure the connection information needs to be isolated on a per user level. It isn't today at least.

@kevin-bates
Copy link
Member

Probably best just to replicate the conversation - thank you for your response.

EG is for parity with Kernel Gateway environments in which each user spawns their own instance of the gateway.

I would rather avoid each user having to spin their own kernel gateway.

OK - fair enough, Just be aware that in this case, the EG server will require the total resources consumed by each active (and local) kernel across all users.

This also gives users the flexibility to choose between using dockerized kernels or conda kernels or spark or just the local kernel.

Keep in mind that unless EG is running in a matching containerized environment, docker or k8s kernels cannot be launched.

I think conda kernels also get launched using LocalProcessProxy.

Correct, conda kernels essentially fabricate the kernelspec so, in that case, they can only be local kernels.

I'd like to understand why you're not leveraging either of the container solutions that use Kubernetes or Docker-related process proxy implementations.

Honestly for small setups (with 1-2servers 3-6 users) managing Kubernetes is not really easy (or necessary).
Not every user using the server (through jupyterhub) knows docker or wants to use it (Dockerized kernels come with their own hurdles of mounting volumes etc)
Some people really love their conda.
some don't care and just want the main python kernel.

I see. I misunderstood this comment from your description: So I can launch containers/kernels on behalf of users.
So it sounds like you have EG hosted in a containerized env, but want to better "isolate" users that want to run their kernels locally for whatever reason, etc. That makes sense - provided you're aware of the resource ramifications that this introduces.

I think you should also be setting the impersonation_enabled trait.

yes~ partially my reason for naming it user impersonate. I think we should do this impersonation only if EG_IMPERSONATION_ENABLED=True. I think user authorization check is already done by _enforce_authorization in BaseProcessProxyABC in launch_process. which LocalProcessProxy inherits and calls super in launch_process. I will recheck just in case. (authorized list is extracted from the kernel config)

Correct. The impersonation that exists today is really only used by YARN/Spark, but this capability at the local level seems useful, and, yes, we should only perform setuid if enabled. I also think we should require that the gateway user be unauthorized from launching local kernels.

(authorized list is extracted from the kernel config)

It is also configured at a global level and "unauthorized" users take precedence no matter where they are defined, whereas authorized users at the kernel level can override globally configured authorized users.

I'd rather not introduce a dependency on jupyterhub

Yep~ That was my lazy way of testing if this implementation worked. I will replace it with something standalone later.

Right on. Thanks.

@cceyda
Copy link
Author

cceyda commented May 21, 2021

aha, The EG I'm running is not contained right now (running on bare metal).
so I guess it makes more sense to use localprocessproxy & dockerprocessproxy (custom launcher).
whereas a contained EG would have no reason to allow localprocessproxy & launched kernels would all be remote.

I probably have an unconventional setup. Roughly depicted here:
image
*we plan on scaling up later thus chose EG over kernel gateway.

@kevin-bates
Copy link
Member

Thanks for the diagram. Got a few questions, observations:

  1. EG can't spawn "contained" kernels w/o also being "contained" itself. Are you finding otherwise?
  2. I'm curious how Hub knows which "server" to hit for user B? I thought it was one-to-one between a user and its spawned server.
  3. The Python and Conda Kernel ovals will share the resources available on the EG bare metal server from which they are spawned - so that should be taken into consideration.
  4. Are the Docker sqauares using DockerProcessProxy or DockerSwarmProcessProxy? If the former, they will run on the same host as EG (assuming EG was contained) while DockerSwarmProcessProxy launches the kernel as a docker service - which can land on a different host.

@cceyda
Copy link
Author

cceyda commented May 21, 2021

  1. EG can't spawn "contained" kernels w/o also being "contained" itself. Are you finding otherwise?

I'm using a custom ProcessProxy for launching docker containers (custom images), which also adds some logic about volumes to mount.
why can't EG spawn containers unless contained? Is it a security thing?

  1. I'm curious how Hub knows which "server" to hit for user B? I thought it was one-to-one between a user and its spawned server.

I'm using a custom spawner with jupyerhub, it takes the server address as input. If c.JupyterHub.allow_named_servers=True users can have more than one jupyterlab instance.

  1. The Python and Conda Kernel ovals will share the resources available on the EG bare metal server from which they are spawned - so that should be taken into consideration.

Yes, in my case it is okay if resources are shared (it ends up that way anyway, whether we use jupyter or not). Also containerization is mostly for the sake of reproducibility/convenience and not security isolation.

  1. Are the Docker sqauares using DockerProcessProxy or DockerSwarmProcessProxy? If the former, they will run on the same host as EG (assuming EG was contained) while DockerSwarmProcessProxy launches the kernel as a docker service - which can land on a different host.

I prefer using; one EG per machine + one JupyterLab(per user per machine) because it is easier than sharing the files between machines. So DockerProcessProxy logic has been enough. Also didn't bother setting docker swarm since I heard it is being superseded by Kubernetes. (Idealistically I want to use kubernetes, but don't have the energy to set everything up😅 )

@kevin-bates
Copy link
Member

I'm using a custom ProcessProxy for launching docker containers (custom images), which also adds some logic about volumes to mount.

Nice. Are you thinking about contributing that back to the project at all?

why can't EG spawn containers unless contained? Is it a security thing?

I thought it was a virtual network thing - but that might just be kubernetes and swarm and the docker process proxy just followed suit. Since they'll (DPP) always be on the same host, I could see that working although, in the big picture of EG, we want kernels off-host.

If you could address the build issues (make lint for starters), that would be great.

@cceyda cceyda changed the title LocalProcessProxy Kernels: user impersonate & working_dir [WIP] LocalProcessProxy Kernels: user impersonate & working_dir May 23, 2021
@cceyda
Copy link
Author

cceyda commented May 23, 2021

Nice. Are you thinking about contributing that back to the project at all?

I keep meaning to do that but my timeline is chaotic. I thought better to start with this smaller PR and work up to that one.

If you could address the build issues (make lint for starters), that would be great.

I will mark this as WIP, address the issues you mentioned later next weekend then ping you back for review 😄

@kevin-bates kevin-bates added this to the v3.0 milestone Jan 25, 2022
@kevin-bates kevin-bates modified the milestones: v3.0, Future Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants