Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inline function suip2a() (ip_addr.h) not working for ipv6 #379

Closed
topotomaso opened this issue Oct 23, 2015 · 2 comments
Closed

inline function suip2a() (ip_addr.h) not working for ipv6 #379

topotomaso opened this issue Oct 23, 2015 · 2 comments

Comments

@topotomaso
Copy link

I observed that the inline function suip2a() (ip_addr.h) contains a silly bug and therefor does not work for IPv6 conversions. This silly bug is preventing sip_trace() to work in the onsend_route block for IPv6 sip messages.
Rem.: The function suip2a() is used only by the siptrace module.

Test background (just for understanding the use case):

  • using kamailio 4.1.3 (with an additional patch to let onsend_route be called also for replies taken from kamailio 4.2.x)
  • kamailio is used as loadbalancer in stateless mode (using forward() only)
  • sip tracing is used by explictly calling sip_trace() in onsend_route, route and onreply routing block to be able to implement an interface specific sip tracing (no SIP transaction based sip tracing is applied by using the siptrace's trace_flag)

The silly bug:

In suip2a() the function ip6tosbuf() is called with an odd buffer length argument of value "sizeof(buf)-4" which is equal to "IP6_MAX_STR_SIZE -1". ip6tosbuf() however needs at least a buffer length of IP6_MAX_STR_SIZE (otherwise it returns immediately without writing anything into the buffer).
So regarding IPv6 adresses the ascii output of suip2a() is alway an empty IPv6 reference ([]).

static inline int ip6tosbuf(unsigned char* ip6, char* buff, int len)
{
    int offset;
    register unsigned char a,b,c;
    register unsigned char d;
    register unsigned short hex4;
    int r;
    #define HEXDIG(x) (((x)>=10)?(x)-10+'A':(x)+'0')


    offset=0;
    if (unlikely(len<IP6_MAX_STR_SIZE))
        return 0;
    ...
}

#define SUIP2A_MAX_STR_SIZE  (IP6_MAX_STR_SIZE + 2 /* [] */ + 1 /* \0 */)
/* returns an asciiz string containing the ip
 *  (<ipv4_addr> or [<ipv6_addr>])
 */
static inline char* suip2a(union sockaddr_union* su, int su_len)
{
    static char buf[SUIP2A_MAX_STR_SIZE];
    int offs;

    if (unlikely(su->s.sa_family==AF_INET6)){
        if (unlikely(su_len<sizeof(su->sin6)))
            return "<addr. error>";
        buf[0]='[';
        offs=1+ip6tosbuf((unsigned char*)su->sin6.sin6_addr.s6_addr, &buf[1],
                            sizeof(buf)-4);
        buf[offs]=']';
        offs++;
    }else
    if (unlikely(su_len<sizeof(su->sin)))
        return "<addr. error>";
    else
        offs=ip4tosbuf((unsigned char*)&su->sin.sin_addr, buf, sizeof(buf)-2);
    buf[offs]=0;
    return buf;
}

A possible verfied patch looks like this (using this patch sip_trace() is working now as expected in this use case).

--- ip_addr.h   2014-04-24 12:02:35.000000000 +0200
+++ ip_addr.h   2015-10-22 18:48:40.553177206 +0200
@@ -737,14 +737,14 @@
            return "<addr. error>";
        buf[0]='[';
        offs=1+ip6tosbuf((unsigned char*)su->sin6.sin6_addr.s6_addr, &buf[1],
-                           sizeof(buf)-4);
+                           IP6_MAX_STR_SIZE);
        buf[offs]=']';
        offs++;
    }else
    if (unlikely(su_len<sizeof(su->sin)))
        return "<addr. error>";
    else
-       offs=ip4tosbuf((unsigned char*)&su->sin.sin_addr, buf, sizeof(buf)-2);
+       offs=ip4tosbuf((unsigned char*)&su->sin.sin_addr, buf, IP4_MAX_STR_SIZE);
    buf[offs]=0;
    return buf;
 }

@miconda
Copy link
Member

miconda commented Oct 23, 2015

Thanks for report, troubleshooting and proposing the fix.

Can you make a pull request with your patch? It will be easier to merge. Just see the commit message guidelines at:

@miconda
Copy link
Member

miconda commented Oct 26, 2015

Fixed in master branch, will be backported.

@miconda miconda closed this as completed Oct 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants