Fix regression of Match directive processing

Resolves: RHEL-76317
This commit is contained in:
Dmitry Belyavskiy 2025-01-27 12:34:57 +01:00
parent 84c0936017
commit 35bf325387
2 changed files with 472 additions and 1 deletions

View File

@ -0,0 +1,463 @@
diff --git a/misc.c b/misc.c
index afdf5142..1b4b55c5 100644
--- a/misc.c
+++ b/misc.c
@@ -107,6 +107,27 @@ rtrim(char *s)
}
}
+/*
+ * returns pointer to character after 'prefix' in 's' or otherwise NULL
+ * if the prefix is not present.
+ */
+const char *
+strprefix(const char *s, const char *prefix, int ignorecase)
+{
+ size_t prefixlen;
+
+ if ((prefixlen = strlen(prefix)) == 0)
+ return s;
+ if (ignorecase) {
+ if (strncasecmp(s, prefix, prefixlen) != 0)
+ return NULL;
+ } else {
+ if (strncmp(s, prefix, prefixlen) != 0)
+ return NULL;
+ }
+ return s + prefixlen;
+}
+
/* set/unset filedescriptor to non-blocking */
int
set_nonblock(int fd)
diff --git a/misc.h b/misc.h
index 11340389..efecdf1a 100644
--- a/misc.h
+++ b/misc.h
@@ -56,6 +56,7 @@ struct ForwardOptions {
char *chop(char *);
void rtrim(char *);
void skip_space(char **);
+const char *strprefix(const char *, const char *, int);
char *strdelim(char **);
char *strdelimw(char **);
int set_nonblock(int);
diff --git a/readconf.c b/readconf.c
index 3d9cc6db..9f559269 100644
--- a/readconf.c
+++ b/readconf.c
@@ -710,7 +710,7 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
struct passwd *pw, const char *host_arg, const char *original_host,
int final_pass, int *want_final_pass, const char *filename, int linenum)
{
- char *arg, *oattrib, *attrib, *cmd, *host, *criteria;
+ char *arg, *oattrib = NULL, *attrib = NULL, *cmd, *host, *criteria;
const char *ruser;
int r, this_result, result = 1, attributes = 0, negate;
@@ -731,7 +731,8 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
debug2("checking match for '%s' host %s originally %s",
full_line, host, original_host);
- while ((oattrib = attrib = argv_next(acp, avp)) != NULL) {
+ while ((attrib = argv_next(acp, avp)) != NULL) {
+ attrib = oattrib = xstrdup(attrib);
/* Terminate on comment */
if (*attrib == '#') {
argv_consume(acp);
@@ -777,9 +778,23 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
this_result ? "" : "not ", oattrib);
continue;
}
+
+ /* Keep this list in sync with below */
+ if (strprefix(attrib, "host=", 1) != NULL ||
+ strprefix(attrib, "originalhost=", 1) != NULL ||
+ strprefix(attrib, "user=", 1) != NULL ||
+ strprefix(attrib, "localuser=", 1) != NULL ||
+ strprefix(attrib, "localnetwork=", 1) != NULL ||
+ strprefix(attrib, "tagged=", 1) != NULL ||
+ strprefix(attrib, "exec=", 1) != NULL) {
+ arg = strchr(attrib, '=');
+ *(arg++) = '\0';
+ } else {
+ arg = argv_next(acp, avp);
+ }
+
/* All other criteria require an argument */
- if ((arg = argv_next(acp, avp)) == NULL ||
- *arg == '\0' || *arg == '#') {
+ if (arg == NULL || *arg == '\0' || *arg == '#') {
error("Missing Match criteria for %s", attrib);
result = -1;
goto out;
@@ -856,6 +871,8 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
criteria == NULL ? "" : criteria,
criteria == NULL ? "" : "\"");
free(criteria);
+ free(oattrib);
+ oattrib = attrib = NULL;
}
if (attributes == 0) {
error("One or more attributes required for Match");
@@ -865,6 +882,7 @@ match_cfg_line(Options *options, const char *full_line, int *acp, char ***avp,
out:
if (result != -1)
debug2("match %sfound", result ? "" : "not ");
+ free(oattrib);
free(host);
return result;
}
diff --git a/servconf.c b/servconf.c
index 89b8413e..dd774f46 100644
--- a/servconf.c
+++ b/servconf.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: servconf.c,v 1.418 2024/09/15 03:09:44 djm Exp $ */
+/* $OpenBSD: servconf.c,v 1.419 2024/09/25 01:24:04 djm Exp $ */
/*
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
* All rights reserved
@@ -1033,7 +1033,7 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
int line, struct connection_info *ci)
{
int result = 1, attributes = 0, port;
- char *arg, *attrib;
+ char *arg, *attrib = NULL, *oattrib;
if (ci == NULL)
debug3("checking syntax for 'Match %s'", full_line);
@@ -1047,7 +1047,8 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
ci->laddress ? ci->laddress : "(null)", ci->lport);
}
- while ((attrib = argv_next(acp, avp)) != NULL) {
+ while ((oattrib = argv_next(acp, avp)) != NULL) {
+ attrib = xstrdup(oattrib);
/* Terminate on comment */
if (*attrib == '#') {
argv_consume(acp); /* mark all arguments consumed */
@@ -1062,11 +1063,13 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
*arg != '\0' && *arg != '#')) {
error("'all' cannot be combined with other "
"Match attributes");
- return -1;
+ result = -1;
+ goto out;
}
if (arg != NULL && *arg == '#')
argv_consume(acp); /* consume remaining args */
- return 1;
+ result = 1;
+ goto out;
}
/* Criterion "invalid-user" also has no argument */
if (strcasecmp(attrib, "invalid-user") == 0) {
@@ -1078,11 +1081,26 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
debug("matched invalid-user at line %d", line);
continue;
}
+
+ /* Keep this list in sync with below */
+ if (strprefix(attrib, "user=", 1) != NULL ||
+ strprefix(attrib, "group=", 1) != NULL ||
+ strprefix(attrib, "host=", 1) != NULL ||
+ strprefix(attrib, "address=", 1) != NULL ||
+ strprefix(attrib, "localaddress=", 1) != NULL ||
+ strprefix(attrib, "localport=", 1) != NULL ||
+ strprefix(attrib, "rdomain=", 1) != NULL) {
+ arg = strchr(attrib, '=');
+ *(arg++) = '\0';
+ } else {
+ arg = argv_next(acp, avp);
+ }
+
/* All other criteria require an argument */
- if ((arg = argv_next(acp, avp)) == NULL ||
- *arg == '\0' || *arg == '#') {
+ if (arg == NULL || *arg == '\0' || *arg == '#') {
error("Missing Match criteria for %s", attrib);
- return -1;
+ result = -1;
+ goto out;
}
if (strcasecmp(attrib, "user") == 0) {
if (ci == NULL || (ci->test && ci->user == NULL)) {
@@ -1105,7 +1123,8 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
match_test_missing_fatal("Group", "user");
switch (match_cfg_line_group(arg, line, ci->user)) {
case -1:
- return -1;
+ result = -1;
+ goto out;
case 0:
result = 0;
}
@@ -1141,7 +1160,8 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
result = 0;
break;
case -2:
- return -1;
+ result = -1;
+ goto out;
}
} else if (strcasecmp(attrib, "localaddress") == 0){
if (ci == NULL || (ci->test && ci->laddress == NULL)) {
@@ -1166,13 +1186,15 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
result = 0;
break;
case -2:
- return -1;
+ result = -1;
+ goto out;
}
} else if (strcasecmp(attrib, "localport") == 0) {
if ((port = a2port(arg)) == -1) {
error("Invalid LocalPort '%s' on Match line",
arg);
- return -1;
+ result = -1;
+ goto out;
}
if (ci == NULL || (ci->test && ci->lport == -1)) {
result = 0;
@@ -1200,16 +1222,21 @@ match_cfg_line(const char *full_line, int *acp, char ***avp,
debug("user %.100s matched 'RDomain %.100s' at "
"line %d", ci->rdomain, arg, line);
} else {
- error("Unsupported Match attribute %s", attrib);
- return -1;
+ error("Unsupported Match attribute %s", oattrib);
+ result = -1;
+ goto out;
}
+ free(attrib);
+ attrib = NULL;
}
if (attributes == 0) {
error("One or more attributes required for Match");
return -1;
}
- if (ci != NULL)
+ out:
+ if (ci != NULL && result != -1)
debug3("match %sfound", result ? "" : "not ");
+ free(attrib);
return result;
}
diff --git a/regress/cfginclude.sh b/regress/cfginclude.sh
index d442cdd6..97fd816f 100644
--- a/regress/cfginclude.sh
+++ b/regress/cfginclude.sh
@@ -1,4 +1,4 @@
-# $OpenBSD: cfginclude.sh,v 1.4 2024/09/03 05:58:56 djm Exp $
+# $OpenBSD: cfginclude.sh,v 1.5 2024/09/27 01:05:54 djm Exp $
# Placed in the Public Domain.
tid="config include"
@@ -10,7 +10,7 @@ cat > $OBJ/ssh_config.i << _EOF
Match host a
Hostname aa
-Match host b # comment
+Match host=b # comment
Hostname bb
Include $OBJ/ssh_config.i.*
@@ -18,7 +18,7 @@ Match host c
Include $OBJ/ssh_config.i.*
Hostname cc
-Match host m
+Match host=m !user xxxyfake
Include $OBJ/ssh_config.i.* # comment
Host d
@@ -41,7 +41,7 @@ Match host xxxxxx
_EOF
cat > $OBJ/ssh_config.i.1 << _EOF
-Match host a
+Match host=a
Hostname aaa
Match host b
@@ -64,10 +64,10 @@ cat > $OBJ/ssh_config.i.2 << _EOF
Match host a
Hostname aaaa
-Match host b
+Match host=b !user blahblahfake
Hostname bbbb
-Match host c
+Match host=c
Hostname cccc
Host d
@@ -142,7 +142,7 @@ trial a aa
# cleanup
rm -f $OBJ/ssh_config.i $OBJ/ssh_config.i.* $OBJ/ssh_config.out
-# $OpenBSD: cfginclude.sh,v 1.4 2024/09/03 05:58:56 djm Exp $
+# $OpenBSD: cfginclude.sh,v 1.5 2024/09/27 01:05:54 djm Exp $
# Placed in the Public Domain.
tid="config include"
diff --git a/regress/cfgmatch.sh b/regress/cfgmatch.sh
index 05a66685..2737a5f9 100644
--- a/regress/cfgmatch.sh
+++ b/regress/cfgmatch.sh
@@ -1,4 +1,4 @@
-# $OpenBSD: cfgmatch.sh,v 1.13 2021/06/08 06:52:43 djm Exp $
+# $OpenBSD: cfgmatch.sh,v 1.14 2024/09/27 01:05:54 djm Exp $
# Placed in the Public Domain.
tid="sshd_config match"
@@ -26,7 +26,7 @@ start_client()
kill $client_pid
fatal "timeout waiting for background ssh"
fi
- done
+ done
}
stop_client()
@@ -119,40 +119,42 @@ stop_client
# requires knowledge of actual group memberships user running the test).
params="user:user:u1 host:host:h1 address:addr:1.2.3.4 \
localaddress:laddr:5.6.7.8 rdomain:rdomain:rdom1"
-cp $OBJ/sshd_proxy_bak $OBJ/sshd_config
-echo 'Banner /nomatch' >>$OBJ/sshd_config
-for i in $params; do
- config=`echo $i | cut -f1 -d:`
- criteria=`echo $i | cut -f2 -d:`
- value=`echo $i | cut -f3 -d:`
- cat >>$OBJ/sshd_config <<EOD
- Match $config $value
- Banner /$value
+for separator in " " "=" ; do
+ cp $OBJ/sshd_proxy_bak $OBJ/sshd_config
+ echo 'Banner /nomatch' >>$OBJ/sshd_config
+ for i in $params; do
+ config=`echo $i | cut -f1 -d:`
+ criteria=`echo $i | cut -f2 -d:`
+ value=`echo $i | cut -f3 -d:`
+ cat >>$OBJ/sshd_config <<EOD
+ Match ${config}${separator}${value}
+ Banner /$value
EOD
-done
+ done
-${SUDO} ${SSHD} -f $OBJ/sshd_config -T >/dev/null || \
- fail "validate config for w/out spec"
-
-# Test matching each criteria.
-for i in $params; do
- testcriteria=`echo $i | cut -f2 -d:`
- expected=/`echo $i | cut -f3 -d:`
- spec=""
- for j in $params; do
- config=`echo $j | cut -f1 -d:`
- criteria=`echo $j | cut -f2 -d:`
- value=`echo $j | cut -f3 -d:`
- if [ "$criteria" = "$testcriteria" ]; then
- spec="$criteria=$value,$spec"
- else
- spec="$criteria=1$value,$spec"
+ ${SUDO} ${SSHD} -f $OBJ/sshd_config -T >/dev/null || \
+ fail "validate config for w/out spec"
+
+ # Test matching each criteria.
+ for i in $params; do
+ testcriteria=`echo $i | cut -f2 -d:`
+ expected=/`echo $i | cut -f3 -d:`
+ spec=""
+ for j in $params; do
+ config=`echo $j | cut -f1 -d:`
+ criteria=`echo $j | cut -f2 -d:`
+ value=`echo $j | cut -f3 -d:`
+ if [ "$criteria" = "$testcriteria" ]; then
+ spec="$criteria=$value,$spec"
+ else
+ spec="$criteria=1$value,$spec"
+ fi
+ done
+ trace "test spec $spec"
+ result=`${SUDO} ${SSHD} -f $OBJ/sshd_config -T -C "$spec" | \
+ awk '$1=="banner"{print $2}'`
+ if [ "$result" != "$expected" ]; then
+ fail "match $config expected $expected got $result"
fi
done
- trace "test spec $spec"
- result=`${SUDO} ${SSHD} -f $OBJ/sshd_config -T -C "$spec" | \
- awk '$1=="banner"{print $2}'`
- if [ "$result" != "$expected" ]; then
- fail "match $config expected $expected got $result"
- fi
done
diff --git a/regress/servcfginclude.sh b/regress/servcfginclude.sh
index 518a703d..f67c3caa 100644
--- a/regress/servcfginclude.sh
+++ b/regress/servcfginclude.sh
@@ -4,14 +4,14 @@ tid="server config include"
cat > $OBJ/sshd_config.i << _EOF
HostKey $OBJ/host.ssh-ed25519
-Match host a
+Match host=a
Banner /aa
Match host b
Banner /bb
Include $OBJ/sshd_config.i.* # comment
-Match host c
+Match host=c
Include $OBJ/sshd_config.i.* # comment
Banner /cc
@@ -25,7 +25,7 @@ Match Host e
Banner /ee
Include $OBJ/sshd_config.i.*
-Match Host f
+Match Host=f
Include $OBJ/sshd_config.i.*
Banner /ff
@@ -47,13 +47,13 @@ Match host b
Match host c
Banner /ccc
-Match Host d
+Match Host=d
Banner /ddd
Match Host e
Banner /eee
-Match Host f
+Match Host=f
Banner /fff
_EOF
@@ -61,13 +61,13 @@ cat > $OBJ/sshd_config.i.2 << _EOF
Match host a
Banner /aaaa
-Match host b
+Match host=b
Banner /bbbb
Match host c # comment
Banner /cccc
-Match Host d
+Match Host=d
Banner /dddd
Match Host e

View File

@ -39,7 +39,7 @@
%{?static_openssl:%global static_libcrypto 1}
%global openssh_ver 9.9p1
%global openssh_rel 4
%global openssh_rel 5
Summary: An open source implementation of SSH protocol version 2
Name: openssh
@ -199,6 +199,9 @@ Patch1018: openssh-8.7p1-openssl-log.patch
# upstream cf3e48ee8ba1beeccddd2f203b558fa102be67a2
# upstream 0c3927c45f8a57b511c874c4d51a8c89414f74ef
Patch1019: openssh-9.9p1-mlkembe.patch
# upstream 3f02368e8e9121847727c46b280efc280e5eb615
# upstream 67a115e7a56dbdc3f5a58c64b29231151f3670f5
Patch1020: openssh-9.9p1-match-regression.patch
License: BSD-3-Clause AND BSD-2-Clause AND ISC AND SSH-OpenSSH AND ssh-keyscan AND sprintf AND LicenseRef-Fedora-Public-Domain AND X11-distribute-modifications-variant
Requires: /sbin/nologin
@ -383,6 +386,7 @@ gpgv2 --quiet --keyring %{SOURCE3} %{SOURCE1} %{SOURCE0}
%patch -P 1017 -p1 -b .help
%patch -P 1018 -p1 -b .openssl-log
%patch -P 1019 -p1 -b .mlkembe
%patch -P 1020 -p1 -b .match
%patch -P 100 -p1 -b .coverity
@ -662,6 +666,10 @@ test -f %{sysconfig_anaconda} && \
%attr(0755,root,root) %{_libdir}/sshtest/sk-dummy.so
%changelog
* Mon Jan 27 2025 Dmitry Belyavskiy <dbelyavs@redhat.com> - 9.9p1-5
- Fix regression of Match directive processing
Resolves: RHEL-76317
* Tue Oct 29 2024 Troy Dawson <tdawson@redhat.com> - 9.9p1-4.1
- Bump release for October 2024 mass rebuild:
Resolves: RHEL-64018