Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 19 Aug 2024 04:02:29 GMT
From:      Eugene Grosbein <eugen@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 8132e959099f - main - libalias: fix subtle racy problem in outside-inside forwarding
Message-ID:  <202408190402.47J42TTP028366@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by eugen:

URL: https://cgit.FreeBSD.org/src/commit/?id=8132e959099f0c533f698d8fbc17386f9144432f

commit 8132e959099f0c533f698d8fbc17386f9144432f
Author:     Eugene Grosbein <eugen@FreeBSD.org>
AuthorDate: 2024-08-19 03:34:37 +0000
Commit:     Eugene Grosbein <eugen@FreeBSD.org>
CommitDate: 2024-08-19 03:34:37 +0000

    libalias: fix subtle racy problem in outside-inside forwarding
    
    sys/netinet/libalias/alias_db.c has internal static function UseLink()
    that passes a link to CleanupLink() to verify if the link has expired.
    If so, UseLink() may return NULL.
    
    _FindLinkIn()'s usage of UseLink() is not quite correct.
    
    Assume there is "redirect_port udp" configured to forward incoming
    traffic for specific port to some internal address.
    Such a rule creates partially specified permanent link.
    
    After first such packet libalias creates new fully specifiled
    temporary LINK_UDP with default timeout 60 seconds.
    Also, in case of low traffic libalias may assign "timestamp"
    for this new temporary link way in the past because
    LibAliasTime is updated seldom and can keep old value
    for tens of seconds, and it will be used for the temporary link.
    
    It may happen that next incoming packet for redirected port
    passed to _FindLinkIn() results in a call to UseLink()
    that returns NULL due to detected expiration.
    Immediate return of NULL results in broken translation:
    either a packet is dropped (deny_incoming mode) or delivered to
    original destination address instead of internal one.
    
    Fix it with additional check for NULL to proceed with a search
    for original partially specified link. In case of UDP,
    it also recreates temporary fully specified link
    with a call to ReLink().
    
    Practical examples are "redirect_port udp" rules for unidirectional
    SYSLOG protocol (port 514) or some low volume VPN encapsulated in UDP.
    
    Thanks to Peter Much for initial analysis and first version of a patch.
    
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    PR:             269770
    MFC after:      1 week
---
 sys/netinet/libalias/alias_db.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sys/netinet/libalias/alias_db.c b/sys/netinet/libalias/alias_db.c
index 167201fa1b8f..d516b6cda96c 100644
--- a/sys/netinet/libalias/alias_db.c
+++ b/sys/netinet/libalias/alias_db.c
@@ -868,8 +868,15 @@ _FindLinkIn(struct libalias *la, struct in_addr dst_addr,
 	case 0:
 		LIST_FOREACH(lnk, &grp->full, all.in) {
 			if (lnk->dst_addr.s_addr == dst_addr.s_addr &&
-			    lnk->dst_port == dst_port)
-				return (UseLink(la, lnk));
+			    lnk->dst_port == dst_port) {
+				struct alias_link *found;
+
+				found = UseLink(la, lnk);
+				if (found != NULL)
+					return (found);
+				/* link expired */
+				break;
+			}
 		}
 		break;
 	case LINK_UNKNOWN_DEST_PORT:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202408190402.47J42TTP028366>