1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
|
From: "Roger A. Light" <roger@atchoo.org>
Date: Tue, 10 Aug 2021 20:48:21 +0100
Subject: Fix CONNECT performance with many user-properties.
Origin: https://github.com/eclipse/mosquitto/commit/9d6a73f9f72005c2f19a262f15d28327eedea91f
Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=575314
Bug: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/637
Bug-Debian: https://bugs.debian.org/1001028
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2021-41039
An MQTT v5 client connecting with a large number of user-property properties
could cause excessive CPU usage, leading to a loss of performance and
possible denial of service. This has been fixed.
---
lib/property_mosq.c | 14 ++++++------
test/broker/01-connect-575314.py | 49 ++++++++++++++++++++++++++++++++++++++++
test/broker/Makefile | 1 +
test/broker/test.py | 1 +
4 files changed, 58 insertions(+), 7 deletions(-)
create mode 100644 test/broker/01-connect-575314.py
diff --git a/lib/property_mosq.c b/lib/property_mosq.c
index 0cfc9f9..5f0f24c 100644
--- a/lib/property_mosq.c
+++ b/lib/property_mosq.c
@@ -959,14 +959,14 @@ int mosquitto_property_check_all(int command, const mosquitto_property *properti
if(rc) return rc;
/* Check for duplicates */
- tail = p->next;
- while(tail){
- if(p->identifier == tail->identifier
- && p->identifier != MQTT_PROP_USER_PROPERTY){
-
- return MOSQ_ERR_DUPLICATE_PROPERTY;
+ if(p->identifier != MQTT_PROP_USER_PROPERTY){
+ tail = p->next;
+ while(tail){
+ if(p->identifier == tail->identifier){
+ return MOSQ_ERR_DUPLICATE_PROPERTY;
+ }
+ tail = tail->next;
}
- tail = tail->next;
}
p = p->next;
diff --git a/test/broker/01-connect-575314.py b/test/broker/01-connect-575314.py
new file mode 100644
index 0000000..4a8f314
--- /dev/null
+++ b/test/broker/01-connect-575314.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+
+# Check for performance of processing user-property on CONNECT
+
+from mosq_test_helper import *
+
+def do_test():
+ rc = 1
+ props = mqtt5_props.gen_string_pair_prop(mqtt5_props.PROP_USER_PROPERTY, "key", "value")
+ for i in range(0, 20000):
+ props += mqtt5_props.gen_string_pair_prop(mqtt5_props.PROP_USER_PROPERTY, "key", "value")
+ connect_packet_slow = mosq_test.gen_connect("connect-user-property", proto_ver=5, properties=props)
+ connect_packet_fast = mosq_test.gen_connect("a"*65000, proto_ver=5)
+ connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5)
+
+ port = mosq_test.get_port()
+ broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port)
+
+ try:
+ t_start = time.monotonic()
+ sock = mosq_test.do_client_connect(connect_packet_slow, connack_packet, port=port)
+ t_stop = time.monotonic()
+ sock.close()
+
+ t_diff_slow = t_stop - t_start
+
+ t_start = time.monotonic()
+ sock = mosq_test.do_client_connect(connect_packet_fast, connack_packet, port=port)
+ t_stop = time.monotonic()
+ sock.close()
+
+ t_diff_fast = t_stop - t_start
+ # 20 is chosen as a factor that works in plain mode and running under
+ # valgrind. The slow performance manifests as a factor of >100. Fast is <10.
+ if t_diff_slow / t_diff_fast < 20:
+ rc = 0
+ except mosq_test.TestError:
+ pass
+ finally:
+ broker.terminate()
+ broker.wait()
+ (stdo, stde) = broker.communicate()
+ if rc:
+ print(stde.decode('utf-8'))
+ exit(rc)
+
+
+do_test()
+exit(0)
diff --git a/test/broker/Makefile b/test/broker/Makefile
index e1501b4..1ee2dd2 100644
--- a/test/broker/Makefile
+++ b/test/broker/Makefile
@@ -20,6 +20,7 @@ ptest : test-compile
test : test-compile 01 02 03 04 05 06 07 08 09 10 11 12 13 14
01 :
+ ./01-connect-575314.py
./01-connect-allow-anonymous.py
./01-connect-bad-packet.py
./01-connect-connack-2163.py
diff --git a/test/broker/test.py b/test/broker/test.py
index 91a4ca4..3dc0058 100755
--- a/test/broker/test.py
+++ b/test/broker/test.py
@@ -5,6 +5,7 @@ import ptest
tests = [
#(ports required, 'path'),
+ (1, './01-connect-575314.py'),
(1, './01-connect-allow-anonymous.py'),
(1, './01-connect-bad-packet.py'),
(1, './01-connect-connack-2163.py'),
|
