laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/33325 )

Change subject: Forbid partial VTY configurations of ASPs with name asp-clnt-* ......................................................................

Forbid partial VTY configurations of ASPs with name asp-clnt-*

A recent commit (83db938859428ce3835239e2692d93c557b750e1) changed the behavior of default "sctp-role" and "role" values of ASPs named asp-clnt-* which are used by osmo_sccp_simple_client_on_ss7_id(), in order to avoid having different default values when than function is used, which is totally confusing for end users.

As it was already informed when submitting the mentioned commit, it changes the behavior on that specific case, and that made some users start having problems without a proper "your config is wrong!" error.

This commit addresses it by basically forbiding that exact use case, that is, partially defining an asp-clnt-* ASP through VTY without explicitly configuring a "role" and "sctp-role". With this patch, function osmo_sccp_simple_client_on_ss7_id() will print a proper error informing the user and will return NULL, potentially triggering an early program exit which can then easily be fixed by using proper configuration.

Related: SYS#6486 Change-Id: I65b5fad2ec06a9d9c521f1e3ce8aab633da95a47 --- M TODO-RELEASE M include/osmocom/sigtran/osmo_ss7.h M src/osmo_ss7_vty.c M src/sccp_user.c 4 files changed, 67 insertions(+), 14 deletions(-)

Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved

diff --git a/TODO-RELEASE b/TODO-RELEASE index 05e12a6..ede41b5 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -8,3 +8,4 @@ # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line libosmocore >1.8.0 uses new osmo_prim_operation_name() +osmo_sccp_simple_client_on_ss7_id() behavior change: ASPs asp-clnt-* defined through VTY must explicitly configure "role" and "sctp-role" \ No newline at end of file diff --git a/include/osmocom/sigtran/osmo_ss7.h b/include/osmocom/sigtran/osmo_ss7.h index eff16b9..f413983 100644 --- a/include/osmocom/sigtran/osmo_ss7.h +++ b/include/osmocom/sigtran/osmo_ss7.h @@ -430,6 +430,8 @@ enum osmo_ss7_asp_admin_state adm_state; bool is_server; enum osmo_ss7_asp_role role; + bool role_set_by_vty; + bool sctp_role_set_by_vty;

struct osmo_ss7_asp_peer local; struct osmo_ss7_asp_peer remote; diff --git a/src/osmo_ss7_vty.c b/src/osmo_ss7_vty.c index 09e5f0a..ebec5f6 100644 --- a/src/osmo_ss7_vty.c +++ b/src/osmo_ss7_vty.c @@ -715,6 +715,8 @@ return CMD_WARNING; } else OSMO_ASSERT(0); + + asp->cfg.role_set_by_vty = true; return CMD_SUCCESS; }

@@ -733,6 +735,8 @@ asp->cfg.is_server = true; else OSMO_ASSERT(0); + + asp->cfg.sctp_role_set_by_vty = true; return CMD_SUCCESS; }

diff --git a/src/sccp_user.c b/src/sccp_user.c index aac6ee0..fb14e75 100644 --- a/src/sccp_user.c +++ b/src/sccp_user.c @@ -646,24 +646,42 @@ osmo_ss7_as_add_asp(as, asp->cfg.name); }

- /* If ASP was configured through VTY it may be explicitly configured as - * SCTP server. It may be a bit confusing since this function is to create - * a "SCCP simple client", but this allows users of this API such as - * osmo-hnbgw to support SCTP-role server if properly configured through VTY. - */ - if (asp->cfg.is_server) { - struct osmo_xua_server *xs; - LOGP(DLSCCP, LOGL_NOTICE, - "%s: Requesting an SCCP simple client on ASP %s configured with 'sctp-role server'\n", - name, asp->cfg.name); - xs = osmo_ss7_xua_server_find(ss7, prot, asp->cfg.local.port); - if (!xs) { + /* Extra sanity checks if the ASP asp-clnt-* was pre-configured over VTY: */ + if (!asp->simple_client_allocated) { + /* Forbid ASPs defined through VTY that are not entirely + * configured. "role" and "sctp-role" must be explicitly provided: + */ + if (!asp->cfg.role_set_by_vty) { LOGP(DLSCCP, LOGL_ERROR, - "%s: Requesting an SCCP simple client on ASP %s configured with 'sctp-role server' " - "but no matching xUA server was configured!\n", + "%s: ASP %s defined in VTY but 'role' was not set there, please set it.\n", name, asp->cfg.name); goto out_asp; } + if (!asp->cfg.sctp_role_set_by_vty) { + LOGP(DLSCCP, LOGL_ERROR, + "%s: ASP %s defined in VTY but 'sctp-role' was not set there, please set it.\n", + name, asp->cfg.name); + goto out_asp; + } + + /* If ASP was configured through VTY it may be explicitly configured as + * SCTP server. It may be a bit confusing since this function is to create + * a "SCCP simple client", but this allows users of this API such as + * osmo-hnbgw to support SCTP-role server if properly configured through VTY. + */ + if (asp->cfg.is_server) { + struct osmo_xua_server *xs; + LOGP(DLSCCP, LOGL_NOTICE, + "%s: Requesting an SCCP simple client on ASP %s configured with 'sctp-role server'\n", + name, asp->cfg.name); + xs = osmo_ss7_xua_server_find(ss7, prot, asp->cfg.local.port); + if (!xs) { + LOGP(DLSCCP, LOGL_ERROR, "%s: Requesting an SCCP simple client on ASP %s configured " + "with 'sctp-role server' but no matching xUA server was configured!\n", + name, asp->cfg.name); + goto out_asp; + } + } }

/* Restart ASP */