[Asterisk-code-review] res pjsip session: Fix in-dialog authentication. (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res_pjsip_session: Fix in-dialog authentication.
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/535/1/res/res_pjsip/pjsip_outbound_auth.c
File res/res_pjsip/pjsip_outbound_auth.c:

Line 65: 	if (tsx->method.id != PJSIP_INVITE_METHOD) {
       : 		/* Not an INVITE that needs authentication */
       : 		return PJ_FALSE;
       : 	}
       : 
       : 	inv = pjsip_dlg_get_inv_session(dlg);
       : 	if (PJSIP_INV_STATE_CONFIRMED <= inv->state) {
       : 		/*
       : 		 * We cannot handle reINVITE authentication at this
       : 		 * time because the reINVITE transaction is still in
       : 		 * progress.  Authentication will get handled by the
       : 		 * session state change callback.
       : 		 */
       : 		ast_debug(1, "A reINVITE is being challenged.\n");
       : 		return PJ_FALSE;
       : 	}
       : 	ast_debug(1, "Initial INVITE is being challenged.\n");
> The code previously had nothing special - ie: it was not INVITE session spe
I've mostly answered this question in the commit message and the comments added in this routine.  There are several reasons this "generic" code didn't work.

1) The outbound_auth_mod module is only inserted into the transaction stack by res_pjsip_session.c for the outgoing call dialog so nothing else would execute it.

2) This callback code is only setup on outgoing calls.  When I initially tried to set it up for incoming calls it simply caused a crash.  For some reason the dlg pointer was garbage.

3) Re-INVITEs cannot be handled here because the re-INVITE transaction has not completed and the pjsip stack cannot handle the overlapping INVITE transactions.  The pjsip stack explicitly checks for this situation and refuses to send the authenticated re-INVITE.

4) Authentication handling is easily processed in session_inv_on_tsx_state_changed() for both incoming and outgoing calls for everything except the initial INVITE transaction.  Handling the non-INVITE methods here would result in duplicate handling in the outgoing call case.

5) This is where the initial INVITE authentication needs to happen because you need to restart the transaction and the pjsip stack has pjsip_inv_uac_restart() expressly for this purpose.  Handling the initial INVITE authentication in session_inv_on_tsx_state_changed() is probably too late and allowing the transaction to terminate is likely to tear down everyting else associated with the call.

I would like to pull this module into res_pjsip_session.c and eliminate this file in res_pjsip.

Hmm.  Maybe I should revert the changes to this file and pull the needed guts of this file into res_pjsip_session.c.  Then in master I could just delete this file and the associated prototypes in res_pjsip.h.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12bdd7ddccc819b4ce4b091e826d1e26334601b0
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes