Address UDP connect message endian issue #1415

mriswyth · 2022-11-04T17:59:02Z

Accept big-endian and little-endian format for both legacy and new messages since the value is sent in host byte order.

PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":

You are under no obligation whatsoever to provide any bug fixes, patches, or
upgrades to the features, functionality or performance of the source code
("Enhancements") to anyone; however, if you choose to make your Enhancements
available either publicly, or directly to Lawrence Berkeley National
Laboratory, without imposing a separate written license agreement for such
Enhancements, then you hereby grant the following license: a non-exclusive,
royalty-free perpetual license to install, use, modify, prepare derivative
works, incorporate into other computer software, distribute, and sublicense
such enhancements or derivative works thereof, in binary and source code form.

The complete iperf3 license is available in the LICENSE file in the
top directory of the iperf3 source tree.

Version of iperf3 (or development branch, such as master or
3.1-STABLE) to which this pull request applies: master
Issues fixed (if any):
Mixed big-endian and little-endian UDP test fails #1414
Brief description of code changes (suitable for use as a commit message):
Accept UDP connect message replies in little-endian or big-endian order.
There was discussion and concern about accepting two values in the original pull request Ignore out of order packts during UDP connection in Reverse Mode #1260, this makes that problem worse. This is a solution that allows mixed-endian and mixed-version operations if the client is updated.

mriswyth · 2022-11-04T18:03:07Z

Here's the output running against the same server binary that shows the failed output in #1414.

./iperf3 -c FE80::848D:84FF:FEFE:9165%eno1 -n 1 -u --debug --verbose
iperf 3.12
Linux bthompson.rajant.com 6.0.5-200.fc36.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Oct 26 15:55:21 UTC 2022 x86_64
Control connection MSS 1428
Setting UDP block size to 1428
send_parameters:
{
        "udp":  true,
        "omit": 0,
        "time": 0,
        "num":  1,
        "blockcount":   0,
        "parallel":     1,
        "len":  1428,
        "bandwidth":    1048576,
        "pacing_timer": 1000,
        "client_version":       "3.12"
}
Time: Fri, 04 Nov 2022 18:01:18 GMT
Connecting to host FE80::848D:84FF:FEFE:9165%eno1, port 5201
      Cookie: feepi4jyp4up4bj2fw54nidyqe27qh76x6wk
      Target Bitrate: 1048576
SNDBUF is 212992, expecting 0
RCVBUF is 212992, expecting 0
Setting application pacing to 131072
Sending Connect message to Socket 5
Connect received for Socket 5, sz=4, buf=36373839, i=0, max_len_wait_for_reply=4
[  5] local fe80::bf96:250b:5c8e:66f9 port 56896 connected to fe80::848d:84ff:fefe:9165 port 5201
Starting Test: protocol: UDP, 1 streams, 1428 byte blocks, omitting 0 seconds, 1 bytes to send, tos 0
sent 1428 bytes of 1428, total 1428
send_results
{
        "cpu_util_total":       17.9589679968755,
        "cpu_util_user":        6.8784892135915641,
        "cpu_util_system":      11.080478783283938,
        "sender_has_retransmits":       0,
        "streams":      [{
                        "id":   1,
                        "bytes":        1428,
                        "retransmits":  18446744073709551615,
                        "jitter":       0,
                        "errors":       0,
                        "packets":      1,
                        "start_time":   0,
                        "end_time":     0.000505
                }]
}
get_results
{
        "cpu_util_total":       3.8469305331179324,
        "cpu_util_user":        0,
        "cpu_util_system":      3.836833602584814,
        "sender_has_retransmits":       1.8446744073709552e+19,
        "streams":      [{
                        "id":   1,
                        "bytes":        0,
                        "retransmits":  1.8446744073709552e+19,
                        "jitter":       0,
                        "errors":       0,
                        "packets":      0,
                        "start_time":   0,
                        "end_time":     0.008745
                }]
}
interval_len 0.000505 bytes_transferred 1428
interval forces keep
[ ID] Interval           Transfer     Bitrate         Total Datagrams
[  5]   0.00-0.00   sec  1.39 KBytes  22.6 Mbits/sec  1  
- - - - - - - - - - - - - - - - - - - - - - - - -
Test Complete. Summary Results:
[ ID] Interval           Transfer     Bitrate         Jitter    Lost/Total Datagrams
[  5]   0.00-0.00   sec  1.39 KBytes  22.6 Mbits/sec  0.000 ms  0/1 (0%)  sender
[  5]   0.00-0.01   sec  0.00 Bytes  0.00 bits/sec  0.000 ms  0/0 (0%)  receiver
CPU Utilization: local/sender 18.0% (6.9%u/11.1%s), remote/receiver 3.8% (0.0%u/3.8%s)

Accept big-endian and little-endian format for both legacy and new messages since the value is sent in host byte order.

BrainSlayer · 2023-02-11T11:04:35Z

see my pull request for a better solution. this commit here does not fix the wrong reply message. it just checks for both types on client side. so if you use it on a big endian host as server and the client is still a older version it will still fail. so both client and server side must use the correct endian type message which is little endian for historic reasons

davidBar-On · 2023-12-05T12:27:32Z

src/iperf.h

+#define UDP_CONNECT_MSG 0x36373839                 // "6789" - legacy value was 123456789
+#define UDP_CONNECT_REPLY 0x39383736               // "9876" - legacy value was 987654321
+#define LEGACY_UDP_CONNECT_REPLY 987654321         // Old servers may still reply with legacy value
+#define LEGACY_UDP_CONNECT_REPLY_ENDIAN 2976439866 // Old servers may still reply with legacy value


Per issue #1608, reverse Endian of UDP_CONNECT_REPLY should also be considered. therefore should also add:

#define UDP_CONNECT_REPLY_ENDIAN 0x36373839

davidBar-On · 2023-12-05T12:28:30Z

src/iperf_udp.c

@@ -592,9 +592,9 @@ iperf_udp_connect(struct iperf_test *test)
            printf("Connect received for Socket %d, sz=%d, buf=%x, i=%d, max_len_wait_for_reply=%d\n", s, sz, buf, i, max_len_wait_for_reply);
        }
        i += sz;
-    } while (buf != UDP_CONNECT_REPLY && buf != LEGACY_UDP_CONNECT_REPLY && i < max_len_wait_for_reply);
+    } while (buf != UDP_CONNECT_REPLY && buf != LEGACY_UDP_CONNECT_REPLY && buf != UDP_CONNECT_MSG && buf != LEGACY_UDP_CONNECT_REPLY_ENDIAN && i < max_len_wait_for_reply);


Check also for UDP_CONNECT_REPLY_ENDIAN.

davidBar-On · 2023-12-05T12:28:39Z

src/iperf_udp.c


-    if (buf != UDP_CONNECT_REPLY  && buf != LEGACY_UDP_CONNECT_REPLY) {
+    if (buf != UDP_CONNECT_REPLY && buf != LEGACY_UDP_CONNECT_REPLY && buf != UDP_CONNECT_MSG && buf != LEGACY_UDP_CONNECT_REPLY_ENDIAN) {


Check also for UDP_CONNECT_REPLY_ENDIAN.

this is already covered by my pull request. but the iperf3 maintainer seem to ignore all pull requests and bug fixes since a long time. i fixed it in my own tree and i'm happy with it. every effort here is pointless. check my pull request. its a very simple solution and covers all problems mentioned here

this is already covered by my pull request.

If I understand the pull request correctly, it only covers the endian case of LEGACY_UDP_CONNECT_REPLY but not the endian case of UDP_CONNECT_REPLY, which is what my suggested changes cover.

but the iperf3 maintainer seem to ignore all pull requests and bug fixes since a long time.

This may be because the iperf3 team was busy with releasing the multi-thread version. As this issue seems to be a major issue (that was caused because of my pull request ...) I hope that it (or a version of it) will be merged soon.

yes but there is another pull request made by me which covers everything. i posted it many months ago. this one here is not from me and was also posted a long time ago

OK. Thanks. I assume you are referring to PR #1468. By the way, I see that #1564 is another suggested PR to fix the issue.

The following is copied from the first comment in this PR...

Accept UDP connect message replies in little-endian or big-endian order.
There was discussion and concern about accepting two values in the original pull request #1260, this makes that problem worse. This is a solution that allows mixed-endian and mixed-version operations if the client is updated.

It's been over a year since I put this together so I really don't remember the specifics of what I found from the original change PR, but it was reasonable enough to make me try to limit this change. I work in an environment where updating the client is much more reasonable and frequent than updating the server. 3.12 is broken for mixed endian no matter what so something has to be updated. I was trying to limit the scope of damage that I was re-introducing that #1260 was attempting to fix. Since the author of #1260 is now part of the conversation I can assume he has a better handle on how many issues fixing everything will introduce.

If those concerns aren't real I also prefer a solution that addresses both client and server. That's an additional patch that I'm carrying on my product on top of this one since I'm unaware of ever having #1260 occur and cross-endian and version compatibility is important to me.

bmah888 · 2023-12-07T19:20:25Z

Thanks for the PR! I chose to merge a slightly simpler fix from PR #1468.

--------------------- * Notable user-visible changes * BREAKING CHANGE: iperf3's authentication features, when used with OpenSSL prior to 3.2.0, contain a vulnerability to a side-channel timing attack. To address this flaw, a change has been made to the padding applied to encrypted strings. This change is not backwards compatible with older versions of iperf3 (before 3.17). To restore the older (vulnerable) behavior, and hence backwards-compatibility, use the --use-pkcs1-padding flag. The iperf3 team thanks Hubert Kario from RedHat for reporting this issue and providing feedback on the fix. (CVE-2024-26306)(PR#1695) * iperf3 no longer changes its current working directory in --daemon mode. This results in more predictable behavior with relative paths, in particular finding key and credential files for authentication. (PR#1672) * A new --json-stream option has been added to enable a streaming output format, consisting of a series of JSON objects (for the start of the test, each measurement interval, and the end of the test) separated by newlines (#444, #923, #1098). * UDP tests now work correctly between different endian hosts (#1415). * The --fq-rate parameter now works for --reverse tests (#1632, PR#1667). * The statistics reporting interval is now available in the --json start test object (#1663). * A negative time test duration is now properly flagged as an error (IS#1662 / PR#1666). * Notable developer-visible changes * Fixes have been made to better (unofficially) support builds on Android (#1641 / #1651) and VxWorks (#1595). * iperf3 now builds correctly on architectures without native support for 64-bit atomic types, by linking with the libatomic library (#1611).

Address UDP connect message endian issue

1bfed6c

Accept big-endian and little-endian format for both legacy and new messages since the value is sent in host byte order.

mriswyth force-pushed the udp_endian_issue branch from 087fa8b to 1bfed6c Compare November 7, 2022 17:57

bmah888 linked an issue Feb 9, 2023 that may be closed by this pull request

Mixed big-endian and little-endian UDP test fails #1414

Closed

bmah888 mentioned this pull request Feb 9, 2023

big endian bug #1467

Closed

davidBar-On reviewed Dec 5, 2023

View reviewed changes

davidBar-On mentioned this pull request Dec 5, 2023

iperf3 -u -c fails for server with opposite byte-order #1608

Closed

bmah888 closed this Dec 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address UDP connect message endian issue #1415

Address UDP connect message endian issue #1415

mriswyth commented Nov 4, 2022 •

edited

mriswyth commented Nov 4, 2022

BrainSlayer commented Feb 11, 2023

davidBar-On Dec 5, 2023

davidBar-On Dec 5, 2023

davidBar-On Dec 5, 2023

BrainSlayer Dec 5, 2023

davidBar-On Dec 6, 2023

BrainSlayer Dec 6, 2023

davidBar-On Dec 6, 2023

mriswyth Dec 6, 2023

bmah888 commented Dec 7, 2023


		if (buf != UDP_CONNECT_REPLY && buf != LEGACY_UDP_CONNECT_REPLY) {
		if (buf != UDP_CONNECT_REPLY && buf != LEGACY_UDP_CONNECT_REPLY && buf != UDP_CONNECT_MSG && buf != LEGACY_UDP_CONNECT_REPLY_ENDIAN) {

Address UDP connect message endian issue #1415

Address UDP connect message endian issue #1415

Conversation

mriswyth commented Nov 4, 2022 • edited

mriswyth commented Nov 4, 2022

BrainSlayer commented Feb 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmah888 commented Dec 7, 2023

mriswyth commented Nov 4, 2022 •

edited