You're welcome!
The same bug was also found in the smp_fetch_dst_is_local() function
(fc_dst_is_local sample fetch).
Attached is the patch that addresses the 2 bugs, it will probably be
included in the next batch of releases.
2.5 and 2.7 are also affected
> Unfortunately, this option doesn't work in my context, because my logs
> show that it contains a different port (not the one I need)
Oh ok :/
> But I can wait for a release with the bug fixed, it's not urgent.
Thank you for you patience :)
Aurelien
From 69aaeeb8a54e60ddd66481f91354f0d99c75ec05 Mon Sep 17 00:00:00 2001
From: Aurelien DARRAGON <adarra...@haproxy.com>
Date: Fri, 10 Mar 2023 16:53:43 +0100
Subject: [PATCH] BUG/MINOR: tcp_sample: fix a bug in fc_dst_port and
fc_dst_is_local sample fetches
There is a bug in the function smp_fetch_dport() which affects the 'f' case, also
known as 'fc_dst_port' sample fetch.
conn_get_src() is used to retrieve the address prior to calling conn_dst().
But this is wrong: conn_get_dst() should be used instead.
Because of that, conn_dst() may return unexpected results since the dst address
is not guaranteed to be set depending on the conn state at the time the sample
fetch is used.
This was reported by Corin Langosch on the ML:
during his tests he noticed that using fc_dst_port in a log-format string
resulted in the correct value being printed in the logs but when he used it
in an ACL, the ACL did not evaluate properly.
This can be easily reproduced with the following test conf:
|frontend test-http
| bind 127.0.0.1:8080
| mode http
|
| acl test fc_dst_port eq 8080
| http-request return status 200 if test
| http-request return status 500 if !test
A request on 127.0.0.1:8080 should normally return 200 OK, but here it
will return a 500.
The same bug was also found in smp_fetch_dst_is_local() (fc_dst_is_local sample
fetch) by reading the code: the fix was applied twice.
This needs to be backported up to 2.5
[both sample fetches were introduced in 2.5 with 888cd70 ("MINOR: tcp-sample:
Add samples to get original info about client connection")]
---
src/tcp_sample.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/tcp_sample.c b/src/tcp_sample.c
index 925b93291..12eb25c4e 100644
--- a/src/tcp_sample.c
+++ b/src/tcp_sample.c
@@ -176,7 +176,7 @@ int smp_fetch_dst_is_local(const struct arg *args, struct sample *smp, const cha
if (kw[0] == 'f') { /* fc_dst_is_local */
struct connection *conn = objt_conn(smp->sess->origin);
- if (conn && conn_get_src(conn))
+ if (conn && conn_get_dst(conn))
dst = conn_dst(conn);
}
else /* dst_is_local */
@@ -232,10 +232,10 @@ smp_fetch_dport(const struct arg *args, struct sample *smp, const char *kw, void
if (conn && conn_get_dst(conn))
dst = conn_dst(conn);
}
- else if (kw[0] == 'f') { /* fc_dst_post */
+ else if (kw[0] == 'f') { /* fc_dst_port */
struct connection *conn = objt_conn(smp->sess->origin);
- if (conn && conn_get_src(conn))
+ if (conn && conn_get_dst(conn))
dst = conn_dst(conn);
}
else /* dst_port */
--
2.34.1
