aboutsummaryrefslogtreecommitdiffstats
path: root/openbsc/src/osmo-bsc_nat/bsc_nat.c
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2018-06-08 13:10:13 +0200
committerPau Espin Pedrol <pespin@sysmocom.de>2018-06-08 13:30:41 +0200
commitd3e15ad21e06a1cf20af900b8ff7ffd4dba785c4 (patch)
tree195a2572975b571c4a264f53fe7998a51d723342 /openbsc/src/osmo-bsc_nat/bsc_nat.c
parent9612004c0794cf1d606814e962be0e3a85c143c4 (diff)
bsc-nat: Avoid heap-use-after-free on bsc auth failure
Previous to this patch, if ipaccess_auth_bsc() failed finding the requested auth token, it would call bsc_close_connection() on it. However, it would not report callers that the bsc conn was closed. Since ipaccess_auth_bsc is called in the following path: [osmo_wqueue_bfd_cb->ipaccess_bsc_read_cb->forward_sccp_to_msc->ipaccess_auth_bsc] It needs to notify the lower layers (wqueue) that the conn/osmo_fd has been freed an it should avoid keep using/forwarding it again. This patch fixes this issue by moving the conn closing one layer down the stack (from ipaccess_auth_bsc to forward_sccp_to_msc), and in there we now close the conn and provide required information to the callers. Fixes following Asan report: Unit_Name='foobar' <0015> openbsc/openbsc/src/osmo-bsc_nat/bsc_nat.c:1061 No bsc found for token 'foobar' len 6 on fd: 11. ================================================================= ==18946==ERROR: AddressSanitizer: heap-use-after-free on address 0x616001f8b81c at pc 0x7ffff6067540 bp 0x7fffffffe170 sp 0x7fffffffe168 READ of size 4 at 0x616001f8b81c thread T0 #0 0x7ffff606753f in osmo_wqueue_bfd_cb libosmocore/src/write_queue.c:65 #1 0x7ffff605206b in osmo_fd_disp_fds libosmocore/src/select.c:217 #2 0x7ffff6052305 in osmo_select_main libosmocore/src/select.c:257 #3 0x421c8e in main openbsc/openbsc/src/osmo-bsc_nat/bsc_nat.c:1714 #4 0x7ffff47ffb44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44) #5 0x406438 (/bin/osmo-bsc_nat+0x406438) Fixes: SYS#4250 Change-Id: Ifb39a045b98bc2043a98a9787fc61cbcddc368e0
Diffstat (limited to 'openbsc/src/osmo-bsc_nat/bsc_nat.c')
-rw-r--r--openbsc/src/osmo-bsc_nat/bsc_nat.c51
1 files changed, 32 insertions, 19 deletions
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index fb2ec8347..e912f60b4 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -958,21 +958,23 @@ void bsc_close_connection(struct bsc_connection *connection)
talloc_free(connection);
}
-static void bsc_maybe_close(struct bsc_connection *bsc)
+/* Returns true if bsc_close_connection() was called, false otherwise */
+static bool bsc_maybe_close(struct bsc_connection *bsc)
{
struct nat_sccp_connection *sccp;
if (!bsc->nat->blocked)
- return;
+ return false;
/* are there any connections left */
llist_for_each_entry(sccp, &bsc->nat->sccp_connections, list_entry)
if (sccp->bsc == bsc)
- return;
+ return false;
/* nothing left, close the BSC */
LOGP(DNAT, LOGL_NOTICE, "Cleaning up BSC %d in blocking mode.\n",
bsc->cfg ? bsc->cfg->nr : -1);
bsc_close_connection(bsc);
+ return true;
}
static void ipaccess_close_bsc(void *data)
@@ -1021,7 +1023,8 @@ static int verify_key(struct bsc_connection *conn, struct bsc_config *conf, cons
return osmo_constant_time_cmp(vec.res, key, 8) == 0;
}
-static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc)
+/* Returns true if connection was successfully authenticated, false otherwise. */
+static bool ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc)
{
struct bsc_config *conf;
const char *token = (const char *) TLVP_VAL(tvp, IPAC_IDTAG_UNITNAME);
@@ -1032,21 +1035,19 @@ static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc
if (bsc->cfg) {
LOGP(DNAT, LOGL_ERROR, "Reauth on fd %d bsc nr %d\n",
bsc->write_queue.bfd.fd, bsc->cfg->nr);
- return;
+ return true;
}
if (len <= 0) {
LOGP(DNAT, LOGL_ERROR, "Token with length zero on fd: %d\n",
bsc->write_queue.bfd.fd);
- bsc_close_connection(bsc);
- return;
+ return false;
}
if (token[len - 1] != '\0') {
LOGP(DNAT, LOGL_ERROR, "Token not null terminated on fd: %d\n",
bsc->write_queue.bfd.fd);
- bsc_close_connection(bsc);
- return;
+ return false;
}
/*
@@ -1061,8 +1062,7 @@ static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc
LOGP(DNAT, LOGL_ERROR,
"No bsc found for token '%s' len %d on fd: %d.\n", token,
bsc->write_queue.bfd.fd, len);
- bsc_close_connection(bsc);
- return;
+ return false;
}
/* We have set a key and expect it to be present */
@@ -1070,8 +1070,7 @@ static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc
LOGP(DNAT, LOGL_ERROR,
"Wrong key for bsc nr %d fd: %d.\n", conf->nr,
bsc->write_queue.bfd.fd);
- bsc_close_connection(bsc);
- return;
+ return false;
}
rate_ctr_inc(&conf->stats.ctrg->ctr[BCFG_CTR_NET_RECONN]);
@@ -1081,6 +1080,7 @@ static void ipaccess_auth_bsc(struct tlv_parsed *tvp, struct bsc_connection *bsc
LOGP(DNAT, LOGL_NOTICE, "Authenticated bsc nr: %d on fd %d\n",
conf->nr, bsc->write_queue.bfd.fd);
start_ping_pong(bsc);
+ return true;
}
static void handle_con_stats(struct nat_sccp_connection *con)
@@ -1098,7 +1098,14 @@ static void handle_con_stats(struct nat_sccp_connection *con)
rate_ctr_inc(&ctrg->ctr[id]);
}
-static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg)
+/*!
+ * Forward messages to msc and verify received authentication messages.
+ * \param[in] bsc Pointer to bsc_connection structure from which the message was received.
+ * \param[in] msg The msg received to be forwarded to the msc.
+ * \param[out] bsc_conn_closed Whether bsc_close_connection(bsc) was called inside the function.
+ * \returns 0 on success, -1 on error.
+ */
+static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg, bool *bsc_conn_closed)
{
int con_filter = 0;
char *imsi = NULL;
@@ -1107,6 +1114,7 @@ static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg)
int con_type;
struct bsc_nat_parsed *parsed;
struct bsc_filter_reject_cause cause;
+ *bsc_conn_closed = false;
/* Parse and filter messages */
parsed = bsc_nat_parse(msg);
@@ -1216,7 +1224,7 @@ static int forward_sccp_to_msc(struct bsc_connection *bsc, struct msgb *msg)
con_filter = con->con_local;
}
remove_sccp_src_ref(bsc, msg, parsed);
- bsc_maybe_close(bsc);
+ *bsc_conn_closed = bsc_maybe_close(bsc);
break;
case SCCP_MSG_TYPE_UDT:
/* simply forward everything */
@@ -1277,8 +1285,12 @@ exit:
"message with malformed TLVs\n");
return ret;
}
- if (TLVP_PRESENT(&tvp, IPAC_IDTAG_UNITNAME))
- ipaccess_auth_bsc(&tvp, bsc);
+ if (TLVP_PRESENT(&tvp, IPAC_IDTAG_UNITNAME)) {
+ if (!ipaccess_auth_bsc(&tvp, bsc)) {
+ bsc_close_connection(bsc);
+ *bsc_conn_closed = true;
+ }
+ }
}
}
@@ -1296,6 +1308,7 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd)
struct msgb *msg = NULL;
struct ipaccess_head *hh;
struct ipaccess_head_ext *hh_ext;
+ bool fd_closed = false;
int ret;
ret = ipa_msg_recv_buffered(bfd->fd, &msg, &bsc->pending_msg);
@@ -1344,8 +1357,8 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd)
/* FIXME: Currently no PONG is sent to the BSC */
/* FIXME: Currently no ID ACK is sent to the BSC */
- forward_sccp_to_msc(bsc, msg);
- return 0;
+ forward_sccp_to_msc(bsc, msg, &fd_closed);
+ return fd_closed ? -EBADF : 0;
close_fd:
bsc_close_connection(bsc);