OpenSIPS accepts broken Contact header, then fails to start

[saghul]

(Tested on OpenSIPS 1.11 as of commit 95f5f79)

OpenSIPS accepts a registration with a broken Contact header and saves it in the location table. Example REGISTER:

REGISTER sip:sip2sip.info SIP/2.0
Via: SIP/2.0/UDP 127.0.1.1:5060;branch=z9hG4bK-1605-1-0
Max-Forwards: 70
From: "sipp" <sip:saghul@sip2sip.info>;tag=1
To: "sipp" <sip:saghul@sip2sip.info>
Call-ID: reg///1-1605@127.0.1.1
CSeq: 7 REGISTER
Contact: <sip:sipp@;rinstance=1234>
Expires: 3600
Content-Length: 0
User-Agent: SIPp

After this, if OpenSIPS is restarted, it fails to start with the following log:

DBG:core:parse_uri: bad host in uri (error at char ; in state 4) parsed: <sip:sipp@>(XX) /<sip:sipp@;rinstance=1234> (XX)
ERROR:usrloc:compute_next_hop: failed to parse URI of next hop: '                              '
ERROR:usrloc:new_ucontact: failed to resolve next hop
ERROR:usrloc:mem_insert_ucontact: failed to create new contact
ERROR:usrloc:preload_udomain: inserting contact failed

[razvancrainea]

Hi Saul!

Not sure whether the problem is that opensips accepts the bogus Contact header, or it does not start if the contact in the DB is bogus. Probably both?
The first problem can be solved by using the sipmsg_validate() function. Not sure if we should explicitly validate the contact before inserting it in the database.
However, the second problem seems more serious, since OpenSIPS does not start, no matter where that Contact header ended up in the database.

Will look into this next week.

Best regards,
Răzvan

[saghul]

Hi Razvan!

Thanks for the tip!

As for the fix, I think OpenSIPS should make sure the data is correct
before putting it on the DB, so that REGISTER should have been rejected.

Now, data might end up bogus on the DB due to bugs, so ignoring the broken
records on startup sounds like a sensible thing to do IMHO.
On Dec 18, 2015 6:09 PM, "Răzvan Crainea" notifications@github.com wrote:

Hi Saul!

Not sure whether the problem is that opensips accepts the bogus Contact
header, or it does not start if the contact in the DB is bogus. Probably
both?
The first problem can be solved by using the sipmsg_validate() function.
Not sure if we should explicitly validate the contact before inserting it
in the database.
However, the second problem seems more serious, since OpenSIPS does not
start, no matter where that Contact header ended up in the database.

Will look into this next week.

Best regards,
Răzvan

—
Reply to this email directly or view it on GitHub
#729 (comment).

[ionutrazvanionita]

Hi @saghul,

The only way I was able to save that contact in the DB was using fix_nated_register() function, the received contact attribute being set. This way the contact is not being checked when OpenSIPS starts again. If received not set, the contact is checked in compute_next_hop() (ucontact.c:59) using parse_uri(). In conclusion, the only way I am able to replicate this is calling fix_nated_register() when saving the contact and setting the received attribute to NULL after it has been inserted in the DB. Do you have other method to put only the contact from the OpenSIPS?

[saghul]

Hi @ionutrazvanionita,

You're right, I was using nat_traversal, so I guess the same applies.

[ionutrazvanionita]

Can you tell me what are you using from nat_traversal and/or nathelper modules?

[saghul]

Actually, I was wrong. We are using a bit of custom logic to set the received_uri AVP manually. So it is set.

[ionutrazvanionita]

ok i see. i will add a check in usrloc which will do parse_uri() for each contact added, such that invalid contacts won't be inserted, even though the received is ok.

[saghul]

👍

[ionutrazvanionita]

7f37f05 . Also, at startup, if a broken contact it's found, you receive an error which gives the aor, contact, received and contact_id (if trunk version used) such that you can delete it from the database without closing OpenSIPS.

[saghul]

Nice, thanks! I'll make a new build and test it out.

[saghul]

Tested it, works just fine, thanks @ionutrazvanionita!