ghantoos / lshell

lshell is a shell coded in Python, that lets you restrict a user's environment to limited sets of commands, choose to enable/disable any command over SSH (e.g. SCP, SFTP, rsync, etc.), log user's commands, implement timing restriction, and more.
GNU General Public License v3.0
435 stars 112 forks source link

prevent escaping lshell using sudo_noexec #122

Closed lberra closed 8 years ago

lberra commented 8 years ago

at the moment it is possible to escape lshell if an allowed command can execute an arbitrary non allowed one: example: find ~ -name .lhistory -exec bash \; (yes default config disallows ";" but it can be enabled) i prevented this by creating an alias: aliases : {'find':'LD_PRELOAD=/usr/lib/sudo/sudo_noexec.so find'} but it would be cool having an option for this

path_noxec: '/path/to/noexec.so'
allowed_noexec: ['find', 'vi' ]

which would preload the noexec library to the restricted program

ghantoos commented 8 years ago

:+1: Amazing idea!! Thank you for sharing this! I've been wanting to work on adding noexec to prevent such commands to use the shell escape, but never thought of it this way.

I will work on it to integrate it. I'll keep you posted.

ghantoos commented 8 years ago

I am going to use the LD_PRELOAD for all the allowed commands, regardless of the fact that they allow shell escapes (e.g. vim, find) or not. I have a first patch that seems functional. I will commit it soon, it would be great if you could test it out.

ghantoos commented 8 years ago

I have tested the patch on Debian and CentOS. It would be great if you could test it too.

lberra commented 8 years ago

i am sorry but this is easily bypassed: echo a && find . -name .lhistory -exec bash \; would become LD_PRELOAD=/usr/libexec/sudo_noexec.so echo a && find . -name .lhistory -exec bash \; which would set the noexec to echo, but not on find setting LD_PRELOAD before subprocess.call wont work, since we have shell=True i tried a different approach, autocreating aliases also i made the path to the shlib configurable if not autodetected see #123

ghantoos commented 8 years ago

Nice again @lberra ! I will take a look at your PR. Ideally, I would like to avoid adding a new variable. Some administrator might not know which command allows shell-escapes, and miss them.

I'm looking into forcing the LD_PRELOAD as a global environment variable. This would make it available for all the commands on a command line. More to follow soon. :)

lberra commented 8 years ago

if you want to set it global do something like

if os.path.isfile(noexec): return 'export LD_PRELOAD=%s;' % noexec

remember that setting it globally will break shell scripts

we could invert the settings, making allowed run commands through noexec and allowed_exec bypass the restriction putting a big fat warning

ghantoos commented 8 years ago

I have tested the following, and it seems to work:

    retcode = subprocess.call("%s" % cmd,
                              env={'LD_PRELOAD':ld_preload},
                              shell=True)

I'll commit it in a minute, if you still have some time to test it out. :)

ghantoos commented 8 years ago

@lberra I have forced the git push, to overwrite master, you might need to re-clone. Sorry for that.

ghantoos commented 8 years ago

I'm not totally awake, lot's of --force. :)

ghantoos commented 8 years ago

we could invert the settings, making allowed run commands through noexec and allowed_exec bypass the restriction putting a big fat warning

I wonder if it should be allowed at all. Wouldn't that just reopen the door for escapes? In which scenario do you think it would be helpful?

lberra commented 8 years ago

setting LD_PRELOAD breaks shell scripts that execute external commands i could find useful creating some custom shell script and allowing the user to run it (in this case i could just reset LD_PRELOAD inside the script) it may also break other things: man comes to mind

ghantoos commented 8 years ago

On Feb 6, 2016 14:43, "lberra" notifications@github.com wrote:

setting LD_PRELOAD breaks shell scripts that execute external commands i could find useful creating some custom shell script and allowing the user to run it (in this case i could just reset LD_PRELOAD inside the script) it may also break other things: man comes to mind

Very true. I went too fast on a solution. I will revert my changes, and rethink the solution. As I see it, it will be a mix of your suggestion and mine: two types of allowed commands (exec and no exec), no aliases but env variable setting the LD_PRELOAD. With the warning you suggested. Also, I might reuse your list of sudo_noexec.so, it's quite generic.

ghantoos commented 8 years ago

Here is the plan. I prefer having the default command list to use "allowed" (instead of "allowed_noexec" in your PR). This will prevent from breaking the configuration with previous versions of lshell.

Instead, I will be adding another variable to explicitly allow commands to use exec (e.g. shell scripts). Otherwise, it seems that you alias solution is the best way to achieve want you have described.

To sum up, I will be integrating your PR, and working on top of it to introduce aliases for allowed commands, and a new variable called "allowed_shell_escape" for the exception list.

ghantoos commented 8 years ago

Thank you so much for your help on this @lberra !! This is very much appreciated!

lmarkov commented 8 years ago

Hello, I updated may configuration to new version 0.9.18 and I have problems with this new check. I have many custom scripts that can't be executed because of this restrictions. Can you suggest me how to bypass this check or allow access for the scripts without to put every one in the list of the configuration file?

ghantoos commented 8 years ago

@lmarkov there is currently no way to bypass the script without adding them to the allowed_shell_escape variable.

There is actually a tiny bug preventing you from bypassing the standard noexec path. I will correct this now. If you clone the latest code, you will be able to set another path_noexec, for example on my Debian: path_noexec : '/lib/x86_64-linux-gnu/libc.so.6'.

I will add the possibility to disable this, with a big security warning.

lmarkov commented 8 years ago

I commented all lines in def set_noexec(self): in file checkconfig.py. I think that this is solution for so far.

ghantoos commented 8 years ago

@lmarkov I have just comited the possibility to disable the LD_PRELOAD by setting the value of path_noexec to ''. See https://github.com/ghantoos/lshell/commit/d9e7eea92952397a256c5a079023b2a7df76b363 and https://github.com/ghantoos/lshell/issues/133.