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

Reply via email to