This could never possibly have worked. When iterating over the
different IEs to encode, we must of course use the tag of the current
iterator item, and not the hard-coded value of the second tag in the
list.
Change-Id: I148799c5bdb95f70118691c1150330ebac4fdf21
In 2018, I4723361e1094b358310541a7dc4c5c921c778a15 introuced a
check against an integer unterflow. However, the fheck got the
logic wrong, with the result of breaking the function completely:
It would always only detect the first tag within the IPA request
and then take the branch that assumes an integer underflow.
Change-Id: I344975d0bda565ff196a1c0c69305cd349b98a19
ipa_ccm_idtag_parse_off is broken, and can only be used with
len_offset=1 on ID Request messages, otherwise won't work correctly.
Modify ipa_ccm_idtag_parse to at least parse those correctly, and
document the limitations.
Those two functions are already deprecated and only used in openbsc by 3
callers:
* ipa_ccm_idtag_parse in ussd_read_cb(): Broken, that function can only
work for Requests and it's used to parse a Response.
* ipa_ccm_idtag_parse_off in forward_sccp_to_msc (NAT): Broken, it can
only be used to parse Requests and it's used to parse a Response.
Furthermore, len_offset=2 is passed which makes no sense and most
probably it fails always, or can even make the program crash.
* ipa_ccm_idtag_parse_off in (answer_challenge): This one is fine and
could actually be replaced with ipa_ccm_id_get_parse after this commit
is merged.
Change-Id: I6efc852dfc041192f554e41a58290a0f63298021
This reverts commit 1261db1505.
The patch broke openbsc's external tests, and currently it is unclear
whether it is just an error in the test or if openbsc makes wrong
assumptions about the length value. Let's revert the patch to unblock
the master-openbsc jenkins job.
Related: OS#3851
Change-Id: I9adea35ff6de36c1611c7f85dde1b15bc1c0e786
IPA CCM is using a somewhat weird TLV encoding scheme:
* 16bit length (of tag and value)
* 8bit tag
* value
Our existing code mapping the CCM to 'struct tlv_parse' used the plain
length value without accounting for the one-byte tag.
This patch ensures we only report the length of the "value" part,
excluding the tag.
Change-Id: I435aaa33605bd48635715a2c81aa2d231c1abf51
In the previous commit we deprecated ipa_ccm_idtag_parse() but
also removed ipa_ccm_idtag_parse_off(), for which I couldn't find
any users. However, legacy openbsc.git still uses this function,
so let's re-introiduce it in its original form.
Change-Id: Ibfe53b04340eb355c8bfb8453a2af1522a4b6baf
In the past, the function ipa_ccm_idtag_parse() was used to parse
the payload of IPA CCM ID RESP packets. However, the function was
based on a possible misunderstanding of the message encoding, and
callers actually counted the first (upper) length nibble as part
of the header and passed a pointer to the second
(lower) length nibble of the first TLV into this function. As such,
it was unfixable, and had to be replaced with a new function called
ipa_ccm_id_resp_parse(). At the same time, we also add
ipa_ccm_id_get_parse() to parse the slightly different format of
the IPA CCM ID GET payload.
We can never be 100% sure what is "correct", as our understanding
of the protocol is entirely based on protocol analysis, without any
official documentation available.
This patch also introduces unit test coverage for both of the new
functions.
Revert "ipa: Add libosmogsm.map entry for ipa_ccm_idtag_parse_off"
This reverts commit 7f31c90b80.
Revert "ipa: Properly parse LV stream of a ID_GET request"
This reverts commit f558ed4bb9.
It introduced a function/behavior that was not originally intended:
The parse of IPA CCM ID GET (8bit length followed by 1 byte tag
and variable-length payload) instead of the IPA CCM ID RESP (16bit
length followed by 1 byte tag and variable-length payload).
Change-Id: I1834d90fbcdbfcb05f5b8cfe39bfe9543737ef8f
don't blindly trust the tag-length value in an IPA CCM ID GET
message. This could result in a remotely-triggered integer underflow.
Change-Id: I4723361e1094b358310541a7dc4c5c921c778a15
fix for some spelling issues found by lintian
Signed-off-by: Thorsten Alteholz <osmocom@alteholz.de>
Change-Id: I69976ecae6939d9ff51bfe4ce7374890c6563b82
Let's fix some erroneous/accidential references to wrong license,
update copyright information where applicable and introduce a
SPDX-License-Identifier to all files.
Change-Id: I39af26c6aaaf5c926966391f6565fc5936be21af
libosmocore offers the ipa API as general IPA Multiplex, which is e.g. used for
GSUP in osmo-msc. Looking at talloc reports, it is confusing to see "Abis/IP"
as msgb comment, because osmo-msc does not have an Abis interface.
Rename to "IPA Multiplex" as a more general description.
Change-Id: I3714dd21707bec0c4bcd0871e6ee8ff32d56b125
Considering the various styles and implications found in the sources, edit
scores of files to follow the same API doc guidelines around the doxygen
grouping and the \file tag.
Many files now show a short description in the generated API doc that was so
far only available as C comment.
The guidelines and reasoning behind it is documented at
https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API_documentation
In some instances, remove file comments and add to the corresponding group
instead, to be shared among several files (e.g. bitvec).
Change-Id: Ifa70e77e90462b5eb2b0457c70fd25275910c72b
Especially for short descriptions, it is annoying to have to type \brief for
every single API doc.
Drop all \brief and enable the AUTOBRIEF feature of doxygen, which always takes
the first sentence of an API doc as the brief description.
Change-Id: I11a8a821b065a128108641a2a63fb5a2b1916e87
It's a pity that even with this patch we still are fare away from having
the whole API documented. However, at least we have a more solid
foundation. Updates not only extend the documentation, but also make
sure it is rendered properly in the doxygen HTML.
Change-Id: I1344bd1a6869fb00de7c1899a8db93bba9bafce3
We are allocating a buffer on the stack without initializing it, and
then passing it into ipa_ccm_make_id_resp(). There is no real danger
from this, as the buffer is only uninitialized if num_ies is 0, but
let's memset() it for good style
Change-Id: If5761a47b8cba73ddcc02a88cfa5c87c1970c04e
Fixes: coverity CID#167040
The ipa.c file already contained code to parse an ID RESPONSE into the
'struct ipaccess_unit', but it didn't so far contain code to put
together an ID RESPONSE packet based on that structure. Let's change
that with ipa_ccm_make_id_resp() and a helper wrapper
ipa_ccm_make_id_resp_from_req().
Change-Id: Icbcd8827a75fd5f3393351c1ca372de85275ad35
FreeBSD 11.0 uses clang version 3.8.0 which spits various warnings
during libosmocore compilation. Let's clean this up a bit.
Change-Id: Ic14572e6970bd0b8916604fabf807f1608fa07e5
Output more information in case of IPA protocol errors to make debugging
easier.
Change-Id: I7632d6e679e076bfbec9abc12da4a46cc27ccea1
Related: SYS#3028
Include the headers mentioned by the manpage
ipa.c:346:9: warning: implicit declaration of function 'recv' is invalid in C99
[-Wimplicit-function-declaration]
ret = recv(fd, msg->tail, needed, 0);
For some reason the structure is closer to be a LV (length
and value). The value is actually a tag but it is counted
inside the length. Introduce an overload of the parse function
to provide an offset for the length. This will be taken from
the returned length.
Currently, the ipa_send function returns -1 in one execution branch
to indicate an error and -EIO in another. This is not consistent and
can lead to a misinterpretation of the error code, since -1 is -EPERM
and in general, EPERM is not returned by write(2).
This patch changes the return code to -errno instead of -1 for the
case that write(2) fails for same reason. So -rc is always a sensible
error value if there is a failure.
Sponsored-by: On-Waves ehf
As we are breaking builds by moving functions from libosmo-abis to
libosmocore anyway, we might as well give functions more appropriate
names. ipaccess is a company, while IPA is the multiplex protocol, and
CCM is the protocol used for establishing identities on the IPA
multiplex.
Harald Welte
Pau Espin
Oliver Smith
Neels Hofmeyr
Neels Hofmeyr