Support transmission of environment variables

[lorenzph]

The SSH protocol allows the client to transmit environment variables to
the server. This is particularly useful if the user wants to modify the
environment of an executed command without having to reexecute the
actual command from a shell.

This patch extends the Client and Channel interface to allow
the transmission of environment variables to the server side.

In order to use this feature the SSH server must accept environment
variables from the client (e.g. the AcceptEnv configuration directive of
OpenSSH).

[coveralls]

Coverage increased (+0.13%) when pulling 2a56886 on lorenzph:env-support into 84995f9 on paramiko:master.

[lndbrg]

Looks good.

[mkunzmann]

Hi,
whats the status with this pull request?
Is there any chance to see this getting merged in the near future.

[bitprophet]

[bitprophet]

Grump. Channel.update_environment_variables is not actually working for me in practice, despite the Channel object happily executing .exec_command. I get an exception upset about Channel closed..

The exception seems to do with the eventing stuff, _wait_for_event is falling down past the event_ready check and thus defaulting to assuming something is wrong, even when there is no stored exception. (...what?)

More digging:

• event_ready is never set to True outside of _request_success()
• self.event.set() is only called on _request_success() or `_set_closed()
• Those two together seem to imply why _wait_for_event behaves the way it does; if the event was .set() but event_ready is False, it implies a closure event.
• Sure enough, _set_closed is being called in my situation...
• ... by _request_failed via MSG_CHANNEL_FAILURE - so the server is trying to tell us it wasn't able to set the event.
  • so right off the bat this implies a bug in Paramiko re: failure handling, I should have been told about this and not just "oh lol the channel is closed now"
• Why it sent failure: probably because the default sshd_config on my server flavor says AcceptEnv LANG LC_* and my code was attempting to set something not one of those...
  • Confirmed, with AcceptEnv * things work much better

Take-aways:

• The docs need an update to warn users of this, because AcceptEnv * is arguably bad security practice and certainly appears to not be a common default.
• The changelog entry I left needs a tweak because, silly me, I was assuming this would be an acceptable generic method for setting the env, in place of crap like exec_command("FOO=bar BIZ=baz mycommand").
  • My guess is that the main differentiator between that and what this PR does, security-wise, is that SSH env channel commands affect the server process handling the request, which may be running as root, while the actual shell command is running after privileges have been dropped to those of the connecting user. I should really know more about this.
• There's a bug re: MSG_CHANNEL_FAILURE handling. Don't know the best way to solve it offhand so I need to find/make a ticket for it.
• Update the message so it does NOT want a reply; this prevents exceptions, resulting in an annoying silent failure that is nonetheless in line with how OpenSSH's client behaves.

[bitprophet]

Seems one of the specific problems with AcceptEnv is it opens one up to exploits like the one described under LD_PRELOAD Exploit here: http://www.dankalia.com/tutor/01005/0100501004.htm

It's still unclear to me exactly how this sort of attack works against the sshd, because I don't know exactly what happens between recipient of "please exec this command for me" and creation of the user-owned subprocess. I'm struggling to figure out how the root-privileged sshd is the one executing an actual, LD_PRELOAD-honoring program, vs that execution occurring after privileges are dropped. Kind of assume it's just using syscalls which wouldn't be affected...?

Put another way, why is this exploit not simply a vanilla local privilege escalation? Why does it have to go via an sshd?

[bitprophet]

(Note - I am just digging into the security implications of this for my own edification; it's not possible to change the rest of the world so I still need to drop those warnings/notices into the docs. Just wondering whether I can feasibly use this particular feature myself in higher-level code or not...)

[bitprophet]

Took another tack to see exactly how the env vars set with the env message are being used:

• the message is interpreted in session.c by session_input_channel_req(), calling session_env_req()
• session_env_req() simply updates s->env with the given name & value
• The only references to ->env in the code are on session close (to free the memory), the above setting bits, and in do_setup_env() (still in session.c)
• do_setup_env() makes many calls, including one for each of those ->env members, to child_set_env(), passing in an env array (generated via xcalloc) to be mutated
• child_set_env() simply adds a new string to that env array of the format, surprise, name=var.

So all of this so far is, as expected, updating a memory value holding the env var map. Question is what's done with this array?

• do_setup_env is only called once, in (again still in session.c) do_child()
• do_child is quite meaty, doing all the things one would expect for a command execute (scrub sensitive memory from parent, close file descriptors, load shell path from user's passwd entry, change directory to user's home directory from same, etc)
  • Notably, the password entry data is also put into the session object (s) before do_child is called, so there's other "setup-y" things happening before we get here; and I'm still unclear on what our privileges look like at this point in time.
• At one point, we see a strange line, environ = env, despite environ not being used after that point. Why? Turns out there's some fun global BS going on - do_child, and other functions around the codebase, use extern char **environ.
  • However, I looked at all the other references to it, and I'm not seeing it really referenced anywhere that matters for remote exec - it's all in commented-out-via-ifdef or client-related code.
• After the env is set up, do_rc_files is called, which executes e.g. ~/.ssh/rc - an arbitrary script. However, this seems to involve popen and no reference to the stored env array is used at all.
• At the end of do_child, we see the expected execve() call, which uses the env.
  • Thus implying that this code is expected to run unprivileged...meaning the session env setup is...never used in an unprivileged manner?
  • So I'm still at a loss as to how one could run the exploit in this manner. Was it an older OpenSSH bug that has been fixed since? Or am I still missing something really obvious?

[radssh]

Looks like the standard reference implementation (OpenSSH client) sends the env requests with want_reply=False. The PR is sending True in the request message, which is the source of the MSG_CHANNEL_FAILURE that triggers the channel to close when the server declines based on non-matching AcceptEnv entries.

While it would be nice to be able to tell the client which (if any) Env requests failed, that would rely on fixing the uncovered buggy behavior of Channel._request_failed(). In the interim, changing the request's want_reply=False looks like it might be an adequate solution for this, and buy time to properly address the Channel._request_failed() handling, which looks like a big enough gaffe to warrant its own tracking issue :)

[bitprophet]

Yea, privileges are dropped well before any of that stuff happens; once the sshd main process accepts connections it authenticates, drops privileges, sets up user context for the connected user, and spawns a new child that does the rest of the work re: handling subsequent messages from the client. So everything from acceptance of the env messages, to exec-related setup, to actual exec, should be happening with effective unprivileged uid.

Which brings me back to: how is this an exploitable channel if it's functionally identical in terms of privileges to a user running things in a local shell? Re: the specific LD_PRELOAD exploit, it would seem like prevention is solely up to the loader, and that it's been fixed in most loaders for some time: http://stackoverflow.com/questions/426230/what-is-the-ld-preload-trick

So I'm starting to wonder if the info out there re: why AcceptEnv is considered insecure (I started at http://serverfault.com/questions/427522/why-is-acceptenv-considered-insecure...maybe all this poking has been a SO-induced time waster of sorts. Though I now know the OpenSSH codebase much better...) is inaccurate.

---

Digging deeper:

Darwin's sshd_config manpage says:

Be warned that some environment variables could be used to bypass restricted user environments.

Which is pretty vague and I'm interpreting it to mean "hey, if you have your login or ssh keys or etc configured such that users can only run certain programs / run in a certain environment, AcceptEnv + env messages could work around that". Good to know, but not an argument against someone explicitly setting AcceptEnv * if they trust their users / aren't trying to limit them in a specific manner.

Ubuntu's manpage has identical language...as does OpenBSD's: http://man.openbsd.org/sshd_config

EDIT: ah, ok. Part of the problem is that the SO poster conflated AcceptEnv with PermitUserEnv, which are distinct options; it is the latter that includes language about LD_PRELOAD (and it does so on all platforms that I have checked). Now, because I'm curious, I'll dig into that a bit to see what the distinction is; if my above analysis is accurate, it would need to affect the sshd before the privilege drop, in order to be problematic.

[bitprophet]

@radssh Good catch on the reference client implementation. I'd guess that matching it on our end would turn rejections into silent failures? Certainly seems the case with ssh; all env settings result in debug log lines like:

debug1: Sending env FOOBAR = whatever
debug2: channel 0: request env confirm 0

even with a default/limited AcceptEnv on the server side.

So that sounds like another worthwhile change to make to this, if orthogonal to my own personal quandary :D

[bitprophet]

Checking on PermitUserEnv; there's two things it impacts, public key environment= clauses, and ~/.ssh/environment files. However, when I dug, the env data found in either are only referenced within the same do_setup_env function that's called after privileges are dropped.

So for better or worse it appears both of these sshd options have similar security impact.

---

More generic googling about this issue finds a seeming confirmation of my assumptions here: http://serverfault.com/a/527648 - the idea here is that if you're locking down SSH to specific commands or subsystems, allowing LD_PRELOAD to be set could let users break out of it.

Even in that case I'm wondering exactly what limited access could include setuid apps without full shell access. SFTP server locking users down to password-based SFTP and the ability to execute passwd (to change their SFTP password) is one that comes to mind, I guess. SFTP upload malicious .so file, set LD_PRELOAD to that file, run passwd, and bam, you're root.

But at least it seems I'm not missing something: in the "typical" use case of "your users are all shell users", AcceptEnv * is not really a security risk.

[bitprophet]

Enacted my checklist above, I'm all done with this again, for now.

[bitprophet]

Originally worked on this on master for 2.1.0 release, but now that I am going to pop out a 1.18 release, backporting it there too. Should thus appear in both 1.18 and 2.1.