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

[BUG] error_route not triggered for parsing error on first line of SIP message #2533

Closed
markallen01666 opened this issue May 28, 2021 · 6 comments

Comments

@markallen01666
Copy link

markallen01666 commented May 28, 2021

This is either a bug or a feature request:

We were seeing an issue with a corrupted SIP message (example below). To handle it we wanted to use error_route triggered by the parsing error, but because the error was with the first line of the message, error_route was not activated.

From https://github.com/OpenSIPS/opensips/blob/master/receive.c#L145 etc, it appears that this is a deliberate decision. The code reads:

if (parse_msg(in_buff.s,len, msg)!=0){
tmp=ip_addr2a(&(rcv_info->src_ip));
LM_ERR("Unable to parse msg received from [%s:%d]\n", tmp, rcv_info->src_port); /* if a REQUEST msg was detected (first line was successfully parsed) we should trigger the error route */
if ( msg->first_line.type==SIP_REQUEST && sroutes->error.a!=NULL ) {
if (existing_context == NULL) prepare_context( ctx, parse_error );
current_processing_ctx = ctx;
run_error_route(msg, 1);
}
goto parse_error;
}

However, this contradicts the documentation (https://www.opensips.org/Documentation/Script-Routes-3-1#toc5) which reads...

The error route is executed automatically when a parsing error occurs during SIP request processing

...there is no mention that if the error is with parsing the first line of the SIP message, error_route doesn't activate.

To resolve this I think that there are three possible solutions:

  • The test for first line being SIP should be removed (could the user choose to test for a non-SIP message using sipmsg_validate()?)
  • A parameter could be added to allow the coder to turn on triggering of error_route on failure of parsing the first line if they choose to
  • It could be left as it is, but documentation updated to make it clear that it doesn't work if the first line is corrupted

Here's an example. What we are seeing is this message which starts with part of a previous SIP message (due to dialer error I believe)...

Contact: <sip:1234 at abc.def.com:5060>
User-Agent: MWWRTC 3.4.21042
Accept: application/sdp,application/dtmf-relay,text/plain

...preceding...

SIP/2.0 200 OK
Via: SIP/2.0/TCP 192.168.1.23:5060;received=192.168.1.23;rport=42385;branch=z9hG4bK22a8.4fa6d127.0;i=b4986fe4
...etc...

...so OpenSIPS is failing to parse the message (the colon at the end of 'Contact:' seems to be the thing that breaks it).

My list post about this - http://lists.opensips.org/pipermail/users/2021-May/044779.html
Previous report of this issue - https://opensips.org/pipermail/users/2019-February/040605.html

Relevant log entries:

ERROR:core:parse_method: invalid character :
DBG:core:tcp_read_req: tcp_read_req end
INFO:core:parse_first_line: failed to parse the method
INFO:core:parse_first_line: bad message
DBG:core:parse_msg: invalid message
ERROR:core:parse_msg: message=<Contact:
<sip:1234 at abc.def.com:5060>#15#012User-Agent:
MWWRTC 3.4.21042#015#012Accept:
application/sdp,application/dtmf-relay,text/plain#015#012SIP/2.0 200
OK#015#012Via: SIP/2.0/TCP
192.168.1.23:5060;received=192.168.1.23;rport=42385;branch=z9hG4bK22a8.4fa6d127.0;i=b4986fe4#015#012Via:
SIP/2.0/WSS
98kaag0xmybq.invalid;received=4.56.78.110;branch=z9hG4bKU6O3fJQGeLvuACMTXTArJgJW73rOD5dU;rport=52570#015#012From:
<sip:1234 at abc.def.com>;tag=Lyk010G476K7xcKrE84M#015#012To: <
sip:1234 at abc.def.com>;tag=af78-6213d386c3edcd02707b0c0aa8423d3a#015#012Call-ID:
666b5e7c-cef3-f306-4b79-60d3160dc5d0#015#012CSeq: 28825
REGISTER#015#012Contact: <sips:1234 at 98kaag0xmybq.invalid
;transport=wss>;expires=60;received="sip:4.56.78.110:52570"#15#012Server:
OpenSIPS (3.1.1 (x86_64/linux))#15#012Content-Length: 0#015#012#015#012>
ERROR:core:receive_msg: Unable to parse msg received from [192.168.1.23:5060
]
ERROR:core:tcp_handle_req: receive_msg failed

@github-actions
Copy link

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

@github-actions github-actions bot added the stale label Jul 18, 2021
@bogdan-iancu
Copy link
Member

Hi @markallen01666 , thanks for the detailed report here. Now, the "is request" test is a quite a must as the error route is expected to be called for a request and not for a reply - especially because in the error route you may send back a SIP reply, which should be done only for a request. So, I say, it is mandatory to know, at least, if the msg you received is request or reply. This is why the code is like this.

@bogdan-iancu bogdan-iancu self-assigned this Jul 20, 2021
@stale stale bot removed the stale label Jul 20, 2021
@markallen01666
Copy link
Author

Thanks for getting back on this. In our case we did hit a problem when the first line was corrupted, and because we thought the SIP message was passing validation (not activating the error route) we did spend a chunk of time looking elsewhere for the problems we were seeing. Could the documentation be amended to make it clear that the error route is not executed if the problem is in the first line of the SIP message?

@github-actions
Copy link

github-actions bot commented Aug 5, 2021

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

@github-actions github-actions bot added the stale label Aug 5, 2021
@bogdan-iancu
Copy link
Member

@markallen01666 , I added a note on this, see https://www.opensips.org/Documentation/Script-Routes-3-3#toc5 . Thank you!

@markallen01666
Copy link
Author

markallen01666 commented Aug 13, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants