- 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@

7 files changed, 149 insertions, 139 deletions

diff --git a/ChangeLog b/ChangeLog
index 2b0ca0b8..91a6b649 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 69e8e6f6..a8eeb0e3 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 <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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 c3c80994..e4c8c323 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 <netinet/ip.h>
 #include <netinet/tcp.h>
 
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
@@ -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 fceb3065..d4df619c 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 <ylo@cs.hut.fi>
@@ -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 f7b912ef..9340effd 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 <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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 b62bd351..3056c454 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 13f384a9..5aa5dcc8 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 <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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);