support =- for removing methods from algorithms lists, e.g.

Ciphers=-*cbc; suggested by Cristian Ionescu-Idbohrn in bz#2671
"I like it" markus@

8 files changed, 120 insertions, 59 deletions

diff --git a/usr.bin/ssh/compat.c b/usr.bin/ssh/compat.c
index 97d040a5004..56125010e94 100644
--- a/usr.bin/ssh/compat.c
+++ b/usr.bin/ssh/compat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.c,v 1.99 2016/05/24 02:31:57 dtucker Exp $ */
+/* $OpenBSD: compat.c,v 1.100 2017/02/03 23:01:19 djm Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -35,6 +35,7 @@
 #include "compat.h"
 #include "log.h"
 #include "match.h"
+#include "kex.h"
 
 int compat13 = 0;
 int compat20 = 0;
@@ -248,42 +249,14 @@ proto_spec(const char *spec)
 	return ret;
 }
 
-/*
- * Filters a proposal string, excluding any algorithm matching the 'filter'
- * pattern list.
- */
-static char *
-filter_proposal(char *proposal, const char *filter)
-{
-	Buffer b;
-	char *orig_prop, *fix_prop;
-	char *cp, *tmp;
-
-	buffer_init(&b);
-	tmp = orig_prop = xstrdup(proposal);
-	while ((cp = strsep(&tmp, ",")) != NULL) {
-		if (match_pattern_list(cp, filter, 0) != 1) {
-			if (buffer_len(&b) > 0)
-				buffer_append(&b, ",", 1);
-			buffer_append(&b, cp, strlen(cp));
-		} else
-			debug2("Compat: skipping algorithm \"%s\"", cp);
-	}
-	buffer_append(&b, "\0", 1);
-	fix_prop = xstrdup((char *)buffer_ptr(&b));
-	buffer_free(&b);
-	free(orig_prop);
-
-	return fix_prop;
-}
-
 char *
 compat_cipher_proposal(char *cipher_prop)
 {
 	if (!(datafellows & SSH_BUG_BIGENDIANAES))
 		return cipher_prop;
 	debug2("%s: original cipher proposal: %s", __func__, cipher_prop);
-	cipher_prop = filter_proposal(cipher_prop, "aes*");
+	if ((cipher_prop = match_filter_list(cipher_prop, "aes*")) == NULL)
+		fatal("match_filter_list failed");
 	debug2("%s: compat cipher proposal: %s", __func__, cipher_prop);
 	if (*cipher_prop == '\0')
 		fatal("No supported ciphers found");
@@ -296,7 +269,8 @@ compat_pkalg_proposal(char *pkalg_prop)
 	if (!(datafellows & SSH_BUG_RSASIGMD5))
 		return pkalg_prop;
 	debug2("%s: original public key proposal: %s", __func__, pkalg_prop);
-	pkalg_prop = filter_proposal(pkalg_prop, "ssh-rsa");
+	if ((pkalg_prop = match_filter_list(pkalg_prop, "ssh-rsa")) == NULL)
+		fatal("match_filter_list failed");
 	debug2("%s: compat public key proposal: %s", __func__, pkalg_prop);
 	if (*pkalg_prop == '\0')
 		fatal("No supported PK algorithms found");
@@ -310,10 +284,14 @@ compat_kex_proposal(char *p)
 		return p;
 	debug2("%s: original KEX proposal: %s", __func__, p);
 	if ((datafellows & SSH_BUG_CURVE25519PAD) != 0)
-		p = filter_proposal(p, "curve25519-sha256@libssh.org");
+		if ((p = match_filter_list(p,
+		    "curve25519-sha256@libssh.org")) == NULL)
+			fatal("match_filter_list failed");
 	if ((datafellows & SSH_OLD_DHGEX) != 0) {
-		p = filter_proposal(p, "diffie-hellman-group-exchange-sha256");
-		p = filter_proposal(p, "diffie-hellman-group-exchange-sha1");
+		if ((p = match_filter_list(p,
+		    "diffie-hellman-group-exchange-sha256,"
+		    "diffie-hellman-group-exchange-sha1")) == NULL)
+			fatal("match_filter_list failed");
 	}
 	debug2("%s: compat KEX proposal: %s", __func__, p);
 	if (*p == '\0')
diff --git a/usr.bin/ssh/kex.c b/usr.bin/ssh/kex.c
index 4147b230203..9b7753b441c 100644
--- a/usr.bin/ssh/kex.c
+++ b/usr.bin/ssh/kex.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.c,v 1.127 2016/10/10 19:28:48 markus Exp $ */
+/* $OpenBSD: kex.c,v 1.128 2017/02/03 23:01:19 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  *
@@ -191,7 +191,8 @@ kex_names_cat(const char *a, const char *b)
 /*
  * Assemble a list of algorithms from a default list and a string from a
  * configuration file. The user-provided string may begin with '+' to
- * indicate that it should be appended to the default.
+ * indicate that it should be appended to the default or '-' that the
+ * specified names should be removed.
  */
 int
 kex_assemble_names(const char *def, char **list)
@@ -202,14 +203,18 @@ kex_assemble_names(const char *def, char **list)
 		*list = strdup(def);
 		return 0;
 	}
-	if (**list != '+') {
-		return 0;
+	if (**list == '+') {
+		if ((ret = kex_names_cat(def, *list + 1)) == NULL)
+			return SSH_ERR_ALLOC_FAIL;
+		free(*list);
+		*list = ret;
+	} else if (**list == '-') {
+		if ((ret = match_filter_list(def, *list + 1)) == NULL)
+			return SSH_ERR_ALLOC_FAIL;
+		free(*list);
+		*list = ret;
 	}
 
-	if ((ret = kex_names_cat(def, *list + 1)) == NULL)
-		return SSH_ERR_ALLOC_FAIL;
-	free(*list);
-	*list = ret;
 	return 0;
 }
 
diff --git a/usr.bin/ssh/match.c b/usr.bin/ssh/match.c
index 8fd1c9b0451..1a9c5288881 100644
--- a/usr.bin/ssh/match.c
+++ b/usr.bin/ssh/match.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: match.c,v 1.33 2016/11/06 05:46:37 djm Exp $ */
+/* $OpenBSD: match.c,v 1.34 2017/02/03 23:01:19 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -282,3 +282,32 @@ match_list(const char *client, const char *server, u_int *next)
 	free(s);
 	return NULL;
 }
+
+/*
+ * Filters a comma-separated list of strings, excluding any entry matching
+ * the 'filter' pattern list. Caller must free returned string.
+ */
+char *
+match_filter_list(const char *proposal, const char *filter)
+{
+	size_t len = strlen(proposal) + 1;
+	char *fix_prop = malloc(len);
+	char *orig_prop = strdup(proposal);
+	char *cp, *tmp;
+
+	if (fix_prop == NULL || orig_prop == NULL)
+		return NULL;
+
+	tmp = orig_prop;
+	*fix_prop = '\0';
+	while ((cp = strsep(&tmp, ",")) != NULL) {
+		if (match_pattern_list(cp, filter, 0) != 1) {
+			if (*fix_prop != '\0')
+				strlcat(fix_prop, ",", len);
+			strlcat(fix_prop, cp, len);
+		}
+	}
+	free(orig_prop);
+	return fix_prop;
+}
+
diff --git a/usr.bin/ssh/match.h b/usr.bin/ssh/match.h
index db97ca8f7a2..937ba041277 100644
--- a/usr.bin/ssh/match.h
+++ b/usr.bin/ssh/match.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: match.h,v 1.16 2015/05/04 06:10:48 djm Exp $ */
+/* $OpenBSD: match.h,v 1.17 2017/02/03 23:01:19 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -20,6 +20,7 @@ int	 match_hostname(const char *, const char *);
 int	 match_host_and_ip(const char *, const char *, const char *);
 int	 match_user(const char *, const char *, const char *, const char *);
 char	*match_list(const char *, const char *, u_int *);
+char	*match_filter_list(const char *, const char *);
 
 /* addrmatch.c */
 int	 addr_match_list(const char *, const char *);
diff --git a/usr.bin/ssh/readconf.c b/usr.bin/ssh/readconf.c
index e8f63e381e3..97b3550388b 100644
--- a/usr.bin/ssh/readconf.c
+++ b/usr.bin/ssh/readconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: readconf.c,v 1.267 2017/02/03 05:05:56 djm Exp $ */
+/* $OpenBSD: readconf.c,v 1.268 2017/02/03 23:01:19 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1179,7 +1179,7 @@ parse_int:
 		arg = strdelim(&s);
 		if (!arg || *arg == '\0')
 			fatal("%.200s line %d: Missing argument.", filename, linenum);
-		if (!ciphers_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' && !ciphers_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%.200s line %d: Bad SSH2 cipher spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (*activep && options->ciphers == NULL)
@@ -1190,7 +1190,7 @@ parse_int:
 		arg = strdelim(&s);
 		if (!arg || *arg == '\0')
 			fatal("%.200s line %d: Missing argument.", filename, linenum);
-		if (!mac_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' && !mac_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%.200s line %d: Bad SSH2 Mac spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (*activep && options->macs == NULL)
@@ -1202,7 +1202,8 @@ parse_int:
 		if (!arg || *arg == '\0')
 			fatal("%.200s line %d: Missing argument.",
 			    filename, linenum);
-		if (!kex_names_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' &&
+		    !kex_names_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%.200s line %d: Bad SSH2 KexAlgorithms '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (*activep && options->kex_algorithms == NULL)
@@ -1216,7 +1217,8 @@ parse_keytypes:
 		if (!arg || *arg == '\0')
 			fatal("%.200s line %d: Missing argument.",
 			    filename, linenum);
-		if (!sshkey_names_valid2(*arg == '+' ? arg + 1 : arg, 1))
+		if (*arg != '-' &&
+		    !sshkey_names_valid2(*arg == '+' ? arg + 1 : arg, 1))
 			fatal("%s line %d: Bad key types '%s'.",
 				filename, linenum, arg ? arg : "<NONE>");
 		if (*activep && *charptr == NULL)
diff --git a/usr.bin/ssh/servconf.c b/usr.bin/ssh/servconf.c
index 08e66a98690..434b639b430 100644
--- a/usr.bin/ssh/servconf.c
+++ b/usr.bin/ssh/servconf.c
@@ -1,5 +1,5 @@
 
-/* $OpenBSD: servconf.c,v 1.303 2017/02/03 05:05:56 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.304 2017/02/03 23:01:19 djm Exp $ */
 /*
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
  *                    All rights reserved
@@ -1125,7 +1125,8 @@ process_server_config_line(ServerOptions *options, char *line,
 		if (!arg || *arg == '\0')
 			fatal("%s line %d: Missing argument.",
 			    filename, linenum);
-		if (!sshkey_names_valid2(*arg == '+' ? arg + 1 : arg, 1))
+		if (*arg != '-' &&
+		    !sshkey_names_valid2(*arg == '+' ? arg + 1 : arg, 1))
 			fatal("%s line %d: Bad key types '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (*activep && *charptr == NULL)
@@ -1384,7 +1385,7 @@ process_server_config_line(ServerOptions *options, char *line,
 		arg = strdelim(&cp);
 		if (!arg || *arg == '\0')
 			fatal("%s line %d: Missing argument.", filename, linenum);
-		if (!ciphers_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' && !ciphers_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%s line %d: Bad SSH2 cipher spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (options->ciphers == NULL)
@@ -1395,7 +1396,7 @@ process_server_config_line(ServerOptions *options, char *line,
 		arg = strdelim(&cp);
 		if (!arg || *arg == '\0')
 			fatal("%s line %d: Missing argument.", filename, linenum);
-		if (!mac_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' && !mac_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%s line %d: Bad SSH2 mac spec '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (options->macs == NULL)
@@ -1407,7 +1408,8 @@ process_server_config_line(ServerOptions *options, char *line,
 		if (!arg || *arg == '\0')
 			fatal("%s line %d: Missing argument.",
 			    filename, linenum);
-		if (!kex_names_valid(*arg == '+' ? arg + 1 : arg))
+		if (*arg != '-' &&
+		    !kex_names_valid(*arg == '+' ? arg + 1 : arg))
 			fatal("%s line %d: Bad SSH2 KexAlgorithms '%s'.",
 			    filename, linenum, arg ? arg : "<NONE>");
 		if (options->kex_algorithms == NULL)
diff --git a/usr.bin/ssh/ssh_config.5 b/usr.bin/ssh/ssh_config.5
index 591365f34cb..016adbc7343 100644
--- a/usr.bin/ssh/ssh_config.5
+++ b/usr.bin/ssh/ssh_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: ssh_config.5,v 1.240 2016/10/15 19:56:25 jmc Exp $
-.Dd $Mdocdate: October 15 2016 $
+.\" $OpenBSD: ssh_config.5,v 1.241 2017/02/03 23:01:19 djm Exp $
+.Dd $Mdocdate: February 3 2017 $
 .Dt SSH_CONFIG 5
 .Os
 .Sh NAME
@@ -415,6 +415,10 @@ If the specified value begins with a
 .Sq +
 character, then the specified ciphers will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified ciphers (including wildcards) will be removed
+from the default set instead of replacing them.
 .Pp
 The supported ciphers are:
 .Bd -literal -offset indent
@@ -784,6 +788,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified key types will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified key types (including wildcards) will be removed
+from the default set instead of replacing them.
 The default for this option is:
 .Bd -literal -offset 3n
 ecdsa-sha2-nistp256-cert-v01@openssh.com,
@@ -807,6 +815,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified key types will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified key types (including wildcards) will be removed
+from the default set instead of replacing them.
 The default for this option is:
 .Bd -literal -offset 3n
 ecdsa-sha2-nistp256-cert-v01@openssh.com,
@@ -1027,6 +1039,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified methods will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified methods (including wildcards) will be removed
+from the default set instead of replacing them.
 The default is:
 .Bd -literal -offset indent
 curve25519-sha256,curve25519-sha256@libssh.org,
@@ -1102,6 +1118,10 @@ If the specified value begins with a
 .Sq +
 character, then the specified algorithms will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified algorithms (including wildcards) will be removed
+from the default set instead of replacing them.
 .Pp
 The algorithms that contain
 .Qq -etm
@@ -1264,6 +1284,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the key types after it will be appended to the default
 instead of replacing it.
+If the specified value begins with a
+.Sq -
+character, then the specified key types (including wildcards) will be removed
+from the default set instead of replacing them.
 The default for this option is:
 .Bd -literal -offset 3n
 ecdsa-sha2-nistp256-cert-v01@openssh.com,
diff --git a/usr.bin/ssh/sshd_config.5 b/usr.bin/ssh/sshd_config.5
index 620dfd077ad..53ee5601f7d 100644
--- a/usr.bin/ssh/sshd_config.5
+++ b/usr.bin/ssh/sshd_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: sshd_config.5,v 1.241 2017/01/06 16:28:12 jmc Exp $
-.Dd $Mdocdate: January 6 2017 $
+.\" $OpenBSD: sshd_config.5,v 1.242 2017/02/03 23:01:19 djm Exp $
+.Dd $Mdocdate: February 3 2017 $
 .Dt SSHD_CONFIG 5
 .Os
 .Sh NAME
@@ -438,6 +438,10 @@ If the specified value begins with a
 .Sq +
 character, then the specified ciphers will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified ciphers (including wildcards) will be removed
+from the default set instead of replacing them.
 .Pp
 The supported ciphers are:
 .Pp
@@ -650,6 +654,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified key types will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified key types (including wildcards) will be removed
+from the default set instead of replacing them.
 The default for this option is:
 .Bd -literal -offset 3n
 ecdsa-sha2-nistp256-cert-v01@openssh.com,
@@ -844,6 +852,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified methods will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified methods (including wildcards) will be removed
+from the default set instead of replacing them.
 The supported algorithms are:
 .Pp
 .Bl -item -compact -offset indent
@@ -934,6 +946,10 @@ If the specified value begins with a
 .Sq +
 character, then the specified algorithms will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified algorithms (including wildcards) will be removed
+from the default set instead of replacing them.
 .Pp
 The algorithms that contain
 .Qq -etm
@@ -1281,6 +1297,10 @@ Alternately if the specified value begins with a
 .Sq +
 character, then the specified key types will be appended to the default set
 instead of replacing them.
+If the specified value begins with a
+.Sq -
+character, then the specified key types (including wildcards) will be removed
+from the default set instead of replacing them.
 The default for this option is:
 .Bd -literal -offset 3n
 ecdsa-sha2-nistp256-cert-v01@openssh.com,