[PATCH] openbsc[master]: oap_client: reject all messages in disabled/uninitialized state

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.org
Fri Dec 9 00:38:27 UTC 2016


Review 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>



More information about the gerrit-log mailing list