Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ims_registrar_scscf: fix multiple contacts in NOTIFY #2246

Merged
merged 1 commit into from
Apr 21, 2020

Conversation

alexyosifov
Copy link
Contributor

  • Prevent multiple contacts for NOTIFY message in
    Message body tag after
    RE-REGISTRATION procedure.

Pre-Submission Checklist

  • Commit message has the format required by CONTRIBUTING guide
  • Commits are split per component (core, individual modules, libs, utils, ...)
  • Each component has a single commit (if not, squash them into one commit)
  • No commits to README files for modules (changes must be done to docbook files
    in doc/ subfolder, the README file is autogenerated)

Type Of Change

  • Small bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds new functionality)
  • Breaking change (fix or feature that would change existing functionality)

Checklist:

  • PR should be backported to stable branches
  • Tested changes locally
  • Related to issue #XXXX (replace XXXX with an open issue number)

Description

@henningw
Copy link
Contributor

Looks fine to me, you can merge.

Just a note for the archive - would probably make sense to review static int handle_ruri_alias(struct sip_msg *msg) in nathelper.c, which does a kind of similar function internally like the new added extract_alias_ip_port, to see if this could be generalized and maybe added to the core.

@ngvoice
Copy link
Member

ngvoice commented Mar 12, 2020

Question:
If the Proxy-CSCF (or any other AS) sends a subscribe and expects a NOTIFY, then the condition would never be true, right?

This should be adressed in my opinion.

@kamailio-sync
Copy link

kamailio-sync commented Mar 12, 2020 via email

@kamailio-sync
Copy link

kamailio-sync commented Mar 12, 2020 via email

@alexyosifov
Copy link
Contributor Author

alexyosifov commented Mar 12, 2020

Hi,
After reception of SUBSCRIBE msg, create_notifications() method is invoked with event_type=IMS_REGISTRAR_SUBSCRIBE and there is a full-contact match of IP, port, and alias. This prevents sending multiple NOTIFY msgs after RE-REGISTRATION. In this case, single NOTIFY msg will be sent with the right content (after the fix in this pull request).

Thanks!

@henningw
Copy link
Contributor

@ngvoice does this adress your concerns? If not, probably make sense to discuss e.g. on the list with @alexyosifov all the details and then update this ticket with the outcome.

@ngvoice
Copy link
Member

ngvoice commented Mar 13, 2020 via email

@henningw
Copy link
Contributor

@ngvoice - do you managed to do the local tests, any update?

@alexyosifov
Copy link
Contributor Author

@ngvoice, Hi,
Any update with the local tests?

@ngvoice
Copy link
Member

ngvoice commented Apr 7, 2020

Sorry for the delay. Due to the Corona virus, the access to the Lab is a limited. In fact, it's good business for our Lab-Network vendors, as we had to replicate the Lab to various colleagues at home... ;-)

However, the result of testing is exactly as expected. This is the NOTIFY for the SUBSCRIBE from the UE to the S-CSCF (this one is correct):

2020/04/07 14:56:47.851519 172.18.0.24:6060 -> 192.168.178.92:5060
NOTIFY sip:40e02947-53d8-48c3-a1b2-eca3dd2635fd@192.168.4.6:42923 SIP/2.0
Via: SIP/2.0/UDP 172.18.0.24:6060;branch=z9hG4bK1d7b.c2da92c5000000000000000000000000.0
To: <tel:+494046899711>;tag=3291254491
From: <tel:+494046899711>;tag=ee326f8ce5448eb29429c3445faa64d5-ae1c
CSeq: 70029013 NOTIFY
Call-ID: 3291254482_2340697020@192.168.4.6
Route: <sip:mo@192.168.178.92;r2=on;lr=on;ftag=3291254491>, <sip:mo@192.168.178.92:5062;r2=on;lr=on;ftag=3291254491>
Content-Length: 1779
User-Agent: Kamailio S-CSCF
Contact: <sip:scscf-1.ims.mnc001.mcc001.3gppnetwork.org>
Event: reg
Max-Forwards: 70
Subscription-State: active;expires=600000
Content-Type: application/reginfo+xml

<?xml version="1.0"?>
<reginfo xmlns="urn:ietf:params:xml:ns:reginfo" version="2" state="full">
 <registration aor="tel:+494046899711" id="0x7f8844b8f5a8" state="active">
  <contact id="0x7f8844b9e9e0" state="active" event="registered" expires="600000">
   <uri>sip:40e02947-53d8-48c3-a1b2-eca3dd2635fd@192.168.4.6:42923</uri>
  <unknown-param name="+sip.instance">"&lt;urn:gsma:imei:86153804-193985-0&gt;"</unknown-param>
  <unknown-param name="video"></unknown-param>
  <unknown-param name="+g.3gpp.smsip"></unknown-param>
  <unknown-param name="+g.3gpp.icsi-ref">""</unknown-param>
  </contact>
 </registration>
 <registration aor="sip:494046899711@ims.mnc001.mcc001.3gppnetwork.org" id="0x7f8844b8faa0" state="active">
  <contact id="0x7f8844b9e9e0" state="active" event="registered" expires="600000">
   <uri>sip:40e02947-53d8-48c3-a1b2-eca3dd2635fd@192.168.4.6:42923</uri>
  <unknown-param name="+sip.instance">"&lt;urn:gsma:imei:86153804-193985-0&gt;"</unknown-param>
  <unknown-param name="video"></unknown-param>
  <unknown-param name="+g.3gpp.smsip"></unknown-param>
  <unknown-param name="+g.3gpp.icsi-ref">""</unknown-param>
  </contact>
 </registration>
 <registration aor="sip:+494046899711@ims.mnc001.mcc001.3gppnetwork.org" id="0x7f8844b8ffa8" state="active">
  <contact id="0x7f8844b9e9e0" state="active" event="registered" expires="600000"
[kamailio-notify-bug.zip](https://github.com/kamailio/kamailio/files/4445391/kamailio-notify-bug.zip)

>
   <uri>sip:40e02947-53d8-48c3-a1b2-eca3dd2635fd@192.168.4.6:42923</uri>
  <unknown-param name="+sip.instance">"&lt;urn:gsma:imei:86153804-193985-0&gt;"</unknown-param>
  <unknown-param name="video"></unknown-param>
  <unknown-param name="+g.3gpp.smsip"></unknown-param>
  <unknown-param name="+g.3gpp.icsi-ref">""</unknown-param>
  </contact>
 </registration>
</reginfo>

But for the P-CSCF to the S-CSCF, we are missing significant parts, which makes the NOTIFY invalid:

2020/04/07 14:56:47.747234 172.18.0.24:6060 -> 192.168.178.92:5060
NOTIFY sip:pcscf-int-1.ims.mnc001.mcc001.3gppnetwork.org SIP/2.0
Via: SIP/2.0/UDP 172.18.0.24:6060;branch=z9hG4bK6f36.eb2f0cc3000000000000000000000000.0
To: <sip:pcscf-int-1.ims.mnc001.mcc001.3gppnetwork.org>;tag=bbaebbb44e94e479868b46460b1d800b-d988
From: <sip:494046899711@ims.mnc001.mcc001.3gppnetwork.org>;tag=ee326f8ce5448eb29429c3445faa64d5-ae1c
CSeq: 13 NOTIFY
Call-ID: 7fb844aa6636e93a-173@192.168.178.92
Content-Length: 450
User-Agent: Kamailio S-CSCF
Contact: <sip:scscf-1.ims.mnc001.mcc001.3gppnetwork.org>
Event: reg
Max-Forwards: 70
Subscription-State: active;expires=3700
Content-Type: application/reginfo+xml

<?xml version="1.0"?>
<reginfo xmlns="urn:ietf:params:xml:ns:reginfo" version="2" state="full">
 <registration aor="tel:+494046899711" id="0x7f8844b8f5a8" state="active">
 </registration>
 <registration aor="sip:494046899711@ims.mnc001.mcc001.3gppnetwork.org" id="0x7f8844b8faa0" state="active">
 </registration>
 <registration aor="sip:+494046899711@ims.mnc001.mcc001.3gppnetwork.org" id="0x7f8844b8ffa8" state="active">
 </registration>
</reginfo>

@alexyosifov
Copy link
Contributor Author

Looks fine to me, you can merge.

Just a note for the archive - would probably make sense to review static int handle_ruri_alias(struct sip_msg *msg) in nathelper.c, which does a kind of similar function internally like the new added extract_alias_ip_port, to see if this could be generalized and maybe added to the core.

Hi, I will check this function handle_ruri_alias to see if it the same functionality as the new one extract_alias_ip_port. And the idea for generalization and added the core is good.

@alexyosifov
Copy link
Contributor Author

alexyosifov commented Apr 8, 2020

@ngvoice
Hi,
thank you for the quick response!
I see the problem right now but to be honest with you - in our test lab we never met this behavior of the P-CSCF. The P-CSCF sends SUBSCRIBE generated from itself (with Contact: <sip: pcscf-int-1 .ims.mnc001.mcc001.3gppnetwork.org> and Call-ID 7fb844aa6636e93a-173@192.168.178.92) and there is a problem with my fix. The fix is ​​working for the SUBSCRIBE coming from the UE.
Could you please give us some more details about the P-CSCF configuration for the current test. Especially for the part with the SUBSCRIBE generated from the P-CSCF.

Thank you!

@alexyosifov
Copy link
Contributor Author

Hi,
I just tested my change and reginfo enabled in PCSCF configuration. I got the same result as you.
But the problem is that without my change there is an issue with the NOTIFY sent to the UE. If there are two or more contacts (for example after re-registration) the NOTIFY sent to the UE includes ALL these contacts. If there is more than one contact the UE starts re-registering after the least expiation time from all of the contacts in the NOTIFY. I had a look on the code I cannot find a way to fix the issue with the subscribe sent from PCSCF (reginfo enabled) and my fix for the NOTIFY. Would you please have a look and let me know if you have any suggestions?
Here is an example of NOTIFY for the SUBSCRIBE from the UE to the S-CSCF after re-registration. For each there is two contacts - the old one (expires=163 sec) and the new one (expires=299 sec). This is the problem - the UE starts re-registering with the least time - 163 sec.

image

Best regards!

- Prevent multiple contacts for NOTIFY message in
  Message body <registration> tag after
  RE-REGISTRATION procedure.
@alexyosifov alexyosifov force-pushed the ims_registrar_scscf_notify_fix branch from 58d289c to fa8b794 Compare April 21, 2020 09:27
@alexyosifov
Copy link
Contributor Author

Hi,
I did some fixes. Could you please check them.

Thanks!

@ngvoice ngvoice merged commit 058edd7 into kamailio:master Apr 21, 2020
@alexyosifov alexyosifov deleted the ims_registrar_scscf_notify_fix branch April 21, 2020 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants