STUN Binding Request verification truncates username & fails

LarryHemenway commented 4 years ago

FreeSWITCH Version 1.10.2-release.4~64bit (-release.4 64bit)

When sending in a STUN username of greater than 34 bytes, the STUN Binding request is ignored because it truncates the username.

The problem is in switch_rtp.c handle_ice() function:

static void handle_ice(switch_rtp_t *rtp_session, switch_rtp_ice_t *ice, void *data, switch_size_t len)
{
    ....
    char username[34] = { 0 };
    unsigned char buf[512] = { 0 };

For example, my STUN Binding request is

dTg40bESPYEAj58h:HkOwvylni/1IHG4YWcO+tC1PBu6QjWV+

but it gets truncated to

dTg40bESPYEAj58h:HkOwvylni/1IHG4YW

The function 'icecmp' compares the truncated value stored in username above to the full username stored in ice->user_ice and returns failure. The request is then ignored.

I looked at a RFC5389 STUN and found the following

The value of USERNAME is a variable-length value. It MUST contain a UTF-8 [RFC3629] encoded sequence of less than 513 bytes, and MUST have been processed using SASLprep [RFC4013].

The easiest solution would be to increase the username to conform to RFC5389 and make it 513 bytes. After doing that, the buf that contains the copied data would also need to increase. I have less analysis on this, but I think it makes sense to bump it up to a typical MTU of 1500. Combined, the updates would be

static void handle_ice(switch_rtp_t *rtp_session, switch_rtp_ice_t *ice, void *data, switch_size_t len)
{
    ....
    char username[513] = { 0 };   // conform with RFC5389
    unsigned char buf[1500] = { 0 }; // a typical MTU size

Other Potential Issue

When I type:

freeswitch@88d481f5770a> sofia loglevel stun 5

I get the following:

USAGE:
--------------------------------------------------------------------------------
sofia global siptrace <on|off>
sofia        capture  <on|off>
             watchdog <on|off>

sofia profile <name> [start | stop | restart | rescan] [wait]
                     flush_inbound_reg [<call_id> | <[user]@domain>] [reboot]
                     check_sync [<call_id> | <[user]@domain>]
                     [register | unregister] [<gateway name> | all]
                     killgw <gateway name>
                     [stun-auto-disable | stun-enabled] [true | false]]
                     siptrace <on|off>
                     capture  <on|off>
                     watchdog <on|off>

sofia <status|xmlstatus> profile <name> [reg [<contact str>]] | [pres <pres str>] | [user <user@domain>]
sofia <status|xmlstatus> gateway <name>

sofia loglevel <all|default|tport|iptsec|nea|nta|nth_client|nth_server|nua|soa|sresolv|stun> [0-9]
sofia tracelevel <console|alert|crit|err|warning|notice|info|debug>

sofia help
--------------------------------------------------------------------------------

Background:

We noticed this when using a test tool that uses GStreamer webrtc. Our tool is responding to BIND requests and sending good responses back, but its BIND requests are ignored by FreeSWITCH. After making a private build of FreeSWITCH with the updates above, everything works.

Other notes

The username appears to use the ufrag values for the SDP offer & answer with format "ufrag:ufrag". Looking at the draft-ietf-rtcweb-jsep-26 it indicates that handling ufrag should conform to draft-ietf-mmusic-ice-sip-sdp-24

For compatibility with the 512 character limitation for the STUN username attribute value and for bandwidth conservation considerations, the ice-ufrag attribute MUST NOT be longer than 32 characters when sending, but an implementation MUST accept up to 256 characters when receiving.

So, in theory, if FreeSWITCH conforms to never make a ufrag value of length > 32 bytes, we could make the username something like:

32 + 256 + 1 + 1 = 290 bytes

where

32 = FreeSWITCH ufrag value
256 = potential size of client's ufrag value
1 = the colon separator
1 = null terminator (safety)

LarryHemenway commented 4 years ago

Looking at the code, I believe these values need to be updated too:

SWITCH_DECLARE(switch_status_t) switch_rtp_activate_ice(switch_rtp_t *rtp_session, char *login, char *rlogin,
                                                        const char *password, const 
char *rpassword, ice_proto_t proto,
                                                    switch_core_media_ice_type_t 
type, ice_t *ice_params)
{
    char ice_user[80]; // make 513?
    char user_ice[80]; // make 513?
    char luser_ice[80]; // make 256?

ice_user & user_ice both appear to have the full "ufrag:ufrag" , but luser_ice appears to have only up the to ":" of user_ice:

ice_user +k8f:fGyeQ8r1tdGIkDLN, user_ice fGyeQ8r1tdGIkDLN:+k8f, luser_ice fGyeQ8r1tdGIkDLN+k8f

mjerris commented 4 years ago

feel free to toss us a pull request. stun trace in sofia has nothing to do with stun/ice in the media stack so that’s why that setting has no effect.

signalwire / freeswitch