sim-card
/
osmo-remsim
mirror of https://gerrit.osmocom.org/osmo-remsim
9
0
Fork 0
Code Issues Releases Wiki Activity

server: Detect duplicate client/bankd connection; drop new ones 

we're dropping the current (new) connection as we don't really know which
is the "right" one. Dropping the new gives the old connection time to
timeout, or to continue to operate.  If we were to drop the old
connection, this could interrupt a perfectly working connection and opens
some kind of DoS.

Related: OS#5527
Change-Id: I00387dbc19d689415470e2f08df08a47a78b81c0
changes/27/28027/2
Harald Welte 2022-05-03 16:22:37 +02:00
parent 900ee725b5
commit 65006583c1
4 changed files with 61 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
asn1/RSPRO.asn
View File

@ -87,6 +87,7 @@ ResultCode ::= ENUMERATED {
illegalSlotId (3),
unsupportedProtocolVersion (4),
unknownSlotmap (5),
identityInUse (6),
-- no card is present in given slot
cardNotPresent (100),

1
include/osmocom/rspro/ResultCode.h
View File

@ -25,6 +25,7 @@ typedef enum ResultCode {
ResultCode_illegalSlotId = 3,
ResultCode_unsupportedProtocolVersion = 4,
ResultCode_unknownSlotmap = 5,
ResultCode_identityInUse = 6,
ResultCode_cardNotPresent = 100,
ResultCode_cardUnresponsive = 101,
ResultCode_cardTransmissionError = 102

12
src/rspro/ResultCode.c
View File

@ -89,15 +89,17 @@ static const asn_INTEGER_enum_map_t asn_MAP_ResultCode_value2enum_1[] = {
{ 3, 13, "illegalSlotId" },
{ 4, 26, "unsupportedProtocolVersion" },
{ 5, 14, "unknownSlotmap" },
{ 6, 13, "identityInUse" },
{ 100, 14, "cardNotPresent" },
{ 101, 16, "cardUnresponsive" },
{ 102, 21, "cardTransmissionError" }
/* This list is extensible */
};
static const unsigned int asn_MAP_ResultCode_enum2value_1[] = {
6, /* cardNotPresent(100) */
8, /* cardTransmissionError(102) */
7, /* cardUnresponsive(101) */
7, /* cardNotPresent(100) */
9, /* cardTransmissionError(102) */
8, /* cardUnresponsive(101) */
6, /* identityInUse(6) */
2, /* illegalBankId(2) */
1, /* illegalClientId(1) */
3, /* illegalSlotId(3) */
@ -109,8 +111,8 @@ static const unsigned int asn_MAP_ResultCode_enum2value_1[] = {
static const asn_INTEGER_specifics_t asn_SPC_ResultCode_specs_1 = {
asn_MAP_ResultCode_value2enum_1, /* "tag" => N; sorted by tag */
asn_MAP_ResultCode_enum2value_1, /* N => "tag"; sorted by N */
9, /* Number of elements in the maps */
10, /* Extensions before this member */
10, /* Number of elements in the maps */
11, /* Extensions before this member */
1, /* Strict enumeration */
0, /* Native long size */
0

60
src/server/rspro_server.c
View File

@ -76,6 +76,7 @@ enum remsim_server_client_fsm_state {
CLNTC_ST_WAIT_CONF_RES, /* waiting for ConfigClientRes */
CLNTC_ST_CONNECTED_BANKD,
CLNTC_ST_CONNECTED_CLIENT,
CLNTC_ST_REJECTED,
};
enum remsim_server_client_event {
@ -119,6 +120,7 @@ static void clnt_st_init(struct osmo_fsm_inst *fi, uint32_t event, void *data)
static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *data)
{
struct rspro_client_conn *conn = fi->priv;
struct rspro_client_conn *previous_conn;
const RsproPDU_t *pdu = data;
const ConnectClientReq_t *cclreq = NULL;
const ConnectBankReq_t *cbreq = NULL;
@ -141,11 +143,6 @@ static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *
return;
}
/* reparent us from srv->connections to srv->clients */
pthread_rwlock_wrlock(&conn->srv->rwlock);
llist_del(&conn->list);
llist_add_tail(&conn->list, &conn->srv->clients);
pthread_rwlock_unlock(&conn->srv->rwlock);
if (!cclreq->clientSlot) {
#if 0
@ -161,7 +158,6 @@ static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
#endif
} else {
/* FIXME: check for unique-ness */
rspro2client_slot(&conn->client.slot, cclreq->clientSlot);
osmo_fsm_inst_update_id_f(fi, "C%u:%u", conn->client.slot.client_id,
conn->client.slot.slot_nr);
@ -169,6 +165,30 @@ static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *
conn->client.slot.client_id,
conn->client.slot.slot_nr);
LOGPFSML(fi, LOGL_INFO, "Client connected from %s:%s\n", ip_str, port_str);
/* check for unique-ness */
previous_conn = client_conn_by_slot(conn->srv, &conn->client.slot);
if (previous_conn && previous_conn != conn) {
/* we're dropping the current (new) connection as we don't really know which
* is the "right" one. Dropping the new gives the old connection time to
* timeout, or to continue to operate. If we were to drop the old
* connection, this could interrupt a perfectly working connection and opens
* some kind of DoS. */
LOGPFSML(fi, LOGL_ERROR, "New client connection from %s:%s, but we already "
"have a connection from %s:%u. Dropping new connection.\n",
ip_str, port_str, previous_conn->peer->addr, previous_conn->peer->port);
resp = rspro_gen_ConnectClientRes(&conn->srv->comp_id, ResultCode_identityInUse);
client_conn_send(conn, resp);
osmo_fsm_inst_state_chg(fi, CLNTC_ST_REJECTED, 1, 2);
return;
}
/* reparent us from srv->connections to srv->clients */
pthread_rwlock_wrlock(&conn->srv->rwlock);
llist_del(&conn->list);
llist_add_tail(&conn->list, &conn->srv->clients);
pthread_rwlock_unlock(&conn->srv->rwlock);
resp = rspro_gen_ConnectClientRes(&conn->srv->comp_id, ResultCode_ok);
client_conn_send(conn, resp);
osmo_fsm_inst_state_chg(fi, CLNTC_ST_CONNECTED_CLIENT, 0, 0);
@ -183,7 +203,6 @@ static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
return;
}
/* FIXME: check for unique-ness */
conn->bank.bank_id = cbreq->bankId;
conn->bank.num_slots = cbreq->numberOfSlots;
osmo_fsm_inst_update_id_f(fi, "B%u", conn->bank.bank_id);
@ -196,6 +215,23 @@ static void clnt_st_established(struct osmo_fsm_inst *fi, uint32_t event, void *
"as they must be able to reach the bankd!\n", ip_str);
}
/* check for unique-ness */
previous_conn = bankd_conn_by_id(conn->srv, conn->bank.bank_id);
if (previous_conn && previous_conn != conn) {
/* we're dropping the current (new) connection as we don't really know which
* is the "right" one. Dropping the new gives the old connection time to
* timeout, or to continue to operate. If we were to drop the old
* connection, this could interrupt a perfectly working connection and opens
* some kind of DoS. */
LOGPFSML(fi, LOGL_ERROR, "New bankd connection from %s:%s, but we already "
"have a connection from %s:%u. Dropping new connection.\n",
ip_str, port_str, previous_conn->peer->addr, previous_conn->peer->port);
resp = rspro_gen_ConnectBankRes(&conn->srv->comp_id, ResultCode_identityInUse);
client_conn_send(conn, resp);
osmo_fsm_inst_state_chg(fi, CLNTC_ST_REJECTED, 1, 2);
return;
}
/* reparent us from srv->connections to srv->banks */
pthread_rwlock_wrlock(&conn->srv->rwlock);
llist_del(&conn->list);
@ -445,6 +481,10 @@ static int server_client_fsm_timer_cb(struct osmo_fsm_inst *fi)
case 1:
/* No ClientConnectReq received:disconnect */
return 1; /* ask core to terminate FSM */
case 2:
/* Timeout after rejecting client */
LOGPFSML(fi, LOGL_NOTICE, "Closing connection of rejected peer\n");
return 1; /* ask core to terminate FSM */
default:
OSMO_ASSERT(0);
}
@ -471,7 +511,7 @@ static const struct osmo_fsm_state server_client_fsm_states[] = {
.name = "ESTABLISHED",
.in_event_mask = S(CLNTC_E_CLIENT_CONN) | S(CLNTC_E_BANK_CONN),
.out_state_mask = S(CLNTC_ST_CONNECTED_CLIENT) | S(CLNTC_ST_WAIT_CONF_RES) |
S(CLNTC_ST_CONNECTED_BANKD),
S(CLNTC_ST_CONNECTED_BANKD) | S(CLNTC_ST_REJECTED),
.action = clnt_st_established,
},
[CLNTC_ST_WAIT_CONF_RES] = {
@ -493,6 +533,10 @@ static const struct osmo_fsm_state server_client_fsm_states[] = {
.action = clnt_st_connected_bankd,
.onenter = clnt_st_connected_bankd_onenter,
},
[CLNTC_ST_REJECTED] = {
.name = "REJECTED",
/* no events permitted, no action required */
}
};