pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/29659 ) Change subject: osmux: Introduce osmux peer-behind-nat (on|off) and rework conn activation ...................................................................... osmux: Introduce osmux peer-behind-nat (on|off) and rework conn activation Change-Id: I7654ddf51d197a4107e55f4e406053b2e4a02f83 --- M include/osmocom/mgcp/mgcp.h M include/osmocom/mgcp/osmux.h M src/libosmo-mgcp/mgcp_osmux.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_vty.c 5 files changed, 131 insertions(+), 50 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/59/29659/1 diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h index c6d628e..41baaac 100644 --- a/include/osmocom/mgcp/mgcp.h +++ b/include/osmocom/mgcp/mgcp.h @@ -173,6 +173,8 @@ uint16_t osmux_port; /* Pad circuit with dummy AMR frames if no payload to transmit is available */ bool osmux_dummy; + /* Whether peer is behind NAT (Retrieve remote addr from 1st received Osmux packet)*/ + bool osmux_peer_behind_nat; /* domain name of the media gateway */ char domain[255+1]; diff --git a/include/osmocom/mgcp/osmux.h b/include/osmocom/mgcp/osmux.h index be5ee6a..044a33f 100644 --- a/include/osmocom/mgcp/osmux.h +++ b/include/osmocom/mgcp/osmux.h @@ -1,5 +1,7 @@ #pragma once +#include <stdint.h> + #include <osmocom/core/socket.h> #include <osmocom/netif/osmux.h> @@ -12,6 +14,7 @@ int osmux_init_conn(struct mgcp_conn_rtp *conn); int conn_osmux_enable(struct mgcp_conn_rtp *conn); void conn_osmux_disable(struct mgcp_conn_rtp *conn); +int conn_osmux_event_rx_crcx_mdcx(struct mgcp_conn_rtp *conn); int osmux_xfrm_to_osmux(char *buf, int buf_len, struct mgcp_conn_rtp *conn); int osmux_send_dummy(struct mgcp_conn_rtp *conn); diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c index e20868b..ccb1572 100644 --- a/src/libosmo-mgcp/mgcp_osmux.c +++ b/src/libosmo-mgcp/mgcp_osmux.c @@ -264,18 +264,34 @@ if (!mgcp_conn_rtp_is_osmux(conn_rtp)) continue; - /* Current implementation sets remote address & port for - * the conn based on src address received on the socket - * for the CID, in order to workaround NATs. - * Once the conn is fully established (remote address is - * known), validate the remote address doesn't change: */ + /* If Osmux peer is behind NAT: + * + state OSMUX_STATE_ACTIVATING: we cannot validate the remote address & port for + * the conn since we actually set conn's remote IP address from the info we received + * from this source address. NOTE: This means if Osmux peer is behind NAT we cannot + * have more than 1 osmux trunk since it's not possible to differentiate based on + * remote address. + * + state OSMUX_STATE_ENABLED: the conn is fully established (remote address is known), + * it is now posssible to validate the remote address doesn't change. + * If Osmux peer is not behind NAT: + * We can always validate the remote IP address is matching the one we received in + * CRCX/MDCX, we can support several parallel Osmux trunks since we can differentiate same CID + * based on remote IP addr+port. However, if Osmux peer is not behind NAT it means it will go + * into OSMUX_STATE_ENABLED as soon as the remote address is known, meaning if the conn is not + * in tha state we cannot (nor want) validate the IP address, we want to skip it until it is + * ready (to avoid 3rd party data injections). + * TLDR: When in OSMUX_STATE_ENABLED we can always validate remote address+port. + * When in OSMUX_STATE_ACTIVATING, in case peer behind NAT we select it, + * in case peer NOT behind NAT we skip it. + */ if (conn_rtp->osmux.state == OSMUX_STATE_ENABLED) { h = osmux_xfrm_input_get_deliver_cb_data(conn_rtp->osmux.in); if (osmo_sockaddr_cmp(&h->rem_addr, rem_addr) != 0) continue; - } - /* else: select based on CID only, to learn rem addr in NAT-based scenarios. - * FIXME: This should be configurable, have some sort of "osmux nat (on|off)" */ + } else if (!trunk->cfg->osmux_peer_behind_nat) { + LOGPCONN(conn, DOSMUX, LOGL_DEBUG, "osmux_conn_lookup(rem_addr=%s local_cid=%d): Skipping because not (yet) ENABLED\n", + osmo_sockaddr_to_str(rem_addr), local_cid); + continue; /* skip, read above */ + } /* else: continue CID validation below: */ if (conn_rtp->osmux.local_cid == local_cid) return conn_rtp; @@ -326,34 +342,69 @@ return msg; } -/* Updates conn osmux state and returns 0 if it can process messages, -1 otherwise */ -static int conn_osmux_state_check(struct mgcp_conn_rtp *conn, - bool sending) +/* To be called every time some AMR data is received on a connection */ +static void conn_osmux_event_data_received(struct mgcp_conn_rtp *conn, const struct osmo_sockaddr *rem_addr) { + const struct mgcp_config *cfg; switch(conn->osmux.state) { case OSMUX_STATE_ACTIVATING: + /* If peer is not behind NAT, transition to OSMUX_STATE_ENABLED is done through + * conn_osmux_rx_mdcx() whenever an CRCX/MDCX with the remote address is received. + */ + cfg = conn->conn->endp->trunk->cfg; + if (!cfg->osmux_peer_behind_nat) + return; + /* We have to wait until we received the remote CID from CRCX/MDCX */ + if (!conn->osmux.remote_cid_present) + return; + + /* Update remote address with the src address of the package we received */ + conn->end.addr = *rem_addr; + /* mgcp_rtp_end_remote_addr_available() is now true and we can enable the conn: */ if (conn_osmux_enable(conn) < 0) { LOGPCONN(conn->conn, DOSMUX, LOGL_ERROR, - "Could not enable osmux for conn on %s: %s\n", - sending ? "sent" : "received", + "Could not enable osmux conn: %s\n", mgcp_conn_dump(conn->conn)); - return -1; } - LOGPCONN(conn->conn, DOSMUX, LOGL_INFO, - "Osmux %s CID %u towards %s is now enabled\n", - sending ? "sent" : "received", - sending ? conn->osmux.remote_cid : conn->osmux.local_cid, - osmo_sockaddr_to_str(&conn->end.addr)); + return; + case OSMUX_STATE_ENABLED: + return; + default: + return; + } +} + +/* To be called every time an CRCX/MDCX is received. + * returns: 0 if conn can continue, negative if an error ocurred during setup */ +int conn_osmux_event_rx_crcx_mdcx(struct mgcp_conn_rtp *conn) +{ + struct mgcp_config *cfg; + + switch (conn->osmux.state) { + case OSMUX_STATE_ACTIVATING: + /* If peer is behind NAT, we have to wait until 1st osmux frame is received + * to discover peer's real remote address */ + cfg = conn->conn->endp->trunk->cfg; + if (cfg->osmux_peer_behind_nat) + return 0; + /* Keep waiting to receive remote CID through CRCX/MDCX */ + if (!conn->osmux.remote_cid_present) + return 0; + /* keep waiting to receive remote address through CRCX/MDCX */ + if (!mgcp_rtp_end_remote_addr_available(&conn->end)) + return 0; + /* Since the peer is not behind NAT, we can enable the conn right away since the proper + * remote address is now known: */ + if (conn_osmux_enable(conn) < 0) + return -1; + /* We have already transitioned to OSMUX_STATE_ENABLED here */ return 0; case OSMUX_STATE_ENABLED: return 0; default: - LOGPCONN(conn->conn, DOSMUX, LOGL_ERROR, - "Osmux %s in conn %s without full negotiation, state %d\n", - sending ? "sent" : "received", - mgcp_conn_dump(conn->conn), conn->osmux.state); - return -1; + OSMO_ASSERT(NULL); } + return 0; } /* Old versions of osmux used to send dummy packets [0x23 0x<CID>] to punch the @@ -371,7 +422,7 @@ goto out; } - conn_osmux_state_check(conn, false); + conn_osmux_event_data_received(conn, rem_addr); /* Only needed to punch hole in firewall, it can be dropped */ out: msgb_free(msg); @@ -419,14 +470,14 @@ rem = msg->len; continue; } + + conn_osmux_event_data_received(conn_src, &rem_addr); mgcp_conn_watchdog_kick(conn_src->conn); - if (conn_osmux_state_check(conn_src, false) == 0) { - rtpconn_osmux_rate_ctr_inc(conn_src, OSMUX_CHUNKS_RX_CTR); - rtpconn_osmux_rate_ctr_add(conn_src, OSMUX_OCTETS_RX_CTR, - osmux_chunk_length(msg, rem)); - osmux_xfrm_output_sched(conn_src->osmux.out, osmuxh); - } + rtpconn_osmux_rate_ctr_inc(conn_src, OSMUX_CHUNKS_RX_CTR); + rtpconn_osmux_rate_ctr_add(conn_src, OSMUX_OCTETS_RX_CTR, + osmux_chunk_length(msg, rem)); + osmux_xfrm_output_sched(conn_src->osmux.out, osmuxh); rem = msg->len; } out: @@ -535,6 +586,8 @@ * \returns 0 on success, -1 on ERROR */ int conn_osmux_enable(struct mgcp_conn_rtp *conn) { + OSMO_ASSERT(conn->osmux.state == OSMUX_STATE_ACTIVATING); + OSMO_ASSERT(conn->osmux.remote_cid_present == true); /*! If osmux is enabled, initialize the output handler. This handler is * used to reconstruct the RTP flow from osmux. The RTP SSRC is * allocated based on the circuit ID (conn_net->osmux.cid), which is unique @@ -548,14 +601,6 @@ static const uint32_t rtp_ssrc_winlen = UINT32_MAX / (OSMUX_CID_MAX + 1); bool osmux_dummy = trunk->cfg->osmux_dummy; - /* Check if osmux is enabled for the specified connection */ - if (conn->osmux.state != OSMUX_STATE_ACTIVATING) { - LOGPCONN(conn->conn, DOSMUX, LOGL_ERROR, - "conn:%s didn't negotiate Osmux, state %d\n", - mgcp_conn_dump(conn->conn), conn->osmux.state); - return -1; - } - /* Wait until we have the connection information from MDCX */ if (!mgcp_rtp_end_remote_addr_available(&conn->end)) { LOGPCONN(conn->conn, DOSMUX, LOGL_INFO, @@ -586,7 +631,10 @@ scheduled_from_osmux_tx_rtp_cb, conn); conn->osmux.state = OSMUX_STATE_ENABLED; - + LOGPCONN(conn->conn, DOSMUX, LOGL_INFO, + "Osmux CID %u towards %s is now enabled\n", + conn->osmux.remote_cid, + osmo_sockaddr_to_str(&conn->end.addr)); return 0; } @@ -638,10 +686,6 @@ * endpoint may have already punched the hole in the firewall. This * approach is simple though. */ - - if (conn_osmux_state_check(conn, true) < 0) - return 0; - buf_len = sizeof(struct osmux_hdr) + osmo_amr_bytes(AMR_FT_0); osmuxh = (struct osmux_hdr *) alloca(buf_len); memset(osmuxh, 0, buf_len); diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index fdac665..f3ebe60 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -1086,6 +1086,14 @@ goto error2; } + /* Notify Osmux conn that CRCX was received */ + if (mgcp_conn_rtp_is_osmux(conn)) { + if (conn_osmux_event_rx_crcx_mdcx(conn) < 0) { + LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, "CRCX: Osmux handling failed!\n"); + goto error2; + } + } + LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG, "CRCX: Creating connection: port: %u\n", conn->end.local_port); @@ -1271,13 +1279,19 @@ "MDCX: wilcard in MDCX is not supported!\n"); goto error3; } else if (conn->osmux.remote_cid_present && - remote_osmux_cid != (int) conn->osmux.remote_cid) { - LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, - "MDCX: changing already allocated CID is not supported!\n"); - goto error3; + remote_osmux_cid != conn->osmux.remote_cid) { + LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, + "MDCX: changing already allocated CID is not supported!\n"); + goto error3; + } else { + conn->osmux.remote_cid_present = true; + conn->osmux.remote_cid = remote_osmux_cid; + if (conn_osmux_event_rx_crcx_mdcx(conn) < 0) { + LOGPCONN(conn->conn, DLMGCP, LOGL_ERROR, + "MDCX: Osmux handling failed!\n"); + goto error3; + } } - conn->osmux.remote_cid_present = true; - conn->osmux.remote_cid = remote_osmux_cid; } /* MDCX may have provided a new remote address, which means we may need diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c index a3af94a..faa6479 100644 --- a/src/libosmo-mgcp/mgcp_vty.c +++ b/src/libosmo-mgcp/mgcp_vty.c @@ -156,6 +156,8 @@ g_cfg->osmux_port, VTY_NEWLINE); vty_out(vty, " osmux dummy %s%s", g_cfg->osmux_dummy ? "on" : "off", VTY_NEWLINE); + vty_out(vty, " osmux peer-behind-nat %s%s", + g_cfg->osmux_peer_behind_nat ? "on" : "off", VTY_NEWLINE); } if (g_cfg->conn_timeout) @@ -1646,6 +1648,21 @@ return CMD_SUCCESS; } +DEFUN(cfg_mgcp_osmux_peer_behind_nat, + cfg_mgcp_osmux_peer_behind_nat_cmd, + "osmux peer-behind-nat (on|off)", + OSMUX_STR "Define whether peer is behind NAT\n" + "Peer is behind NAT\n" + "Peer is NOT behind NAT\n") +{ + if (strcmp(argv[0], "on") == 0) + g_cfg->osmux_peer_behind_nat = true; + else if (strcmp(argv[0], "off") == 0) + g_cfg->osmux_peer_behind_nat = false; + + return CMD_SUCCESS; +} + DEFUN(cfg_mgcp_domain, cfg_mgcp_domain_cmd, "domain NAME", @@ -1736,6 +1753,7 @@ install_element(MGCP_NODE, &cfg_mgcp_osmux_batch_size_cmd); install_element(MGCP_NODE, &cfg_mgcp_osmux_port_cmd); install_element(MGCP_NODE, &cfg_mgcp_osmux_dummy_cmd); + install_element(MGCP_NODE, &cfg_mgcp_osmux_peer_behind_nat_cmd); install_element(MGCP_NODE, &cfg_mgcp_allow_transcoding_cmd); install_element(MGCP_NODE, &cfg_mgcp_no_allow_transcoding_cmd); install_element(MGCP_NODE, &cfg_mgcp_domain_cmd); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/29659 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I7654ddf51d197a4107e55f4e406053b2e4a02f83 Gerrit-Change-Number: 29659 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-MessageType: newchange