[Asterisk-code-review] res pjsip pubsub: More accurately persist packet. (asterisk[certified/13.1])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_pjsip_pubsub: More accurately persist packet.
......................................................................


res_pjsip_pubsub: More accurately persist packet.

The pjsip_rx_data structure has a pkt_info.packet field on it that is
the packet that was read from the transport. For datagram transports,
the packet read from the transport will correspond to the SIP message
that arrived. For streamed transports, however, it is possible to read
multiple SIP messages in one packet.

In a recent case, Asterisk crashed on a system where TCP was being used.
This is because at some point, a read from the TCP socket resulted in a
200 OK response as well as an incoming SUBSCRIBE request being stored in
rdata->pkt_info.packet. When the SUBSCRIBE was processed, the
combination 200 OK and SUBSCRIBE was saved in persistent storage. Later,
a restart of Asterisk resulted in the crash because the persistent
subscription recreation code ended up building the 200 OK response
instead of a SUBSCRIBE request, and we attempted to access
request-specific data.

The fix here is to use the pjsip_msg_print() function in order to
persist SUBSCRIBE requests. This way, rather than using the raw socket
data, we use the parsed SIP message that PJSIP has given us. If we
receive multiple SIP messages from a single read, we will be sure only
to save off the relevant SIP message. There also is a safeguard put in
place to make sure that if we do end up reconstructing a SIP response,
it will not cause a crash.

ASTERISK-25306 #close
Reported by Mark Michelson

Change-Id: I4bf16f7b76a2541d10b55de82bcd14c6e542afb2
---
M res/res_pjsip_pubsub.c
1 file changed, 8 insertions(+), 1 deletion(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Ashley Sanders: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index ff688eb..3eb0eeb 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -558,7 +558,7 @@
 		expires = expires_hdr ? expires_hdr->ivalue : DEFAULT_PUBLISH_EXPIRES;
 		sub_tree->persistence->expires = ast_tvadd(ast_tvnow(), ast_samp2tv(expires, 1));
 
-		ast_copy_string(sub_tree->persistence->packet, rdata->pkt_info.packet,
+		pjsip_msg_print(rdata->msg_info.msg, sub_tree->persistence->packet,
 				sizeof(sub_tree->persistence->packet));
 		ast_copy_string(sub_tree->persistence->src_name, rdata->pkt_info.src_name,
 				sizeof(sub_tree->persistence->src_name));
@@ -1333,6 +1333,13 @@
 		return 0;
 	}
 
+	if (rdata.msg_info.msg->type != PJSIP_REQUEST_MSG) {
+		ast_log(LOG_NOTICE, "Endpoint %s persisted a SIP response instead of a subscribe request. Unable to reload subscription.\n",
+				ast_sorcery_object_get_id(endpoint));
+		ast_sorcery_delete(ast_sip_get_sorcery(), persistence);
+		return 0;
+	}
+
 	request_uri = pjsip_uri_get_uri(rdata.msg_info.msg->line.req.uri);
 	resource_size = pj_strlen(&request_uri->user) + 1;
 	resource = alloca(resource_size);

-- 
To view, visit https://gerrit.asterisk.org/1045
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4bf16f7b76a2541d10b55de82bcd14c6e542afb2
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.1
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>