Author: rmudgett Date: Wed Feb 17 14:21:18 2016 New Revision: 2337 URL: http://svnview.digium.com/svn/libpri?view=rev&rev=2337 Log: q931.c: Tighten mandatory ie checks.
Libpri was lax in checking if a missing channel identification ie is mandatory for the SETUP ACKNOWLEDGE, PROCEEDING, ALERTING, and CONNECT messages. That ie is mandatory when those messages are the first response to a SETUP message sent by the CPE side. * Made those messages check if a missing channel identification ie is mandatory and send a STATUS with cause 96 "Mandatory information element is missing" in response. Libpri did not care if a mandatory ie had a coding error. * Made coding errors in mandatory ie's send a STATUS with cause 100 "Invalid information element contents" in response. * Fixed detection of coding errors in channel identification ie. SWP-8721 SWP-8722 Modified: branches/1.4/pri_internal.h branches/1.4/q931.c Modified: branches/1.4/pri_internal.h URL: http://svnview.digium.com/svn/libpri/branches/1.4/pri_internal.h?view=diff&rev=2337&r1=2336&r2=2337 ============================================================================== --- branches/1.4/pri_internal.h (original) +++ branches/1.4/pri_internal.h Wed Feb 17 14:21:18 2016 @@ -647,10 +647,13 @@ unsigned char char_set; } display; - /* AOC charge requesting on Setup */ + /*! AOC charge requesting on Setup */ int aoc_charging_request; - unsigned int slotmap_size:1;/* TRUE if the slotmap is E1 (32 bits). */ + /*! TRUE if the slotmap is E1 (32 bits). */ + unsigned int slotmap_size:1; + /*! TRUE if need to see the channel id ie in first response to SETUP. */ + unsigned int channel_id_ie_mandatory:1; /*! Control the RESTART reception to the upper layer. */ struct { Modified: branches/1.4/q931.c URL: http://svnview.digium.com/svn/libpri/branches/1.4/q931.c?view=diff&rev=2337&r1=2336&r2=2337 ============================================================================== --- branches/1.4/q931.c (original) +++ branches/1.4/q931.c Wed Feb 17 14:21:18 2016 @@ -40,6 +40,12 @@ #include <stdio.h> #include <limits.h> +enum mandatory_ie_status { + MAND_STATUS_OK, + MAND_STATUS_CONTENT_ERROR, + MAND_STATUS_MISSING, +}; + #define MAX_MAND_IES 10 struct msgtype { @@ -50,13 +56,27 @@ static struct msgtype msgs[] = { /* Call establishment messages */ - { Q931_ALERTING, "ALERTING" }, - { Q931_CALL_PROCEEDING, "CALL PROCEEDING" }, - { Q931_CONNECT, "CONNECT" }, + { Q931_ALERTING, "ALERTING", { Q931_CHANNEL_IDENT } }, + { Q931_CALL_PROCEEDING, "CALL PROCEEDING", { Q931_CHANNEL_IDENT } }, + { Q931_CONNECT, "CONNECT", { Q931_CHANNEL_IDENT } }, { Q931_CONNECT_ACKNOWLEDGE, "CONNECT ACKNOWLEDGE" }, + /* + * Very early in libpri SVN history (SVN -r154) a change was explicitly + * made to not care about a missing Progress Indicator ie. There is no + * explanation in the commit message why this was done even though + * Q.931 and ECMA-143 clearly show that the ie is mandatory for a + * PROGRESS message. + * + * Hazzarding a guess, I think it was because of the + * ALERTING_NO_PROGRESS support for Mitel switches. (SVN -r44) + */ +#ifdef ALERTING_NO_PROGRESS + { Q931_PROGRESS, "PROGRESS" }, +#else { Q931_PROGRESS, "PROGRESS", { Q931_PROGRESS_INDICATOR } }, +#endif { Q931_SETUP, "SETUP", { Q931_BEARER_CAPABILITY, Q931_CHANNEL_IDENT } }, - { Q931_SETUP_ACKNOWLEDGE, "SETUP ACKNOWLEDGE" }, + { Q931_SETUP_ACKNOWLEDGE, "SETUP ACKNOWLEDGE", { Q931_CHANNEL_IDENT } }, /* Call disestablishment messages */ { Q931_DISCONNECT, "DISCONNECT", { Q931_CAUSE } }, @@ -93,7 +113,7 @@ { Q931_ANY_MESSAGE, "ANY MESSAGE" }, }; -static int post_handle_q931_message(struct pri *ctrl, struct q931_mh *mh, struct q931_call *c, int missingmand); +static int post_handle_q931_message(struct pri *ctrl, struct q931_mh *mh, struct q931_call *c, enum mandatory_ie_status mand_status); static void nt_ptmp_handle_q931_message(struct pri *ctrl, struct q931_mh *mh, struct q931_call *c, int *allow_event, int *allow_posthandle); struct msgtype att_maintenance_msgs[] = { @@ -1258,6 +1278,14 @@ int pos = 0; int need_extended_channel_octets;/*!< TRUE if octets 3.2 and 3.3 need to be present. */ + call->channel_id_ie_mandatory = 0; + + if (ie->len < 1) { + /* Must have at least one payload octet. */ + pri_error(ctrl, "!! Channel Identification ie too short\n"); + return -1; + } + call->restart.count = 0; if (ie->data[0] & 0x08) { @@ -1308,10 +1336,24 @@ pos++; if (ie->data[0] & 0x40) { - /* DS1 specified -- stop here */ - call->ds1no = ie->data[1] & 0x7f; + /* Explicitly defined DS1 (One or more 3.1 octets expected) */ + if (ctrl->switchtype == PRI_SWITCH_QSIG) { + pri_error(ctrl, "!! Q.SIG does not support Channel Identification octet 3.1\n"); + return -1; + } + if (ie->len <= pos) { + pri_error(ctrl, "!! Missing expected Channel Identification octet 3.1\n"); + return -1; + } + + /* Extract explicit DS1 reference. */ + call->ds1no = ie->data[pos] & 0x7f; call->ds1explicit = 1; - pos++; + + /* Advance to 3.2 octet if present. */ + do { + ++pos; + } while (pos < ie->len && !(ie->data[pos - 1] & 0x80)); } else { call->ds1explicit = 0; } @@ -1324,8 +1366,12 @@ return 0; } - if (need_extended_channel_octets && pos + 2 < len) { - /* More coming */ + if (need_extended_channel_octets && pos + 2 <= ie->len) { + /* + * Octet 3.2 and at least one octet 3.3 are present. + * + * Process octet 3.2 + */ if ((ie->data[pos] & 0x0f) != 3) { /* Channel type/mapping is not for B channel units. */ pri_error(ctrl, "!! Unexpected Channel Type %d\n", ie->data[pos] & 0x0f); @@ -1335,14 +1381,25 @@ pri_error(ctrl, "!! Invalid CCITT coding %d\n", (ie->data[pos] & 0x60) >> 5); return -1; } - if (ie->data[pos] & 0x10) { - /* Expect Slot Map */ + if (ie->data[pos++] & 0x10) { + /* Process 3.3 octets as a Slot Map */ call->slotmap = 0; - pos++; call->slotmap_size = (ie->len - pos > 3) ? 1 : 0; - for (x = 0; x < (call->slotmap_size ? 4 : 3); ++x) { + + /* + * We will be tolerant of partial slot maps that do not + * specify the upper channels. We will assume they + * are not requested. + * + * We will only read up to 24 or 32 bit channel maps. + */ + for (x = call->slotmap_size ? 4 : 3; x--;) { call->slotmap <<= 8; - call->slotmap |= ie->data[x + pos]; + call->slotmap |= ie->data[pos++]; + if (ie->len <= pos) { + /* No more ie contents. */ + break; + } } if (msgtype == Q931_RESTART) { @@ -1357,8 +1414,11 @@ } } } else { - pos++; - /* Only expect a particular channel */ + /* + * Process 3.3 octets as either a single channel or a channel list + * + * Get single channel + */ call->channelno = ie->data[pos] & 0x7f; if (ctrl->chan_mapping_logical && call->channelno > 15) { call->channelno++; @@ -1385,6 +1445,9 @@ } } } + } else if (need_extended_channel_octets) { + pri_error(ctrl, "!! Missing expected Channel Identification octets 3.2 or 3.3\n"); + return -1; } return 0; } @@ -1518,21 +1581,29 @@ (ie->data[0] & 0x08) ? "Exclusive" : "Preferred", (ie->data[0] & 0x04) ? 1 : 0); pri_message(ctrl, "%c ChanSel: %s\n", - prefix, msg_chan_sel[(ie->data[0] & 0x03) | ((ie->data[0] >> 3) & 0x04)]); + prefix, msg_chan_sel[(ie->data[0] & 0x03) | ((ie->data[0] & 0x20) >> 3)]); pos = 1; len -= 2; if (ie->data[0] & 0x40) { - /* Explicitly defined DS1 */ - do { - pri_message(ctrl, "%c Ext: %d DS1 Identifier: %d \n", - prefix, (ie->data[pos] & 0x80) >> 7, ie->data[pos] & 0x7f); - ++pos; - } while (!(ie->data[pos - 1] & 0x80) && pos < len); + /* Explicitly defined DS1 (One or more 3.1 octets expected) */ + if (ctrl->switchtype == PRI_SWITCH_QSIG) { + pri_message(ctrl, "%c Octet 3.1 specified when Q.SIG!\n", + prefix); + } + if (pos < len) { + do { + pri_message(ctrl, "%c Ext: %d DS1 Identifier: %d\n", + prefix, (ie->data[pos] & 0x80) >> 7, ie->data[pos] & 0x7f); + ++pos; + } while (!(ie->data[pos - 1] & 0x80) && pos < len); + } else { + pri_message(ctrl, "%c Octet 3.1 is missing!\n", prefix); + } } else { /* Implicitly defined DS1 */ } if (pos < len) { - /* Still more information here */ + /* Octet 3.2 present */ pri_message(ctrl, "%c Ext: %d Coding: %d %s Specified Channel Type: %d\n", prefix, (ie->data[pos] & 0x80) >> 7, (ie->data[pos] & 60) >> 5, @@ -1540,6 +1611,7 @@ ++pos; } if (pos < len) { + /* One or more 3.3 octets present */ if (!(ie->data[pos - 1] & 0x10)) { /* Number specified */ do { @@ -2864,7 +2936,7 @@ break; default: pri_error(ctrl, "XXX Invalid Progress indicator value received: %02x\n",(ie->data[1] & 0x7f)); - break; + return -1; } return 0; } @@ -6316,6 +6388,9 @@ c->chanflags = FLAG_PREFERRED; } } + if (ctrl->localtype == PRI_CPE) { + c->channel_id_ie_mandatory = 1; + } c->slotmap = -1; c->nonisdn = req->nonisdn; @@ -7510,12 +7585,13 @@ int res; int r; int mandies[MAX_MAND_IES]; - int missingmand; + int is_mandatory; int codeset, cur_codeset; int last_ie[8]; int cref; int allow_event; int allow_posthandle; + enum mandatory_ie_status mand_status; ctrl = link->ctrl; memset(last_ie, 0, sizeof(last_ie)); @@ -7595,7 +7671,7 @@ q931_clr_subcommands(ctrl); q931_display_clear(c); - /* Handle IEs */ + /* Determine which ies are mandatory for this message. */ memset(mandies, 0, sizeof(mandies)); for (x = 0; x < ARRAY_LEN(msgs); ++x) { if (msgs[x].msgnum == mh->msg) { @@ -7603,6 +7679,25 @@ break; } } + for (x = 0; x < ARRAY_LEN(mandies); ++x) { + if (mandies[x]) { + /* Check mandatory channel identification ie exceptions */ + if (mandies[x] != Q931_CHANNEL_IDENT + /* Always mandatory for RESUME_ACKNOWLEDGE */ + || mh->msg == Q931_RESUME_ACKNOWLEDGE + /* Mandatory in Net -> CPE direction for SETUP */ + || (mh->msg == Q931_SETUP && ctrl->localtype == PRI_CPE) + /* Mandatory for first SETUP response message in Net -> CPE direction. */ + || c->channel_id_ie_mandatory) { + /* ie is mandatory for this message */ + continue; + } + /* ie is not mandatory for this message */ + mandies[x] = 0; + } + } + mand_status = MAND_STATUS_OK; + /* Do real IE processing */ len -= (h->crlen + 3); codeset = cur_codeset = 0; @@ -7619,10 +7714,16 @@ q931_display_clear(c); return -1; } + + /* Check if processing a mandatory ie. */ + is_mandatory = 0; for (y = 0; y < ARRAY_LEN(mandies); ++y) { - if (mandies[y] == Q931_FULL_IE(cur_codeset, ie->ie)) + if (mandies[y] == Q931_FULL_IE(cur_codeset, ie->ie)) { mandies[y] = 0; - } + is_mandatory = 1; + } + } + /* Special processing for codeset shifts */ switch (ie->ie & 0xf8) { case Q931_LOCKING_SHIFT: @@ -7665,6 +7766,15 @@ /* Fall through */ default: y = q931_handle_ie(cur_codeset, ctrl, c, mh->msg, ie); + if (y < 0 && is_mandatory) { + /* Error processing mandatory ie. */ + mand_status = MAND_STATUS_CONTENT_ERROR; + pri_error(ctrl, "%s: Error in mandatory IE %d (cs%d, %s)\n", + msg2str(mh->msg), + ie->ie, + cur_codeset, + ie2str(Q931_FULL_IE(codeset, ie->ie))); + } /* XXX Applicable to codeset 0 only? XXX */ if (!cur_codeset && !(ie->ie & 0xf0) && (y < 0)) { /* @@ -7681,19 +7791,21 @@ break; } } - missingmand = 0; + + /* Check for missing mandatory ies. */ for (x = 0; x < ARRAY_LEN(mandies); ++x) { if (mandies[x]) { - /* check if there is no channel identification when we're configured as network -> that's not an error */ - if (((ctrl->localtype != PRI_NETWORK) || (mh->msg != Q931_SETUP) || (mandies[x] != Q931_CHANNEL_IDENT)) && - ((mh->msg != Q931_PROGRESS) || (mandies[x] != Q931_PROGRESS_INDICATOR))) { - pri_error(ctrl, "XXX Missing handling for mandatory IE %d (cs%d, %s) XXX\n", Q931_IE_IE(mandies[x]), Q931_IE_CODESET(mandies[x]), ie2str(mandies[x])); - missingmand++; - } - } - } - - if (!missingmand) { + /* This mandatory ie was not processed. */ + mand_status = MAND_STATUS_MISSING; + pri_error(ctrl, "%s: Missing mandatory IE %d (cs%d, %s)\n", + msg2str(mh->msg), + Q931_IE_IE(mandies[x]), + Q931_IE_CODESET(mandies[x]), + ie2str(mandies[x])); + } + } + + if (mand_status == MAND_STATUS_OK) { switch (mh->msg) { case Q931_SETUP: case Q931_CONNECT: @@ -7729,7 +7841,7 @@ } if (allow_posthandle) { - res = post_handle_q931_message(ctrl, mh, c, missingmand); + res = post_handle_q931_message(ctrl, mh, c, mand_status); if (res == Q931_RES_HAVEEVENT && !allow_event) { res = 0; } @@ -8599,7 +8711,7 @@ * \param ctrl D channel controller. * \param mh Q.931 message header. * \param c Q.931 call leg. - * \param missingmand Number of missing mandatory ie's. + * \param mand_status Mandatory ie status * * \note * When this function returns c may be destroyed so you can no @@ -8609,19 +8721,32 @@ * \retval Q931_RES_HAVEEVENT if have an event. * \retval -1 on error. */ -static int post_handle_q931_message(struct pri *ctrl, struct q931_mh *mh, struct q931_call *c, int missingmand) +static int post_handle_q931_message(struct pri *ctrl, struct q931_mh *mh, struct q931_call *c, enum mandatory_ie_status mand_status) { int res; int changed; + int mand_cause; struct apdu_event *cur = NULL; struct pri_subcommand *subcmd; struct q931_call *master_call; + mand_cause = 0; + switch (mand_status) { + case MAND_STATUS_OK: + break; + case MAND_STATUS_CONTENT_ERROR: + mand_cause = PRI_CAUSE_INVALID_IE_CONTENTS; + break; + case MAND_STATUS_MISSING: + mand_cause = PRI_CAUSE_MANDATORY_IE_MISSING; + break; + } + switch(mh->msg) { case Q931_RESTART: q931_display_subcmd(ctrl, c); - if (missingmand) { - q931_status(ctrl, c, PRI_CAUSE_MANDATORY_IE_MISSING); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); pri_destroycall(ctrl, c); break; } @@ -8679,9 +8804,8 @@ return Q931_RES_HAVEEVENT; case Q931_SETUP: q931_display_subcmd(ctrl, c); - - if (missingmand) { - q931_release_complete(ctrl, c, PRI_CAUSE_MANDATORY_IE_MISSING); + if (mand_status != MAND_STATUS_OK) { + q931_release_complete(ctrl, c, mand_cause); break; } /* Must be new call */ @@ -8724,6 +8848,10 @@ return Q931_RES_HAVEEVENT; case Q931_ALERTING: q931_display_subcmd(ctrl, c); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); + break; + } stop_t303(c->master_call); if (c->newcall) { q931_release_complete(ctrl, c, newcall_rel_comp_cause(c)); @@ -8769,6 +8897,10 @@ return Q931_RES_HAVEEVENT; case Q931_CONNECT: q931_display_subcmd(ctrl, c); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); + break; + } stop_t303(c->master_call); if (c->newcall) { q931_release_complete(ctrl, c, newcall_rel_comp_cause(c)); @@ -8847,16 +8979,15 @@ } break; case Q931_PROGRESS: - if (missingmand) { - q931_status(ctrl, c, PRI_CAUSE_MANDATORY_IE_MISSING); - pri_destroycall(ctrl, c); - break; - } ctrl->ev.e = PRI_EVENT_PROGRESS; ctrl->ev.proceeding.cause = c->cause; /* Fall through */ case Q931_CALL_PROCEEDING: q931_display_subcmd(ctrl, c); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); + break; + } stop_t303(c->master_call); ctrl->ev.proceeding.subcmds = &ctrl->subcmds; if (c->newcall) { @@ -8926,9 +9057,8 @@ break; case Q931_STATUS: q931_display_subcmd(ctrl, c); - if (missingmand) { - q931_status(ctrl, c, PRI_CAUSE_MANDATORY_IE_MISSING); - pri_destroycall(ctrl, c); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); break; } if (c->newcall) { @@ -9034,9 +9164,9 @@ case Q931_RELEASE: q931_display_subcmd(ctrl, c); c->hangupinitiated = 1; - if (missingmand) { - /* Force cause to be mandatory IE missing */ - c->cause = PRI_CAUSE_MANDATORY_IE_MISSING; + if (mand_status != MAND_STATUS_OK) { + /* Force mandatory ie cause */ + c->cause = mand_cause; } /* @@ -9080,9 +9210,9 @@ case Q931_DISCONNECT: q931_display_subcmd(ctrl, c); c->hangupinitiated = 1; - if (missingmand) { + if (mand_status != MAND_STATUS_OK) { /* Still let user call release */ - c->cause = PRI_CAUSE_MANDATORY_IE_MISSING; + c->cause = mand_cause; } if (c->newcall) { q931_release_complete(ctrl, c, newcall_rel_comp_cause(c)); @@ -9242,6 +9372,10 @@ break; case Q931_SETUP_ACKNOWLEDGE: q931_display_subcmd(ctrl, c); + if (mand_status != MAND_STATUS_OK) { + q931_status(ctrl, c, mand_cause); + break; + } stop_t303(c->master_call); if (c->newcall) { q931_release_complete(ctrl, c, newcall_rel_comp_cause(c)); @@ -9467,9 +9601,9 @@ master_call = c->master_call; switch (master_call->hold_state) { case Q931_HOLD_STATE_HOLD_REQ: - if (missingmand) { + if (mand_status != MAND_STATUS_OK) { /* Still, let hold rejection continue. */ - c->cause = PRI_CAUSE_MANDATORY_IE_MISSING; + c->cause = mand_cause; } ctrl->ev.e = PRI_EVENT_HOLD_REJ; ctrl->ev.hold_rej.channel = q931_encode_channel(c); @@ -9591,9 +9725,9 @@ pri_schedule_del(ctrl, master_call->hold_timer); master_call->hold_timer = 0; - if (missingmand) { + if (mand_status != MAND_STATUS_OK) { /* Still, let retrive rejection continue. */ - c->cause = PRI_CAUSE_MANDATORY_IE_MISSING; + c->cause = mand_cause; } ctrl->ev.e = PRI_EVENT_RETRIEVE_REJ; ctrl->ev.retrieve_rej.channel = q931_encode_channel(c); -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits