harshsxn / jss7

Automatically exported from code.google.com/p/jss7
0 stars 2 forks source link

CAP InitialDP empty (zero length) IMSI #312

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Some equipment vendors send badly encoded InitialDP with empty (zero length) 
IMSI field. This contradicts the specification.

But we need to accept such messages and fix it. Accepted IMSI will be "null" if 
a field has zero length.

Original issue reported on code.google.com by serg.vet...@gmail.com on 2 Aug 2013 at 4:50

GoogleCodeExporter commented 9 years ago
Fixed now:

https://code.google.com/p/jss7/source/detail?r=a4d7e0dad398124e4c62424baadd5318a
48e17e5

Original comment by serg.vet...@gmail.com on 2 Aug 2013 at 6:21

GoogleCodeExporter commented 9 years ago
Assuming this is BER, zero length is explicitly permitted by X.690 in rules for 
length encoding
"""
8.1.3.4 In the short form, the length octets shall consist of a single octet in 
which bit 8 is zero and bits 7 to 1 encode the number of octets in the contents 
octets (which may be zero)
"""
and rules for OCTET STRING encoding
"""
8.7.2 The primitive encoding contains zero, one or more contents octets equal 
in value to the octets in the data value

"""

I'm not sure I correctly understand the issue you were solving without a trace, 
but it might be better to have generic support for zero length in jASN rather 
than consider that a specific workaround for a single case.

Original comment by vil...@users.sourceforge.net on 5 Aug 2013 at 7:46

GoogleCodeExporter commented 9 years ago
The problem is:
IMSI field is coded in InitialDP message as follow:
9f 32 00 
This is [50] tag (IMSI) and zero length
But IMSI generally must have several digits in it's content

Original comment by serg.vet...@gmail.com on 5 Aug 2013 at 5:30

GoogleCodeExporter commented 9 years ago
I see, so my previous comment seems relevant. Even though, such encoding of 
IMSI is not very useful (it does not carry the actual IMSI value) I just wanted 
to bring the fact that it is actually valid BER encoding to your attention.

So my suggestion was that it might be better to make a fix for such cases (zero 
length) in jASN instead of an isolated length check of a single field of CAP 
IDP. Otherwise, the same problem may occur with other messages/fields again.

Original comment by vil...@users.sourceforge.net on 6 Aug 2013 at 2:25

GoogleCodeExporter commented 9 years ago
Hello!

> I just wanted to bring the fact that it is actually valid BER encoding to 
your attention.
I agree with you that it is ok from BER encoding point of view.
But it contradicts MAP specification that does not allow zero length IMSI field.

> So my suggestion was that it might be better 
> to make a fix for such cases (zero length) in jASN instead of an isolated 
length check 
> of a single field of CAP IDP
Sorry, what do you exactly suggest? I am not sure that it is possible to fix at 
jasn level. Some fields can has zero length, some can not. How jasn can sort it?

Original comment by serg.vet...@gmail.com on 6 Aug 2013 at 7:28

GoogleCodeExporter commented 9 years ago
I assumed jASN takes care of decoding base types, not only reading the bytes 
(from experience with other ASN.1 tools) without actually looking at the code. 
Sorry for muddying the waters.

Having looked at the code now, though :)
http://code.google.com/p/jss7/source/browse/cap/cap-impl/src/main/java/org/mobic
ents/protocols/ss7/cap/service/circuitSwitchedCall/InitialDPRequestImpl.java#610
does not seem to do anything useful.

This would be more readable and save a method call.

diff --git 
a/cap/cap-impl/src/main/java/org/mobicents/protocols/ss7/cap/service/circuitSwit
chedCall/InitialDPRequestImpl.java 
b/cap/cap-impl/src/main/java/org/mobicents/protocols/ss7/cap/service/circuitSwit
chedCall/InitialDPRequestImpl.java
index 93e9d8d..4f0b410 100644
--- 
a/cap/cap-impl/src/main/java/org/mobicents/protocols/ss7/cap/service/circuitSwit
chedCall/InitialDPRequestImpl.java
+++ 
b/cap/cap-impl/src/main/java/org/mobicents/protocols/ss7/cap/service/circuitSwit
chedCall/InitialDPRequestImpl.java
@@ -606,9 +606,7 @@ public class InitialDPRequestImpl extends 
CircuitSwitchedCallMessageImpl impleme
                         break;
                     case _ID_iMSI:
                         int len = ais.readLength();
-                        if (len == 0) {
-                            ais.advanceElementData(len);
-                        } else {
+                        if (len != 0) {
                             this.imsi = new IMSIImpl();
                             ((IMSIImpl) this.imsi).decodeData(ais, len);
                         }

Original comment by vil...@users.sourceforge.net on 7 Aug 2013 at 8:41

GoogleCodeExporter commented 9 years ago
Hello!

If you exclude "ais.advanceElementData(len);" from code, decoding will not 
correctly work.

Original comment by serg.vet...@gmail.com on 7 Aug 2013 at 2:18

GoogleCodeExporter commented 9 years ago

Original comment by serg.vet...@gmail.com on 7 Sep 2013 at 9:40

GoogleCodeExporter commented 9 years ago

Original comment by amit.bha...@gmail.com on 3 Nov 2013 at 12:10

GoogleCodeExporter commented 9 years ago

Original comment by amit.bha...@gmail.com on 3 Nov 2013 at 12:20