From e9fc72edd6c313b670558cd5219601c38a949b67 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Tue, 15 Oct 2013 12:14:12 +1100 Subject: [PATCH] - djm@cvs.openbsd.org 2013/10/14 23:28:23 [canohost.c misc.c misc.h readconf.c sftp-server.c ssh.c] refactor client config code a little: add multistate option partsing to readconf.c, similar to servconf.c's existing code. move checking of options that accept "none" as an argument to readconf.c add a lowercase() function and use it instead of explicit tolower() in loops part of a larger diff that was ok markus@ --- ChangeLog | 9 +++ canohost.c | 13 +--- misc.c | 10 ++- misc.h | 4 +- readconf.c | 199 ++++++++++++++++++++++++-------------------------- sftp-server.c | 25 +++++-- ssh.c | 28 +++---- 7 files changed, 149 insertions(+), 139 deletions(-) diff --git a/ChangeLog b/ChangeLog index 2b0ca0b8d..91a6b6497 100644 --- a/ChangeLog +++ b/ChangeLog @@ -37,6 +37,15 @@ [readconf.c readconf.h ssh-keysign.c ssh.c ssh_config.5] add a "Match" keyword to ssh_config that allows matching on hostname, user and result of arbitrary commands. "nice work" markus@ + - djm@cvs.openbsd.org 2013/10/14 23:28:23 + [canohost.c misc.c misc.h readconf.c sftp-server.c ssh.c] + refactor client config code a little: + add multistate option partsing to readconf.c, similar to servconf.c's + existing code. + move checking of options that accept "none" as an argument to readconf.c + add a lowercase() function and use it instead of explicit tolower() in + loops + part of a larger diff that was ok markus@ 20131010 - (dtucker) OpenBSD CVS Sync diff --git a/canohost.c b/canohost.c index 69e8e6f6d..a8eeb0e35 100644 --- a/canohost.c +++ b/canohost.c @@ -1,4 +1,4 @@ -/* $OpenBSD: canohost.c,v 1.67 2013/05/17 00:13:13 djm Exp $ */ +/* $OpenBSD: canohost.c,v 1.68 2013/10/14 23:28:22 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -48,7 +48,6 @@ static char * get_remote_hostname(int sock, int use_dns) { struct sockaddr_storage from; - int i; socklen_t fromlen; struct addrinfo hints, *ai, *aitop; char name[NI_MAXHOST], ntop[NI_MAXHOST], ntop2[NI_MAXHOST]; @@ -99,13 +98,9 @@ get_remote_hostname(int sock, int use_dns) return xstrdup(ntop); } - /* - * Convert it to all lowercase (which is expected by the rest - * of this software). - */ - for (i = 0; name[i]; i++) - if (isupper(name[i])) - name[i] = (char)tolower(name[i]); + /* Names are stores in lowercase. */ + lowercase(name); + /* * Map it back to an IP address and check that the given * address actually is an address of this host. This is diff --git a/misc.c b/misc.c index c3c809943..e4c8c3238 100644 --- a/misc.c +++ b/misc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.c,v 1.91 2013/07/12 00:43:50 djm Exp $ */ +/* $OpenBSD: misc.c,v 1.92 2013/10/14 23:28:23 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2005,2006 Damien Miller. All rights reserved. @@ -43,6 +43,7 @@ #include #include +#include #include #include #include @@ -1017,6 +1018,13 @@ iptos2str(int iptos) snprintf(iptos_str, sizeof iptos_str, "0x%02x", iptos); return iptos_str; } + +void +lowercase(char *s) +{ + for (; *s; s++) + *s = tolower((u_char)*s); +} void sock_set_v6only(int s) { diff --git a/misc.h b/misc.h index fceb30655..d4df619cd 100644 --- a/misc.h +++ b/misc.h @@ -1,4 +1,4 @@ -/* $OpenBSD: misc.h,v 1.49 2013/06/01 13:15:52 dtucker Exp $ */ +/* $OpenBSD: misc.h,v 1.50 2013/10/14 23:28:23 djm Exp $ */ /* * Author: Tatu Ylonen @@ -36,6 +36,8 @@ void sanitise_stdfd(void); void ms_subtract_diff(struct timeval *, int *); void ms_to_timeval(struct timeval *, int); time_t monotime(void); +void lowercase(char *s); + void sock_set_v6only(int); struct passwd *pwcopy(struct passwd *); diff --git a/readconf.c b/readconf.c index f7b912ef6..9340effd0 100644 --- a/readconf.c +++ b/readconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: readconf.c,v 1.206 2013/10/14 22:22:02 djm Exp $ */ +/* $OpenBSD: readconf.c,v 1.207 2013/10/14 23:28:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -555,6 +555,61 @@ parse_token(const char *cp, const char *filename, int linenum, return oBadOption; } +/* Multistate option parsing */ +struct multistate { + char *key; + int value; +}; +static const struct multistate multistate_flag[] = { + { "true", 1 }, + { "false", 0 }, + { "yes", 1 }, + { "no", 0 }, + { NULL, -1 } +}; +static const struct multistate multistate_yesnoask[] = { + { "true", 1 }, + { "false", 0 }, + { "yes", 1 }, + { "no", 0 }, + { "ask", 2 }, + { NULL, -1 } +}; +static const struct multistate multistate_addressfamily[] = { + { "inet", AF_INET }, + { "inet6", AF_INET6 }, + { "any", AF_UNSPEC }, + { NULL, -1 } +}; +static const struct multistate multistate_controlmaster[] = { + { "true", SSHCTL_MASTER_YES }, + { "yes", SSHCTL_MASTER_YES }, + { "false", SSHCTL_MASTER_NO }, + { "no", SSHCTL_MASTER_NO }, + { "auto", SSHCTL_MASTER_AUTO }, + { "ask", SSHCTL_MASTER_ASK }, + { "autoask", SSHCTL_MASTER_AUTO_ASK }, + { NULL, -1 } +}; +static const struct multistate multistate_tunnel[] = { + { "ethernet", SSH_TUNMODE_ETHERNET }, + { "point-to-point", SSH_TUNMODE_POINTOPOINT }, + { "true", SSH_TUNMODE_DEFAULT }, + { "yes", SSH_TUNMODE_DEFAULT }, + { "false", SSH_TUNMODE_NO }, + { "no", SSH_TUNMODE_NO }, + { NULL, -1 } +}; +static const struct multistate multistate_requesttty[] = { + { "true", REQUEST_TTY_YES }, + { "yes", REQUEST_TTY_YES }, + { "false", REQUEST_TTY_NO }, + { "no", REQUEST_TTY_NO }, + { "force", REQUEST_TTY_FORCE }, + { "auto", REQUEST_TTY_AUTO }, + { NULL, -1 } +}; + /* * Processes a single option line as used in the configuration files. This * only sets those values that have not already been set. @@ -572,6 +627,7 @@ process_config_line(Options *options, struct passwd *pw, const char *host, long long val64; size_t len; Forward fwd; + const struct multistate *multistate_ptr; if (activep == NULL) { /* We are processing a command line directive */ cmdline = 1; @@ -595,8 +651,7 @@ process_config_line(Options *options, struct passwd *pw, const char *host, if (keyword == NULL || !*keyword || *keyword == '\n' || *keyword == '#') return 0; /* Match lowercase keyword */ - for (i = 0; i < strlen(keyword); i++) - keyword[i] = tolower(keyword[i]); + lowercase(keyword); opcode = parse_token(keyword, filename, linenum, options->ignored_unknown); @@ -626,17 +681,23 @@ parse_time: case oForwardAgent: intptr = &options->forward_agent; -parse_flag: + parse_flag: + multistate_ptr = multistate_flag; + parse_multistate: arg = strdelim(&s); if (!arg || *arg == '\0') - fatal("%.200s line %d: Missing yes/no argument.", filename, linenum); - value = 0; /* To avoid compiler warning... */ - if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0) - value = 1; - else if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0) - value = 0; - else - fatal("%.200s line %d: Bad yes/no argument.", filename, linenum); + fatal("%s line %d: missing argument.", + filename, linenum); + value = -1; + for (i = 0; multistate_ptr[i].key != NULL; i++) { + if (strcasecmp(arg, multistate_ptr[i].key) == 0) { + value = multistate_ptr[i].value; + break; + } + } + if (value == -1) + fatal("%s line %d: unsupported option \"%s\".", + filename, linenum, arg); if (*activep && *intptr == -1) *intptr = value; break; @@ -719,27 +780,13 @@ parse_flag: case oVerifyHostKeyDNS: intptr = &options->verify_host_key_dns; - goto parse_yesnoask; + multistate_ptr = multistate_yesnoask; + goto parse_multistate; case oStrictHostKeyChecking: intptr = &options->strict_host_key_checking; -parse_yesnoask: - arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%.200s line %d: Missing yes/no/ask argument.", - filename, linenum); - value = 0; /* To avoid compiler warning... */ - if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0) - value = 1; - else if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0) - value = 0; - else if (strcmp(arg, "ask") == 0) - value = 2; - else - fatal("%.200s line %d: Bad yes/no/ask argument.", filename, linenum); - if (*activep && *intptr == -1) - *intptr = value; - break; + multistate_ptr = multistate_yesnoask; + goto parse_multistate; case oCompression: intptr = &options->compression; @@ -1080,22 +1127,9 @@ parse_int: break; case oAddressFamily: - arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%s line %d: missing address family.", - filename, linenum); intptr = &options->address_family; - if (strcasecmp(arg, "inet") == 0) - value = AF_INET; - else if (strcasecmp(arg, "inet6") == 0) - value = AF_INET6; - else if (strcasecmp(arg, "any") == 0) - value = AF_UNSPEC; - else - fatal("Unsupported AddressFamily \"%s\"", arg); - if (*activep && *intptr == -1) - *intptr = value; - break; + multistate_ptr = multistate_addressfamily; + goto parse_multistate; case oEnableSSHKeysign: intptr = &options->enable_ssh_keysign; @@ -1134,27 +1168,8 @@ parse_int: case oControlMaster: intptr = &options->control_master; - arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%.200s line %d: Missing ControlMaster argument.", - filename, linenum); - value = 0; /* To avoid compiler warning... */ - if (strcmp(arg, "yes") == 0 || strcmp(arg, "true") == 0) - value = SSHCTL_MASTER_YES; - else if (strcmp(arg, "no") == 0 || strcmp(arg, "false") == 0) - value = SSHCTL_MASTER_NO; - else if (strcmp(arg, "auto") == 0) - value = SSHCTL_MASTER_AUTO; - else if (strcmp(arg, "ask") == 0) - value = SSHCTL_MASTER_ASK; - else if (strcmp(arg, "autoask") == 0) - value = SSHCTL_MASTER_AUTO_ASK; - else - fatal("%.200s line %d: Bad ControlMaster argument.", - filename, linenum); - if (*activep && *intptr == -1) - *intptr = value; - break; + multistate_ptr = multistate_controlmaster; + goto parse_multistate; case oControlPersist: /* no/false/yes/true, or a time spec */ @@ -1186,25 +1201,8 @@ parse_int: case oTunnel: intptr = &options->tun_open; - arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%s line %d: Missing yes/point-to-point/" - "ethernet/no argument.", filename, linenum); - value = 0; /* silence compiler */ - if (strcasecmp(arg, "ethernet") == 0) - value = SSH_TUNMODE_ETHERNET; - else if (strcasecmp(arg, "point-to-point") == 0) - value = SSH_TUNMODE_POINTOPOINT; - else if (strcasecmp(arg, "yes") == 0) - value = SSH_TUNMODE_DEFAULT; - else if (strcasecmp(arg, "no") == 0) - value = SSH_TUNMODE_NO; - else - fatal("%s line %d: Bad yes/point-to-point/ethernet/" - "no argument: %s", filename, linenum, arg); - if (*activep) - *intptr = value; - break; + multistate_ptr = multistate_tunnel; + goto parse_multistate; case oTunnelDevice: arg = strdelim(&s); @@ -1253,24 +1251,9 @@ parse_int: goto parse_flag; case oRequestTTY: - arg = strdelim(&s); - if (!arg || *arg == '\0') - fatal("%s line %d: missing argument.", - filename, linenum); intptr = &options->request_tty; - if (strcasecmp(arg, "yes") == 0) - value = REQUEST_TTY_YES; - else if (strcasecmp(arg, "no") == 0) - value = REQUEST_TTY_NO; - else if (strcasecmp(arg, "force") == 0) - value = REQUEST_TTY_FORCE; - else if (strcasecmp(arg, "auto") == 0) - value = REQUEST_TTY_AUTO; - else - fatal("Unsupported RequestTTY \"%s\"", arg); - if (*activep && *intptr == -1) - *intptr = value; - break; + multistate_ptr = multistate_requesttty; + goto parse_multistate; case oIgnoreUnknown: charptr = &options->ignored_unknown; @@ -1596,8 +1579,16 @@ fill_default_options(Options * options) options->request_tty = REQUEST_TTY_AUTO; if (options->proxy_use_fdpass == -1) options->proxy_use_fdpass = 0; - /* options->local_command should not be set by default */ - /* options->proxy_command should not be set by default */ +#define CLEAR_ON_NONE(v) \ + do { \ + if (v != NULL && strcasecmp(v, "none") == 0) { \ + free(v); \ + v = NULL; \ + } \ + } while(0) + CLEAR_ON_NONE(options->local_command); + CLEAR_ON_NONE(options->proxy_command); + CLEAR_ON_NONE(options->control_path); /* options->user will be set in the main program if appropriate */ /* options->hostname will be set in the main program if appropriate */ /* options->host_key_alias should not be set by default */ diff --git a/sftp-server.c b/sftp-server.c index b62bd3510..3056c454e 100644 --- a/sftp-server.c +++ b/sftp-server.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sftp-server.c,v 1.100 2013/10/14 14:18:56 jmc Exp $ */ +/* $OpenBSD: sftp-server.c,v 1.101 2013/10/14 23:28:23 djm Exp $ */ /* * Copyright (c) 2000-2004 Markus Friedl. All rights reserved. * @@ -230,6 +230,8 @@ flags_from_portable(int pflags) } else if (pflags & SSH2_FXF_WRITE) { flags = O_WRONLY; } + if (pflags & SSH2_FXF_APPEND) + flags |= O_APPEND; if (pflags & SSH2_FXF_CREAT) flags |= O_CREAT; if (pflags & SSH2_FXF_TRUNC) @@ -256,6 +258,8 @@ string_from_portable(int pflags) PAPPEND("READ") if (pflags & SSH2_FXF_WRITE) PAPPEND("WRITE") + if (pflags & SSH2_FXF_APPEND) + PAPPEND("APPEND") if (pflags & SSH2_FXF_CREAT) PAPPEND("CREATE") if (pflags & SSH2_FXF_TRUNC) @@ -279,6 +283,7 @@ struct Handle { int use; DIR *dirp; int fd; + int flags; char *name; u_int64_t bytes_read, bytes_write; int next_unused; @@ -302,7 +307,7 @@ static void handle_unused(int i) } static int -handle_new(int use, const char *name, int fd, DIR *dirp) +handle_new(int use, const char *name, int fd, int flags, DIR *dirp) { int i; @@ -320,6 +325,7 @@ handle_new(int use, const char *name, int fd, DIR *dirp) handles[i].use = use; handles[i].dirp = dirp; handles[i].fd = fd; + handles[i].flags = flags; handles[i].name = xstrdup(name); handles[i].bytes_read = handles[i].bytes_write = 0; @@ -382,6 +388,14 @@ handle_to_fd(int handle) return -1; } +static int +handle_to_flags(int handle) +{ + if (handle_is_ok(handle, HANDLE_FILE)) + return handles[handle].flags; + return 0; +} + static void handle_update_read(int handle, ssize_t bytes) { @@ -668,7 +682,7 @@ process_open(u_int32_t id) if (fd < 0) { status = errno_to_portable(errno); } else { - handle = handle_new(HANDLE_FILE, name, fd, NULL); + handle = handle_new(HANDLE_FILE, name, fd, flags, NULL); if (handle < 0) { close(fd); } else { @@ -754,7 +768,8 @@ process_write(u_int32_t id) if (fd < 0) status = SSH2_FX_FAILURE; else { - if (lseek(fd, off, SEEK_SET) < 0) { + if (!(handle_to_flags(handle) & O_APPEND) && + lseek(fd, off, SEEK_SET) < 0) { status = errno_to_portable(errno); error("process_write: seek failed"); } else { @@ -971,7 +986,7 @@ process_opendir(u_int32_t id) if (dirp == NULL) { status = errno_to_portable(errno); } else { - handle = handle_new(HANDLE_DIR, path, 0, dirp); + handle = handle_new(HANDLE_DIR, path, 0, 0, dirp); if (handle < 0) { closedir(dirp); } else { diff --git a/ssh.c b/ssh.c index 13f384a92..5aa5dcc89 100644 --- a/ssh.c +++ b/ssh.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh.c,v 1.382 2013/10/14 22:22:04 djm Exp $ */ +/* $OpenBSD: ssh.c,v 1.383 2013/10/14 23:28:23 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -723,6 +723,14 @@ main(int ac, char **av) channel_set_af(options.address_family); + /* Tidy and check options */ + if (options.host_key_alias != NULL) + lowercase(options.host_key_alias); + if (options.proxy_command != NULL && + strcmp(options.proxy_command, "-") == 0 && + options.proxy_use_fdpass) + fatal("ProxyCommand=- and ProxyUseFDPass are incompatible"); + /* reinit */ log_init(argv0, options.log_level, SYSLOG_FACILITY_USER, !use_syslog); @@ -779,24 +787,6 @@ main(int ac, char **av) free(cp); } - /* force lowercase for hostkey matching */ - if (options.host_key_alias != NULL) { - for (p = options.host_key_alias; *p; p++) - if (isupper(*p)) - *p = (char)tolower(*p); - } - - if (options.proxy_command != NULL && - strcmp(options.proxy_command, "none") == 0) { - free(options.proxy_command); - options.proxy_command = NULL; - } - if (options.control_path != NULL && - strcmp(options.control_path, "none") == 0) { - free(options.control_path); - options.control_path = NULL; - } - if (options.control_path != NULL) { cp = tilde_expand_filename(options.control_path, original_real_uid);