pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/39723?usp=email ) Change subject: mgw: Split MGCP LocalConnectionOptions parsing from updating endpoint ...................................................................... mgw: Split MGCP LocalConnectionOptions parsing from updating endpoint Split current code into 2 steps, so first read-only parsing can be done, then when all parsing and checks have been done, finally can be applied o the object. Change-Id: If75731fb92329dca6d092ffc7ff17cb354d28c0d --- M include/osmocom/mgcp/mgcp_endp.h M include/osmocom/mgcp/mgcp_protocol.h M src/libosmo-mgcp/mgcp_endp.c M src/libosmo-mgcp/mgcp_msg.c M src/libosmo-mgcp/mgcp_protocol.c 5 files changed, 259 insertions(+), 244 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/23/39723/1 diff --git a/include/osmocom/mgcp/mgcp_endp.h b/include/osmocom/mgcp/mgcp_endp.h index fbe0850..8a2c8a2 100644 --- a/include/osmocom/mgcp/mgcp_endp.h +++ b/include/osmocom/mgcp/mgcp_endp.h @@ -149,6 +149,7 @@ const struct mgcp_trunk *trunk); struct mgcp_endpoint *mgcp_endp_find_specific(const char *epname, const struct mgcp_trunk *trunk); +void mgcp_endp_update_lco(struct mgcp_endpoint *endp, const struct mgcp_lco *lco); void mgcp_endp_release(struct mgcp_endpoint *endp); struct mgcp_conn *mgcp_endp_get_conn(struct mgcp_endpoint *endp, const char *id); diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h index 2ef4c3e..9f79334 100644 --- a/include/osmocom/mgcp/mgcp_protocol.h +++ b/include/osmocom/mgcp/mgcp_protocol.h @@ -42,12 +42,26 @@ mgcp_codecset_reset(&sdp->cset); } +/* Local connection options */ +struct mgcp_lco { + bool present; + char *codec; /* talloc-allocated to some parent */ + int pkt_period_min; /* time in ms */ + int pkt_period_max; /* time in ms */ +}; +static inline void mgcp_lco_init(struct mgcp_lco *lco) +{ + *lco = (struct mgcp_lco){}; +} +char *get_lco_identifier(const char *options); +int check_local_cx_options(void *ctx, const char *options); #define MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET (-2) #define MGCP_PARSE_HDR_PARS_OSMUX_CID_WILDCARD (-1) struct mgcp_parse_hdr_pars { - const char *local_options; + const char *lco_string; + struct mgcp_lco lco; const char *callid; const char *connid; enum mgcp_connection_mode mode; @@ -59,15 +73,14 @@ static inline void mgcp_parse_hdr_pars_init(struct mgcp_parse_hdr_pars *hpars) { - *hpars = (struct mgcp_parse_hdr_pars){ - .local_options = NULL, - .callid = NULL, - .connid = NULL, - .mode = MGCP_CONN_NONE, - .remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET, - .have_sdp = false, - .x_osmo_ign = 0, - }; + hpars->lco_string = NULL; + mgcp_lco_init(&hpars->lco); + hpars->callid = NULL; + hpars->connid = NULL; + hpars->mode = MGCP_CONN_NONE; + hpars->remote_osmux_cid = MGCP_PARSE_HDR_PARS_OSMUX_CID_UNSET; + hpars->have_sdp = false; + hpars->x_osmo_ign = 0; } /* Internal structure while parsing a request */ @@ -111,18 +124,8 @@ int mgcp_cause; }; -/* Local connection options */ -struct mgcp_lco { - char *string; - char *codec; - int pkt_period_min; /* time in ms */ - int pkt_period_max; /* time in ms */ -}; - char *mgcp_debug_get_last_endpoint_name(void); -char *get_lco_identifier(const char *options); -int check_local_cx_options(void *ctx, const char *options); struct mgcp_rtp_end; struct mgcp_endpoint; diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c index 1d2287c..a4ab860 100644 --- a/src/libosmo-mgcp/mgcp_endp.c +++ b/src/libosmo-mgcp/mgcp_endp.c @@ -764,6 +764,22 @@ return NULL; } +/* Helps assigning a new lco structure, since "codec" is talloc allocated. */ +void mgcp_endp_update_lco(struct mgcp_endpoint *endp, const struct mgcp_lco *lco) +{ + /* First free old talloc allocated codec string: */ + talloc_free(endp->local_options.codec); + endp->local_options.codec = NULL; + + if (lco) { + endp->local_options = *lco; + if (lco->codec) + endp->local_options.codec = talloc_strdup(endp, lco->codec); + } else { + endp->local_options = (struct mgcp_lco){0}; + } +} + /*! release endpoint, all open connections are closed. * \param[in] endp endpoint to release */ void mgcp_endp_release(struct mgcp_endpoint *endp) @@ -785,10 +801,7 @@ /* Reset endpoint parameters and states */ talloc_free(endp->callid); endp->callid = NULL; - talloc_free(endp->local_options.string); - endp->local_options.string = NULL; - talloc_free(endp->local_options.codec); - endp->local_options.codec = NULL; + mgcp_endp_update_lco(endp, NULL); if (endp->trunk->trunk_type == MGCP_TRUNK_E1) { uint8_t ts = e1_ts_nr_from_epname(endp->name); diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c index af4f9d0..afeadd5 100644 --- a/src/libosmo-mgcp/mgcp_msg.c +++ b/src/libosmo-mgcp/mgcp_msg.c @@ -101,6 +101,197 @@ return MGCP_CONN_NONE; } +/*! Helper function for check_local_cx_options() to get a pointer of the next + * lco option identifier + * \param[in] lco string + * \returns pointer to the beginning of the LCO identifier, NULL on failure */ +char *get_lco_identifier(const char *options) +{ + char *ptr; + unsigned int count = 0; + + /* Jump to the end of the lco identifier */ + ptr = strstr(options, ":"); + if (!ptr) + return NULL; + + /* Walk backwards until the pointer points to the beginning of the + * lco identifier. We know that we stand at the beginning when we + * are either at the beginning of the memory or see a space or + * comma. (this is tolerant, it will accept a:10, b:11 as well as + * a:10,b:11) */ + while (1) { + /* Endless loop protection */ + if (count > 10000) + return NULL; + else if (ptr < options || *ptr == ' ' || *ptr == ',') { + ptr++; + break; + } + ptr--; + count++; + } + + /* Check if we got any result */ + if (*ptr == ':') + return NULL; + + return ptr; +} + +/*! Check the LCO option. This function checks for multiple appearance of LCO +* options, which is illegal +* \param[in] ctx talloc context +* \param[in] lco string +* \returns 0 on success, -1 on failure */ +int check_local_cx_options(void *ctx, const char *options) +{ + int i; + char *options_copy; + char *lco_identifier; + char *lco_identifier_end; + char *next_lco_identifier; + + char **lco_seen; + unsigned int lco_seen_n = 0; + + if (!options) + return -1; + + lco_seen = + (char **)talloc_zero_size(ctx, strlen(options) * sizeof(char *)); + options_copy = talloc_strdup(ctx, options); + lco_identifier = options_copy; + + do { + /* Move the lco_identifier pointer to the beginning of the + * current lco option identifier */ + lco_identifier = get_lco_identifier(lco_identifier); + if (!lco_identifier) + goto error; + + /* Look ahead to the next LCO option early, since we + * will parse destructively */ + next_lco_identifier = strstr(lco_identifier + 1, ","); + + /* Pinch off the end of the lco field identifier name + * and see if we still got something, also check if + * there is some value after the colon. */ + lco_identifier_end = strstr(lco_identifier, ":"); + if (!lco_identifier_end) + goto error; + if (*(lco_identifier_end + 1) == ' ' + || *(lco_identifier_end + 1) == ',' + || *(lco_identifier_end + 1) == '\0') + goto error; + *lco_identifier_end = '\0'; + if (strlen(lco_identifier) == 0) + goto error; + + /* Check if we have already seen the current field identifier + * before. If yes, we must bail, an LCO must only appear once + * in the LCO string */ + for (i = 0; i < lco_seen_n; i++) { + if (strcasecmp(lco_seen[i], lco_identifier) == 0) + goto error; + } + lco_seen[lco_seen_n] = lco_identifier; + lco_seen_n++; + + /* The first identifier must always be found at the beginnning + * of the LCO string */ + if (lco_seen[0] != options_copy) + goto error; + + /* Go to the next lco option */ + lco_identifier = next_lco_identifier; + } while (lco_identifier); + + talloc_free(lco_seen); + talloc_free(options_copy); + return 0; +error: + talloc_free(lco_seen); + talloc_free(options_copy); + return -1; +} + +/* Set the LCO from a string (see RFC 3435). +* The string is stored in the 'string' field. A NULL string is handled exactly +* like an empty string, the 'string' field is never NULL after this function +* has been called. */ +static int mgcp_parse_lco(void *ctx, struct mgcp_lco *lco, const char *options) +{ + const char *lco_id; + char codec[17]; + char nt[17]; + int len; + + if (!options) + return 0; + if (strlen(options) == 0) + return 0; + + lco->present = true; + + /* Make sure the encoding of the LCO is consistent before we proceed */ + if (check_local_cx_options(ctx, options) != 0) { + LOGP(DLMGCP, LOGL_ERROR, + "local CX options: Internal inconsistency in Local Connection Options!\n"); + return -524; + } + + lco_id = options; + while ((lco_id = get_lco_identifier(lco_id))) { + switch (tolower(lco_id[0])) { + case 'p': + if (sscanf(lco_id + 1, ":%d-%d", + &lco->pkt_period_min, &lco->pkt_period_max) == 1) + lco->pkt_period_max = lco->pkt_period_min; + break; + case 'a': + /* FIXME: LCO also supports the negotiation of more than one codec. + * (e.g. a:PCMU;G726-32) But this implementation only supports a single + * codec only. Ignoring all but the first codec. */ + if (sscanf(lco_id + 1, ":%16[^,;]", codec) == 1) { + talloc_free(lco->codec); + /* MGCP header is case insensive, and we'll need + codec in uppercase when using it later: */ + len = strlen(codec); + lco->codec = talloc_size(ctx, len + 1); + osmo_str_toupper_buf(lco->codec, len + 1, codec); + } + break; + case 'n': + if (lco_id[1] == 't' && sscanf(lco_id + 2, ":%16[^,]", nt) == 1) + break; + /* else: fall through to print notice log */ + default: + LOGP(DLMGCP, LOGL_NOTICE, + "LCO: unhandled option: '%c'/%d in \"%s\"\n", + *lco_id, *lco_id, options); + break; + } + + lco_id = strchr(lco_id, ','); + if (!lco_id) + break; + } + + LOGP(DLMGCP, LOGL_DEBUG, + "local CX options: lco->pkt_period_max: %d, lco->codec: %s\n", + lco->pkt_period_max, lco->codec); + + /* Check if the packetization fits the 20ms raster */ + if (lco->pkt_period_min % 20 && lco->pkt_period_max % 20) { + LOGP(DLMGCP, LOGL_ERROR, + "local CX options: packetization interval is not a multiple of 20ms!\n"); + return -535; + } + + return 0; +} + /*! Analyze and parse the the hader of an MGCP messeage string. * \param[out] pdata caller provided memory to store the parsing results. * \param[in] data mgcp message string. @@ -177,6 +368,7 @@ { struct mgcp_parse_hdr_pars *hp = &pdata->hpars; char *line; + int rc; mgcp_parse_hdr_pars_init(hp); @@ -188,7 +380,24 @@ switch (toupper(line[0])) { case 'L': - hp->local_options = (const char *)line + 3; + hp->lco_string = (const char *)line + 3; + rc = mgcp_parse_lco(pdata, &hp->lco, hp->lco_string); + if (rc < 0) { + LOG_MGCP_PDATA(pdata, LOGL_NOTICE, "Invalid LocalConnectionOptions line: '%s'", line); + switch (pdata->rq->verb) { + case MGCP_VERB_CRCX: + rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_crcx_ctr_group, + MGCP_CRCX_FAIL_INVALID_CONN_OPTIONS)); + break; + case MGCP_VERB_MDCX: + rate_ctr_inc(rate_ctr_group_get_ctr(pdata->rq->trunk->ratectr.mgcp_mdcx_ctr_group, + MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS)); + break; + default: + break; + } + return rc; + } break; case 'C': hp->callid = (const char *)line + 3; diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c index 06da91a..49a4e83 100644 --- a/src/libosmo-mgcp/mgcp_protocol.c +++ b/src/libosmo-mgcp/mgcp_protocol.c @@ -455,199 +455,6 @@ return create_ok_response(rq->trunk, rq->endp, 200, "AUEP", rq->pdata->trans); } -/*! Helper function for check_local_cx_options() to get a pointer of the next - * lco option identifier - * \param[in] lco string - * \returns pointer to the beginning of the LCO identifier, NULL on failure */ -char *get_lco_identifier(const char *options) -{ - char *ptr; - unsigned int count = 0; - - /* Jump to the end of the lco identifier */ - ptr = strstr(options, ":"); - if (!ptr) - return NULL; - - /* Walk backwards until the pointer points to the beginning of the - * lco identifier. We know that we stand at the beginning when we - * are either at the beginning of the memory or see a space or - * comma. (this is tolerant, it will accept a:10, b:11 as well as - * a:10,b:11) */ - while (1) { - /* Endless loop protection */ - if (count > 10000) - return NULL; - else if (ptr < options || *ptr == ' ' || *ptr == ',') { - ptr++; - break; - } - ptr--; - count++; - } - - /* Check if we got any result */ - if (*ptr == ':') - return NULL; - - return ptr; -} - -/*! Check the LCO option. This function checks for multiple appearence of LCO - * options, which is illegal - * \param[in] ctx talloc context - * \param[in] lco string - * \returns 0 on success, -1 on failure */ -int check_local_cx_options(void *ctx, const char *options) -{ - int i; - char *options_copy; - char *lco_identifier; - char *lco_identifier_end; - char *next_lco_identifier; - - char **lco_seen; - unsigned int lco_seen_n = 0; - - if (!options) - return -1; - - lco_seen = - (char **)talloc_zero_size(ctx, strlen(options) * sizeof(char *)); - options_copy = talloc_strdup(ctx, options); - lco_identifier = options_copy; - - do { - /* Move the lco_identifier pointer to the beginning of the - * current lco option identifier */ - lco_identifier = get_lco_identifier(lco_identifier); - if (!lco_identifier) - goto error; - - /* Look ahead to the next LCO option early, since we - * will parse destructively */ - next_lco_identifier = strstr(lco_identifier + 1, ","); - - /* Pinch off the end of the lco field identifier name - * and see if we still got something, also check if - * there is some value after the colon. */ - lco_identifier_end = strstr(lco_identifier, ":"); - if (!lco_identifier_end) - goto error; - if (*(lco_identifier_end + 1) == ' ' - || *(lco_identifier_end + 1) == ',' - || *(lco_identifier_end + 1) == '\0') - goto error; - *lco_identifier_end = '\0'; - if (strlen(lco_identifier) == 0) - goto error; - - /* Check if we have already seen the current field identifier - * before. If yes, we must bail, an LCO must only appear once - * in the LCO string */ - for (i = 0; i < lco_seen_n; i++) { - if (strcasecmp(lco_seen[i], lco_identifier) == 0) - goto error; - } - lco_seen[lco_seen_n] = lco_identifier; - lco_seen_n++; - - /* The first identifier must always be found at the beginnning - * of the LCO string */ - if (lco_seen[0] != options_copy) - goto error; - - /* Go to the next lco option */ - lco_identifier = next_lco_identifier; - } while (lco_identifier); - - talloc_free(lco_seen); - talloc_free(options_copy); - return 0; -error: - talloc_free(lco_seen); - talloc_free(options_copy); - return -1; -} - -/* Set the LCO from a string (see RFC 3435). - * The string is stored in the 'string' field. A NULL string is handled exactly - * like an empty string, the 'string' field is never NULL after this function - * has been called. */ -static int set_local_cx_options(void *ctx, struct mgcp_lco *lco, - const char *options) -{ - char *lco_id; - char codec[17]; - char nt[17]; - int len; - - if (!options) - return 0; - if (strlen(options) == 0) - return 0; - - /* Make sure the encoding of the LCO is consistant before we proceed */ - if (check_local_cx_options(ctx, options) != 0) { - LOGP(DLMGCP, LOGL_ERROR, - "local CX options: Internal inconsistency in Local Connection Options!\n"); - return 524; - } - - talloc_free(lco->string); - lco->string = talloc_strdup(ctx, options); - - lco_id = lco->string; - while ((lco_id = get_lco_identifier(lco_id))) { - switch (tolower(lco_id[0])) { - case 'p': - if (sscanf(lco_id + 1, ":%d-%d", - &lco->pkt_period_min, &lco->pkt_period_max) == 1) - lco->pkt_period_max = lco->pkt_period_min; - break; - case 'a': - /* FIXME: LCO also supports the negotiation of more than one codec. - * (e.g. a:PCMU;G726-32) But this implementation only supports a single - * codec only. Ignoring all but the first codec. */ - if (sscanf(lco_id + 1, ":%16[^,;]", codec) == 1) { - talloc_free(lco->codec); - /* MGCP header is case insensive, and we'll need - codec in uppercase when using it later: */ - len = strlen(codec); - lco->codec = talloc_size(ctx, len + 1); - osmo_str_toupper_buf(lco->codec, len + 1, codec); - } - break; - case 'n': - if (lco_id[1] == 't' && sscanf(lco_id + 2, ":%16[^,]", nt) == 1) - break; - /* else: fall throught to print notice log */ - default: - LOGP(DLMGCP, LOGL_NOTICE, - "LCO: unhandled option: '%c'/%d in \"%s\"\n", - *lco_id, *lco_id, lco->string); - break; - } - - lco_id = strchr(lco_id, ','); - if (!lco_id) - break; - } - - LOGP(DLMGCP, LOGL_DEBUG, - "local CX options: lco->pkt_period_max: %i, lco->codec: %s\n", - lco->pkt_period_max, lco->codec); - - /* Check if the packetization fits the 20ms raster */ - if (lco->pkt_period_min % 20 && lco->pkt_period_max % 20) { - LOGP(DLMGCP, LOGL_ERROR, - "local CX options: packetization interval is not a multiple of 20ms!\n"); - return 535; - } - - return 0; -} - uint32_t mgcp_rtp_packet_duration(const struct mgcp_endpoint *endp, const struct mgcp_rtp_end *rtp) { @@ -907,6 +714,10 @@ /* Update endp->x_osmo_ign: */ endp->x_osmo_ign |= hpars->x_osmo_ign; + /* Set local connection options, if present */ + if (hpars->lco.present) + mgcp_endp_update_lco(endp, &hpars->lco); + if (!endp->callid) { /* Claim endpoint resources. This will also set the callid, * creating additional connections will only be possible if @@ -953,19 +764,6 @@ } /* else: -1 (wildcard) */ } - /* Set local connection options, if present */ - if (hpars->local_options) { - rc = set_local_cx_options(trunk->endpoints, - &endp->local_options, hpars->local_options); - if (rc != 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, - "CRCX: invalid local connection options!\n"); - error_code = rc; - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_INVALID_CONN_OPTIONS)); - goto error2; - } - } - /* Handle codec information and decide for a suitable codec */ rc = handle_codec_info(conn_rtp, rq); mgcp_codecset_summary(&conn_rtp->end.cset, mgcp_conn_dump(conn)); @@ -1120,6 +918,10 @@ return create_err_response(endp, endp, 400, "MDCX", pdata->trans); } + /* Set local connection options, if present */ + if (hpars->lco.present) + mgcp_endp_update_lco(endp, &hpars->lco); + mgcp_conn_watchdog_kick(conn); if (hpars->mode == MGCP_CONN_NONE) { @@ -1130,19 +932,6 @@ return create_err_response(endp, endp, 517, "MDCX", pdata->trans); } - /* Set local connection options, if present */ - if (hpars->local_options) { - rc = set_local_cx_options(trunk->endpoints, - &endp->local_options, hpars->local_options); - if (rc != 0) { - LOGPCONN(conn, DLMGCP, LOGL_ERROR, - "MDCX: invalid local connection options!\n"); - error_code = rc; - rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_INVALID_CONN_OPTIONS)); - goto error3; - } - } - conn_rtp = mgcp_conn_get_conn_rtp(conn); OSMO_ASSERT(conn_rtp); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/39723?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If75731fb92329dca6d092ffc7ff17cb354d28c0d Gerrit-Change-Number: 39723 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt;