OpenSIPS / opensips

OpenSIPS is a GPL implementation of a multi-functionality SIP Server that targets to deliver a high-level technical solution (performance, security and quality) to be used in professional SIP server platforms.
https://opensips.org
Other
1.28k stars 581 forks source link

[FEATURE] STIR/SHAKEN - French version - Allow to change ORIG populating order #3268

Open StellaTeam opened 11 months ago

StellaTeam commented 11 months ago

Is your feature request related to a problem? Please describe.

On the contrary of STIR/SHAKEN original (ATIS) version, the french STIR/SHAKEN version (MAN : Mécanisme d'Authentification des Numéros) populate ORIG value from "From" header, then, if invalid or unavailable, from "PAI" header.

So, because of the order change, in order to comply with these differences, we have to use script statements and function arguments, which are probably much less efficient that when done from module internals.

Describe the solution you'd like

Being able to set a stir_shaken module parameter to change "orig" derivation from SIP order. Something like "orig_derivationOrder_reverse" : false (default - pai then from) / true (from then pai)

Implementation

stir_shaken.c

adding the "reverse order" parameter as a definition of the module

then changing get_orig_tn_from_msg function to reflect both alternatives

This might look like something like that (probably partially false, and certainly suboptimal)

The code is quite bigger and more ugly because:

static int get_orig_tn_from_msg(struct sip_msg *msg, str *orig_tn)
{
        struct to_body *body;
        struct sip_uri parsed_uri;

        if (parse_headers(msg, HDR_PAI_F | HDR_FROM_F, 0) < 0) {
                LM_ERR("Failed to parse headers\n");
                return -1;
        }

// If derivation is in reverse mode trying to first parse "From" header and extracting a valid E164 origin from it

    if(orig_derivationOrder_reverse) {
        if (parse_from_header(msg) < 0) {
            LM_ERR("Unable to parse From header\n");
            return -1;
        }
        body = get_from(msg);
        if (parse_uri(body->uri.s, body->uri.len, &parsed_uri) < 0) {
            LM_ERR("Failed to parse %s URI: %.*s\n", "From",
                   body->uri.len, body->uri.s);
            return -1;
        }

        if ((parsed_uri.type != SIP_URI_T && parsed_uri.type != TEL_URI_T &&
            parsed_uri.type != SIPS_URI_T && parsed_uri.type != TELS_URI_T) ||
            (e164_strict_mode &&
              (parsed_uri.type == SIP_URI_T || parsed_uri.type == SIPS_URI_T) &&
              str_strcmp(&parsed_uri.user_param, _str("user=phone")))) {
// If no valid E164 found in "From" header, trying to get one from PAI if present
            if (msg->pai) {
                if (parse_pai_header(msg) < 0) {
                    LM_ERR("Unable to parse P-Asserted-Identity header\n");
                    return -1;
                }
                body = get_pai(msg);
                if ((parsed_uri.type != SIP_URI_T && parsed_uri.type != TEL_URI_T &&
                    parsed_uri.type != SIPS_URI_T && parsed_uri.type != TELS_URI_T) ||
                    (e164_strict_mode &&
                      (parsed_uri.type == SIP_URI_T || parsed_uri.type == SIPS_URI_T) &&
                      str_strcmp(&parsed_uri.user_param, _str("user=phone")))) {
                        LM_ERR("'tel:' URI or 'sip:' URI %srequired\n",
                            e164_strict_mode ? "with ';user=phone' parameter " : "");
                        return -3;
                    }
            }
        }

// if not in reverse derivation mode, using PAI if available and falling back to "From" if PAI is missing
    } else {

        if (msg->pai) {
            if (parse_pai_header(msg) < 0) {
                LM_ERR("Unable to parse P-Asserted-Identity header\n");
                return -1;
            }
            body = get_pai(msg);
        } else {
            if (parse_from_header(msg) < 0) {
                LM_ERR("Unable to parse From header\n");
                return -1;
            }
            body = get_from(msg);
        }

        if (parse_uri(body->uri.s, body->uri.len, &parsed_uri) < 0) {
            LM_ERR("Failed to parse %s URI: %.*s\n", msg->pai ? "PAI" : "From",
                   body->uri.len, body->uri.s);
            return -1;
        }

        if ((parsed_uri.type != SIP_URI_T && parsed_uri.type != TEL_URI_T &&
            parsed_uri.type != SIPS_URI_T && parsed_uri.type != TELS_URI_T) ||
            (e164_strict_mode &&
              (parsed_uri.type == SIP_URI_T || parsed_uri.type == SIPS_URI_T) &&
              str_strcmp(&parsed_uri.user_param, _str("user=phone")))) {
            LM_ERR("'tel:' URI or 'sip:' URI %srequired\n",
                e164_strict_mode ? "with ';user=phone' parameter " : "");
            return -3;
        }
    }

        *orig_tn = parsed_uri.user;

        return 0;
}

Describe alternatives you've considered

Using fn parameter from auth/check STIR/SHAKEN functions in script, but much less efficient.

Thank you

github-actions[bot] commented 10 months ago

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.

github-actions[bot] commented 9 months ago

Marking as closed due to lack of progress for more than 30 days. If this issue is still relevant, please re-open it with additional details.

StellaTeam commented 9 months ago

Hello,

The issue has been automatically closed because of inactivity.

Just for me to know, what's the OpenSIPS team position about this feature?

Don't hesitate if you need any additionnal information about the requirements.

Thank you

liviuchircu commented 9 months ago

Hello @StellaTeam!

It's interesting to see the differences between the ATIS spec (which prioritizes PAI -> From) and the MAN spec (which prefers From -> PAI). The implementation suggestion is also sensible, there are plenty of ways to tackle it -- maybe a modparam (some kind of "mode"?) would be an intuitive choice.

Using fn parameter from auth/check STIR/SHAKEN functions in script, but much less efficient.

Still, why do you consider that the simple orig function parameter is much less efficient? On the contrary, by setting it you completely skip the get_orig_tn_from_msg() part, so the provided number immediately jumps to the next step, of checking validity.

To me, providing $fU or $fn (whichever is the E.164) as the auth/validate function parameter seems like an optimal solution for implementing MAN STIR/SHAKEN, without even having to change the code.

StellaTeam commented 9 months ago

Hello @liviuchircu

Thank you for your reply.

Using fn parameter and script functions actually do works, and i use it as i had no other alternatives. But i use to think compiled functions are much more optimized than scripts actions... that's what i mean with "much less efficient". Maybe i'm wrong..

I only suggested to alter stir/shaken code in order to keep OpenSIPS performance at its best :)

If you consider using script actions to make the check and validation is as efficient as using stir/shaken compiled function, that's good for me, and we can forget this issue ;)

Thank you

github-actions[bot] commented 9 months ago

Any updates here? No progress has been made in the last 15 days, marking as stale. Will close this issue if no further updates are made in the next 30 days.