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

passing $MOSH_KEY in environment a shaky choice security-wise #156

Open
mitchblank opened this issue Apr 12, 2012 · 9 comments · May be fixed by #521
Open

passing $MOSH_KEY in environment a shaky choice security-wise #156

mitchblank opened this issue Apr 12, 2012 · 9 comments · May be fixed by #521
Labels
Milestone

Comments

@mitchblank
Copy link

Background: in the process image argv[] and envp[] are stored in the same way, next to each other. In "classic" UNIXes /usr/bin/ps was typically setgid "kmem" (or similar group), which allowed it to dig around in /dev/kmem to read information about the active processes. This included the ability to read the process arguments AND the environment, of all users on the system.

These days these "privileged ps hacks" are largely behind us: UNIX systems have all come up with different ways of querying such information (/proc on Linux, etc) I think all(?) of these consider a process's environment only to be readable by its uid. Thus, security-sensitive data like passwords in the environment aren't leaked.

However, the old ways aren't 100% dead. Just as an example, here's an example from an AIX 5.2 machine I have access to, running as a non-root user:

$ ps ewwwax | grep cron | grep -v grep
 352378      - A     0:20 /usr/sbin/cron _=/usr/sbin/cron LANG=en_US PATH=/usr/bin:/etc:/usr/sbin:/usr/ucb:/usr/bin/X11:/sbin:/home/java14/jre/bin:/home/java14/bin LC__FASTMSG=true LOCPATH=/usr/lib/nls/loc ODMDIR=/etc/objrepos PWD=/ TZ=PST8PDT NLSPATH=/usr/lib/nls/msg/%L/%N:/usr/lib/nls/msg/%L/%N.cat 

Today, mosh seems to be only on Linux/OSX/FreeBSD but if it catches on I'm afraid it will end up on some platform that still uses the old-school UNIX "everybody can see anybody's environment variables" model. Since $MOSH_KEY is basically the keys-to-the-kingdom this would be a humungous hole.

There's a well established pattern for dealing with this: send the key over a pipe. So the call looks like:

$ mosh-client --key-fd=3 10.1.2.3 60001

...and the parent process has a pipe open on fd=3 when it exec's mosh-client which it can write the key to.

@kmcallister
Copy link
Contributor

We're aware of this issue, but thank you for giving us a report about AIX. We've been discussing what to do about it; at minimum we will add a warning in the build instructions.

@mitchblank
Copy link
Author

I guess one question would be is it worth to have the client split into separate perl and C++ halves at all? Just glancing at the perl script it doesn't look like it's doing anything that would be heinous to implement in C++. Then you could possibly roll the whole thing up into one binary (even the server could be just a command line flag like "--run-as-server" similar to how tools like rdist/rsync work)

Anyway -- great tool guys. Solves a real problem in a creative way. I'm looking forward to seeing its evolution.

@kmcallister
Copy link
Contributor

Indeed, we have a prototype of the wrapper in C++, and we've discussed merging it with mosh-client. It would solve this problem among others.

@kmcallister
Copy link
Contributor

We're considering merging the wrapper with mosh-client in Mosh 1.3. That would eliminate the env var key passing in the common case. I expect we would still support it for those users who launch mosh-server by some means other than SSH. But I'm uneasy about turning env var key passing into a more obscure, less tested alternative. Maybe we should also implement --key-fd=3 or such.

@cgull
Copy link
Member

cgull commented Feb 24, 2015

For the record, some while back we discovered (on IRC?) that OpenBSD 5.2 has this exact security exposure of leaking the environment to other local users (it was fixed shortly after that release, though). So this is not a theoretical security flaw. Ironically I'd implemented my pull request #521 before we discovered the OpenBSD issue.

@cgull
Copy link
Member

cgull commented Oct 30, 2015

The other night I realized that procfs on Linux allows anyone with permission to easily get access to that pipe and maybe read the key before mosh-client does. Urrgh. I do not love procfs. I think this is still a smaller exposure than using environment variables, but the difference is lot less clear cut than it was to me before.

@jeffschaller
Copy link

In response to mitchblank's earlier comment -- I got here via https://unix.stackexchange.com/a/369568/117549 -- AIX, at least by 6100-09, and also confirmed on 7.2, now prevents non-root users from seeing the environment of other users' processes with the "ps ewwwax" command.

@fidergo-stephane-gourichon
Copy link

Hello. Continuing on @cgull comment.

Issue

Yes /proc can be used to snoop around process activity, including messing with file descriptors passed between processed (e.g. /proc and directory permissions [LWN.net] -- not the same issue).

Solution

The good news is there appears to be a solution: call prctl(PR_SET_DUMPABLE, 0).
This forbids process to produce a core dump, and has additional effect of making entries /proc/$PID/* belong to root.

This is used by programs such as ssh-agent. It's easy to confirm with:

strace -eprctl ssh-agent -D >/dev/null

which outputs:

prctl(PR_SET_DUMPABLE, 0)               = 0

See discussion on:

Calling prctl(PR_SET_DUMPABLE, 0) may fix the issue raised by @cgull on #issuecomment-152403721.

What about mosh?

git log --all -Sprctl shows that mosh does not it so far.

Setting dumpable to false with all its effects seems reasonable for a process that handles security keys, isn't it? ssh-agent doing it is a reason to consider.

What do you think?

@keithw
Copy link
Member

keithw commented Jan 26, 2018

FYI mosh-client and mosh-server do use setrlimit( RLIMIT_CORE, ... ) to set rlim_cur to 0, to prevent dumping core containing the session key. See

mosh/src/crypto/crypto.cc

Lines 291 to 308 in ddb5fd9

/* Disable dumping core, as a precaution to avoid saving sensitive data
to disk. */
void Crypto::disable_dumping_core( void ) {
struct rlimit limit;
if ( 0 != getrlimit( RLIMIT_CORE, &limit ) ) {
/* We don't throw CryptoException because this is called very early
in main(), outside of 'try'. */
perror( "getrlimit(RLIMIT_CORE)" );
exit( 1 );
}
saved_core_rlimit = limit.rlim_cur;
limit.rlim_cur = 0;
if ( 0 != setrlimit( RLIMIT_CORE, &limit ) ) {
perror( "setrlimit(RLIMIT_CORE)" );
exit( 1 );
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants