[Home]

Summary:ASTERISK-27342: [patch] libpri: Calling Party Subaddress extended octet 3 encoding incorrectly handled
Reporter:Marin Odrljin (modrljin)Labels:patch
Date Opened:2017-10-13 10:58:35Date Closed:2019-05-07 03:07:27
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_dahdi
Versions:13.11.2 Frequency of
Occurrence		Constant
Related
Issues:
Environment:Debian 8Attachments:( 0) debug_log_subaddr-nok.txt
( 1) debug_log_subaddr-nok1.txt
Description:There is a problem with starting Stasis application for incoming calls via PRI interface when there is Subaddress IE in Q.931 SETUP message.

I'm not sure if the problem is in libpri, dahdi or asterisk source.

I have attached a debug log 'debug_log_subaddr-nok.txt'.

Please, check rows 19 and 20 where is 'Calling Party Subaddress'. If you check Q.931 specification [Q.931|https://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-Q.931-199805-I!!PDF-E&type=items] page 71-72 (4.5.11), you'll see how octet 4 should be decoded. Spec says if type of subaddress (octet 3) is NSAP ITU-T X.213 and ISO/IEC 8348, address shall be formatted as specified by octet 4 which contains Authority and Format Identifier (AFI). When you check octet 3 from my debug log [6d 0c 01 83 30 37 38 36 31 34 34 33 34 31] it is 01 what means it is NSAP type so octet 4 must be skipped in address information (or better to say treated differently), but unfortunately this is not a case in provided example. You can see that byte 83 is taken into subaddress information and result is decoded address 'ƒ0786144341'. I believe this is the root of errors that are happening later in code when building JSON in stasis_channels.c, json.c, res_stasis.c and other classes. The error message is quite logical 'Error building JSON from {s: ...} Invalid UTF-8 string'. Since JSON can't be created and it is needed for Stasis application, call never goes there (row 60-64 from debug log: 'res_stasis.c:159 stasis_start_to_json: Failed to pack JSON for StasisStart message') and it is disconnected after.

The same problem probably exists with SETUP IE 'Called Party Subaddress' (Q.931 spec 4.5.9, page 68-69) and also in CONNECT IE 'Connected Subaddress'.

I have checked class [q931.c|https://github.com/asterisk/libpri/blob/master/q931.c] in libpri and saw that in the function 'receive_subaddr_helper' which is called by 'receive_calling_party_subaddr' and 2 others, all bytes from octet 4 are always copied into q931_subbaddres->data. It would be easy to skip octet 4 if type is NSAP, but I'm not sure that this is correct place to make changes because this one byte is then lost in structure and in the case of transmitting to other channel it will bring problems. Maybe this can be prevented if we add new field  'char afi' into 'q931_party_subaddress' struct and when type is NSAP to remove first byte from 'data' into it.

If this is not the solution, then this byte must be removed when data is copied into asterisk objects when building json for Stasis, but I have no idea where is this and that's why not sure where the correction should be done, in libpri or somehere in asterisk.
Comments:By: Asterisk Team (asteriskteam) 2017-10-13 10:58:36.039-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].
By: Richard Mudgett (rmudgett) 2017-10-13 13:45:21.065-0500

{noformat}
[Oct 13 08:33:59] VERBOSE[43961] chan_dahdi.c: PRI Span: 1 < [6d 0c 01 83 30 37 38 36 31 34 34 33 34 31]
[Oct 13 08:33:59] VERBOSE[43961] chan_dahdi.c: PRI Span: 1 < Calling Party Subaddress (len=14) [ Ext: 0  Type: NSAP (X.213/ISO 8348 AD2) (0)  O: 0  '�0786144341' ]
{noformat}

6d Octet 1 (ie identifier) Calling party subaddress
0c Octet 2 (ie length) 12 more octets
01 Octet 3 (ext=0 (Octet 3 is extended to the next octet and is not defined by Q.931), Type=0-NSAP, Odd/even=0-Even Spare=1 (Q.931 does not define these bits and they are supposed to be zero))
83 Octet 3a (Q.931 does not define this octet)
30 First octet 4

The switch you are attached to is sending the Calling Party Subaddress with an encoding not defined in Q.931 but using the defined extension mechanism.  You should be able to alter the q931.c:receive_subaddr_helper() routine to check the ext bit in octet 3 and discard the extra octet 3 since Q.931 doesn't know how to interpret the extended octet.
By: Richard Mudgett (rmudgett) 2017-10-13 14:07:58.918-0500

q931.c:dump_subaddr_helper() would also need to be updated so the libpri Q.931 level message decode displays correctly.
By: Marin Odrljin (modrljin) 2017-10-16 03:09:51.292-0500

And I don't need to store this extra octet somewhere? What will happen if this subaddress IE has to be forwarded to a new call (I'm not so famliliar with libpri so I'm just guessing)? Without this octet data it is not the same anymore and in q931.c:transmit_subaddr_helper() it won't included.
By: Richard Mudgett (rmudgett) 2017-10-16 11:22:23.497-0500

Storing that extra octet is useless as you have nowhere to pass that information.  Asterisk and libpri don't know what to do with it and cannot pass it to you either.  The switch is sending something using a standard (or proprietary extension) that libpri knows nothing about.  I don't expect sending that subaddress out in a new call without the extended octet to be a problem or anything to worry about.

What switch type are you using?
By: Marin Odrljin (modrljin) 2017-10-16 13:46:54.741-0500

This call is coming from Swisscom PSTN to our Aculab Groomer II router and then transparently forwarded to Asterisk. This is the case of calls for paid lines, 0900 business numbers and Swiss Tariff Change calls. Basically we are making our own solution to replace Groomer II as a router and that's why we would like to have a possibility to have complete control. Probably it could be usefull to forward all octets if it is needed.
By: Marin Odrljin (modrljin) 2017-10-20 04:35:47.455-0500

Hi Richard, we have managed to change those two functions and get it working properly. Can I just attach the patch here for your initial check or we have to do it through git / gerrit (in this case we'll do it somewhere next week because we have never did it before and now don't have time for it)?
By: Richard Mudgett (rmudgett) 2017-10-20 11:12:35.946-0500

You can attach patches to the issue as a contribution [1] after signing the license agreement if you have not already signed it.  However, it is faster to get a patch reviewed [2] and into the project if you go through gerrit.  Otherwise, you have to wait for someone else to come along to push it through the process.

[1] https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process
[2] https://wiki.asterisk.org/wiki/display/AST/Code+Review
By: Bruno Babic (bbabic) 2017-10-23 04:24:42.095-0500

I've submitted patch for review to fix this issue.
https://gerrit.asterisk.org/6867 Patch for issue ASTERISK-27342
By: Marin Odrljin (modrljin) 2017-10-23 11:12:35.836-0500

Everything seemed fine to me after we have created a patch, but now I have noticed that first character is cut off from the subaddress while sending it to Stasis app.
Caller subbadress '0786144341' is forwarded as '786144341'. Now the question is why since in q931.c it is decoded fine what is visible in 'debug_log_subaddr-nok1.txt'.
By: Richard Mudgett (rmudgett) 2017-10-23 13:03:23.706-0500

I've updated the patch.  It should take care of the missing subaddress digit in the patch.
By: Marin Odrljin (modrljin) 2017-10-24 05:26:28.609-0500

Bruno and me have tested it with your patch update and now it works fine. Thanks.
By: Marin Odrljin (modrljin) 2019-05-07 03:07:27.931-0500

Provided patch has fixed the issue