- Requests after a DNS failover are constructed from the outgoing

message buffer of the failed branch instead of from the incomming
request.
- reparse_on_dns_failover module parameter is introduced.
Closes SER-300

diff --git a/modules/tm/doc/params.xml b/modules/tm/doc/params.xml
index 4b7b07d..4c4ff78 100644 (file)
--- a/modules/tm/doc/params.xml
+++ b/modules/tm/doc/params.xml
@@ -477,4 +477,42 @@ modparam("tm", "cancel_b_method", 1)
        </example>
     </section>
 
        </example>
     </section>
 

+    <section id="reparse_on_dns_failover">
+       <title><varname>reparse_on_dns_failover</varname> (integer)</title>
+       <para>
+               If set to 1, the SIP message after a DNS failover is constructed
+               from the outgoing message buffer of the failed branch instead of
+               from the received request.
+       </para>
+       <para>
+               It must be set if multiple branches are installed, the SIP message is
+               modified differently in them, and at least one of them can result
+               in DNS failover. If the parameter is not set the per-branch modifications
+               are lost after the failover.
+       </para>
+       <para>
+               Note: If the parameter is set, branch route block and TMCB_REQUEST_FWDED
+               callback are not called in case of the failover.
+       </para>
+       <para>
+               Disadvantage: only the via header is replaced in the message buffer, so
+               the outgoing socket address is not corrected in any other part of the message.
+               It is dangerous on multihomed hosts: when the new SIP request after
+               the DNS failover is sent via different interface than the first request,
+               the message can contain incorrect ip address in the Record-Route header
+               for instance.
+       </para>
+       <para>
+               Default value is 1.
+       </para>
+       <example>
+           <title>Set <varname>reparse_on_dns_failover</varname> parameter</title>
+           <programlisting>
+...
+modparam("tm", "reparse_on_dns_failover", 0)
+...
+           </programlisting>
+       </example>
+    </section>
+

 </section>
 </section>

diff --git a/modules/tm/lw_parser.c b/modules/tm/lw_parser.c
index 8756db6..b6153c6 100644 (file)
--- a/modules/tm/lw_parser.c
+++ b/modules/tm/lw_parser.c
@@ -255,3 +255,27 @@ char *lw_next_line(char *buf, char *buf_end)
 
        return c;
 }
 
        return c;
 }

+
+#ifdef USE_DNS_FAILOVER
+/* returns the pointer to the first VIA header */
+char *lw_find_via(char *buf, char *buf_end)
+{
+       char            *p;
+       unsigned int    val;
+
+       /* skip the first line */
+       p = eat_line(buf, buf_end - buf);
+
+       while (buf_end - p > 4) {
+               val = LOWER_DWORD(READ(p));
+               if ((val == _via1_) || (val == _via2_)
+               || ((LOWER_BYTE(*p) == 'v')             /* compact header */
+                       && ((*(p+1) == ' ') || (*(p+1) == ':')) )
+                               ) return p;
+
+               p = lw_next_line(p, buf_end);
+       }
+       /* not found */
+       return 0;
+}
+#endif

diff --git a/modules/tm/lw_parser.h b/modules/tm/lw_parser.h
index 902084f..6ab81fe 100644 (file)
--- a/modules/tm/lw_parser.h
+++ b/modules/tm/lw_parser.h
@@ -43,4 +43,9 @@ char *lw_get_hf_name(char *begin, char *end,
 /* returns a pointer to the next line */
 char *lw_next_line(char *buf, char *buf_end);
 
 /* returns a pointer to the next line */
 char *lw_next_line(char *buf, char *buf_end);
 

+#ifdef USE_DNS_FAILOVER
+/* returns the pointer to the first VIA header */
+char *lw_find_via(char *buf, char *buf_end);
+#endif
+

 #endif /* _LW_PARSER_H */
 #endif /* _LW_PARSER_H */

diff --git a/modules/tm/t_fwd.c b/modules/tm/t_fwd.c
index b3dc391..5bce80c 100644 (file)
--- a/modules/tm/t_fwd.c
+++ b/modules/tm/t_fwd.c
@@ -72,6 +72,11 @@
  *              t_relay_cancel() introduced -- can be used to relay CANCELs
  *              at the beginning of the script. (Miklos)
  * 2007-06-04  running transaction are canceled hop by hop (andrei)
  *              t_relay_cancel() introduced -- can be used to relay CANCELs
  *              at the beginning of the script. (Miklos)
  * 2007-06-04  running transaction are canceled hop by hop (andrei)

+ *  2007-08-37  In case of DNS failover the new SIP message is constructed
+ *              from the message buffer of the failed branch instead of
+ *              applying the lumps again, because the per-branch lumps are not saved,
+ *              thus, are not available. Set reparse_on_dns_failover to 0 to
+ *              revert the change. (Miklos)

  */
 
 #include "defs.h"
  */
 
 #include "defs.h"


@@ -103,6 +108,8 @@
 #ifdef USE_DNS_FAILOVER
 #include "../../dns_cache.h"
 #include "../../cfg_core.h" /* cfg_get(core, core_cfg, use_dns_failover) */
 #ifdef USE_DNS_FAILOVER
 #include "../../dns_cache.h"
 #include "../../cfg_core.h" /* cfg_get(core, core_cfg, use_dns_failover) */

+#include "../../msg_translator.h"
+#include "lw_parser.h"

 #endif
 #ifdef USE_DST_BLACKLIST
 #include "../../dst_blacklist.h"
 #endif
 #ifdef USE_DST_BLACKLIST
 #include "../../dst_blacklist.h"


@@ -235,6 +242,82 @@ error01:
        return shbuf;
 }
 
        return shbuf;
 }
 

+#ifdef USE_DNS_FAILOVER
+/* Similar to print_uac_request(), but this function uses the outgoing message buffer of
+   the failed branch to construt the new message in case of DNS failover.
+
+   WARNING: only the first VIA header is replaced in the buffer, the rest
+   of the message is untuched, thus, the send socket is corrected only in the VIA HF.
+*/
+static char *print_uac_request_from_buf( struct cell *t, struct sip_msg *i_req,
+       int branch, str *uri, unsigned int *len, struct dest_info* dst,
+       char *buf, short buf_len)
+{
+       char *shbuf;
+       str branch_str;
+       char *via, *old_via_begin, *old_via_end;
+       unsigned int via_len;
+
+       shbuf=0;
+
+       /* ... we calculate branch ... */       
+       if (!t_calc_branch(t, branch, i_req->add_to_branch_s,
+                       &i_req->add_to_branch_len ))
+       {
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: branch computation failed\n");
+               goto error00;
+       }
+       branch_str.s = i_req->add_to_branch_s;
+       branch_str.len = i_req->add_to_branch_len;
+
+       /* find the beginning of the first via header in the buffer */
+       old_via_begin = lw_find_via(buf, buf+buf_len);
+       if (!old_via_begin) {
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: beginning of via header not found\n");
+               goto error00;
+       }
+       /* find the end of the first via header in the buffer */
+       old_via_end = lw_next_line(old_via_begin, buf+buf_len);
+       if (!old_via_end) {
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: end of via header not found\n");
+               goto error00;
+       }
+
+       /* create the new VIA HF */
+       via = create_via_hf(&via_len, i_req, dst, &branch_str);
+       if (!via) {
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: via building failed\n");
+               goto error00;
+       }
+
+       /* allocate memory for the new buffer */
+       *len = buf_len + via_len - (old_via_end - old_via_begin);
+       shbuf=(char *)shm_malloc(*len);
+       if (!shbuf) {
+               ser_error=E_OUT_OF_MEM;
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: no shmem\n");
+               goto error01;
+       }
+
+       /* construct the new buffer */
+       memcpy(shbuf, buf, old_via_begin-buf);
+       memcpy(shbuf+(old_via_begin-buf), via, via_len);
+       memcpy(shbuf+(old_via_begin-buf)+via_len, old_via_end, (buf+buf_len)-old_via_end);
+
+#ifdef DBG_MSG_QA
+       if (shbuf[*len-1]==0) {
+               LOG(L_ERR, "ERROR: print_uac_request_from_buf: sanity check failed\n");
+               abort();
+       }
+#endif
+
+error01:
+       pkg_free(via);
+error00:
+       return shbuf;
+}
+#endif
+

 /* introduce a new uac, which is blind -- it only creates the
    data structures and starts FR timer, but that's it; it does
    not print messages and send anything anywhere; that is good
 /* introduce a new uac, which is blind -- it only creates the
    data structures and starts FR timer, but that's it; it does
    not print messages and send anything anywhere; that is good


@@ -375,6 +458,76 @@ error:
 
 
 #ifdef USE_DNS_FAILOVER
 
 
 #ifdef USE_DNS_FAILOVER

+/* Similar to add_uac(), but this function uses the outgoing message buffer of
+   the failed branch to construt the new message in case of DNS failover.
+*/
+static int add_uac_from_buf( struct cell *t, struct sip_msg *request, str *uri, int proto,
+                       char *buf, short buf_len)
+{
+
+       int ret;
+       unsigned short branch;
+       char *shbuf;
+       unsigned int len;
+
+       branch=t->nr_of_outgoings;
+       if (branch==MAX_BRANCHES) {
+               LOG(L_ERR, "ERROR: add_uac_from_buf: maximum number of branches exceeded\n");
+               ret=ser_error=E_TOO_MANY_BRANCHES;
+               goto error;
+       }
+
+       /* check existing buffer -- rewriting should never occur */
+       if (t->uac[branch].request.buffer) {
+               LOG(L_CRIT, "ERROR: add_uac_from_buf: buffer rewrite attempt\n");
+               ret=ser_error=E_BUG;
+               goto error;
+       }
+
+       if (uri2dst(&t->uac[branch].dns_h, &t->uac[branch].request.dst,
+                               request, uri, proto) == 0)
+       {
+               ret=ser_error=E_BAD_ADDRESS;
+               goto error;
+       }
+       
+       /* check if send_sock is ok */
+       if (t->uac[branch].request.dst.send_sock==0) {
+               LOG(L_ERR, "ERROR: add_uac_from_buf: can't fwd to af %d, proto %d "
+                       " (no corresponding listening socket)\n",
+                       t->uac[branch].request.dst.to.s.sa_family, 
+                       t->uac[branch].request.dst.proto );
+               ret=ser_error=E_NO_SOCKET;
+               goto error;
+       }
+
+       /* now message printing starts ... */
+       shbuf=print_uac_request_from_buf( t, request, branch, uri, 
+                                                       &len, &t->uac[branch].request.dst,
+                                                       buf, buf_len);
+       if (!shbuf) {
+               ret=ser_error=E_OUT_OF_MEM;
+               goto error;
+       }
+
+       /* things went well, move ahead and install new buffer! */
+       t->uac[branch].request.buffer=shbuf;
+       t->uac[branch].request.buffer_len=len;
+       t->uac[branch].uri.s=t->uac[branch].request.buffer+
+               request->first_line.u.request.method.len+1;
+       t->uac[branch].uri.len=uri->len;
+       membar_write(); /* to allow lockless ops (e.g. which_cancel()) we want
+                                          to be sure everything above is fully written before
+                                          updating branches no. */
+       t->nr_of_outgoings=(branch+1);
+
+       /* done! */
+       ret=branch;
+               
+error:
+       return ret;
+}
+

 /* introduce a new uac to transaction, based on old_uac and a possible
  *  new ip address (if the dns name resolves to more ips). If no more
  *   ips are found => returns -1.
 /* introduce a new uac to transaction, based on old_uac and a possible
  *  new ip address (if the dns name resolves to more ips). If no more
  *   ips are found => returns -1.


@@ -420,12 +573,23 @@ int add_uac_dns_fallback( struct cell *t, struct sip_msg* msg,
                        /* copy the dns handle into the new uac */
                        dns_srv_handle_cpy(&t->uac[t->nr_of_outgoings].dns_h,
                                                                &old_uac->dns_h);
                        /* copy the dns handle into the new uac */
                        dns_srv_handle_cpy(&t->uac[t->nr_of_outgoings].dns_h,
                                                                &old_uac->dns_h);

-                       /* add_uac will use dns_h => next_hop will be ignored.
-                        * Unfortunately we can't reuse the old buffer, the branch id
-                        *  must be changed and the send_socket might be different =>
-                        *  re-create the whole uac */
-                       ret=add_uac(t,  msg, &old_uac->uri, 0, 0, 
+
+                       if (cfg_get(tm, tm_cfg, reparse_on_dns_failover))
+                               /* Reuse the old buffer and only replace the via header.
+                                * The drowback is that the send_socket is not corrected
+                                * in the rest of the message, only in the VIA HF (Miklos) */
+                               ret=add_uac_from_buf(t,  msg, &old_uac->uri, 
+                                                       old_uac->request.dst.proto,
+                                                       old_uac->request.buffer,
+                                                       old_uac->request.buffer_len);
+                       else
+                               /* add_uac will use dns_h => next_hop will be ignored.
+                                * Unfortunately we can't reuse the old buffer, the branch id
+                                *  must be changed and the send_socket might be different =>
+                                *  re-create the whole uac */
+                               ret=add_uac(t,  msg, &old_uac->uri, 0, 0, 

                                                        old_uac->request.dst.proto);
                                                        old_uac->request.dst.proto);

+

                        if (ret<0){
                                /* failed, delete the copied dns_h */
                                dns_srv_handle_put(&t->uac[t->nr_of_outgoings].dns_h);
                        if (ret<0){
                                /* failed, delete the copied dns_h */
                                dns_srv_handle_put(&t->uac[t->nr_of_outgoings].dns_h);


@@ -1099,3 +1263,16 @@ int t_replicate(struct sip_msg *p_msg,  struct proxy_l *proxy, int proto )
        */
        return t_relay_to(p_msg, proxy, proto, 1 /* replicate */);
 }
        */
        return t_relay_to(p_msg, proxy, proto, 1 /* replicate */);
 }

+
+/* fixup function for reparse_on_dns_failover modparam */
+int reparse_on_dns_failover_fixup(void *handle, str *name, void **val)
+{
+#ifdef USE_DNS_FAILOVER
+       if ((int)(long)(*val) && mhomed) {
+               LOG(L_WARN, "WARNING: reparse_on_dns_failover_fixup:"
+               "reparse_on_dns_failover is enabled on a "
+               "multihomed host -- check the readme of tm module!\n");
+       }
+#endif
+       return 0;
+}

diff --git a/modules/tm/t_fwd.h b/modules/tm/t_fwd.h
index 1b83007..d842304 100644 (file)
--- a/modules/tm/t_fwd.h
+++ b/modules/tm/t_fwd.h
@@ -73,6 +73,7 @@ int t_send_branch( struct cell *t, int branch, struct sip_msg* p_msg ,
                                        struct proxy_l * proxy, int lock_replies);
 int t_relay_cancel(struct sip_msg* p_msg);
 
                                        struct proxy_l * proxy, int lock_replies);
 int t_relay_cancel(struct sip_msg* p_msg);
 

+int reparse_on_dns_failover_fixup(void *handle, str *name, void **val);

 
 #endif
 
 
 #endif
 

diff --git a/modules/tm/tm.c b/modules/tm/tm.c
index 234eb35..f04b2c1 100644 (file)
--- a/modules/tm/tm.c
+++ b/modules/tm/tm.c
@@ -106,6 +106,7 @@
 #include "../../route_struct.h"
 #include "../../route.h"
 #include "../../cfg/cfg.h"
 #include "../../route_struct.h"
 #include "../../route.h"
 #include "../../cfg/cfg.h"

+#include "../../globals.h"

 
 #include "config.h"
 #include "sip_msg.h"
 
 #include "config.h"
 #include "sip_msg.h"


@@ -381,6 +382,7 @@ static param_export_t params[]={
        {"blst_methods_add",    PARAM_INT, &default_tm_cfg.tm_blst_methods_add   },
        {"blst_methods_lookup", PARAM_INT, &default_tm_cfg.tm_blst_methods_lookup},
        {"cancel_b_method",     PARAM_INT, &default_tm_cfg.cancel_b_flags},
        {"blst_methods_add",    PARAM_INT, &default_tm_cfg.tm_blst_methods_add   },
        {"blst_methods_lookup", PARAM_INT, &default_tm_cfg.tm_blst_methods_lookup},
        {"cancel_b_method",     PARAM_INT, &default_tm_cfg.cancel_b_flags},

+       {"reparse_on_dns_failover", PARAM_INT, &default_tm_cfg.reparse_on_dns_failover},

        {0,0,0}
 };
 
        {0,0,0}
 };
 


@@ -612,6 +614,14 @@ static int mod_init(void)
                return -1;
        }
 
                return -1;
        }
 

+#ifdef USE_DNS_FAILOVER
+       if (default_tm_cfg.reparse_on_dns_failover && mhomed) {
+               LOG(L_WARN, "WARNING: mod_init: "
+                       "reparse_on_dns_failover is enabled on a "
+                       "multihomed host -- check the readme of tm module!\n");
+       }
+#endif
+

        /* declare the configuration */
        if (cfg_declare("tm", tm_cfg_def, &default_tm_cfg, cfg_size(tm),
                         &tm_cfg)) {
        /* declare the configuration */
        if (cfg_declare("tm", tm_cfg_def, &default_tm_cfg, cfg_size(tm),
                         &tm_cfg)) {

diff --git a/msg_translator.c b/msg_translator.c
index d966af8..06e3879 100644 (file)
--- a/msg_translator.c
+++ b/msg_translator.c
@@ -59,6 +59,8 @@
  * 2006-04-20  build_req_from_sip_req, via_builder and lump_* functions
  *              use now struct dest_info; lumps & via comp param support
  *              (rfc3486) (andrei)
  * 2006-04-20  build_req_from_sip_req, via_builder and lump_* functions
  *              use now struct dest_info; lumps & via comp param support
  *              (rfc3486) (andrei)

+ * 2007-08-31  id_builder() and via_builder() are grouped into one function:
+ *             create_via_hf() -- tm module needs them as well (Miklos)

  *
  */
 /* Via special params:
  *
  */
 /* Via special params:


@@ -1427,20 +1429,8 @@ char * build_req_buf_from_sip_req( struct sip_msg* msg,
        struct lump* anchor;
        struct lump* via_insert_param;
        str branch;
        struct lump* anchor;
        struct lump* via_insert_param;
        str branch;

-       str extra_params;
-       struct hostport hp;

 
 

-#ifdef USE_TCP
-       char* id_buf;
-       unsigned int id_len;
-
-
-       id_buf=0;
-       id_len=0;
-#endif

        via_insert_param=0;
        via_insert_param=0;

-       extra_params.len=0;
-       extra_params.s=0;

        uri_len=0;
        buf=msg->buf;
        len=msg->len;
        uri_len=0;
        buf=msg->buf;
        len=msg->len;


@@ -1451,25 +1441,6 @@ char * build_req_buf_from_sip_req( struct sip_msg* msg,
        rport_buf=0;
        line_buf=0;
 
        rport_buf=0;
        line_buf=0;
 

-#ifdef USE_TCP
-       /* add id if tcp */
-       if (msg->rcv.proto==PROTO_TCP
-#ifdef USE_TLS
-                       || msg->rcv.proto==PROTO_TLS
-#endif
-                       ){
-               if  ((id_buf=id_builder(msg, &id_len))==0){
-                       LOG(L_ERR, "ERROR: build_req_buf_from_sip_req:"
-                                                       " id_builder failed\n");
-                       goto error00; /* we don't need to free anything,
-                                        nothing alloc'ed yet*/
-               }
-               DBG("build_req_from_req: id added: <%.*s>, rcv proto=%d\n",
-                               (int)id_len, id_buf, msg->rcv.proto);
-               extra_params.s=id_buf;
-               extra_params.len=id_len;
-       }
-#endif

             /* Calculate message body difference and adjust
              * Content-Length
              */
             /* Calculate message body difference and adjust
              * Content-Length
              */


@@ -1480,11 +1451,11 @@ char * build_req_buf_from_sip_req( struct sip_msg* msg,
                goto error00;
        }
 
                goto error00;
        }
 

+       /* create a the via header */

        branch.s=msg->add_to_branch_s;
        branch.len=msg->add_to_branch_len;
        branch.s=msg->add_to_branch_s;
        branch.len=msg->add_to_branch_len;

-       set_hostport(&hp, msg);
-       line_buf = via_builder( &via_len, send_info, &branch,
-                                                       extra_params.len?&extra_params:0, &hp);
+
+       line_buf = create_via_hf( &via_len, msg, send_info, &branch);

        if (!line_buf){
                LOG(L_ERR,"ERROR: build_req_buf_from_sip_req: no via received!\n");
                goto error00;
        if (!line_buf){
                LOG(L_ERR,"ERROR: build_req_buf_from_sip_req: no via received!\n");
                goto error00;


@@ -1615,11 +1586,6 @@ char * build_req_buf_from_sip_req( struct sip_msg* msg,
 #endif
 
        *returned_len=new_len;
 #endif
 
        *returned_len=new_len;

-       /* cleanup */
-#ifdef USE_TCP
-       if (id_buf) pkg_free(id_buf); /* it's not in a lump => we don't need it
-                                                                        anymore */
-#endif

        return new_buf;
 
 error01:
        return new_buf;
 
 error01:


@@ -1629,9 +1595,6 @@ error02:
 error03:
        if (rport_buf) pkg_free(rport_buf);
 error00:
 error03:
        if (rport_buf) pkg_free(rport_buf);
 error00:

-#ifdef USE_TCP
-       if (id_buf) pkg_free(id_buf);
-#endif

        *returned_len=0;
        return 0;
 }
        *returned_len=0;
        return 0;
 }


@@ -2189,6 +2152,60 @@ char* via_builder( unsigned int *len,
        return line_buf;
 }
 
        return line_buf;
 }
 

+/* creates a via header honoring the protocol of the incomming socket
+ * msg is an optional parameter */
+char* create_via_hf( unsigned int *len,
+       struct sip_msg *msg,
+       struct dest_info* send_info /* where to send the reply */,
+       str* branch)
+{
+       char* via;
+       str extra_params;
+       struct hostport hp;
+#ifdef USE_TCP
+       char* id_buf;
+       unsigned int id_len;
+
+
+       id_buf=0;
+       id_len=0;
+#endif
+       extra_params.len=0;
+       extra_params.s=0;
+
+
+#ifdef USE_TCP
+       /* add id if tcp */
+       if (msg
+       && ((msg->rcv.proto==PROTO_TCP)
+#ifdef USE_TLS
+                       || (msg->rcv.proto==PROTO_TLS)
+#endif
+                       )){
+               if  ((id_buf=id_builder(msg, &id_len))==0){
+                       LOG(L_ERR, "ERROR: create_via_hf:"
+                                                       " id_builder failed\n");
+                       return 0; /* we don't need to free anything,
+                                        nothing alloc'ed yet*/
+               }
+               DBG("create_via_hf: id added: <%.*s>, rcv proto=%d\n",
+                               (int)id_len, id_buf, msg->rcv.proto);
+               extra_params.s=id_buf;
+               extra_params.len=id_len;
+       }
+#endif
+
+       set_hostport(&hp, msg);
+       via = via_builder( len, send_info, branch,
+                                                       extra_params.len?&extra_params:0, &hp);
+
+#ifdef USE_TCP
+       /* we do not need id_buf any more, the id is already in the new via header */
+       if (id_buf) pkg_free(id_buf);
+#endif
+       return via;
+}
+

 /* builds a char* buffer from message headers without body
  * first line is excluded in case of skip_first_line=1
  * error is set -1 if the memory allocation failes
 /* builds a char* buffer from message headers without body
  * first line is excluded in case of skip_first_line=1
  * error is set -1 if the memory allocation failes

diff --git a/msg_translator.h b/msg_translator.h
index 0ee723c..c5e4f83 100644 (file)
--- a/msg_translator.h
+++ b/msg_translator.h
@@ -105,6 +105,12 @@ char* via_builder( unsigned int *len,
        struct dest_info* send_info,
        str *branch, str* extra_params, struct hostport *hp );
 
        struct dest_info* send_info,
        str *branch, str* extra_params, struct hostport *hp );
 

+/* creates a via header honoring the protocol of the incomming socket
+ * msg is an optional parameter */
+char* create_via_hf( unsigned int *len,
+       struct sip_msg *msg,
+       struct dest_info* send_info /* where to send the reply */,
+       str* branch);

 
 int branch_builder( unsigned int hash_index, 
        /* only either parameter useful */
 
 int branch_builder( unsigned int hash_index, 
        /* only either parameter useful */