[PATCH] Expand tilde for UNIX domain socket forwards.

[Lauri Võsandi]

---
channels.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/channels.c b/channels.c
index a84b487..396e192 100644
--- a/channels.c
+++ b/channels.c
@@ -3014,10 +3014,14 @@ channel_setup_fwd_listener_streamlocal(int type, struct Forward *fwd,

debug3("%s: type %d path %s", __func__, type, fwd->listen_path);

+ /* Expand home directory if necessary */
+ char *expanded_path = tilde_expand_filename(fwd->listen_path, getuid());
+
/* Start a Unix domain listener. */
omask = umask(fwd_opts->streamlocal_bind_mask);
- sock = unix_listener(fwd->listen_path, SSH_LISTEN_BACKLOG,
+ sock = unix_listener(expanded_path, SSH_LISTEN_BACKLOG,
fwd_opts->streamlocal_bind_unlink);
+ free(expanded_path);
umask(omask);
if (sock < 0)
return 0;

[Todd C. Miller]

I like the idea but tilde_expand_filename() calls fatal() if it
cannot resolve ~foo. This is not terrible when using -L and -R on
the normal command line but it seems pretty harsh to exit when -L
or -R are used via the ~C escape or the streamlocal-***@openssh.com
request.
Message-Id: <***@courtesan.com>

Perhaps we just need a non-fatal version of tilde_expand_filename().
Message-Id: <***@courtesan.com>

- todd

[Damien Miller]

Post by Todd C. Miller
I like the idea but tilde_expand_filename() calls fatal() if it
cannot resolve ~foo. This is not terrible when using -L and -R on
the normal command line but it seems pretty harsh to exit when -L
request.
Perhaps we just need a non-fatal version of tilde_expand_filename().

Yeah, we should refactor it into a version that returns a ssherr.h code
and (perhaps) leave the existing tilde_expand_filename() as a wrapper.

-d

[Todd C. Miller]

Post by Damien Miller
Yeah, we should refactor it into a version that returns a ssherr.h code
and (perhaps) leave the existing tilde_expand_filename() as a wrapper.

We could do something like this. The name stinks but...

- todd

Index: misc.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
retrieving revision 1.97
diff -u -p -u -r1.97 misc.c
--- misc.c 24 Apr 2015 01:36:00 -0000 1.97
+++ misc.c 19 Aug 2015 02:28:18 -0000
@@ -50,6 +50,7 @@
#include "xmalloc.h"
#include "misc.h"
#include "log.h"
+#include "ssherr.h"
#include "ssh.h"

/* remove newline at end of string */
@@ -499,29 +500,31 @@ freeargs(arglist *args)
* Expands tildes in the file name. Returns data allocated by xmalloc.
* Warning: this calls getpw*.
*/
-char *
-tilde_expand_filename(const char *filename, uid_t uid)
+int
+tilde_expand_filename2(const char *filename, char **expanded, uid_t uid)
{
const char *path, *sep;
char user[128], *ret;
struct passwd *pw;
- u_int len, slash;
+ size_t len, slash;

- if (*filename != '~')
- return (xstrdup(filename));
+ if (*filename != '~') {
+ *expanded = xstrdup(filename);
+ return SSH_ERR_SUCCESS;
+ }
filename++;

path = strchr(filename, '/');
if (path != NULL && path > filename) { /* ~user/path */
slash = path - filename;
if (slash > sizeof(user) - 1)
- fatal("tilde_expand_filename: ~username too long");
+ return SSH_ERR_PATH_TOO_LONG;
memcpy(user, filename, slash);
user[slash] = '\0';
if ((pw = getpwnam(user)) == NULL)
- fatal("tilde_expand_filename: No such user %s", user);
+ return SSH_ERR_UNKNOWN_USER;
} else if ((pw = getpwuid(uid)) == NULL) /* ~/path */
- fatal("tilde_expand_filename: No such uid %ld", (long)uid);
+ return SSH_ERR_UNKNOWN_USER;

/* Make sure directory has a trailing '/' */
len = strlen(pw->pw_dir);
@@ -534,10 +537,29 @@ tilde_expand_filename(const char *filena
if (path != NULL)
filename = path + 1;

- if (xasprintf(&ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX)
- fatal("tilde_expand_filename: Path too long");
+ if (xasprintf(ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX) {
+ free(ret);
+ return SSH_ERR_PATH_TOO_LONG;
+ }
+
+ *expanded = ret;
+ return SSH_ERR_SUCCESS;
+}
+
+/*
+ * Expands tildes in the file name. Returns data allocated by xmalloc.
+ * Warning: this calls getpw*.
+ */
+char *
+tilde_expand_filename(const char *filename, uid_t uid)
+{
+ char *ret;
+ int rc;

- return (ret);
+ rc = tilde_expand_filename2(filename, &ret, uid);
+ if (rc != SSH_ERR_SUCCESS)
+ fatal("%s: %s", __func__, ssh_err(rc));
+ return ret;
}

/*
Index: misc.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.h,v
retrieving revision 1.54
diff -u -p -u -r1.54 misc.h
--- misc.h 15 Jul 2014 15:54:14 -0000 1.54
+++ misc.h 19 Aug 2015 02:27:37 -0000
@@ -49,6 +49,7 @@ char *cleanhostname(char *);
char *colon(char *);
long convtime(const char *);
char *tilde_expand_filename(const char *, uid_t);
+int tilde_expand_filename2(const char *, char **, uid_t);
char *percent_expand(const char *, ...) __attribute__((__sentinel__));
char *tohex(const void *, size_t);
void sanitise_stdfd(void);
Index: ssherr.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssherr.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 ssherr.c
--- ssherr.c 16 Feb 2015 22:13:32 -0000 1.4
+++ ssherr.c 19 Aug 2015 02:19:05 -0000
@@ -135,6 +135,10 @@ ssh_err(int n)
return "Connection corrupted";
case SSH_ERR_PROTOCOL_ERROR:
return "Protocol error";
+ case SSH_ERR_UNKNOWN_USER:
+ return "Unknown user";
+ case SSH_ERR_PATH_TOO_LONG:
+ return "Path too long";
default:
return "unknown error";
}
Index: ssherr.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssherr.h,v
retrieving revision 1.3
diff -u -p -u -r1.3 ssherr.h
--- ssherr.h 30 Jan 2015 01:13:33 -0000 1.3
+++ ssherr.h 19 Aug 2015 02:18:29 -0000
@@ -77,6 +77,8 @@
#define SSH_ERR_CONN_TIMEOUT -53
#define SSH_ERR_CONN_CORRUPT -54
#define SSH_ERR_PROTOCOL_ERROR -55
+#define SSH_ERR_UNKNOWN_USER -56
+#define SSH_ERR_PATH_TOO_LONG -57

/* Translate a numeric error code to a human-readable error string */
const char *ssh_err(int n);

[Ángel González]

The name is not that bad, and the code looks ok.

Post by Todd C. Miller
diff -u -p -u -r1.54 misc.h
--- misc.h 15 Jul 2014 15:54:14 -0000 1.54
+++ misc.h 19 Aug 2015 02:27:37 -0000
@@ -49,7 +49,8 @@
char *cleanhostname(char *);
char *colon(char *);
long convtime(const char *);
char *tilde_expand_filename(const char *, uid_t);
+int tilde_expand_filename2(const char *, char **, uid_t);
char *percent_expand(const char *, ...) __attribute__((__sentinel__));
char *tohex(const void *, size_t);

Note that here you are breaking the alignment of function names. (yep,
completely minor)

[lauri]

Hello,

any updates on this? We're preparing for upgrade to Ubuntu 16.04 and I
was hoping I don't have to patch OpenSSH again :D

Post by Todd C. Miller

Post by Damien Miller
Yeah, we should refactor it into a version that returns a ssherr.h code
and (perhaps) leave the existing tilde_expand_filename() as a wrapper.

We could do something like this. The name stinks but...
- todd
Index: misc.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.c,v
retrieving revision 1.97
diff -u -p -u -r1.97 misc.c
--- misc.c 24 Apr 2015 01:36:00 -0000 1.97
+++ misc.c 19 Aug 2015 02:28:18 -0000
@@ -50,6 +50,7 @@
#include "xmalloc.h"
#include "misc.h"
#include "log.h"
+#include "ssherr.h"
#include "ssh.h"
/* remove newline at end of string */
@@ -499,29 +500,31 @@ freeargs(arglist *args)
* Expands tildes in the file name. Returns data allocated by xmalloc.
* Warning: this calls getpw*.
*/
-char *
-tilde_expand_filename(const char *filename, uid_t uid)
+int
+tilde_expand_filename2(const char *filename, char **expanded, uid_t uid)
{
const char *path, *sep;
char user[128], *ret;
struct passwd *pw;
- u_int len, slash;
+ size_t len, slash;
- if (*filename != '~')
- return (xstrdup(filename));
+ if (*filename != '~') {
+ *expanded = xstrdup(filename);
+ return SSH_ERR_SUCCESS;
+ }
filename++;
path = strchr(filename, '/');
if (path != NULL && path > filename) { /* ~user/path */
slash = path - filename;
if (slash > sizeof(user) - 1)
- fatal("tilde_expand_filename: ~username too long");
+ return SSH_ERR_PATH_TOO_LONG;
memcpy(user, filename, slash);
user[slash] = '\0';
if ((pw = getpwnam(user)) == NULL)
- fatal("tilde_expand_filename: No such user %s", user);
+ return SSH_ERR_UNKNOWN_USER;
} else if ((pw = getpwuid(uid)) == NULL) /* ~/path */
- fatal("tilde_expand_filename: No such uid %ld", (long)uid);
+ return SSH_ERR_UNKNOWN_USER;
/* Make sure directory has a trailing '/' */
len = strlen(pw->pw_dir);
@@ -534,10 +537,29 @@ tilde_expand_filename(const char *filena
if (path != NULL)
filename = path + 1;
- if (xasprintf(&ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX)
- fatal("tilde_expand_filename: Path too long");
+ if (xasprintf(ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX) {
+ free(ret);
+ return SSH_ERR_PATH_TOO_LONG;
+ }
+
+ *expanded = ret;
+ return SSH_ERR_SUCCESS;
+}
+
+/*
+ * Expands tildes in the file name. Returns data allocated by xmalloc.
+ * Warning: this calls getpw*.
+ */
+char *
+tilde_expand_filename(const char *filename, uid_t uid)
+{
+ char *ret;
+ int rc;
- return (ret);
+ rc = tilde_expand_filename2(filename, &ret, uid);
+ if (rc != SSH_ERR_SUCCESS)
+ fatal("%s: %s", __func__, ssh_err(rc));
+ return ret;
}
/*
Index: misc.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/misc.h,v
retrieving revision 1.54
diff -u -p -u -r1.54 misc.h
--- misc.h 15 Jul 2014 15:54:14 -0000 1.54
+++ misc.h 19 Aug 2015 02:27:37 -0000
@@ -49,6 +49,7 @@ char *cleanhostname(char *);
char *colon(char *);
long convtime(const char *);
char *tilde_expand_filename(const char *, uid_t);
+int tilde_expand_filename2(const char *, char **, uid_t);
char *percent_expand(const char *, ...) __attribute__((__sentinel__));
char *tohex(const void *, size_t);
void sanitise_stdfd(void);
Index: ssherr.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssherr.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 ssherr.c
--- ssherr.c 16 Feb 2015 22:13:32 -0000 1.4
+++ ssherr.c 19 Aug 2015 02:19:05 -0000
@@ -135,6 +135,10 @@ ssh_err(int n)
return "Connection corrupted";
return "Protocol error";
+ return "Unknown user";
+ return "Path too long";
return "unknown error";
}
Index: ssherr.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssherr.h,v
retrieving revision 1.3
diff -u -p -u -r1.3 ssherr.h
--- ssherr.h 30 Jan 2015 01:13:33 -0000 1.3
+++ ssherr.h 19 Aug 2015 02:18:29 -0000
@@ -77,6 +77,8 @@
#define SSH_ERR_CONN_TIMEOUT -53
#define SSH_ERR_CONN_CORRUPT -54
#define SSH_ERR_PROTOCOL_ERROR -55
+#define SSH_ERR_UNKNOWN_USER -56
+#define SSH_ERR_PATH_TOO_LONG -57
/* Translate a numeric error code to a human-readable error string */
const char *ssh_err(int n);