On Wed, Dec 14, 2016 at 11:53:32AM +1100, Damien Miller wrote:
Hi,
OpenSSH 7.4 is almost ready for release, so we would appreciate testing
on as many platforms and systems as possible. This release contains some
substantial new features and a number of bugfixes.
Hi,
I tested (or tried) git commit b737e4d7 on three systems, with somewhat
mixed results.
On Mac OSX (macOS?) 10.9, configure failed with:
...
checking OpenSSL header version... 1000208f (OpenSSL 1.0.2h 3 May 2016)
checking OpenSSL library version... 009081df (OpenSSL 0.9.8zg 14 July 2015)
checking whether OpenSSL's headers match the library... no
configure: error: Your OpenSSL headers do not match your
library. Check config.log for details.
A second attempt with configure's openssl-dir pointed at a macports
install in /opt/local built successfully and passed all tests, though
there were some warnings during the build (mostly noticed just because I
configured with -Werror and then manually papered over them; not sure
how important these really are):
- daemon() deprecated (ssh.c, sshd.c)
- utmp, login, logout, logwtmp deprecated (loginrec.c)
- sandbox_init() deprecated (sandbox-darwin.c)
- struct monitor declared in ssh_sandbox_init() parameter list
(sandbox-darwin.c)
- set-but-unused 'flag' variable in sys_tun_open() (port-tun.c)
On Void Linux (which uses LibreSSL, for what it's worth): unable to
compile due to undeclared arc4random*() functions. The symbols exist in
libcrypto so configure's tests for them pass, but they're not declared
in any header files. I'm not sure where exactly these are "supposed" to
be declared, so I don't know if this is a problem with OpenSSH or
LibreSSL or some packaging bungle on Void's part.
On Debian testing: discovered a small-but-significant problem in
auth.c's allowed_user() function. Commit 010359b3 expanded the body of
the loop that checks DenyUsers entries, but didn't add the necessary
braces around it, so it didn't exactly have the intended effect, instead
resulting in only the last entry in DenyUsers actually being enforced.
(Credit to gcc's -Wmisleading-indentation warning here.)
The attached patch 0001-Unbreak-DenyUsers-with-1-user-specified.patch
fixes the bug; the next two patches
(0002-Add-and-use-stop_sshd-helper-function.patch and
0003-Add-regression-test-for-AllowUsers-DenyUsers.patch) add a
regression test that detects it (failing before and passing after patch
0001).
With those patches applied a -Werror build completed successfully and
passed all tests.
(I also noticed a mis-typed error message in dh.c, addressed in the
attached 0004-Fix-mis-worded-error-message-in-choose_dh.patch.)
Thanks,
Zev
>From dd141737924dc6cf456c08fff984a7e567cb07d0 Mon Sep 17 00:00:00 2001
From: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
Date: Wed, 14 Dec 2016 21:48:38 -0600
Subject: [PATCH 1/4] Unbreak DenyUsers with > 1 user specified.
Broken in 010359b32659f455fddd2bd85fd7cc4d7a3b994a by missing braces
around loop body (causing only the last user listed to be denied);
detected by gcc's -Wmisleading-indentation.
---
auth.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/auth.c b/auth.c
index 2de2db4..b2114a9 100644
--- a/auth.c
+++ b/auth.c
@@ -192,7 +192,7 @@ allowed_user(struct passwd * pw)
/* Return false if user is listed in DenyUsers */
if (options.num_deny_users > 0) {
- for (i = 0; i < options.num_deny_users; i++)
+ for (i = 0; i < options.num_deny_users; i++) {
r = match_user(pw->pw_name, hostname, ipaddr,
options.deny_users[i]);
if (r < 0) {
@@ -204,6 +204,7 @@ allowed_user(struct passwd * pw)
pw->pw_name, hostname);
return 0;
}
+ }
}
/* Return false if AllowUsers isn't empty and user isn't listed there */
if (options.num_allow_users > 0) {
--
2.10.2
>From 700290af91bc5c30e1b9a3a4ed95b30b2caddbb6 Mon Sep 17 00:00:00 2001
From: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
Date: Wed, 14 Dec 2016 22:04:04 -0600
Subject: [PATCH 2/4] Add and use stop_sshd helper function.
For tests that want to run sshd multiple times.
---
regress/login-timeout.sh | 2 +-
regress/reexec.sh | 9 +++------
regress/test-exec.sh | 23 ++++++++++++++---------
3 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/regress/login-timeout.sh b/regress/login-timeout.sh
index eb76f55..a3ce81f 100644
--- a/regress/login-timeout.sh
+++ b/regress/login-timeout.sh
@@ -17,7 +17,7 @@ if [ $? -ne 0 ]; then
fail "ssh connect after login grace timeout failed with privsep"
fi
-$SUDO kill `$SUDO cat $PIDFILE`
+stop_sshd
trace "test login grace without privsep"
echo "UsePrivilegeSeparation no" >> $OBJ/sshd_config
diff --git a/regress/reexec.sh b/regress/reexec.sh
index 5c0a7b4..dccde25 100644
--- a/regress/reexec.sh
+++ b/regress/reexec.sh
@@ -39,8 +39,7 @@ echo "InvalidXXX=no" >> $OBJ/sshd_config
copy_tests
-$SUDO kill `$SUDO cat $PIDFILE`
-rm -f $PIDFILE
+stop_sshd
cp $OBJ/sshd_config.orig $OBJ/sshd_config
@@ -54,8 +53,7 @@ rm -f $SSHD_COPY
copy_tests
-$SUDO kill `$SUDO cat $PIDFILE`
-rm -f $PIDFILE
+stop_sshd
verbose "test reexec fallback without privsep"
@@ -67,7 +65,6 @@ rm -f $SSHD_COPY
copy_tests
-$SUDO kill `$SUDO cat $PIDFILE`
-rm -f $PIDFILE
+stop_sshd
fi
diff --git a/regress/test-exec.sh b/regress/test-exec.sh
index 5d48706..74083df 100644
--- a/regress/test-exec.sh
+++ b/regress/test-exec.sh
@@ -293,16 +293,8 @@ md5 () {
}
# End of portable specific functions
-# helper
-cleanup ()
+stop_sshd ()
{
- if [ "x$SSH_PID" != "x" ]; then
- if [ $SSH_PID -lt 2 ]; then
- echo bad pid for ssh: $SSH_PID
- else
- kill $SSH_PID
- fi
- fi
if [ -f $PIDFILE ]; then
pid=`$SUDO cat $PIDFILE`
if [ "X$pid" = "X" ]; then
@@ -325,6 +317,19 @@ cleanup ()
fi
}
+# helper
+cleanup ()
+{
+ if [ "x$SSH_PID" != "x" ]; then
+ if [ $SSH_PID -lt 2 ]; then
+ echo bad pid for ssh: $SSH_PID
+ else
+ kill $SSH_PID
+ fi
+ fi
+ stop_sshd
+}
+
start_debug_log ()
{
echo "trace: $@" >$TEST_REGRESS_LOGFILE
--
2.10.2
>From 67d2a87c5dadc9d1db699504d57e06c254e26ad2 Mon Sep 17 00:00:00 2001
From: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
Date: Wed, 14 Dec 2016 22:38:54 -0600
Subject: [PATCH 3/4] Add regression test for AllowUsers/DenyUsers.
Catches bug fixed in dd141737924dc6cf456c08fff984a7e567cb07d0.
---
regress/Makefile | 3 ++-
regress/README.regress | 1 +
regress/allow-deny-users.sh | 37 +++++++++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+), 1 deletion(-)
create mode 100644 regress/allow-deny-users.sh
diff --git a/regress/Makefile b/regress/Makefile
index bb88068..56719da 100644
--- a/regress/Makefile
+++ b/regress/Makefile
@@ -78,7 +78,8 @@ LTESTS= connect \
hostkey-rotate \
principals-command \
cert-file \
- cfginclude
+ cfginclude \
+ allow-deny-users
# dhgex \
diff --git a/regress/README.regress b/regress/README.regress
index 9b99bda..14a9e35 100644
--- a/regress/README.regress
+++ b/regress/README.regress
@@ -58,6 +58,7 @@ and keys and runs the specified test.
At the time of writing, the individual tests are:
agent-timeout.sh: agent timeout test
agent.sh: simple agent test
+allow-deny-users.sh AllowUsers/DenyUsers test
broken-pipe.sh: broken pipe test
connect-privsep.sh: proxy connect with privsep
connect.sh: simple connect
diff --git a/regress/allow-deny-users.sh b/regress/allow-deny-users.sh
new file mode 100644
index 0000000..217b159
--- /dev/null
+++ b/regress/allow-deny-users.sh
@@ -0,0 +1,37 @@
+# Public Domain
+# Zev Weiss, 2016
+
+tid="AllowUsers/DenyUsers"
+
+me=`whoami`
+other="nobody"
+
+test_auth()
+{
+ deny="$1"
+ allow="$2"
+ should_succeed="$3"
+ failmsg="$4"
+
+ start_sshd -oDenyUsers="$deny" -oAllowUsers="$allow"
+
+ ${SSH} -F $OBJ/ssh_config "$me@somehost" true
+ status=$?
+
+ if (test $status -eq 0 && ! $should_succeed) \
+ || (test $status -ne 0 && $should_succeed); then
+ fail "$failmsg"
+ fi
+
+ stop_sshd
+}
+
+# DenyUsers AllowUsers should_succeed failure_message
+test_auth "" "" true "user in neither DenyUsers nor AllowUsers denied"
+test_auth "$other $me" "" false "user in DenyUsers allowed"
+test_auth "$me $other" "" false "user in DenyUsers allowed"
+test_auth "" "$other" false "user not in AllowUsers allowed"
+test_auth "" "$other $me" true "user in AllowUsers denied"
+test_auth "" "$me $other" true "user in AllowUsers denied"
+test_auth "$me $other" "$me $other" false "user in both DenyUsers and AllowUsers allowed"
+test_auth "$other $me" "$other $me" false "user in both DenyUsers and AllowUsers allowed"
--
2.10.2
>From d59918fb7b84aed9eafa4a5c5ec4008e2f7b7c32 Mon Sep 17 00:00:00 2001
From: Zev Weiss <zev@xxxxxxxxxxxxxxxxx>
Date: Wed, 14 Dec 2016 23:02:42 -0600
Subject: [PATCH 4/4] Fix mis-worded error message in choose_dh().
---
dh.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dh.c b/dh.c
index 194f29b..7a7342c 100644
--- a/dh.c
+++ b/dh.c
@@ -152,7 +152,7 @@ choose_dh(int min, int wantbits, int max)
struct dhgroup dhg;
if ((f = fopen(_PATH_DH_MODULI, "r")) == NULL) {
- logit("WARNING: could open open %s (%s), using fixed modulus",
+ logit("WARNING: could not open %s (%s), using fixed modulus",
_PATH_DH_MODULI, strerror(errno));
return (dh_new_group_fallback(max));
}
--
2.10.2
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
