[asterisk-dev] [Code Review] 3337: Code for DTLS retransmission

[Matt Jordan]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3337/#review11652
-----------------------------------------------------------



http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/3337/#comment21410>

    Ah, the joy of scheduler madness.
    
    Since you can stop the DTLS timer in a separate thread than the scheduler, you have to check here that you're still supposed to handle the scheduled callback. 
    
    ast_mutex_lock(&rtp->dtls_timer_lock);
    if (rtp->dtlstimerid == -1) {
        /* Whoops, deleted */
        ast_mutex_unlock(&rtp->dtls_timer_lock);
        ao2_ref(instance, -1);
        return 0;
    }
    
    ...
    



http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/3337/#comment21409>

    Possible NULL pointer dereference here:
    
    ast_mutex_lock(&rtp->dtls_timer_lock);
    rtp->dtlstimerid = -1;
    ast_mutex_unlock(&rtp->dtls_timer_lock);
    
    /* If rtp can be NULL, we shouldn't have dereferenced it for the mutex lock */
    
    if (rtp && rtp->ssl) {
    	DTLSv1_handle_timeout(rtp->ssl);
    }
    
    
    I'd bail before locking:
    
    if (!rtp) {
        return -1;
    }
    
    ast_mutex_lock(&rtp->dtls_timer_lock);
    ...



http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/3337/#comment21411>

    Put some spaces around that '/' :-)


- Matt Jordan


On April 16, 2014, 3:26 a.m., Nitesh Bansal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3337/
> -----------------------------------------------------------
> 
> (Updated April 16, 2014, 3:26 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds the code to do the DTLS retransmissions in Asterisk.
> 
> 
> Diffs
> -----
> 
>   http://svn.asterisk.org/svn/asterisk/branches/11/res/res_rtp_asterisk.c 412438 
> 
> Diff: https://reviewboard.asterisk.org/r/3337/diff/
> 
> 
> Testing
> -------
> 
> I tested this with a basic SIPP script, which fakes a DTLS INVITE.
> Asterisk thinks that it is a DTLS call and inititates the DTLS handshake. SIPP doesn't respond to DTLS handshake, which causes the DTLS timeout and DTLS retransmission takes place.
> 
> 
> Thanks,
> 
> Nitesh Bansal
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140417/32e694e2/attachment.html>