[PATCH] ssh: reject "all" combined with other Match criteria

[jelly]

Hi,
the "all" keyword in a Match block must appear alone.  sshd enforces
this, but the ssh client does not, so the same configuration file is
accepted by one half of the toolchain and rejected by the other.
In readconf.c:match_cfg_line(), the check that "all" is not combined
with any other criteria is made with "attributes > 1", but the counter
is incremented only after that check:
	if ((negate = (attrib[0] == '!')))
		attrib++;
	/* Criterion "all" has no argument and must appear alone */
	if (strcasecmp(attrib, "all") == 0) {
		if (attributes > 1 || ...) {
			error("'all' cannot be combined with other "
			    "Match attributes");
			result = -1;
			goto out;
		}
		...
		goto out;
	}
	attributes++;
So when "all" is processed it observes only the number of preceding
criteria.  For "Match user foo all" attributes == 1 at that point, the
check is skipped, and the line is silently accepted.  servconf.c
increments before the check (servconf.c:864) and rejects the same
configuration, which is the behaviour the manual describes:
	$ cat cfg
	Match user foo all
		Compression yes
	$ ssh -G -F cfg example.com
	exit=0					# accepted (wrong)
	$ sshd -T -f cfg
	'all' cannot be combined with other Match attributes
	exit=255				# rejected (correct)
The practical effect is that a user can write a Match block intending to
scope options to a subset of connections, and the client will silently
treat it as broader than written (Match x all is equivalent to Match x).
It is a validation inconsistency rather than a security bug, but since
the client and server disagree about which configurations are valid, it
seems worth making consistent.
The fix is to move the increment ahead of the check, matching what
servconf.c already does.
Tested on current master (4001f0a), patched build:
- "Match user foo all" and "Match !user foo all" -> rejected (exit 255)
- "Match all" -> still accepted (exit 0)
- "Match all foo" -> still rejected by the existing argument check
- "Match host x user y" -> accepted (multiple criteria without all:
unaffected)
- "make unit" passes
- regress/sshcfgparse.sh gains coverage for both the standalone and
combined cases; the new case fails if readconf.c alone is reverted
("Match combined with all accepted"), so it really exercises the bug
The same change is pushed to the immortal-jelly/openssh-portable branch
"match-all-alone".
From 99d13f1f75a7ecaaafeabf3603e725c670b12982 Mon Sep 17 00:00:00 2001
From: xuehaolin <xuehaolin at uniontech.com>
Date: Tue, 3 Nov 2026 14:08:27 +0800
Subject: [PATCH] ssh: reject "all" combined with other Match criteria
match_cfg_line() in readconf.c checks whether the "all" keyword is
combined with other criteria with "attributes > 1", but increments
attributes only after that check.  "all" therefore observes the number
of preceding criteria, so "Match user foo all" passes the check and is
silently accepted.
sshd's servconf.c increments the counter before the check and rejects
the same configuration, so the client and server disagree about which
configurations are valid.
Move the increment ahead of the check to match servconf.c, and add
regression test coverage for both the standalone "Match all" case and
the combined "Match user foo all" case.
---
readconf.c             |  2 +-
regress/sshcfgparse.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/readconf.c b/readconf.c
index 3c21bcb..5c6d3ac 100644
--- a/readconf.c
+++ b/readconf.c
@@ -777,6 +777,7 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
		this_result = 1;
		if ((negate = (attrib[0] == '!')))
			attrib++;
+		attributes++;
		/* Criterion "all" has no argument and must appear alone */
		if (strcasecmp(attrib, "all") == 0) {
			if (attributes > 1 ||
@@ -794,7 +795,6 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
				result = negate ? 0 : 1;
			goto out;
		}
-		attributes++;
		/* criteria "final" and "canonical" have no argument */
		if (strcasecmp(attrib, "canonical") == 0 ||
		    strcasecmp(attrib, "final") == 0) {
diff --git a/regress/sshcfgparse.sh b/regress/sshcfgparse.sh
index 784e841..7cfd74e 100644
--- a/regress/sshcfgparse.sh
+++ b/regress/sshcfgparse.sh
@@ -117,5 +117,20 @@ EOD
f=`${SSH} -GF $OBJ/ssh_config.0 host true | awk '/^stdinnull /{print$2}'`
expect_result_present "$f" "yes"
+# "all" must appear alone in a Match block
+verbose "Match all"
+cat >$OBJ/ssh_config.0 <<EOD
+Match all
+	Compression yes
+EOD
+${SSH} -GF $OBJ/ssh_config.0 host >/dev/null 2>&1 || \
+	fail "standalone Match all rejected"
+cat >$OBJ/ssh_config.0 <<EOD
+Match user foo all
+	Compression yes
+EOD
+${SSH} -GF $OBJ/ssh_config.0 host >/dev/null 2>&1 && \
+	fail "Match combined with all accepted"
+
# cleanup
rm -f $OBJ/ssh_config.[012]
--
2.43.0