[sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)

[alexyosifov]

@alexyosifov approved this pull request.



> -        // for Reply and TCP sends from P-CSCF server port, for Reply and UDP sends from P-CSCF client port
-        src_port = dst_proto == PROTO_TCP ? s->port_ps : s->port_pc;
+        // Check send socket
+        struct socket_info * client_sock = grep_sock_info(via_host.af == AF_INET ? &ipsec_listen_addr : &ipsec_listen_addr6, src_port, dst_proto);
+        if(client_sock) {
+            // for Reply and TCP sends from P-CSCF server port, for Reply and UDP sends from P-CSCF client port
+            src_port = dst_proto == PROTO_TCP ? s->port_ps : s->port_pc;
 
-        // for Reply and TCP sends to UE client port, for Reply and UDP sends to UE server port
-        dst_port = dst_proto == PROTO_TCP ? s->port_uc : s->port_us;
+            // for Reply and TCP sends to UE client port, for Reply and UDP sends to UE server port
+            dst_port = dst_proto == PROTO_TCP ? s->port_uc : s->port_us;
+        }
+        else
+        {
+            src_port = s->port_pc;
+            dst_port = s->port_us;
+        }

Why you have to check "send socket" here? You perform the check with zero src_port and dst_proto.
The "send socket" always is present. If you scroll down at row 877 there is the same check and if "send socket" is not present the function returns an error.
I do not see value from this change and I am not sure this will work properly.
Correct me if I am wrong.

> +    // cipher_null, des,  des3_ede, aes
     strcpy(l_enc_algo->alg_name,"cipher_null");
+    if (strncasecmp(r_ealg.s,"aes-cbc",r_ealg.len) == 0) {
+        LM_DBG("Creating security associations: AES\n");
+        strcpy(l_enc_algo->alg_name,"aes");
+        l_enc_algo->alg_key_len = ck.len * 4;
+        string_to_key(l_enc_algo->alg_key, ck);
+    }
+    else if (strncasecmp(r_ealg.s,"des-ede3-cbc",r_ealg.len) == 0) {
+        LM_DBG("Creating security associations: DES, ck.len=%d\n",ck.len);
+        strcpy(l_enc_algo->alg_name,"des3_ede");
+        str ck1;
+        ck1.s = pkg_malloc (128);
+        strncpy(ck1.s,ck.s,32);
+        strncat(ck1.s,ck.s,16);
+        ck1.len=32+16;
+
+        l_enc_algo->alg_key_len = ck1.len * 4;
+        string_to_key(l_enc_algo->alg_key, ck1);
+
+        pkg_free(ck1.s);
+    }

Is it a good idea cipher algorithm to be optional? Just add a new value in _cflags parameter in int ipsec_create(struct sip_msg* m, udomain_t* d, int _cflags) method.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/2731#pullrequestreview-655573226
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.kamailio.org/pipermail/sr-dev/attachments/20210510/afc770cb/attachment.htm>