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.
Related: OS#1592
Change-Id: I16165d228653e8a2689f9df94b77b470c06480c6
diff --git a/openbsc/src/libcommon/gsup_client.c b/openbsc/src/libcommon/gsup_client.c
index 0360e0a..4c1fe61 100644
--- a/openbsc/src/libcommon/gsup_client.c
+++ b/openbsc/src/libcommon/gsup_client.c
@@ -155,6 +155,7 @@
int rc;
struct msgb *msg_tx;
+ /* If the oap_state is disabled, this will reject the messages. */
rc = 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 46dbcdd..e372ede 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>
@@ -224,6 +225,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 45decf8..ab651e6 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 = 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(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(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 = 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(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(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(oap_client_handle(state, msg_rx, &msg_tx) == -1);
+ OSMO_ASSERT(oap_client_handle(state, msg_rx, &msg_tx) < 0);
OSMO_ASSERT(!msg_tx);
state->state = OAP_DISABLED;
- OSMO_ASSERT(oap_client_handle(state, msg_rx, &msg_tx) == -1);
+ OSMO_ASSERT(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.