Changeset 3028 for pjproject


Ignore:
Timestamp:
Dec 8, 2009 1:11:25 PM (14 years ago)
Author:
bennylp
Message:

Fixed ticket #999:

Several bug fixes to the TURN client library and icedemo sample application:

  1. ICE stream transport reports ICE initialization/candidate gathering stage as successful even when TURN client TCP connection has failed.
  2. Bad ChannelData? framing when TCP is used. PJNATH did not properly add padding to the TURN ChannelData? packet if TCP is used and the data is not aligned to four bytes boundary. Similarly incoming ChannelData? with padding (over TCP) may not be handled correctly.
  3. Incoming data over TCP may be delayed. PJNATH only processed one frame (be it request, indication, or ChannelData?) on an incoming stream, so if the stream contains more than one frames, the processing of subsequent frames will be delayed until more stream is received on the TCP transport.
  4. The icedemo sample application overwrites the incoming packet buffer with NULL character ('\0') before printing the message to console. If there is another packet after current packet (as often happens when TCP is used), the subsequent packet will get corrupted.

The combinations of bugs above may cause PJNATH to return "Invalid STUN message length (PJNATH_EINSTUNMSGLEN)" error when processing incoming TURN ChannelData? message over TCP.

And a small enhancement:

  1. Add logging to file option to icedemo sample.

Thanks Sarun Nandakumar for the report.

Location:
pjproject/trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • pjproject/trunk/pjnath/include/pjnath/ice_strans.h

    r2966 r3028  
    623623 
    624624/** 
     625 * Get the number of local candidates for the specified component ID. 
     626 * 
     627 * @param ice_st        The ICE stream transport. 
     628 * @param comp_id       Component ID. 
     629 * 
     630 * @return              The number of candidates. 
     631 */ 
     632PJ_DECL(unsigned) pj_ice_strans_get_cands_count(pj_ice_strans *ice_st, 
     633                                                unsigned comp_id); 
     634 
     635/** 
    625636 * Enumerate the local candidates for the specified component. 
    626637 * 

  • pjproject/trunk/pjnath/include/pjnath/turn_session.h

    r2642 r3028  
    456456 * 
    457457 * @param sess          The TURN client session. 
     458 * @param last_err      Optional error code to be set to the session, 
     459 *                      which would be returned back in the \a info 
     460 *                      parameter of #pj_turn_session_get_info(). If 
     461 *                      this argument value is PJ_SUCCESS, the error 
     462 *                      code will not be set. If the session already 
     463 *                      has an error code set, this function will not 
     464 *                      overwrite that error code either. 
    458465 * 
    459466 * @return              PJ_SUCCESS if the operation has been successful, 
    460467 *                      or the appropriate error code on failure. 
    461468 */ 
    462 PJ_DECL(pj_status_t) pj_turn_session_destroy(pj_turn_session *sess); 
     469PJ_DECL(pj_status_t) pj_turn_session_destroy(pj_turn_session *sess, 
     470                                             pj_status_t last_err); 
    463471 
    464472 

  • pjproject/trunk/pjnath/src/pjnath/ice_strans.c

    r2966 r3028  
    913913 
    914914/* 
     915 * Get number of candidates 
     916 */ 
     917PJ_DEF(unsigned) pj_ice_strans_get_cands_count(pj_ice_strans *ice_st, 
     918                                               unsigned comp_id) 
     919{ 
     920    unsigned i, cnt; 
     921 
     922    PJ_ASSERT_RETURN(ice_st && ice_st->ice && comp_id &&  
     923                     comp_id <= ice_st->comp_cnt, 0); 
     924 
     925    cnt = 0; 
     926    for (i=0; i<ice_st->ice->lcand_cnt; ++i) { 
     927        if (ice_st->ice->lcand[i].comp_id != comp_id) 
     928            continue; 
     929        ++cnt; 
     930    } 
     931 
     932    return cnt; 
     933} 
     934 
     935/* 
    915936 * Enum candidates 
    916937 */ 

  • pjproject/trunk/pjnath/src/pjnath/turn_session.c

    r2774 r3028  
    462462 * Forcefully destroy the TURN session. 
    463463 */ 
    464 PJ_DEF(pj_status_t) pj_turn_session_destroy( pj_turn_session *sess) 
     464PJ_DEF(pj_status_t) pj_turn_session_destroy( pj_turn_session *sess, 
     465                                             pj_status_t last_err) 
    465466{ 
    466467    PJ_ASSERT_RETURN(sess, PJ_EINVAL); 
    467468 
     469    if (last_err != PJ_SUCCESS && sess->last_status == PJ_SUCCESS) 
     470        sess->last_status = last_err; 
    468471    set_state(sess, PJ_TURN_STATE_DEALLOCATED); 
    469472    sess_shutdown(sess, PJ_SUCCESS); 
    … …  
    960963                           PJ_FALSE, PJ_FALSE); 
    961964    if (ch && ch->num != PJ_TURN_INVALID_CHANNEL && ch->bound) { 
     965        unsigned total_len; 
     966 
    962967        /* Peer is assigned a channel number, we can use ChannelData */ 
    963968        pj_turn_channel_data *cd = (pj_turn_channel_data*)sess->tx_pkt; 
    … …  
    965970        pj_assert(sizeof(*cd)==4); 
    966971 
    967         if (pkt_len > sizeof(sess->tx_pkt)-sizeof(*cd)) { 
     972        /* Calculate total length, including paddings */ 
     973        total_len = (pkt_len + sizeof(*cd) + 3) & (~3); 
     974        if (total_len > sizeof(sess->tx_pkt)) { 
    968975            status = PJ_ETOOBIG; 
    969976            goto on_return; 
    … …  
    976983        pj_assert(sess->srv_addr != NULL); 
    977984 
    978         status = sess->cb.on_send_pkt(sess, sess->tx_pkt, pkt_len+sizeof(*cd), 
     985        status = sess->cb.on_send_pkt(sess, sess->tx_pkt, total_len, 
    979986                                      sess->srv_addr, 
    980987                                      pj_sockaddr_get_len(sess->srv_addr)); 
    … …  
    11571164        } else { 
    11581165            if (parsed_len) { 
    1159                 *parsed_len = cd.length + sizeof(cd); 
     1166                /* Apply padding too */ 
     1167                *parsed_len = ((cd.length + 3) & (~3)) + sizeof(cd); 
    11601168            } 
    11611169        } 

  • pjproject/trunk/pjnath/src/pjnath/turn_sock.c

    r2966 r3028  
    274274                     pj_status_t status) 
    275275{ 
    276     char errmsg[PJ_ERR_MSG_SIZE]; 
    277  
    278     if (status != PJ_SUCCESS) { 
    279         pj_strerror(status, errmsg, sizeof(errmsg)); 
    280         PJ_LOG(4,(turn_sock->obj_name, "%s: %s", title, errmsg)); 
    281     } else { 
    282         PJ_LOG(4,(turn_sock->obj_name, "%s", title, errmsg)); 
    283     } 
     276    PJ_PERROR(4,(turn_sock->obj_name, status, title)); 
    284277} 
    285278 
    … …  
    289282{ 
    290283    show_err(turn_sock, title, status); 
    291     if (turn_sock->sess) 
    292         pj_turn_session_destroy(turn_sock->sess); 
     284    if (turn_sock->sess) { 
     285        pj_turn_session_destroy(turn_sock->sess, status); 
     286    } 
    293287} 
    294288 
    … …  
    495489} 
    496490 
     491static pj_uint16_t GETVAL16H(const pj_uint8_t *buf, unsigned pos) 
     492{ 
     493    return (pj_uint16_t) ((buf[pos + 0] << 8) | \ 
     494                          (buf[pos + 1] << 0)); 
     495} 
     496 
     497/* Quick check to determine if there is enough packet to process in the 
     498 * incoming buffer. Return the packet length, or zero if there's no packet. 
     499 */ 
     500static unsigned has_packet(pj_turn_sock *turn_sock, const void *buf, pj_size_t bufsize) 
     501{ 
     502    pj_bool_t is_stun; 
     503 
     504    if (turn_sock->conn_type == PJ_TURN_TP_UDP) 
     505        return bufsize; 
     506 
     507    /* Quickly check if this is STUN message, by checking the first two bits and 
     508     * size field which must be multiple of 4 bytes 
     509     */ 
     510    is_stun = ((((pj_uint8_t*)buf)[0] & 0xC0) == 0) && 
     511              ((GETVAL16H((const pj_uint8_t*)buf, 2) & 0x03)==0); 
     512 
     513    if (is_stun) { 
     514        pj_size_t msg_len = GETVAL16H((const pj_uint8_t*)buf, 2); 
     515        return (msg_len+20 <= bufsize) ? msg_len+20 : 0; 
     516    } else { 
     517        /* This must be ChannelData. */ 
     518        pj_turn_channel_data cd; 
     519 
     520        if (bufsize < 4) 
     521            return 0; 
     522 
     523        /* Decode ChannelData packet */ 
     524        pj_memcpy(&cd, buf, sizeof(pj_turn_channel_data)); 
     525        cd.length = pj_ntohs(cd.length); 
     526 
     527        if (bufsize >= cd.length+sizeof(cd))  
     528            return (cd.length+sizeof(cd)+3) & (~3); 
     529        else 
     530            return 0; 
     531    } 
     532} 
     533 
    497534/* 
    498535 * Notification from ioqueue when incoming UDP packet is received. 
    … …  
    505542{ 
    506543    pj_turn_sock *turn_sock; 
    507     pj_size_t parsed_len; 
    508544    pj_bool_t ret = PJ_TRUE; 
    509545 
    … …  
    512548 
    513549    if (status == PJ_SUCCESS && turn_sock->sess) { 
    514         /* Report incoming packet to TURN session */ 
    515         parsed_len = (unsigned)size; 
    516         pj_turn_session_on_rx_pkt(turn_sock->sess, data,  size, &parsed_len); 
    517         if (parsed_len < (unsigned)size) { 
    518             *remainder = size - parsed_len; 
    519             pj_memmove(data, ((char*)data)+parsed_len, *remainder); 
    520         } else { 
    521             *remainder = 0; 
     550        /* Report incoming packet to TURN session, repeat while we have 
     551         * "packet" in the buffer (required for stream-oriented transports) 
     552         */ 
     553        unsigned pkt_len; 
     554 
     555        //PJ_LOG(5,(turn_sock->pool->obj_name,  
     556        //        "Incoming data, %lu bytes total buffer", size)); 
     557 
     558        while ((pkt_len=has_packet(turn_sock, data, size)) != 0) { 
     559            pj_size_t parsed_len; 
     560            //const pj_uint8_t *pkt = (const pj_uint8_t*)data; 
     561 
     562            //PJ_LOG(5,(turn_sock->pool->obj_name,  
     563            //        "Packet start: %02X %02X %02X %02X",  
     564            //        pkt[0], pkt[1], pkt[2], pkt[3])); 
     565 
     566            //PJ_LOG(5,(turn_sock->pool->obj_name,  
     567            //        "Processing %lu bytes packet of %lu bytes total buffer", 
     568            //        pkt_len, size)); 
     569 
     570            parsed_len = (unsigned)size; 
     571            pj_turn_session_on_rx_pkt(turn_sock->sess, data,  size, &parsed_len); 
     572 
     573            /* parsed_len may be zero if we have parsing error, so use our 
     574             * previous calculation to exhaust the bad packet. 
     575             */ 
     576            if (parsed_len == 0) 
     577                parsed_len = pkt_len; 
     578 
     579            if (parsed_len < (unsigned)size) { 
     580                *remainder = size - parsed_len; 
     581                pj_memmove(data, ((char*)data)+parsed_len, *remainder); 
     582            } else { 
     583                *remainder = 0; 
     584            } 
     585            size = *remainder; 
     586 
     587            //PJ_LOG(5,(turn_sock->pool->obj_name,  
     588            //        "Buffer size now %lu bytes", size)); 
    522589        } 
    523590    } else if (status != PJ_SUCCESS &&  

  • pjproject/trunk/pjsip-apps/src/samples/debug.c

    r2846 r3028  
    2929 *  #include "playfile.c" 
    3030 */ 
    31 #include "jbsim.c" 
     31#include "icedemo.c" 
    3232 

  • pjproject/trunk/pjsip-apps/src/samples/icedemo.c

    r2724 r3028  
    4848        pj_str_t    turn_password; 
    4949        pj_bool_t   turn_fingerprint; 
     50        const char *log_file; 
    5051    } opt; 
    5152 
    … …  
    5758    pj_ice_strans_cfg    ice_cfg; 
    5859    pj_ice_strans       *icest; 
     60    FILE                *log_fhnd; 
    5961 
    6062    /* Variables to store parsed remote ICE info */ 
    … …  
    110112 
    111113    pj_shutdown(); 
     114 
     115    if (icedemo.log_fhnd) { 
     116        fclose(icedemo.log_fhnd); 
     117        icedemo.log_fhnd = NULL; 
     118    } 
     119 
    112120    exit(status != PJ_SUCCESS); 
    113121} 
    … …  
    217225    PJ_UNUSED_ARG(pkt); 
    218226 
    219     ((char*)pkt)[size] = '\0'; 
    220  
    221     PJ_LOG(3,(THIS_FILE, "Component %d: received %d bytes data from %s: \"%s\"", 
     227    // Don't do this! It will ruin the packet buffer in case TCP is used! 
     228    //((char*)pkt)[size] = '\0'; 
     229 
     230    PJ_LOG(3,(THIS_FILE, "Component %d: received %d bytes data from %s: \"%.*s\"", 
    222231              comp_id, size, 
    223232              pj_sockaddr_print(src_addr, ipstr, sizeof(ipstr), 3), 
     233              (unsigned)size, 
    224234              (char*)pkt)); 
    225235} 
    … …  
    236246        (op==PJ_ICE_STRANS_OP_INIT? "initialization" : 
    237247            (op==PJ_ICE_STRANS_OP_NEGOTIATION ? "negotiation" : "unknown_op")); 
    238  
    239     PJ_UNUSED_ARG(ice_st); 
    240248 
    241249    if (status == PJ_SUCCESS) { 
    … …  
    246254        pj_strerror(status, errmsg, sizeof(errmsg)); 
    247255        PJ_LOG(1,(THIS_FILE, "ICE %s failed: %s", opname, errmsg)); 
    248     } 
    249 } 
    250  
     256        pj_ice_strans_destroy(ice_st); 
     257        icedemo.icest = NULL; 
     258    } 
     259} 
     260 
     261/* log callback to write to file */ 
     262static void log_func(int level, const char *data, int len) 
     263{ 
     264    if (icedemo.log_fhnd) 
     265        fwrite(data, len, 1, icedemo.log_fhnd); 
     266    pj_log_write(level, data, len); 
     267} 
    251268 
    252269/* 
    … …  
    258275{ 
    259276    pj_status_t status; 
     277 
     278    if (icedemo.opt.log_file) { 
     279        icedemo.log_fhnd = fopen(icedemo.opt.log_file, "a"); 
     280        pj_log_set_log_func(&log_func); 
     281    } 
    260282 
    261283    /* Initialize the libraries before anything else */ 
    … …  
    11481170    puts(" --max-host, -H N          Set max number of host candidates to N"); 
    11491171    puts(" --regular, -R             Use regular nomination (default aggressive)"); 
     1172    puts(" --log-file, -L FILE       Save output to log FILE"); 
    11501173    puts(" --help, -h                Display this screen."); 
    11511174    puts(""); 
    … …  
    11831206        { "turn-password",      1, 0, 'p'}, 
    11841207        { "turn-fingerprint",   0, 0, 'F'}, 
    1185         { "regular",            0, 0, 'R'} 
     1208        { "regular",            0, 0, 'R'}, 
     1209        { "log-file",           1, 0, 'L'}, 
    11861210    }; 
    11871211    int c, opt_id; 
    … …  
    11911215    icedemo.opt.max_host = -1; 
    11921216 
    1193     while((c=pj_getopt_long(argc,argv, "c:n:s:t:u:p:H:hTFR", long_options, &opt_id))!=-1) { 
     1217    while((c=pj_getopt_long(argc,argv, "c:n:s:t:u:p:H:L:hTFR", long_options, &opt_id))!=-1) { 
    11941218        switch (c) { 
    11951219        case 'c': 
    … …  
    12301254            icedemo.opt.regular = PJ_TRUE; 
    12311255            break; 
     1256        case 'L': 
     1257            icedemo.opt.log_file = pj_optarg; 
     1258            break; 
    12321259        default: 
    12331260            printf("Argument \"%s\" is not valid. Use -h to see help", 
Note: See TracChangeset for help on using the changeset viewer.