This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgReview at https://gerrit.osmocom.org/1395 oap_client: reject all messages in disabled/uninitialized state Fixes the bug indicated in oap_client_test.c: adjust to actually expect the proper behavior. Also adjust for modified return value for message rejection. Instead of -1, just expect < 0. Adjust experr for new error messages. Change-Id: I16165d228653e8a2689f9df94b77b470c06480c6 --- M openbsc/src/libcommon/gsup_client.c M openbsc/src/libcommon/oap_client.c M openbsc/tests/oap/oap_client_test.c M openbsc/tests/oap/oap_client_test.err 4 files changed, 31 insertions(+), 20 deletions(-) git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/95/1395/1 diff --git a/openbsc/src/libcommon/gsup_client.c b/openbsc/src/libcommon/gsup_client.c index 5b05a95..398bfa8 100644 --- a/openbsc/src/libcommon/gsup_client.c +++ b/openbsc/src/libcommon/gsup_client.c @@ -154,6 +154,7 @@ int rc; struct msgb *msg_tx; + /* If the oap_state is disabled, this will reject the messages. */ rc = osmo_oap_client_handle(&gsupc->oap_state, msg_rx, &msg_tx); msgb_free(msg_rx); if (rc < 0) diff --git a/openbsc/src/libcommon/oap_client.c b/openbsc/src/libcommon/oap_client.c index db09630..d9688b5 100644 --- a/openbsc/src/libcommon/oap_client.c +++ b/openbsc/src/libcommon/oap_client.c @@ -21,6 +21,7 @@ */ #include <string.h> +#include <errno.h> #include <osmocom/core/utils.h> #include <osmocom/crypt/auth.h> @@ -223,6 +224,21 @@ return -10; } + switch (state->state) { + case OAP_UNINITIALIZED: + LOGP(DLOAP, LOGL_ERROR, + "Received OAP message %d, but the OAP client is" + " not initialized\n", oap_msg.message_type); + return -ENOTCONN; + case OAP_DISABLED: + LOGP(DLOAP, LOGL_ERROR, + "Received OAP message %d, but the OAP client is" + " disabled\n", oap_msg.message_type); + return -ENOTCONN; + default: + break; + } + switch (oap_msg.message_type) { case OAP_MSGT_CHALLENGE_REQUEST: return handle_challenge(state, &oap_msg, msg_tx); diff --git a/openbsc/tests/oap/oap_client_test.c b/openbsc/tests/oap/oap_client_test.c index d847f15..d83e6f8 100644 --- a/openbsc/tests/oap/oap_client_test.c +++ b/openbsc/tests/oap/oap_client_test.c @@ -52,19 +52,15 @@ fprintf(stderr, "- make sure filling with zeros means uninitialized\n"); OSMO_ASSERT(state->state == OAP_UNINITIALIZED); - fprintf(stderr, "- reject messages in uninitialized state EXPECTING BUGS\n"); + fprintf(stderr, "- reject messages in uninitialized state\n"); memset(&oap_rx, 0, sizeof(oap_rx)); state->client_id = 1; oap_rx.message_type = OAP_MSGT_REGISTER_ERROR; msg_rx = osmo_oap_client_encoded(&oap_rx); - /* ATTENTION: This shows a bug in OAP: the rc should be < 0, but OAP - * happily accepts this message and breaks the uninitialized state. The - * expected rc, state and msg_tx will be fixed along with the fix. */ - OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) == 0 /* BUG, expecting < 0 */); - OSMO_ASSERT(state->state == OAP_REQUESTED_CHALLENGE /* BUG, expecting OAP_UNINITIALIZED */); + OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) < 0); + OSMO_ASSERT(state->state == OAP_UNINITIALIZED); msgb_free(msg_rx); - OSMO_ASSERT(msg_tx /* BUG, expecting NULL */); - msgb_free(msg_tx); + OSMO_ASSERT(!msg_tx); fprintf(stderr, "- reject messages in disabled state\n"); memset(state, 0, sizeof(*state)); @@ -73,14 +69,10 @@ state->client_id = 1; oap_rx.message_type = OAP_MSGT_REGISTER_ERROR; msg_rx = osmo_oap_client_encoded(&oap_rx); - /* ATTENTION: This shows a bug in OAP: the rc should be < 0, but OAP - * happily accepts this message and breaks the uninitialized state. The - * expected rc, state and msg_tx will be fixed along with the fix. */ - OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) == 0 /* BUG, expecting < 0 */); - OSMO_ASSERT(state->state == OAP_REQUESTED_CHALLENGE /* BUG, expecting OAP_DISABLED */); + OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) < 0); + OSMO_ASSERT(state->state == OAP_DISABLED); msgb_free(msg_rx); - OSMO_ASSERT(msg_tx /* BUG, expecting NULL */); - msgb_free(msg_tx); + OSMO_ASSERT(!msg_tx); fprintf(stderr, "- invalid client_id and shared secret\n"); memset(state, 0, sizeof(*state)); @@ -180,11 +172,11 @@ OSMO_ASSERT(state->state == OAP_INITIALIZED); state->state = OAP_UNINITIALIZED; - OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) == -1); + OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) < 0); OSMO_ASSERT(!msg_tx); state->state = OAP_DISABLED; - OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) == -1); + OSMO_ASSERT(osmo_oap_client_handle(state, msg_rx, &msg_tx) < 0); OSMO_ASSERT(!msg_tx); state->state = OAP_INITIALIZED; diff --git a/openbsc/tests/oap/oap_client_test.err b/openbsc/tests/oap/oap_client_test.err index 9e52357..8c538dc 100644 --- a/openbsc/tests/oap/oap_client_test.err +++ b/openbsc/tests/oap/oap_client_test.err @@ -1,8 +1,8 @@ - make sure filling with zeros means uninitialized -- reject messages in uninitialized state EXPECTING BUGS -DLOAP OAP registration failed +- reject messages in uninitialized state +DLOAP Received OAP message 5, but the OAP client is not initialized - reject messages in disabled state -DLOAP OAP registration failed +DLOAP Received OAP message 5, but the OAP client is disabled - invalid client_id and shared secret - reset state - only client_id is invalid @@ -23,6 +23,8 @@ DLOAP OAP: AUTN expected: cec4e3848a33000086781158ca40f136 - all data correct - but refuse to evaluate in uninitialized state +DLOAP Received OAP message 8, but the OAP client is not initialized +DLOAP Received OAP message 8, but the OAP client is disabled - now everything is correct - Expect the challenge response in msg_tx - Receive registration error for the first time. -- To view, visit https://gerrit.osmocom.org/1395 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I16165d228653e8a2689f9df94b77b470c06480c6 Gerrit-PatchSet: 1 Gerrit-Project: openbsc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>