Together-Java / TJ-Bot

TJ-Bot is a Discord Bot used on the Together Java server. It is maintained by the community, anyone can contribute.
https://togetherjava.org
GNU General Public License v3.0
101 stars 87 forks source link

Migrate JShell Bot #91

Closed Zabuzard closed 7 months ago

Zabuzard commented 3 years ago

We may want to migrate the JShell bot. It is actually another repo in this organization, see Together-Java/JShellBot.

For reference, this is how it works as of today:

!jshell <message> by Jshell bot for JShell support (executing Java code)

This bot has tight security restrictions as otherwise its easy for an user to escape the Java sandbox and access the actual system the bot is running on, which would be fatal. Also, the command should not be able to break the rest of the bot (its other commands).

Zabuzard commented 3 years ago

Naturally blocked by #59

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Zabuzard commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

Still relevant and one of the main targets that should be done in the first Migration milestone.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

illuminator3 commented 2 years ago

how do I implement this? last time I made a message command I was yelled at

Zabuzard commented 2 years ago

My 2 cents:

Vast majority of jshell usages in our server are short and not multi-line. So I would say we should have a slash command as well. For the multi-line usages, go with the message-id pattern u introduced with /tag for now.

And once ready for usage: context-commands maybe (right click > execute in jshell).

illuminator3 commented 2 years ago

not making this until we have context menus

illuminator3 commented 2 years ago

blocked by #382

marko-radosavljevic commented 2 years ago

I'm mostly concerned with the security aspect of it. It's critical to have jshell instance properly sandboxed, which is not trivial, especially if we want to keep all the functionality intact. And to be honest, because of the huge impact it can create, I'm not even sure if I would be comfortable reviewing it.

So I would recommend first addressing security aspect, after we agree on the strategy we can move to the implementation details.

Paging @I-Al-Istannen, since he co-authored current jshell bot. Istannen already shared some issues, worries and design ideas on the server that are valuable addition to this issue:

marko-radosavljevic commented 2 years ago

For a sandbox, we can go either containers or microVMs. MicroVMs have some security benefits which would make them preferable for a sandboxed environment, but I'm not familiar with that tech (for e.g. firecracker/kata). Big Virtelization on container sandboxes - "docker as a sandbox for untrusted code".

Since we already use docker in our stack, do we agree that we probably want:

  1. a docker container, so we have an isolated environment, by default we get

    • namespaces:
      • mount namespaces - container has its own filesystem and is unable to access host filesystem
      • network namespaces - container has its own network stack
      • pid namespaces - container has its own isolated pid space, so it can't see host processes just itself and it's children processes
      • user namespaces - container has its own isolated uids and gids. This feature allows for the root user in a container to be mapped to a non uid-0 user outside the container, which can help to mitigate the risks of container breakout. This facility is available but not enabled by default.
    • cgroups - limits resource usage (for e.g. cpu and memory), if process is compromised it can't starve all the resources and thus crash the host.
    • limited capabilities
    • seccomp - restrict which syscalls process can make, decreases the attack surface on the kernel
    • selinux/apparmor - linux security modules that can be used for fine-grained mandatory access control. Rules where subjects (processes or users) are allowed or denied to access objects (files, directories, sockets, etc.)
  2. harden the docker container, to create a sandbox for untrusted code

    • preferably, we completely disable the network stack
    • preferably, we drop all capabilities
    • preferably, read-only filesystem
    • possibly, we configure seccomp policies and selinux/apparmor profiles
  3. each user gets a new container

    • that way each user's execution is independant, and can't be affected by anyone else's
    • keeping execution state is much easier, so you can define a method and then call it with next command, and similar
  4. run jshell process as non-privildeged user inside the container

illuminator3 commented 2 years ago

I'm mostly concerned with the security aspect of it. It's critical to have jshell instance properly sandboxed, which is not trivial, especially if we want to keep all the functionality intact. And to be honest, because of the huge impact it can create, I'm not even sure if I would be comfortable reviewing it.

So I would recommend first addressing security aspect, after we agree on the strategy we can move to the implementation details.

Paging @I-Al-Istannen, since he co-authored current jshell bot. Istannen already shared some issues, worries and design ideas on the server that are valuable addition to this issue:

can't access two out of those links

illuminator3 commented 2 years ago

someone remind me to implement this during the autumn break in germany, nrw

btw the same thing has been done before but for C++ https://github.com/Eelis/geordi

illuminator3 commented 2 years ago

no one reminded me :angwy: will pick up the work now that we have context commands

Alathreon commented 11 months ago

This first completion wasn't good enough, and so JShell was never realed enabled in the bot.