From 8ac992d273bd4ea768132fb799826a220990af06 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 31 Oct 2021 10:29:12 +0100 Subject: [PATCH 1/5] Do not assign the same local and remote IPs Resolves: #430 Signed-off-by: Nikos Mavrogiannopoulos --- src/ip-lease.c | 38 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/src/ip-lease.c b/src/ip-lease.c index 17cf0eaf..2c925ade 100644 --- a/src/ip-lease.c +++ b/src/ip-lease.c @@ -142,11 +142,10 @@ void steal_ip_leases(struct proc_st* proc, struct proc_st *thief) } } -static int is_ipv6_ok(main_server_st *s, struct sockaddr_storage *ip, struct sockaddr_storage *net, struct sockaddr_storage *subnet) +static int is_ipv6_ok(main_server_st *s, struct sockaddr_storage *ip, struct sockaddr_storage *tun, struct sockaddr_storage *subnet) { - /* check that IP & mask don't match network - i.e., the network's IP is outside - * that subnet; we use it in the tun device */ - if (memcmp(SA_IN6_U8_P(subnet), SA_IN6_U8_P(net), 16) == 0) { + /* check that IP & mask don't match tun IP */ + if (ip_cmp(subnet, tun) == 0) { return 0; } @@ -400,6 +399,15 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) for (i=0;iipv6 = talloc_zero(proc, struct ip_lease_st); + if (proc->ipv6 == NULL) + return ERR_MEM; + + /* LIP = network address + 1 */ + memcpy(&proc->ipv6->lip, &network, sizeof(struct sockaddr_in6)); + SA_IN6_U8_P(&proc->ipv6->lip)[15] |= 1; + + proc->ipv6->lip_len = sizeof(struct sockaddr_in6); if (proc->config->explicit_ipv6) { memset(&tmp, 0, sizeof(tmp)); @@ -408,13 +416,10 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) if (ret != 1) { mslog(s, NULL, LOG_ERR, "error reading explicit IP %s", proc->config->explicit_ipv6); - return -1; + ret = ERR_NO_IP; + goto fail; } - proc->ipv6 = talloc_zero(proc, struct ip_lease_st); - if (proc->ipv6 == NULL) - return ERR_MEM; - ((struct sockaddr_in6*)&tmp)->sin6_family = AF_INET6; memcpy(&proc->ipv6->rip, &tmp, sizeof(struct sockaddr_in6)); proc->ipv6->rip_len = sizeof(struct sockaddr_in6); @@ -423,7 +428,7 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) for (i=0;iipv6->sig)[i] = SA_IN6_U8_P(&proc->ipv6->rip)[i] & SA_IN6_U8_P(&subnet_mask)[i]; - if (is_ipv6_ok(s, &tmp, &network, &proc->ipv6->sig) == 0) { + if (is_ipv6_ok(s, &tmp, &proc->ipv6->lip, &proc->ipv6->sig) == 0) { mslog(s, proc, LOG_DEBUG, "cannot assign explicit IP %s; it is in use or invalid", human_addr((void*)&tmp, sizeof(struct sockaddr_in6), buf, sizeof(buf))); ret = ERR_NO_IP; @@ -434,9 +439,6 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) } /* assign "random" IP */ - proc->ipv6 = talloc_zero(proc, struct ip_lease_st); - if (proc->ipv6 == NULL) - return ERR_MEM; proc->ipv6->db = &s->ip_leases; memcpy(&tmp, &network, sizeof(tmp)); @@ -487,8 +489,8 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) } /* check if it exists in the hash table */ - if (is_ipv6_ok(s, &rnd, &network, &proc->ipv6->sig) == 0) { - mslog(s, proc, LOG_DEBUG, "cannot assign local IP %s; it is in use or invalid", + if (is_ipv6_ok(s, &rnd, &proc->ipv6->lip, &proc->ipv6->sig) == 0) { + mslog(s, proc, LOG_DEBUG, "cannot assign IP %s; it is in use or invalid", human_addr((void*)&rnd, sizeof(struct sockaddr_in6), buf, sizeof(buf))); continue; } @@ -504,12 +506,6 @@ int get_ipv6_lease(main_server_st* s, struct proc_st* proc) } while(1); finish: - /* LIP = network address + 1 */ - memcpy(&proc->ipv6->lip, &network, sizeof(struct sockaddr_in6)); - SA_IN6_U8_P(&proc->ipv6->lip)[15] |= 1; - - proc->ipv6->lip_len = sizeof(struct sockaddr_in6); - proc->ipv6->prefix = subnet_prefix; return 0; From ceebc11cc4490f6c6ef435baa36328bf201d5c17 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 31 Oct 2021 10:14:31 +0100 Subject: [PATCH 2/5] tests: check functionality of an IPv6 net with prefix 127 Signed-off-by: Nikos Mavrogiannopoulos --- .gitlab-ci.yml | 2 +- tests/Makefile.am | 2 +- tests/ipv6-small-net | 91 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) create mode 100755 tests/ipv6-small-net diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fc22835d..7a9a3d68 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -72,7 +72,7 @@ Ubuntu16.04: - ./configure --without-nuttcp-tests - make -j$JOBS # this version of openconnect doesn't work with IPv6 only - - make check -j$JOBS XFAIL_TESTS=ipv6-iface + - make check -j$JOBS XFAIL_TESTS="ipv6-iface ipv6-small-net" tags: - shared - linux diff --git a/tests/Makefile.am b/tests/Makefile.am index 790fc593..30f44fcd 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -61,7 +61,7 @@ dist_check_SCRIPTS += haproxy-connect test-iroute test-multi-cookie test-pass-sc test-cookie-invalidation test-user-config test-append-routes test-ban \ multiple-routes json test-udp-listen-host test-max-same-1 test-script-multi-user \ apple-ios ipv6-iface test-namespace-listen disconnect-user disconnect-user2 \ - ping-leases test-ban-local test-client-bypass-protocol + ping-leases test-ban-local test-client-bypass-protocol ipv6-small-net if RADIUS_ENABLED dist_check_SCRIPTS += radius-group radius-otp diff --git a/tests/ipv6-small-net b/tests/ipv6-small-net new file mode 100755 index 00000000..4fc72603 --- /dev/null +++ b/tests/ipv6-small-net @@ -0,0 +1,91 @@ +#!/bin/bash +# +# Copyright (C) 2021 Nikos Mavrogiannopoulos +# +# This file is part of ocserv. +# +# ocserv is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at +# your option) any later version. +# +# ocserv is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with GnuTLS; if not, write to the Free Software Foundation, +# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +SERV="${SERV:-../src/ocserv}" +srcdir=${srcdir:-.} +PIDFILE=ocserv-pid.$$.tmp +CLIPID=oc-pid.$$.tmp +IP=$(which ip) + +. `dirname $0`/common.sh + +eval "${GETPORT}" + +if test -z "${IP}";then + echo "no IP tool is present" + exit 77 +fi + +if test "$(id -u)" != "0";then + echo "This test must be run as root" + exit 77 +fi + + +echo "Testing ocserv and ipv6 iface application... " + +function finish { + set +e + echo " * Cleaning up..." + test -n "${PID}" && kill ${PID} >/dev/null 2>&1 + test -n "${PIDFILE}" && rm -f ${PIDFILE} >/dev/null 2>&1 + test -n "${CLIPID}" && kill $(cat ${CLIPID}) >/dev/null 2>&1 + test -n "${CLIPID}" && rm -f ${CLIPID} >/dev/null 2>&1 + test -n "${CONFIG}" && rm -f ${CONFIG} >/dev/null 2>&1 +} +trap finish EXIT + +ADDRESS=10.201.128.0 +CLI_ADDRESS=10.57.49.0 +VPNNET6=fc04:1b8e:bdca:11ca:576b:80e3:4956:f00c/127 +VPNADDR6=fc04:1b8e:bdca:11ca:576b:80e3:4956:f00d +OCCTL_SOCKET=./ipv6-iface-$$.socket +USERNAME=test + +. `dirname $0`/ns.sh + +update_config ipv6-iface.config +if test "$VERBOSE" = 1;then +DEBUG="-d 3" +fi + +${CMDNS2} ${SERV} -p ${PIDFILE} -f -c ${CONFIG} ${DEBUG} & PID=$! +wait_server $PID + +echo -n "Connecting to setup interface... " +echo "test" | ${CMDNS1} $OPENCONNECT -q $ADDRESS:$PORT -u ${USERNAME} --servercert=d66b507ae074d03b02eafca40d35f87dd81049d3 -s ${srcdir}/scripts/vpnc-script --pid-file=${CLIPID} --passwd-on-stdin -b +if test $? != 0;then + echo "Could not connect to server" + exit 1 +fi + +echo ok + +set -e +echo " * ping remote address" + +${CMDNS2} ping -6 -c 3 ${VPNADDR6} + +echo ok + +kill $PID +wait + +exit 0 From 39954732195237811fa6425c4b2088b8b0587660 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 31 Oct 2021 10:30:20 +0100 Subject: [PATCH 3/5] NEWS: doc update Signed-off-by: Nikos Mavrogiannopoulos --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index 7bcd2212..3c20f361 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,8 @@ * Version 1.1.4 (unreleased) - Added newfstatat() and epoll_pwait() to the accepted list of seccomp calls. This improves compatibility with certain libcs and aarch64. +- Do not allow assigning the same IPv6 as tun device address and to + the client. This allows using /127 as prefix (#430) * Version 1.1.3 (released 2021-06-02) From 296b4fb4fef83317bb36f5d5830b2c6d647b5560 Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 31 Oct 2021 10:50:19 +0100 Subject: [PATCH 4/5] test-explicit-ip: corrected the illegal IP address Signed-off-by: Nikos Mavrogiannopoulos --- tests/user-config-explicit/test4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/user-config-explicit/test4 b/tests/user-config-explicit/test4 index a63001c4..1f977531 100644 --- a/tests/user-config-explicit/test4 +++ b/tests/user-config-explicit/test4 @@ -1 +1 @@ -explicit-ipv6 = fda9:4efe:7e3b:03ea::00 +explicit-ipv6 = fda9:4efe:7e3b:03ea::01 From 11c79189cc57d71849b75c8969136babae15853d Mon Sep 17 00:00:00 2001 From: Nikos Mavrogiannopoulos Date: Sun, 31 Oct 2021 10:23:01 +0100 Subject: [PATCH 5/5] tests: skip leaks in occtl Signed-off-by: Nikos Mavrogiannopoulos --- tests/Makefile.am | 5 +++-- tests/asan.supp | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) create mode 100644 tests/asan.supp diff --git a/tests/Makefile.am b/tests/Makefile.am index 30f44fcd..dcbcc650 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -44,7 +44,7 @@ EXTRA_DIST = certs/ca-key.pem certs/ca.pem ns.sh common.sh certs/server-cert.pem data/disconnect-user2.config data/ping-leases.config data/haproxy-proxyproto.config \ data/haproxy-proxyproto.cfg scripts/proxy-connectscript data/haproxy-proxyproto-v1.config \ data/haproxy-proxyproto-v1.cfg scripts/proxy-connectscript-v1 data/test-multiple-client-ip.config \ - data/test-client-bypass-protocol.config + data/test-client-bypass-protocol.config asan.supp xfail_scripts = dist_check_SCRIPTS = ocpasswd-test @@ -190,7 +190,8 @@ TESTS = $(check_PROGRAMS) $(dist_check_SCRIPTS) $(xfail_scripts) XFAIL_TESTS = $(xfail_scripts) TESTS_ENVIRONMENT = srcdir="$(srcdir)" \ - top_builddir="$(top_builddir)" + top_builddir="$(top_builddir)" \ + LSAN_OPTIONS=suppressions=asan.supp if DISABLE_ASAN_BROKEN_TESTS TESTS_ENVIRONMENT += DISABLE_ASAN_BROKEN_TESTS=1 diff --git a/tests/asan.supp b/tests/asan.supp new file mode 100644 index 00000000..157d8da9 --- /dev/null +++ b/tests/asan.supp @@ -0,0 +1 @@ +leak:occtl