Browse code

ims_ipsec_pcscf: fix issues pointed out in code review

Remove error logs which were not errors at all.
Fix issue in clean_spi_list where the free SPI list would get completely wiped.

Kristiyan Peychev authored on 11/05/2022 14:43:55
Showing 5 changed files
... ...
@@ -945,6 +945,10 @@ int ipsec_reconfig()
945 945
 		return 0;
946 946
 	}
947 947
 
948
+	if(clean_spi_list() != 0) {
949
+		return 1;
950
+	}
951
+
948 952
 	return ipsec_cleanall();
949 953
 }
950 954
 
... ...
@@ -82,7 +82,7 @@ modparam("ims_ipsec_pcscf", "ipsec_listen_addr6", "")
82 82
     <section>
83 83
       <title><varname>ipsec_client_port</varname> (int)</title>
84 84
 
85
-      <para>Start port number which will be bound for incoming (server) IPSec traffic.</para>
85
+      <para>Port number which will be bound for incoming (server) IPSec traffic.</para>
86 86
 
87 87
       <para><emphasis>Default value is 5062.</emphasis></para>
88 88
 
... ...
@@ -100,7 +100,7 @@ modparam("ims_ipsec_pcscf", "ipsec_client_port", 5062)
100 100
     <section>
101 101
       <title><varname>ipsec_server_port</varname> (int)</title>
102 102
 
103
-      <para>Start port number which will be bound for incoming (server) IPSec traffic.</para>
103
+      <para>Port number which will be bound for incoming (server) IPSec traffic.</para>
104 104
 
105 105
       <para><emphasis>Default value is 5063.</emphasis></para>
106 106
 
... ...
@@ -118,9 +118,7 @@ modparam("ims_ipsec_pcscf", "ipsec_server_port", 5063)
118 118
     <section>
119 119
       <title><varname>ipsec_max_connections</varname> (int)</title>
120 120
 
121
-      <para>Maximum IPSec connections for the process. E.g. if ipsec_client_port=5100, ipsec_server_port=6100 and
122
-      ipsec_max_connections=10, all client ports between 5100 and 5109 and all server ports between 6100 and 6109
123
-      will be used for maximum to 10 IPSec connections.</para>
121
+      <para>Maximum simultanious IPSec connections</para>
124 122
 
125 123
       <para><emphasis>Default value is 2.</emphasis></para>
126 124
 
... ...
@@ -138,11 +136,9 @@ modparam("ims_ipsec_pcscf", "ipsec_max_connections", 10)
138 136
     <section>
139 137
       <title><varname>ipsec_reuse_server_port</varname> (int)</title>
140 138
 
141
-      <para>Reuse (1) or not (0) the P-CSCF Server port for Re-registration for one UA.
142
-      When set to 0 - During Re-registration P-CSCF will distribute new P-CSCF client and
143
-      P-CSCF server ports.
144
-      When set to 1 - During Re-registration P-CSCF will reuse the old P-CSCF server port and
145
-      will distribute a new P-CSCF client port.</para>
139
+      <para>Reuse (1) or not (0) the P-CSCF IPSec information for Re-registration for one UA.
140
+      When set to 0 - During Re-registration P-CSCF will create new IPSec tunnels.
141
+      When set to 1 - During Re-registration P-CSCF will reuse the old IPSec tunnels.</para>
146 142
 
147 143
       <para><emphasis>Default value is 1.</emphasis></para>
148 144
 
... ...
@@ -192,6 +188,40 @@ modparam("ims_ipsec_pcscf", "ipsec_spi_id_start", 100)
192 188
         <programlisting format="linespecific">
193 189
 ...
194 190
 modparam("ims_ipsec_pcscf", "ipsec_spi_id_range", 1000)
191
+...
192
+        </programlisting>
193
+      </example>
194
+    </section>
195
+
196
+    <section>
197
+      <title><varname>ipsec_preferred_alg</varname> (string)</title>
198
+
199
+      <para>A name of an authentication algorithm which the Proxy-CSCF will <emphasis>prefer</emphasis> when creating IPSec tunnels.</para>
200
+      <para><emphasis>Default value is empty string (null) - the last algorithm in the Sec-Agree header will be used.</emphasis></para>
201
+
202
+      <example>
203
+        <title><varname>ipsec_preferred_alg</varname> parameter usage</title>
204
+
205
+        <programlisting format="linespecific">
206
+...
207
+modparam("ims_ipsec_pcscf", "ipsec_preferred_alg", "hmac-sha-1-96")
208
+...
209
+        </programlisting>
210
+      </example>
211
+    </section>
212
+
213
+    <section>
214
+      <title><varname>ipsec_preferred_ealg</varname> (string)</title>
215
+
216
+      <para>A name of an encrytion algorithm which the Proxy-CSCF will <emphasis>prefer</emphasis> when creating IPSec tunnels.</para>
217
+      <para><emphasis>Default value is empty string (null) - the last algorithm in the Sec-Agree header will be used. Note that the possibility of it being the "null" algorithm is not insignificant.</emphasis></para>
218
+
219
+      <example>
220
+        <title><varname>ipsec_preferred_ealg</varname> parameter usage</title>
221
+
222
+        <programlisting format="linespecific">
223
+...
224
+modparam("ims_ipsec_pcscf", "ipsec_preferred_ealg", "aes-cbc")
195 225
 ...
196 226
         </programlisting>
197 227
       </example>
... ...
@@ -43,6 +43,8 @@ int ipsec_reuse_server_port = 1;
43 43
 int ipsec_max_connections = 2;
44 44
 int spi_id_start = 100;
45 45
 int spi_id_range = 1000;
46
+str ipsec_preferred_alg= STR_NULL;
47
+str ipsec_preferred_ealg= STR_NULL;
46 48
 int xfrm_user_selector = 143956232;
47 49
 
48 50
 ip_addr_t ipsec_listen_ip_addr;
... ...
@@ -90,6 +92,8 @@ static param_export_t params[] = {
90 92
 	{"ipsec_max_connections",	INT_PARAM, &ipsec_max_connections	},
91 93
 	{"ipsec_spi_id_start",		INT_PARAM, &spi_id_start			},
92 94
 	{"ipsec_spi_id_range",		INT_PARAM, &spi_id_range			},
95
+	{"ipsec_preferred_alg",		PARAM_STR, &ipsec_preferred_alg		},
96
+	{"ipsec_preferred_ealg",	PARAM_STR, &ipsec_preferred_ealg	},
93 97
 	{0, 0, 0}
94 98
 };
95 99
 
... ...
@@ -25,6 +25,9 @@
25 25
 #include "../../core/parser/msg_parser.h"
26 26
 #include "../../core/mem/mem.h"
27 27
 
28
+extern str ipsec_preferred_alg;
29
+extern str ipsec_preferred_ealg;
30
+
28 31
 static uint32_t parse_digits(str value)
29 32
 {
30 33
     uint32_t ret = 0;
... ...
@@ -76,7 +79,7 @@ static int process_sec_agree_param(str name, str value, ipsec_t *ret, char *alg_
76 79
     if(strncasecmp(name.s, "alg", name.len) == 0) {
77 80
         SEC_COPY_STR_PARAM(ret->r_alg, value);
78 81
 
79
-        if(strncasecmp(value.s, "hmac-sha-1-96", value.len) == 0) {
82
+        if(ipsec_preferred_alg.len && STR_EQ(value, ipsec_preferred_alg)) {
80 83
             *alg_found = 1;
81 84
         }
82 85
     }
... ...
@@ -89,7 +92,7 @@ static int process_sec_agree_param(str name, str value, ipsec_t *ret, char *alg_
89 92
     else if(strncasecmp(name.s, "ealg", name.len) == 0) {
90 93
         SEC_COPY_STR_PARAM(ret->r_ealg, value);
91 94
 
92
-        if(strncasecmp(value.s, "aes-cbc", value.len) == 0) {
95
+        if(ipsec_preferred_ealg.len && STR_EQ(value, ipsec_preferred_ealg)) {
93 96
             *ealg_found = 1;
94 97
         }
95 98
     }
... ...
@@ -35,12 +35,51 @@ typedef struct spi_generator{
35 35
 	uint32_t		spi_val;
36 36
 	uint32_t		min_spi;
37 37
 	uint32_t		max_spi;
38
+	uint32_t		sport_start_val;
39
+	uint32_t		cport_start_val;
40
+	uint32_t		port_range;
38 41
 } spi_generator_t;
39 42
 
40 43
 spi_generator_t* spi_data = NULL;
41 44
 
45
+static int init_free_spi()
46
+{
47
+	uint32_t sport_start_val, cport_start_val, port_range, sport, cport, j;
48
+
49
+	if(!spi_data) {
50
+		return 1;
51
+	}
52
+
53
+	sport_start_val = spi_data->sport_start_val;
54
+	cport_start_val = spi_data->cport_start_val;
55
+	port_range = spi_data->port_range;
56
+	//save the initial value for the highly unlikely case where there are no free SPIs
57
+	sport = sport_start_val;
58
+	cport = cport_start_val;
59
+
60
+	spi_data->free_spi = create_list();
61
+	for(j = spi_data->min_spi; j < spi_data->max_spi; j+=2)
62
+	{
63
+		spi_add(&spi_data->free_spi, j, j+1, cport, sport);
64
+		cport++;
65
+		sport++;
66
+
67
+		if(cport >= cport_start_val + port_range) {
68
+			cport = cport_start_val;
69
+		}
70
+
71
+		if(sport >= sport_start_val + port_range) {
72
+			sport = sport_start_val;
73
+		}
74
+	}
75
+
76
+	return 0;
77
+}
78
+
42 79
 int init_spi_gen(uint32_t spi_start_val, uint32_t spi_range, uint32_t sport_start_val, uint32_t cport_start_val, uint32_t port_range)
43 80
 {
81
+    uint32_t j;
82
+
44 83
     if(spi_start_val < 1) {
45 84
         return 1;
46 85
     }
... ...
@@ -49,11 +88,6 @@ int init_spi_gen(uint32_t spi_start_val, uint32_t spi_range, uint32_t sport_star
49 88
         return 2;
50 89
     }
51 90
 
52
-    //save the initial value for the highly unlikely case where there are no free SPIs
53
-    uint32_t sport = sport_start_val;
54
-    uint32_t cport = cport_start_val;
55
-    uint32_t j = 0;
56
-
57 91
     if(spi_data){
58 92
         return 3;
59 93
     }
... ...
@@ -76,27 +110,15 @@ int init_spi_gen(uint32_t spi_start_val, uint32_t spi_range, uint32_t sport_star
76 110
         spi_data->used_spis[j] = create_list();
77 111
     }
78 112
 
79
-    spi_data->free_spi = create_list();
80 113
     spi_data->spi_val  = spi_data->min_spi = spi_start_val;
81 114
     spi_data->max_spi  = spi_start_val + spi_range;
115
+    spi_data->sport_start_val = sport_start_val;
116
+    spi_data->cport_start_val = cport_start_val;
117
+    spi_data->port_range = port_range;
82 118
 
83
-    LM_ERR("spi_data:%p init free spi for IPSEC tunnel min_spi:%u max_api:%u\n", spi_data, spi_data->min_spi, spi_data->max_spi);
84
-
85
-    for(j = spi_data->min_spi; j < spi_data->max_spi; j+=2)
86
-    {
87
-        spi_add(&spi_data->free_spi, j, j+1, cport, sport);
88
-        LM_ERR("spi_data:%p add element to spi free list head:%p\n", spi_data, spi_data->free_spi.head);
89
-        cport++;
90
-        sport++;
91
-
92
-        if(cport >= cport_start_val + port_range) {
93
-            cport = cport_start_val;
94
-        }
95
-
96
-        if(sport >= sport_start_val + port_range) {
97
-            sport = sport_start_val;
98
-        }
99
-    }
119
+	if(init_free_spi() != 0) {
120
+		return 7;
121
+	}
100 122
 
101 123
 	pthread_mutex_unlock(&spi_data->spis_mut);
102 124
 
... ...
@@ -159,6 +181,8 @@ int release_spi(uint32_t spi_cid, uint32_t spi_sid, uint16_t cport, uint16_t spo
159 181
 
160 182
 int clean_spi_list()
161 183
 {
184
+	uint32_t j;
185
+
162 186
 	if(!spi_data){
163 187
 		return 1;
164 188
 	}
... ...
@@ -167,11 +191,13 @@ int clean_spi_list()
167 191
 		return 1;
168 192
 	}
169 193
 
170
-  uint32_t j = 0;
171 194
 	for(j = 0; j < MAX_HASH_SPI; j++) {
172 195
 		destroy_list(&spi_data->used_spis[j]);
173 196
 	}
174
-  destroy_list(&spi_data->free_spi);
197
+
198
+	destroy_list(&spi_data->free_spi);
199
+	init_free_spi();
200
+
175 201
 	spi_data->spi_val = spi_data->min_spi;
176 202
 
177 203
 	pthread_mutex_unlock(&spi_data->spis_mut);
... ...
@@ -181,7 +207,6 @@ int clean_spi_list()
181 207
 
182 208
 int destroy_spi_gen()
183 209
 {
184
-	LM_ERR("destroy spi list \n" );
185 210
 	if(!spi_data){
186 211
 		return 1;
187 212
 	}