[svn-commits] rmudgett: branch 1.4 r2337 - in /branches/1.4: pri_internal.h q931.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Wed Feb 17 14:21:23 CST 2016
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);
More information about the svn-commits
mailing list