diff --git a/ChangeLog b/ChangeLog index 75e99e9..f85e618 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,11 +1,18 @@ 0.8.3dev ======== + 30-Sep-2017: - plugin_fix_fbox_anoncall: better handling of multiple + registered numbers on same device + 27-Sep-2017: - resized URLMAP_SIZE=512, RTPPROXY_SIZE=1024 + - fixed some compile warnings and missing includes + - [dev6] 12-Aug-2017: - performance and code inprovements in plugin_blacklist + - logging timestamp now includes milliseconds 28-May-2017: - updated autogen/automake/libtool/libltdl to a recent version. Personal note: libtool/libltr is a steaming pile of fertilizer - a voodoo driven toolbox + 14-May-2017: - [dev4] 16-Apr-2017: - plugin_blacklist: new plugin to block UACs that cause - excessive failures durign REGISTER attempts. + excessive failures during REGISTER attempts. 27-Feb-2017: - improved memory behavior of some plugins during shutdown - fixed 2 minor memory leaks 02-Aug-2016: - rtpproxy_relay: more robustness when closing sockets. diff --git a/TODO b/TODO index 16bae4c..71ffce2 100644 --- a/TODO +++ b/TODO @@ -1,6 +1,29 @@ TODOs, in random order: ======================= +- REGISTER: updatinf the registration table + currently done upon passing an REGISTER request through, independent of + the outcome of the registration, the registration entry will be put into + the table. + IF a client now tries to register a non-existent number, a fake entry ends + up in the table and may mess things up. + Solution: The lifetime of a REGISTER should be set at receiving the successful + REGISTER Response - before the successful response (at least initially) the + entry should be regarded as invalid. + + 1) + REGISTER request only gives 5 seconds registration time + unsuccessful REGISTER response does nothing (clock ticking) + successful REGISTER response gives full registration time + -> this reduces the window of opportunity + + May want to do further: introduce negative grace time that makes REGISTER + dialog mapping work, but is ignored for all other methods + -> marking records as inactive upon -10 seconds (#define REGISTRATION_GRACE) + -> for REGISTER dialogs, have match if active && urlmatch + -> for all other dialogs, have match if active && expiration>0 && urlmatch + + - check via loop and private IP addresses can comment be used to store a unique ID in there? diff --git a/src/plugin_fix_fbox_anoncall.c b/src/plugin_fix_fbox_anoncall.c index 8fad6c7..d36b773 100644 --- a/src/plugin_fix_fbox_anoncall.c +++ b/src/plugin_fix_fbox_anoncall.c @@ -100,13 +100,21 @@ int PLUGIN_INIT(plugin_def_t *plugin_def) { int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ /* stage contains the PLUGIN_* value - the stage of SIP processing. */ int type; - osip_contact_t *contact; + osip_contact_t *contact=NULL; + osip_uri_t *to_url=NULL; int idx=0; - int param_match_idx=0; - int user_match=0; + int full_match=0; int param_match=0; + int param_match_idx=0; + int to_user_match=0; + int to_user_match_idx=0; char *tmp=NULL; + if (ticket == NULL) { + ERROR("being called with ticket == NULL"); + return STS_FAILURE; + } + type = ticket->direction; DEBUGC(DBCLASS_PLUGIN, "PLUGIN_PROCESS entered: type=%i", type); @@ -128,23 +136,30 @@ int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ return STS_SUCCESS; } + if (ticket->sipmsg && ticket->sipmsg->to && ticket->sipmsg->to->url) { + to_url=ticket->sipmsg->to->url; + } + /* - loop through URLMAP table - compare IP, param, if match - set partial match - compare username, if match - all OK with this Contact header, return from plugin - if no match - probably broken username part in header (as rest matches) - remember urlmap index - end loop - if partial match == 0 - unable to figure out how to fix contact header. - dump some info - if partial match == 1 - replace username part from urlmap[saved_index].true_url - return from plugin + - loop through URLMAP table + - skip if IP does not match + - set full_match if IP and user do match and break out of loop + - param_match++ if uniq= matches + - user_match++ if to: user matches + - end loop + - if full_match, then we do not need to fiddle around with the + contact header, it has not been anonymized. + - if param_match==1, we have found a possibly matching entry + do fixup Contact header + - if to_user_match==1, we have found a possibly matching entry + do fixup Contact header + - if no param_match and no to_user_match, complain + - return from plugin + +Unfortunately, FritzBox does use the same 'uniq=' value for all lines, so if +having a Fritzbox using multiple lines we are screwed here. I may need to +find some additional markers that allow nailing the actual phone number */ @@ -157,12 +172,15 @@ int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ /* Sender IP is in list, fix check and fix Contact header */ DEBUGC(DBCLASS_PLUGIN, "checking for bogus Contact header"); + /* loop through urlmap table */ for (idx=0; idxtimestamp) continue; if (urlmap[idx].true_url == NULL) continue; - /* outgoing response - only look for true_url */ + /* outgoing response - only look at true_url */ + /* 1) check host, skip of no match */ if (contact->url->host && urlmap[idx].true_url->host) { if (osip_strcasecmp(contact->url->host, urlmap[idx].true_url->host) != 0) { @@ -186,12 +204,12 @@ int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ DEBUGC(DBCLASS_PLUGIN, "check username: " "contact->url->username [%s] <-> true_url->username [%s]", contact->url->username, urlmap[idx].true_url->username); - if (osip_strcasecmp(contact->url->username, urlmap[idx].true_url->username) == 0) { - /* MATCH, all OK - return */ - user_match=1; - DEBUGC(DBCLASS_PLUGIN, "username matches"); - break; - } + if (osip_strcasecmp(contact->url->username, urlmap[idx].true_url->username) == 0) { + /* MATCH, all OK - return */ + full_match=1; + DEBUGC(DBCLASS_PLUGIN, "username matches"); + break; + } } else { DEBUGC(DBCLASS_PLUGIN, "NULL username: " "contact->username 0x%p <-> true_url->username 0x%p", @@ -215,7 +233,7 @@ int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ if ((osip_strcasecmp(p1->gname, p2->gname) == 0) && (osip_strcasecmp(p1->gvalue, p2->gvalue) == 0) ) { /* MATCH */ - param_match=1; + param_match += 1; param_match_idx=idx; DEBUGC(DBCLASS_PLUGIN, "uniq param matches"); } @@ -230,32 +248,69 @@ int PLUGIN_PROCESS(int stage, sip_ticket_t *ticket){ p1, p2); } } + } /* */ + + /* 4) search for match on To: user field */ + if (to_url && to_url->username && urlmap[idx].true_url->username) { + DEBUGC(DBCLASS_PLUGIN, "check username: " + "to_url->username [%s] <-> true_url->username [%s]", + to_url->username, urlmap[idx].true_url->username); + if (osip_strcasecmp(to_url->username, urlmap[idx].true_url->username) == 0) { + /* MATCH, all OK - return */ + to_user_match += 1; + to_user_match_idx=idx; + DEBUGC(DBCLASS_PLUGIN, "To: username [%s] matches", to_url->username); + break; + } + } else { + DEBUGC(DBCLASS_PLUGIN, "NULL username: " + "to_url(0x%p)->username(0x%p) <-> true_url->username(0x%p)", + (to_url)?to_url:NULL, + (to_url && to_url->username)?to_url->username:NULL, + urlmap[idx].true_url->username); } - - } // for + + } /* for idx */ + + /* full match (host & user) */ - if (user_match == 1) { + if (full_match == 1) { DEBUGC(DBCLASS_PLUGIN, "PLUGIN_PROCESS exit: got a user@host match - OK"); return STS_SUCCESS; } - /* no partial match (no host, or no user / no param match) */ - if (param_match == 0) { - DEBUGC(DBCLASS_PLUGIN, "PLUGIN_PROCESS exit: bogus outgoing response Contact header from [%s], " - "unable to sanitize!", utils_inet_ntoa(ticket->from.sin_addr)); - return STS_SUCCESS; + /* partial match (uniq=) found */ + if (param_match == 1) { + /* replace the username part from [param_match_idx] -> Contact */ + osip_free(contact->url->username); + osip_uri_set_username(contact->url, + osip_strdup(urlmap[param_match_idx].true_url->username)); + + DEBUGC(DBCLASS_PLUGIN, "sanitized Contact from [%s] (uniq= match)", + utils_inet_ntoa(ticket->from.sin_addr)); + + + /* partial match To: user match found */ + } else if (to_user_match == 1) { + /* replace the username part from [to_user_match_idx] -> Contact */ + osip_free(contact->url->username); + osip_uri_set_username(contact->url, + osip_strdup(urlmap[to_user_match_idx].true_url->username)); + + DEBUGC(DBCLASS_PLUGIN, "sanitized Contact from [%s]" + " (To: user match)", utils_inet_ntoa(ticket->from.sin_addr)); + + + /* no matches at all -> log something */ + } else { + DEBUGC(DBCLASS_PLUGIN, "unable to sanitize bogus outgoing" + " response Contact header from [%s]" + " param_match=%i, to_user_match=%i", + utils_inet_ntoa(ticket->from.sin_addr), + param_match, to_user_match); } - /* param_match - replace the username part from [param_match_idx] -> Contact */ - /* replace contact URI (username part) */ - osip_free(contact->url->username); - osip_uri_set_username(contact->url, - osip_strdup(urlmap[param_match_idx].true_url->username)); - - DEBUGC(DBCLASS_PLUGIN, "sanitized Contact from [%s]", - utils_inet_ntoa(ticket->from.sin_addr)); - } else { DEBUGC(DBCLASS_PLUGIN, "no aclist IP match, returning."); diff --git a/src/plugin_siptrunk.c b/src/plugin_siptrunk.c index af99980..71ab97a 100644 --- a/src/plugin_siptrunk.c +++ b/src/plugin_siptrunk.c @@ -246,6 +246,7 @@ static int plugin_siptrunk_process(sip_ticket_t *ticket) { /* search for an Account entry in registration DB */ for (j=0; jtimestamp) continue; if (compare_url(url, urlmap[j].reg_url) == STS_SUCCESS) { DEBUGC(DBCLASS_PLUGIN, "plugin_siptrunk: found registered client, idx=%i",j); diff --git a/src/plugin_stats.c b/src/plugin_stats.c index d37458d..2156999 100644 --- a/src/plugin_stats.c +++ b/src/plugin_stats.c @@ -394,7 +394,9 @@ static void stats_prepare(void) { } for (i=0; i < URLMAP_SIZE; i++) { - if (urlmap[i].active == 1) { stats_num_reg_clients++; } + if ((urlmap[i].active == 1) && (urlmap[j].expires >= time(NULL))) { + stats_num_reg_clients++; + } } } diff --git a/src/register.c b/src/register.c index 44219d1..46f6f41 100644 --- a/src/register.c +++ b/src/register.c @@ -72,7 +72,7 @@ void register_init(void) { WARN("registration file not found, starting with empty table"); } else { /* read the url table from file */ - DEBUGC(DBCLASS_REG,"loading registration table"); + DEBUGC(DBCLASS_REG,"loading registration table, size=%i",URLMAP_SIZE); for (i=0;i < URLMAP_SIZE; i++) { fgets(buff, sizeof(buff), stream); sts=sscanf(buff, "****:%i:%i", &urlmap[i].active, &urlmap[i].expires); @@ -88,7 +88,7 @@ void register_init(void) { if (strlen(buff) > 0) {\ sts = osip_uri_parse(X, buff); \ if (sts != 0) { \ - ERROR("Unable to parse To URI: %s", buff); \ + ERROR("Unable to parse URI: %s", buff); \ osip_uri_free(X); \ X = NULL; \ } \ @@ -110,12 +110,10 @@ void register_init(void) { } fclose(stream); - /* check for premature abort of reading the registration file */ + /* check for premature abort of reading the registration file, + may happen if URLMAP_SIZE has been resized (bigger) */ if (i < URLMAP_SIZE) { - /* clean up and delete it */ - WARN("registration file corrupt, starting with empty table"); - memset(urlmap, 0, sizeof(urlmap)); - unlink(configuration.registrationfile); + WARN("registration file may be corrupt or URLMAP_SIZE has been resized"); } } } @@ -428,7 +426,7 @@ int register_client(sip_ticket_t *ticket, int force_lcl_masq) { /* * for proxying: force device to be masqueraded - * as with the outbound IP (masq_url) + * with the outbound IP (masq_url) */ if (force_lcl_masq) { struct in_addr addr; @@ -438,9 +436,11 @@ int register_client(sip_ticket_t *ticket, int force_lcl_masq) { return STS_FAILURE; } + /* do ensure that the host part (IP) is kept up to date + * as it might change */ /* host part */ addrstr = utils_inet_ntoa(addr); - DEBUGC(DBCLASS_REG,"masquerading UA %s@%s local %s@%s", + DEBUGC(DBCLASS_REG,"masquerading Contact %s@%s local %s@%s", (url1_contact->username) ? url1_contact->username : "*NULL*", (url1_contact->host) ? url1_contact->host : "*NULL*", (url1_contact->username) ? url1_contact->username : "*NULL*", @@ -455,13 +455,20 @@ int register_client(sip_ticket_t *ticket, int force_lcl_masq) { sprintf(urlmap[i].masq_url->port, "%i", configuration.sip_listen_port); } + + /* A proxied REGISTER request does not yet mean that this is a + * legit registration... So just give a few seconds here, + * enough to receive the REGISTER response, then upon success (2XX) + * the full expiration time will be given to this record. */ + /* &&& this may be reduced to 0 and left solely to the grace period */ +#define EXPIRE_NULL 5 + expires = EXPIRE_NULL; } - /* give some safety margin for the next update */ - if (expires > 0) expires+=30; - - /* update registration timeout */ - urlmap[i].expires=time_now+expires; + /* update registration timeout if we will give additional time */ + if (urlmap[i].expires < time_now+expires) { + urlmap[i].expires=time_now+expires; + } /* * un-REGISTER @@ -481,7 +488,7 @@ int register_client(sip_ticket_t *ticket, int force_lcl_masq) { DEBUGC(DBCLASS_REG, "removing registration for %s@%s at slot=%i", (url2_to->username) ? url2_to->username : "*NULL*", (url2_to->host) ? url2_to->host : "*NULL*", i); - urlmap[i].expires=0; + urlmap[i].expires=time_now+EXPIRE_NULL; break; } } @@ -501,11 +508,12 @@ void register_agemap(void) { int i; time_t t; +#define REGISTER_GRACE 5 /* expire old entries */ time(&t); DEBUGC(DBCLASS_BABBLE,"sip_agemap, t=%i",(int)t); for (i=0; iusername, urlmap[i].masq_url->host); urlmap[i].active=0; @@ -635,12 +643,25 @@ int register_set_expire(sip_ticket_t *ticket) { time_t time_now; osip_header_t *expires_hdr=NULL; osip_uri_param_t *expires_param=NULL; + osip_message_t *response; + + if (ticket == NULL) { + WARN("register_set_expire called with ticket == NULL"); + return STS_FAILURE; + } if (ticket->direction != RESTYP_INCOMING) { WARN("register_set_expire called with != incoming response"); return STS_FAILURE; } + if (ticket->sipmsg == NULL) { + WARN("register_set_expire called with ticket->sipmsg = NULL"); + return STS_FAILURE; + } + + response=ticket->sipmsg; + time(&time_now); DEBUGC(DBCLASS_REG,"REGISTER response, looking for 'Expire' information"); @@ -674,19 +695,24 @@ int register_set_expire(sip_ticket_t *ticket) { DEBUGC(DBCLASS_REG,"Expires=%i, expires_param=%p, expires_hdr=%p", expires, expires_param, expires_hdr); - if (expires > 0) { + /* if 2xx response (success), then set full registration time, + * else if not 2XX, then don't touch existing expires value */ + if (!MSG_IS_STATUS_2XX(response)) { + expires = 0; + } + if (expires > 0) { /* search for an entry */ for (i=0;iurl, urlmap[i].masq_url)==STS_SUCCESS)) break; - } + } /* for i */ /* found a mapping entry */ if (ihost, &tmp_addr) == STS_FAILURE) { DEBUGC(DBCLASS_SIP, "sip_find_direction: cannot resolve host [%s]", urlmap[i].true_url->host); @@ -1077,6 +1080,11 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { if (type == DIRTYP_UNKNOWN) { for (i=0; isipmsg,"REGISTER")) && + (urlmap[i].expires < ticket->timestamp)) continue; + /* RFC3261: * 'To' contains a display name (Bob) and a SIP or SIPS URI * (sip:bob@biloxi.com) towards which the request was originally @@ -1122,6 +1130,10 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { if ((type == DIRTYP_UNKNOWN) && (MSG_IS_REQUEST(ticket->sipmsg))) { for (i=0; isipmsg,"REGISTER")) && + (urlmap[i].expires < ticket->timestamp)) continue; /* incoming request (SIP URI == 'masq') || ((SIP URI == 'reg') && !REGISTER)*/ if ((compare_url(request->req_uri, urlmap[i].masq_url)==STS_SUCCESS) || (!MSG_IS_REGISTER(request) && @@ -1173,6 +1185,10 @@ int sip_find_direction(sip_ticket_t *ticket, int *urlidx) { if (sts == STS_SUCCESS) { for (i=0; isipmsg,"REGISTER")) && + (urlmap[i].expires < ticket->timestamp)) continue; /* incoming response (1st via in list points to a registered UA) */ sts=get_ip_by_host(urlmap[i].true_url->host, &addr_myself); if (sts == STS_FAILURE) {