User names have hitherto been handled neither consistently nor well; this
series alleviates at least some of the issues.
Fear not the long patch series!
Most commits involve a fairly small number of changes; while I could have
consolidated these changes into fewer commits, I think the series as a whole
provides a better narration for what's going.
Besides a few small improvements along the way, the main thrust is:
* Removing user-name handling from `scp' (in favor of using the
handling that is already present in `dropbear'/`dbclient').
* Lazily looking up the current user's name.
* Removing unused code.
Overall, 7 files were changed, with 37 insertions(+) and 158 deletions(-):
cli-main.c | 2 +-
cli-runopts.c | 32 ++++++--------
common-session.c | 1 +
runopts.h | 2 +-
scp.c | 125 ++++++++++---------------------------------------------
scpmisc.c | 31 +-------------
scpmisc.h | 2 -
This is the series:
[01] scp: Insert comma into stderr message
[02] scp: Have `fatal()' append a newline to the message
[03] scp: only pass `-v' when DEBUG_TRACE is set
[04] scp: `-l%s' -> `-l %s'
[05] scp: const/static correctness improvements
[06] scp: Introduce `get_user_name()'
[07] scp: Use "unknown" when the user name cannot be retrieved
[08] scp: style: simplify code by using a tertiary operator
[09] scp: Use `get_user_name()' more often
[10] scp: Simplify code now that the user name is never `NULL'
[11] scp: Remove parsing of `[user@]host'
[12] scp: Remove unused functions
[13] scp: Remove `replacearg()'
[14] runopts: Mark `*cli_runopts.own_user' as `const'
[15] runopts: There's no reason to make a duplicate of "unknown"
[16] runopts: Re-introduce the `get_user_name()' function from `scp'
development
Lastly, for convenience in reviewing the changes, here is the overall patch:
--- a/cli-main.c
+++ b/cli-main.c
@@ -135,7 +135,7 @@
static void cli_proxy_cmd(int *sock_in, int *sock_out) {
int ret;
- fill_passwd(cli_opts.own_user);
+ fill_passwd(get_user_name());
ret = spawn_command(exec_proxy_cmd, cli_opts.proxycmd,
sock_out, sock_in, NULL, NULL);
--- a/cli-runopts.c
+++ b/cli-runopts.c
@@ -36,7 +36,6 @@
static void printhelp();
static void parse_hostname(const char* orighostarg);
static void parse_multihop_hostname(const char* orighostarg, const char*
argv0);
-static void fill_own_user();
#ifdef ENABLE_CLI_PUBKEY_AUTH
static void loadidentityfile(const char* filename, int warnfail);
#endif
@@ -102,6 +101,17 @@
}
+const char *get_user_name() {
+ static const char *user_name = NULL;
+
+ if (user_name == NULL) {
+ struct passwd *pwd = getpwuid(getuid());
+ user_name = pwd ? m_strdup(pwd->pw_name) : "unknown";
+ }
+
+ return user_name;
+}
+
void cli_getopts(int argc, char ** argv) {
unsigned int i, j;
char ** next = 0;
@@ -175,8 +185,6 @@
opts.keepalive_secs = DEFAULT_KEEPALIVE;
opts.idle_timeout_secs = DEFAULT_IDLE_TIMEOUT;
- fill_own_user();
-
for (i = 1; i < (unsigned int)argc; i++) {
/* Handle non-flag arguments such as hostname or commands for
the remote host */
if (argv[i][0] != '-')
@@ -640,7 +648,7 @@
}
if (cli_opts.username == NULL) {
- cli_opts.username = m_strdup(cli_opts.own_user);
+ cli_opts.username = m_strdup(get_user_name());
}
port = strchr(cli_opts.remotehost, '^');
@@ -695,22 +703,6 @@
}
#endif
-static void fill_own_user() {
- uid_t uid;
- struct passwd *pw = NULL;
-
- uid = getuid();
-
- pw = getpwuid(uid);
- if (pw && pw->pw_name != NULL) {
- cli_opts.own_user = m_strdup(pw->pw_name);
- } else {
- dropbear_log(LOG_INFO, "Warning: failed to identify current
user. Trying anyway.");
- cli_opts.own_user = m_strdup("unknown");
- }
-
-}
-
#ifdef ENABLE_CLI_ANYTCPFWD
/* Turn a "[listenaddr:]listenport:remoteaddr:remoteport" string into into a
forwarding
* set, and add it to the forwarding list */
--- a/common-session.c
+++ b/common-session.c
@@ -581,6 +581,7 @@
return ses.authstate.pw_shell;
}
}
+
void fill_passwd(const char* username) {
struct passwd *pw = NULL;
if (ses.authstate.pw_name)
--- a/runopts.h
+++ b/runopts.h
@@ -127,7 +127,6 @@
char *remotehost;
char *remoteport;
- char *own_user;
char *username;
char *cmd;
@@ -165,6 +164,7 @@
extern cli_runopts cli_opts;
void cli_getopts(int argc, char ** argv);
+const char *get_user_name();
#ifdef ENABLE_USER_ALGO_LIST
void parse_ciphers_macs();
--- a/scp.c
+++ b/scp.c
@@ -172,25 +172,21 @@
*/
static void
-arg_setup(char *host, char *remuser, char *cmd)
+arg_setup(const char *user_at_host, const char *cmd)
{
- replacearg(&args, 0, "%s", ssh_program);
- if (remuser != NULL)
- addargs(&args, "-l%s", remuser);
- addargs(&args, "%s", host);
+ addargs(&args, "%s", user_at_host);
addargs(&args, "%s", cmd);
}
int
-do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc)
+do_cmd(const char *user_at_host, const char *cmd, int *fdin, int *fdout, int
argc)
{
int pin[2], pout[2], reserved[2];
if (verbose_mode)
fprintf(stderr,
- "Executing: program %s host %s, user %s, command %s\n",
- ssh_program, host,
- remuser ? remuser : "(unspecified)", cmd);
+ "Executing: program %s, login %s, command %s\n",
+ ssh_program, user_at_host, cmd);
/*
* Reserve two descriptors so that the real pipes won't get
@@ -211,7 +207,7 @@
/* uClinux needs to build the args here before vforking,
otherwise we do it later on. */
#ifdef USE_VFORK
- arg_setup(host, remuser, cmd);
+ arg_setup(user_at_host, cmd);
#endif
/* Fork a child to execute the command on the remote host using ssh. */
@@ -231,7 +227,7 @@
close(pout[1]);
#ifndef USE_VFORK
- arg_setup(host, remuser, cmd);
+ arg_setup(user_at_host, cmd);
#endif
execvp(ssh_program, args.list);
@@ -251,16 +247,10 @@
xfree(args.list[args.num-1]);
args.list[args.num-1]=NULL;
args.num--;
- /* pop host */
+ /* pop user_at_host */
xfree(args.list[args.num-1]);
args.list[args.num-1]=NULL;
args.num--;
- /* pop user */
- if (remuser != NULL) {
- xfree(args.list[args.num-1]);
- args.list[args.num-1]=NULL;
- args.num--;
- }
#endif
/* Parent. Close the other side, and return the local side. */
@@ -282,12 +272,9 @@
BUF *allocbuf(BUF *, int, int);
void lostconn(int);
void nospace(void);
-int okname(char *);
void run_err(const char *,...);
void verifydir(char *);
-struct passwd *pwd;
-uid_t userid;
int errs, remin, remout;
int pflag, iamremote, iamrecursive, targetshouldbedirectory;
@@ -362,7 +349,9 @@
ssh_program = xstrdup(optarg);
break;
case 'v':
+#ifdef DEBUG_TRACE
addargs(&args, "-v");
+#endif
verbose_mode = 1;
break;
case 'q':
@@ -393,9 +382,6 @@
argc -= optind;
argv += optind;
- if ((pwd = getpwuid(userid = getuid())) == NULL)
- fatal("unknown user %u", (u_int) userid);
-
if (!isatty(STDERR_FILENO))
showprogress = 0;
@@ -459,7 +445,8 @@
toremote(char *targ, int argc, char **argv)
{
int i, len;
- char *bp, *host, *src, *suser, *thost, *tuser, *arg;
+ const char *targ_user_at_host;
+ char *bp, *src;
arglist alist;
memset(&alist, '\0', sizeof(alist));
@@ -469,30 +456,17 @@
if (*targ == 0)
targ = ".";
- arg = xstrdup(argv[argc - 1]);
- if ((thost = strrchr(arg, '@'))) {
- /* user@host */
- *thost++ = 0;
- tuser = arg;
- if (*tuser == '\0')
- tuser = NULL;
- } else {
- thost = arg;
- tuser = NULL;
- }
-
- if (tuser != NULL && !okname(tuser)) {
- xfree(arg);
- return;
- }
+ targ_user_at_host = argv[argc - 1];
for (i = 0; i < argc - 1; i++) {
src = colon(argv[i]);
if (src) { /* remote to remote */
freeargs(&alist);
addargs(&alist, "%s", ssh_program);
+#ifdef DEBUG_TRACE
if (verbose_mode)
addargs(&alist, "-v");
+#endif
#if 0
/* Disabled since dbclient won't understand them
and scp works fine without them. */
@@ -504,27 +478,11 @@
*src++ = 0;
if (*src == 0)
src = ".";
- host = strrchr(argv[i], '@');
-
- if (host) {
- *host++ = 0;
- host = cleanhostname(host);
- suser = argv[i];
- if (*suser == '\0')
- suser = pwd->pw_name;
- else if (!okname(suser))
- continue;
- addargs(&alist, "-l");
- addargs(&alist, "%s", suser);
- } else {
- host = cleanhostname(argv[i]);
- }
- addargs(&alist, "%s", host);
+
+ addargs(&alist, "%s", argv[i]); /* [user@]host */
addargs(&alist, "%s", cmd);
addargs(&alist, "%s", src);
- addargs(&alist, "%s%s%s:%s",
- tuser ? tuser : "", tuser ? "@" : "",
- thost, targ);
+ addargs(&alist, "%s:%s", targ_user_at_host, targ);
if (do_local_cmd(&alist) != 0)
errs = 1;
} else { /* local to remote */
@@ -532,8 +490,7 @@
len = strlen(targ) + CMDNEEDS + 20;
bp = xmalloc(len);
(void) snprintf(bp, len, "%s -t %s", cmd, targ);
- host = cleanhostname(thost);
- if (do_cmd(host, tuser, bp, &remin,
+ if (do_cmd(targ_user_at_host, bp, &remin,
&remout, argc) < 0)
exit(1);
if (response() < 0)
@@ -549,7 +506,7 @@
tolocal(int argc, char **argv)
{
int i, len;
- char *bp, *host, *src, *suser;
+ char *bp, *src;
arglist alist;
memset(&alist, '\0', sizeof(alist));
@@ -572,20 +529,10 @@
*src++ = 0;
if (*src == 0)
src = ".";
- if ((host = strrchr(argv[i], '@')) == NULL) {
- host = argv[i];
- suser = NULL;
- } else {
- *host++ = 0;
- suser = argv[i];
- if (*suser == '\0')
- suser = pwd->pw_name;
- }
- host = cleanhostname(host);
len = strlen(src) + CMDNEEDS + 20;
bp = xmalloc(len);
(void) snprintf(bp, len, "%s -f %s", cmd, src);
- if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) {
+ if (do_cmd(argv[i], bp, &remin, &remout, argc) < 0) {
(void) xfree(bp);
++errs;
continue;
@@ -1190,36 +1137,6 @@
killchild(0);
}
-int
-okname(char *cp0)
-{
- int c;
- char *cp;
-
- cp = cp0;
- do {
- c = (int)*cp;
- if (c & 0200)
- goto bad;
- if (!isalpha(c) && !isdigit(c)) {
- switch (c) {
- case '\'':
- case '"':
- case '`':
- case ' ':
- case '#':
- goto bad;
- default:
- break;
- }
- }
- } while (*++cp);
- return (1);
-
-bad: fprintf(stderr, "%s: invalid user name\n", cp0);
- return (0);
-}
-
BUF *
allocbuf(BUF *bp, int fd, int blksize)
{
--- a/scpmisc.c
+++ b/scpmisc.c
@@ -104,16 +104,6 @@
}
char *
-cleanhostname(char *host)
-{
- if (*host == '[' && host[strlen(host) - 1] == ']') {
- host[strlen(host) - 1] = '\0';
- return (host + 1);
- } else
- return host;
-}
-
-char *
colon(char *cp)
{
int flag = 0;
@@ -165,26 +155,6 @@
}
void
-replacearg(arglist *args, u_int which, char *fmt, ...)
-{
- va_list ap;
- char *cp;
- int r;
-
- va_start(ap, fmt);
- r = vasprintf(&cp, fmt, ap);
- va_end(ap);
- if (r == -1)
- fatal("replacearg: argument too long");
-
- if (which >= args->num)
- fatal("replacearg: tried to replace invalid arg %d >= %d",
- which, args->num);
- xfree(args->list[which]);
- args->list[which] = cp;
-}
-
-void
freeargs(arglist *args)
{
u_int i;
@@ -223,6 +193,7 @@
va_start(args, fmt);
vfprintf(stderr, fmt, args);
va_end(args);
+ fputc('\n', stderr);
exit(255);
}
--- a/scpmisc.h
+++ b/scpmisc.h
@@ -21,7 +21,6 @@
void unset_nonblock(int);
void set_nodelay(int);
int a2port(const char *);
-char *cleanhostname(char *);
char *colon(char *);
long convtime(const char *);
@@ -34,7 +33,6 @@
int nalloc;
};
void addargs(arglist *, char *, ...);
-void replacearg(arglist *, u_int, char *, ...);
void freeargs(arglist *);
/* from xmalloc.h */
