Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
Merged #2731 into master. -- 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#event-4788688269___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@henningw commented on this pull request. > @@ -100,8 +100,15 @@ static void string_to_key(char* dst, const str > key_string) } } +static uint choose_nlmsg_seq (void) Was resolved as per other discussions. -- 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#discussion_r638247096___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@miconda , No comments from my side. -- 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#issuecomment-847306443___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
Thanks @miconda , I did the correction and pushed -- 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#issuecomment-844490224___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv pushed 2 commits. 8ef89c258fceb417621cee694a2871066e06dc04 Merge remote-tracking branch 'origin/master' into ipsec_improve f792b215321314d552f7fa90f17ef25b91cc2c09 ims_ipsec_pcscf: use ksr_clock_gettime() -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/kamailio/kamailio/pull/2731/files/7133cb20ad46aafc98e16a571ca641f88c444a09..f792b215321314d552f7fa90f17ef25b91cc2c09 ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv - I added `ksr_clock_gettime()` in the `core/ut.h`, which is portable version of `clock_gettime()` that works for MacOS as well. Push another commit to this PR to use it. Any other comments from devs for this PR? -- 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#issuecomment-843784315___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv commented on 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 ? _listen_addr : _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;
+}
Welcome!
--
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#discussion_r629372001___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@alexyosifov commented on this pull request.
> +// 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);
+}
OK, Thanks!
--
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#discussion_r629355503___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@alexyosifov commented on 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 ? _listen_addr : _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;
+}
I agree. Thanks for the clarification!
--
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#discussion_r629355293___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv commented on this pull request.
> +// 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);
+}
I simply add the algorithm in the original code, nothing more
--
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#discussion_r629336886___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv commented on 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 ? _listen_addr : _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;
+}
Hi @alexyosifov ,
without the patch the ports in case of TCP are always:
src_port = s->port_ps
dst_port = s->port_uc
That are the ones used in case the user agent open the connection.
A pre check is necessary because if this socket is not already open, the use
the ports:
src_port = s->port_pc;
dst_port = s->port_us;
that are the port from Proxy Client (pc) -> User Server (us).
Is a attempt to fallback to the other opened socket.
This case can be happens when INVITE transaction is very long due to long
Ringing time phase.
--
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#discussion_r629336057___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@riccardv commented on this pull request. > @@ -100,8 +100,15 @@ static void string_to_key(char* dst, const str > key_string) } } +static uint choose_nlmsg_seq (void) Hi @henningw , I choose the 1 micro seconds granulation due to the commit comment: "improve nlmsg_seq choice for concurrent multi UEs Registrations at same time" The reason is the previous choice of time(NULL) return a time value on seconds and it can be too large in presence of multiple UE access in the same time. In high load condition it can be happens. IPsec Security Association must have this parameter as a unique identifier, since kamailio can be configured to listen on single local ip:port for IPsec, it can be a problem, resulting in a failure on activation of SA, and then a failure on VoLTE SIP REGISTRATION. A time based choice is a good choice, but 1 second is not enough, so I propose the choice of 1 micro second granularity that can be protect the registrations in a reasonable way. Regarding other platforms that don't have the funtion, feel free to add a pre processor condition on the function -- 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#discussion_r629325968___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@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 ? _listen_addr : _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___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@alexyosifov approved this pull request.
Added some comments
> @@ -846,11 +846,20 @@ int ipsec_forward(struct sip_msg* m, udomain_t* d, int
> _cflags)
// for Reply get the dest proto from the received request
dst_proto = req->rcv.proto;
-// 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
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.
> strcpy(l_enc_algo->alg_name,"cipher_null");
+if (strncasecmp(r_ealg.s,"aes-cbc",r_ealg.len) == 0) {
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-64158___
Kamailio (SER) - Development Mailing List
sr-dev@lists.kamailio.org
https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@henningw commented on this pull request. Thanks for the pull request, i have added one comment regarding the time handling. Are there module any documentation updates necessary as well? > @@ -100,8 +100,15 @@ static void string_to_key(char* dst, const str > key_string) } } +static uint choose_nlmsg_seq (void) Any particular reason why you choose to implement this function instead of the previous time(0) call? The clock_gettime() function is not available at Mac OS, and probably need a wrapper around it as e.g. in the cdp module. If not necessary, we should probably stay with the previous way. -- 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-655517861___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] ims_ipsec_pcscf: more algorithms, SA improvements (#2731)
@alexyosifov - given that you contributed recently to this module, do you have any comment on this PR? -- 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#issuecomment-836553802___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
