From 5d79d98c3767b4cd864a66d66bbf71c56dcff666 Mon Sep 17 00:00:00 2001 From: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> Date: Sun, 12 Jul 2026 21:33:46 +0200 Subject: [PATCH] Apply patch provided by maitainer Signed-off-by: Dimitri Papadopoulos <3350651-DimitriPapadopoulos@users.noreply.gitlab.com> --- doc/requirements/internal/main.md | 34 +++++++++++++ src/icmp-ping.c | 72 +++++++++++++++++--------- src/icmp-ping.h | 12 ++++- tests/icmp-ping-test.c | 84 +++++++++++++++++++++++++++++++ tests/meson.build | 11 ++++ 5 files changed, 186 insertions(+), 27 deletions(-) create mode 100644 tests/icmp-ping-test.c diff --git a/doc/requirements/internal/main.md b/doc/requirements/internal/main.md index 5e5c2018..34be1bd3 100644 --- a/doc/requirements/internal/main.md +++ b/doc/requirements/internal/main.md @@ -196,6 +196,40 @@ iterations rather than hanging or erroring. **Links:** REQ-SECMOD-SESSION (predictable_ips / ipv4_seed, see src/sec-mod-auth.c:570-579) +### REQ-MAIN-NET-004 — With ping-leases enabled, a candidate address is probed with ICMP echo and treated as in-use only on a genuine reply + +**Requirement:** The ICMP echo probe (src/icmp-ping.c) is split into a pure +core, `icmp_echo4()`/`icmp_echo6()`, and an ocserv-facing wrapper, +`icmp_ping4()`/`icmp_ping6()`. `icmp_echo4(addr, timeout_secs)` MUST send a +single ICMP echo request to `addr` from a raw socket, and MUST wait up to +`timeout_secs` seconds, or until it has collected two replies (echo reply + +destination unreachable), for a response; it MUST return the number of +`ICMP_ECHOREPLY`/`ICMP6_ECHO_REPLY` messages matching the request's random +`icmp_id` and received from the exact candidate address, and MUST return 0 +if only a destination-unreachable, a non-matching reply, or no reply at all +arrived before the timeout. `icmp_echo4()`/`icmp_echo6()` MUST NOT depend on +`main_server_st`, vhost config, or `mslog` — any internal error (RNG or +socket setup failure) is reported via `oc_syslog()` instead. The wrapper +`icmp_ping4(s, addr)`/`icmp_ping6(s, addr)` MUST return 0 immediately +without sending any packet when `GETRCONFIG(s)->ping_leases` is false (the +default); otherwise it MUST call the corresponding `icmp_echo*()` with +`PING_TIMEOUT` (3) seconds, `mslog` the in-use/not-in-use outcome, and +return that result unchanged. +**Strength:** MUST +**Status:** DERIVED +**Source:** src/icmp-ping.c (`icmp_echo4`, `icmp_ping4`, `icmp_echo6`, +`icmp_ping6`) +**Acceptance:** unit, local — (a) call `icmp_echo4()` directly against the +loopback address and confirm it returns non-zero (the kernel answers its +own ICMP echo requests over `lo`), with no `main_server_st` involved; (b) +call `icmp_echo4()` against a non-responding address (e.g. a TEST-NET-1 +address unreachable in the test environment) and confirm it returns 0 +within the given timeout, not a hang; (c) confirm `icmp_ping4()`/ +`icmp_ping6()` (and their `GETRCONFIG`/`mslog` dependency) compile out of +the test binary when built with `-DUNDER_TEST`, so the unit test cannot +accidentally depend on them. +**Links:** — + ### REQ-MAIN-NET-003 — Reconnecting client (steal) transfers IP leases without re-fetching from the pool **Requirement:** `steal_ip_leases(proc, thief)` (used when a client diff --git a/src/icmp-ping.c b/src/icmp-ping.c index 8434620e..f5e1339d 100644 --- a/src/icmp-ping.c +++ b/src/icmp-ping.c @@ -167,31 +167,27 @@ static ssize_t recvfrom_timeout(int sockfd, void *buf, size_t len, int flags, return -1; } -int icmp_ping4(main_server_st *s, struct sockaddr_in *addr1) +int icmp_echo4(const struct sockaddr_in *addr1, unsigned int timeout_secs) { struct icmp *pkt; int pingsock, c, e; char packet1[DEFDATALEN + MAXIPLEN + MAXICMPLEN]; - char buf1[64]; time_t now; uint16_t id1; unsigned int gotreply = 0, unreachable = 0; - if (GETRCONFIG(s)->ping_leases == 0) - return 0; - e = gnutls_rnd(GNUTLS_RND_NONCE, &id1, sizeof(id1)); if (e < 0) { - mslog(s, NULL, LOG_ERR, "error in the random generator: %s", - gnutls_strerror(e)); + oc_syslog(LOG_ERR, "error in the random generator: %s", + gnutls_strerror(e)); return 0; } pingsock = socket(AF_INET, SOCK_RAW, 1); if (pingsock == -1) { e = errno; - mslog(s, NULL, LOG_ERR, - "could not open raw socket for ping: %s", strerror(e)); + oc_syslog(LOG_ERR, "could not open raw socket for ping: %s", + strerror(e)); return 0; } @@ -208,7 +204,7 @@ int icmp_ping4(main_server_st *s, struct sockaddr_in *addr1) /* listen for replies */ now = time(NULL); - while (time(NULL) - now < PING_TIMEOUT && + while (time(NULL) - now < (time_t)timeout_secs && (unreachable + gotreply) < 2) { struct sockaddr_in from; socklen_t fromlen = sizeof(from); @@ -246,23 +242,37 @@ int icmp_ping4(main_server_st *s, struct sockaddr_in *addr1) close(pingsock); + return gotreply; +} + +#ifndef UNDER_TEST +int icmp_ping4(main_server_st *s, struct sockaddr_in *addr1) +{ + char buf1[64]; + int gotreply; + + if (GETRCONFIG(s)->ping_leases == 0) + return 0; + + gotreply = icmp_echo4(addr1, PING_TIMEOUT); + if (gotreply > 0) { mslog(s, NULL, LOG_INFO, "pinged %s and is in use", human_addr((void *)addr1, sizeof(struct sockaddr_in), buf1, sizeof(buf1))); - return gotreply; } else { mslog(s, NULL, LOG_INFO, "pinged %s and is not in use", human_addr((void *)addr1, sizeof(struct sockaddr_in), buf1, sizeof(buf1))); - return 0; } -} -int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1) + return gotreply; +} +#endif /* UNDER_TEST */ + +int icmp_echo6(const struct sockaddr_in6 *addr1, unsigned int timeout_secs) { struct icmp6_hdr *pkt; - char buf1[64]; int pingsock, c, e; #if defined(SOL_RAW) && defined(IPV6_CHECKSUM) int sockopt; @@ -272,21 +282,18 @@ int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1) unsigned int gotreply = 0, unreachable = 0; time_t now; - if (GETRCONFIG(s)->ping_leases == 0) - return 0; - e = gnutls_rnd(GNUTLS_RND_NONCE, &id1, sizeof(id1)); if (e < 0) { - mslog(s, NULL, LOG_ERR, "error in the random generator: %s", - gnutls_strerror(e)); + oc_syslog(LOG_ERR, "error in the random generator: %s", + gnutls_strerror(e)); return 0; } pingsock = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6); if (pingsock == -1) { e = errno; - mslog(s, NULL, LOG_ERR, - "could not open raw socket for ping: %s", strerror(e)); + oc_syslog(LOG_ERR, "could not open raw socket for ping: %s", + strerror(e)); return 0; } @@ -306,7 +313,7 @@ int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1) /* listen for replies */ now = time(NULL); - while (time(NULL) - now < PING_TIMEOUT && + while (time(NULL) - now < (time_t)timeout_secs && (unreachable + gotreply) < 2) { struct sockaddr_in6 from; socklen_t fromlen = sizeof(from); @@ -334,15 +341,30 @@ int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1) close(pingsock); + return gotreply; +} + +#ifndef UNDER_TEST +int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1) +{ + char buf1[64]; + int gotreply; + + if (GETRCONFIG(s)->ping_leases == 0) + return 0; + + gotreply = icmp_echo6(addr1, PING_TIMEOUT); + if (gotreply > 0) { mslog(s, NULL, LOG_INFO, "pinged %s and is in use", human_addr((void *)addr1, sizeof(struct sockaddr_in6), buf1, sizeof(buf1))); - return gotreply; } else { mslog(s, NULL, LOG_INFO, "pinged %s and is not in use", human_addr((void *)addr1, sizeof(struct sockaddr_in6), buf1, sizeof(buf1))); - return 0; } + + return gotreply; } +#endif /* UNDER_TEST */ diff --git a/src/icmp-ping.h b/src/icmp-ping.h index 3ae8d59c..2628b30d 100644 --- a/src/icmp-ping.h +++ b/src/icmp-ping.h @@ -23,8 +23,16 @@ #include "main.h" -/* returns the number of positive replies received or - * 0 if no host with this IP exists. */ +/* Pure ICMP echo request/reply probe: no ocserv config or logging + * dependency, so it can be unit tested directly. Returns the number of + * positive replies received within timeout_secs, or 0 if none arrived + * (including the case where a destination-unreachable was received + * instead). */ +int icmp_echo4(const struct sockaddr_in *addr1, unsigned int timeout_secs); +int icmp_echo6(const struct sockaddr_in6 *addr1, unsigned int timeout_secs); + +/* ocserv-facing wrappers: honor the ping-leases config option and log + * the outcome. Same return convention as icmp_echo4()/icmp_echo6(). */ int icmp_ping4(main_server_st *s, struct sockaddr_in *addr1); int icmp_ping6(main_server_st *s, struct sockaddr_in6 *addr1); diff --git a/tests/icmp-ping-test.c b/tests/icmp-ping-test.c new file mode 100644 index 00000000..7581517e --- /dev/null +++ b/tests/icmp-ping-test.c @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2026 Nikos Mavrogiannopoulos + * + * This program 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. + * + * This program 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 this program. If not, see . + */ + +/* Exercises REQ-MAIN-NET-004: icmp_echo4()/icmp_echo6() must send an ICMP + * echo request and correctly distinguish a genuine echo reply from "no + * reply"/"unreachable". UNDER_TEST excludes icmp_ping4()/icmp_ping6() + * (the ocserv-facing wrappers, which need a full main_server_st/vhost + * config tree) from this translation unit, so only the pure send/receive + * logic is under test here. Requires CAP_NET_RAW (root), hence gated as a + * root test. + */ + +#include +#include +#include +#include +#include +#include + +#include "../src/ip-util.c" +#include "../src/icmp-ping.c" + +/* keep the negative-reply tests fast rather than the real 3s PING_TIMEOUT */ +#define TEST_TIMEOUT_SECS 1 + +static void run_test4(const char *desc, const char *ip, int expect_reply) +{ + struct sockaddr_in addr; + int ret; + + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + if (inet_pton(AF_INET, ip, &addr.sin_addr) != 1) { + fprintf(stderr, "FAIL: %s: inet_pton(%s) failed\n", desc, ip); + exit(1); + } + + ret = icmp_echo4(&addr, TEST_TIMEOUT_SECS); + + if (expect_reply && ret <= 0) { + fprintf(stderr, + "FAIL: %s: expected a ping reply from %s, got ret=%d\n", + desc, ip, ret); + exit(1); + } + if (!expect_reply && ret != 0) { + fprintf(stderr, + "FAIL: %s: expected no ping reply from %s, got ret=%d\n", + desc, ip, ret); + exit(1); + } + + printf("PASS: %s (%s) -> %d\n", desc, ip, ret); +} + +int main(void) +{ + /* positive: the kernel answers ICMP echo requests sent to loopback, + * so a genuine ICMP_ECHOREPLY must be observed and reported. */ + run_test4("loopback replies to ping", "127.0.0.1", 1); + + /* negative: a TEST-NET-1 (RFC 5737) address is not assigned to any + * host reachable from the test environment, so no ICMP_ECHOREPLY + * can arrive; icmp_echo4() must not mistake silence (or a + * destination-unreachable) for the address being in use, and must + * not hang past the given timeout. */ + run_test4("non-responding address is treated as free", "192.0.2.1", 0); + + return 0; +} diff --git a/tests/meson.build b/tests/meson.build index d76e194a..b89c49e8 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -287,6 +287,17 @@ if get_option('root-tests') ) endforeach + # icmp-ping-test: exercises icmp_ping4() directly (REQ-MAIN-NET-004). + # Needs CAP_NET_RAW for a raw ICMP socket, hence gated under root-tests + # alongside the higher-level ping-leases shell test below. + icmp_ping_test_exe = executable('icmp-ping-test', 'icmp-ping-test.c', + c_args: ['-DUNDER_TEST'], + dependencies: test_base_deps + [gnutls_dep], + include_directories: test_inc, + ) + test('icmp-ping-test', icmp_ping_test_exe, + env: test_env, workdir: test_workdir, timeout: 30) + root_scripts = [ 'haproxy-connect', 'test-iroute', 'test-multi-cookie', 'test-pass-script', 'idle-timeout', 'idle-timeout-icmpv6', 'test-year-2038',