Hello again, this time I need to improve the test scripts and I return to the ever recurring theme of exact sockaddr lengths.
The scripts for testing pings and tracing are given some small
updates. The first now tests `ping` and `ping6', whereas the
latter tests UDP as well as ICMP.
More alarming is the fact that "tests/tftp.sh" is so severly
GNU/Linux-centric that it fails completely in all aspects
for OpenBSD. It lacks every aspect of robustness and self
diagnosis. I had to impose the switch "-d" on Inetd at all
times in the script, in order make it robust, so the printout
is not as beutiful as was. On the other hand, it does no
longer conceil any false positives.
The changes needed to the test scripts are contaied in
0001-Improve-the-present-test-script.patch
Once the test script for TFTP is functional on GNU/Linux
and OpenBSD, using the tools built by Inetutils itself,
it became evident that "src/inetd.c" is broken in OpenBSD.
It is the old friend of "optimistic calculation of
address structure length" that shows up again. It is
resolved by an additional integer that records the used
sockaddr length for an established connection. The expected
solution is not very intrusive at all.
The few changes are recorded in
0002-src-inetd.c-Use-exact-length-of-address-structure.patch
Best regards,
Mats
From f5a574a9a992db3e74748de8e8a513a4efc92d0a Mon Sep 17 00:00:00 2001 From: Mats Erik Andersson <[email protected]> Date: Sun, 31 Oct 2010 03:51:47 +0100 Subject: [PATCH 1/2] Improve the present test script. --- ChangeLog | 8 +++++++ tests/ping-localhost.sh | 24 +++++++++++++++++++- tests/tftp.sh | 47 ++++++++++++++++++++++++++++++++++------- tests/traceroute-localhost.sh | 19 ++++++++++++++- 4 files changed, 86 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 11e79f0..ed8141a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2010-10-31 Mats Erik Andersson <[email protected]> + + Improve the three test scripts. + + * tests/tftp.sh: Make it robust and portable to GNU/Linux and BSD. + * tests/ping-localhost.sh: Test `ping' and `ping6'. Configurable targets. + * tests/traceroute-localhost.sh: Test UDP and ICMP. Configurable target. + 2010-10-30 Mats Erik Andersson <[email protected]> * src/traceroute.c (trace_init): Change type of TTLP to `int'. diff --git a/tests/ping-localhost.sh b/tests/ping-localhost.sh index 35bf87c..94c7e5c 100755 --- a/tests/ping-localhost.sh +++ b/tests/ping-localhost.sh @@ -19,6 +19,12 @@ PING=${PING:-../ping/ping$EXEEXT} +TARGET=${TARGET:-127.0.0.1} + +PING6=${PING6:-../ping/ping6$EXEEXT} + +TARGET6=${TARGET6:-::1} + if [ $VERBOSE ]; then set -x $PING --version @@ -29,6 +35,20 @@ if [ `id -u` != 0 ]; then exit 77 fi -$PING -c 1 127.0.0.1; errno=$? +errno=0 + +$PING -c 1 $TARGET || errno=$? + +test $errno -eq 0 || echo "Failed at pinging $TARGET." >&2 + +errno2=0 + +# Host might not have been building with `IPV6=1'. +test -x $PING6 && $PING6 -c 1 $TARGET6 || errno2=$? + +test $errno2 -eq 0 || echo "Failed at pinging $TARGET6." >&2 + + +test $errno -eq 0 || exit $errno -exit $errno +exit $errno2 diff --git a/tests/tftp.sh b/tests/tftp.sh index 6b2dc80..f14f758 100755 --- a/tests/tftp.sh +++ b/tests/tftp.sh @@ -22,13 +22,15 @@ TFTP="${TFTP:-../src/tftp$EXEEXT}" TFTPD="${TFTPD:-$PWD/../src/tftpd$EXEEXT}" INETD="${INETD:-../src/inetd$EXEEXT}" -IFCONFIG="${IFCONFIG:-../ifconfig/ifconfig$EXEEXT}" +IFCONFIG="${IFCONFIG:-../ifconfig/ifconfig$EXEEXT --format=unix}" -PORT=7777 +AF=${AF:-inet} +PROTO=${PROTO:-udp} +PORT=${PORT:-7777} INETD_CONF="$PWD/inetd.conf.tmp" -ADDRESSES="`$IFCONFIG | grep 'inet addr:' | \ - sed -e's/inet addr:\([^ ]\+\)[[:blank:]].*$/\1/g'`" +ADDRESSES="`$IFCONFIG | sed -e "/$AF /!d" \ + -e "s/^.*$AF[[:blank:]]\([:.0-9]\{1,\}\)[[:blank:]].*$/\1/g"`" if [ "$VERBOSE" ]; then set -x @@ -37,17 +39,38 @@ if [ "$VERBOSE" ]; then "$INETD" --version fi +# Check that the port is still available +netstat -na | grep -q "^$PROTO .*$PORT " +if test $? -eq 0; then + echo "Desired port $PORT/$PROTO is already in use." + exit 1 +fi + # Create `inetd.conf'. Note: We want $TFTPD to be an absolute file # name because `inetd' chdirs to `/' in daemon mode; ditto for # $INETD_CONF. cat > "$INETD_CONF" <<EOF -$PORT dgram udp wait $USER $TFTPD tftpd -l `pwd`/tftp-test +$PORT dgram $PROTO wait $USER $TFTPD tftpd -l `pwd`/tftp-test EOF # Launch `inetd', assuming it's reachable at all $ADDRESSES. -$INETD "${VERBOSE:+-d}" "$INETD_CONF" & +$INETD -d "$INETD_CONF" & inetd_pid="$!" +test -z "$VERBOSE" || echo "Launched Inetd as process $inetd_pid." >&2 + +# Wait somewhat for the service to settle +sleep 1 + +# Did `inetd' really succeed in establishing a listener? +netstat -na | grep "^$PROTO .*$PORT " +if test $? -ne 0; then + # No it did not + ps "$inetd_pid" >/dev/null 2>&1 && kill "$inetd_pid" + echo "Failed in starting correct Inetd instance." >&2 + exit 1 +fi + if [ -f /dev/urandom ]; then input="/dev/urandom" else @@ -56,7 +79,9 @@ fi rm -fr tftp-test tftp-test-file mkdir tftp-test && \ - dd if="$input" of="tftp-test/tftp-test-file" bs=1024 count=170 + dd if="$input" of="tftp-test/tftp-test-file" bs=1024 count=170 2>/dev/null + +echo "Looks into $ADDRESSES." for addr in $ADDRESSES; do echo "trying with address \`$addr'..." >&2 @@ -69,12 +94,18 @@ for addr in $ADDRESSES; do if [ "$result" -ne 0 ]; then # Failure. + test -z "$VERBOSE" || echo "Failed comparison for $addr." >&2 break + else + test -z "$VERBOSE" || echo "Successful comparison for $addr." >&2 fi done -kill "$inetd_pid" +ps "$inetd_pid" >/dev/null 2>&1 && kill "$inetd_pid" rm -rf tftp-test tftp-test-file "$INETD_CONF" +# Minimal clean up +echo + exit $result diff --git a/tests/traceroute-localhost.sh b/tests/traceroute-localhost.sh index 97d13dc..4a34158 100755 --- a/tests/traceroute-localhost.sh +++ b/tests/traceroute-localhost.sh @@ -19,6 +19,8 @@ TRACEROUTE=${TRACEROUTE:-../src/traceroute$EXEEXT} +TARGET=${TARGET:-127.0.0.1} + if [ $VERBOSE ]; then set -x $TRACEROUTE --version @@ -29,6 +31,19 @@ if [ `id -u` != 0 ]; then exit 77 fi -$TRACEROUTE 127.0.0.1; errno=$? +errno=0 + +$TRACEROUTE --type=udp $TARGET || errno=$? + +test $errno -eq 0 || echo "Failed at UDP tracing." >&2 + +errno2=0 + +$TRACEROUTE --type=icmp $TARGET || errno2=$? + +test $errno2 -eq 0 || echo "Failed at ICMP tracing." >&2 + + +test $errno -eq 0 || exit $errno -exit $errno +exit $errno2 -- 1.7.1
From 71a5c6478ea29872b95db9c3c8d1595b0920ff27 Mon Sep 17 00:00:00 2001 From: Mats Erik Andersson <[email protected]> Date: Sun, 31 Oct 2010 04:29:21 +0100 Subject: [PATCH 2/2] src/inetd.c: Use exact length of address structure. --- ChangeLog | 9 +++++++++ src/inetd.c | 5 ++++- 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/ChangeLog b/ChangeLog index ed8141a..d187dbb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,14 @@ 2010-10-31 Mats Erik Andersson <[email protected]> + Bind socket using knowledge of exact sockaddr length. + + * src/inetd.c: New element `se_addrlen' in `struct servtab'. + (expand_enter): Record in SE_ADDRLEN the actual length of each + established control address. + (setup): Use the exact address length in call to bind(3). + +2010-10-31 Mats Erik Andersson <[email protected]> + Improve the three test scripts. * tests/tftp.sh: Make it robust and portable to GNU/Linux and BSD. diff --git a/src/inetd.c b/src/inetd.c index 272e874..2a30acf 100644 --- a/src/inetd.c +++ b/src/inetd.c @@ -254,6 +254,7 @@ struct servtab sa_family_t se_family; /* address family of the socket */ char se_v4mapped; /* 1 = accept v4mapped connection, 0 = don't */ struct sockaddr_storage se_ctrladdr; /* bound address */ + socklen_t se_addrlen; /* exact address length in use */ unsigned se_refcnt; int se_count; /* number started since se_time */ struct timeval se_time; /* start of se_count */ @@ -585,7 +586,7 @@ setup (struct servtab *sep) syslog (LOG_ERR, "setsockopt (SO_REUSEADDR): %m"); err = bind (sep->se_fd, (struct sockaddr *) &sep->se_ctrladdr, - sizeof (sep->se_ctrladdr)); + sep->se_addrlen); if (err < 0) { /* If we can't bind with AF_INET6 try again with AF_INET. */ @@ -800,7 +801,9 @@ expand_enter (struct servtab *sep) for (rp = result; rp != NULL; rp = rp->ai_next) { + memset (&sep->se_ctrladdr, 0, sizeof (sep->se_ctrladdr)); memcpy (&sep->se_ctrladdr, rp->ai_addr, rp->ai_addrlen); + sep->se_addrlen = rp->ai_addrlen; cp = enter (sep); servent_setup (cp); } -- 1.7.1
signature.asc
Description: Digital signature
