Osmocom Libraries » libosmo-ranap

osmo-iuh iu_helpers.c

/* decode a network address as used inside ASN.1 encoded Iu interface protocols */
int ranap_transp_layer_addr_decode(char *addr, unsigned int addr_len,
                                   const RANAP_TransportLayerAddress_t *trasp_layer_addr)
{
        unsigned char *buf;
        int len;
        const char *rc;

        buf = trasp_layer_addr->buf;
        len = trasp_layer_addr->size;

        if (buf[0] == 0x35 && len > 7)
                rc = inet_ntop(AF_INET, buf + 3, addr, addr_len);
        else if (len > 3)
                rc = inet_ntop(AF_INET, buf, addr, addr_len);
        else
                return -EINVAL;

        if (!rc)
                return -EINVAL;

        return 0;
}

This is so wrong on various levels.

the current failure is mended by trivial https://gerrit.osmocom.org/#/c/osmo-iuh/+/10545 ,
other decoding improvements remain pending.

That fix which changed the condition to '== 7' actually broke nano3G voice calls.
To hackily accomodate both sysmocell and nano3G, it must be '>= 7' instead :/
https://gerrit.osmocom.org/#/c/osmo-iuh/+/11167

neels wrote in the commit message of the last patch:

A proper X.213 NSAP decoding would still be more welcome than this patching back and forth, but this is (another) quick fix without spending too much time on it.

So I looked into what a proper X.213 NSAP decoding would be. The reference for having IPv4 encoded in NSAPA is defined in RFC4538 § 3.1.

It matches exactly what you have found, the 0x350001 followed by the IPv4 is the standard. The header for IPv6 is 0x350000.

=> We can update the code to check for 0x350001 now, not only for 0x35.

The length is always 20 bytes in the document. However, as the address is stored in Osmocom as transportLayerAddress, it gets cut off and that sort of implies what kind of address it has been. I did not find yet where it gets saved and cut up initially (will look into that tomorrow, that is probably the key to why we have different lengths for NSAP addresses that should have the same length), but I did find another piece of code that extracts the IPv4 from the transportLayerAddress in osmo-sgsn/src/gprs/sgsn_libgtp.c:sgsn_ranap_rab_ass_resp():

switch (item->transportLayerAddress->size) {
        case 7:
            /* It must be IPv4 inside a X213 NSAP */
            memcpy(pdp->lib->gsnlu.v, &item->transportLayerAddress->buf[3], 4);
            break;
        case 4:
            /* It must be a raw IPv4 address */
            memcpy(pdp->lib->gsnlu.v, item->transportLayerAddress->buf, 4);
            break;
        case 16:
            /* TODO: It must be a raw IPv6 address */
        case 19:
            /* TODO: It must be IPv6 inside a X213 NSAP */
        default:
            LOGP(DRANAP, LOGL_ERROR, "RAB Assignment Resp: Unknown " 
                "transport layer address size %u\n",
                item->transportLayerAddress->size);
            return -1;
        }

Summary¶

IPv4 Addresses get written into RANAP_TransportLayerAddress_t in various formats. The
ranap_transp_layer_addr_decode() and sgsn_ranap_rab_ass_resp() functions interpret it as follows:

So this is a mixture of writing the real IPv4 address (1.) (possibly with unneeded bytes that follow) and full/cut-off versions of IPv4 Address Encoding in an NSAPA (2. and 3.). IPv6 addresses are not interpreted by both functions (there is a standard for embedding them in NSAP addresses).

A lot of the osmo-iuh C code is generated from ASN1 files. The relevant datatype seems to be defined in RANAP-IEs.asn:

TransportLayerAddress ::= BIT STRING (SIZE (1..160, ...))

How to continue?¶

I thought the TransportLayerAddress would get written somewhere in the Osmocom code, and we could simply write the real address without a header and with a fixed length, and it would be fixed. But it seems that the addresses get sent from the nano3g, sysmocell or other devices instead. Does this mean, where the format does not match the spec, that this is a bug in these devices, and we can't really do anything about it instead of guess the address that has been sent like we are doing now?

Maybe we could move that guessing code into a central location instead of having it twice in both functions?

We need to be able to parse various formats coming from various 3G cells.

We also need to encode either raw IPv4 or a valid X.213 depending on osmo-msc.cfg.

Parsing in one central location sounds good, I wasn't aware that we had two different places?

(Would be even nicer to autodetect which format we should send, but I think that's not easily possible; during RAB assignment, we first send an address and then get one back, so we can't mimick the cell's format)

osmith ping? would be great to get this wrapped up after two years stalled.

neels, I looked into this again and I'm not sure if there is anything left to do here. From the issue description on top:

a) fix the check for the address length, should be len == 7, not len > 7.

Turned out it needed to be >= 7, and you've fixed that already in https://gerrit.osmocom.org/c/osmo-iuh/+/11167.

b) generally be stricter about the lengths.

As we need to support the different formats that the devices send, it seems that we can't be more strict about lengths? I guess we could possibly write the detection more nicely, but I'm not sure if that is worth it or how it should be done.

Can you please close the issue, or write down what needs to be done exactly?

I'm quite far removed from this issue by now, and haven't been involved in any 3G for a while now.

It seems that the "proper" thing would be to write up a proper decoder for X.213,
write a C unit test for it to ensure that it decodes both the 7 byte as well as the padded 20 bytes long versions.

Then looking at this code:

    if (buf[0] == 0x35 && len >= 7)
        rc = inet_ntop(AF_INET, buf + 3, addr, addr_len);
    else if (len > 3)
        rc = inet_ntop(AF_INET, buf, addr, addr_len);
    else
        return -EINVAL;

something like this would look more proper, pseudocode:

    if (decode_x213(&x213, buf, len) == 0) {
         switch(x213.af) {
         case AF_INET:
            inet_ntop(x213.af, x213.in_addr, addr, addr_len);
            break;
         case AF_INET6:
            inet_ntop(x213.af, x213.in6_addr, addr, addr_len);
            break;
         default:
            return -EINVAL;
         }
    } else if (len == 4) {
         inet_ntop(AF_INET, buf, addr, addr_len);
    }

something like that anyway...

Not sure if we really care that much at the moment?