On Mon, 17 Aug 2015, Todd C. Miller wrote:> 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-forward at openssh.com > request. > Message-Id: <aea6cdc1d1b42d07 at courtesan.com> > > 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
2015-Aug-19 02:31 UTC
[PATCH] Expand tilde for UNIX domain socket forwards.
On Tue, 18 Aug 2015 09:41:47 +1000, Damien Miller wrote:> 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
2015-Aug-21 22:49 UTC
[PATCH] Expand tilde for UNIX domain socket forwards.
The name is not that bad, and the code looks ok. On 19/08/15 04:31, Todd C. Miller wrote:> 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)
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 2015-08-19 5:31 GMT+03:00 Todd C. Miller <Todd.Miller at courtesan.com>:> On Tue, 18 Aug 2015 09:41:47 +1000, Damien Miller wrote: > >> 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);-- Lauri V?sandi tel: +372 53329412 e-mail: lauri.vosandi at gmail.com blog: http://lauri.vosandi.com/