< Previous by Date Date Index Next by Date >
  Thread Index  

[reSIProcate] Commit in DtlsTransport.cxx


Hello,
I've committed the fix in DtlsTransport.cxx in SVN main (revision 9183).
Best regards,
Dario Bozzali.


------------------------------------------------------------------------

-----Original Message-----
From: Dario Bozzali 
Sent: mercoledì 15 giugno 2011 14.57
To: resiprocate-devel@xxxxxxxxxxxxxxx
Subject: Change in DtlsTransport.cxx

Hello,
I had a look at file DtlsTransport.cxx in main line and in particular method 
_doHandshake().
In my opinion there is a mistake checking return value of function 
SSL_do_handshake() since it returns values -1, 0 or 1. I think that we should 
check error returned by SSL_get_error().
Maybe we could add also some diagnostic log in _doHandshake(), _read() and 
_write() methods as below (please check code).
 
Now method is:
 
void
DtlsTransport::_doHandshake( void )
{
   DtlsMessage *msg = mHandshakePending.getNext() ;
   SSL *ssl = msg->getSsl() ;
   delete msg ;
   int ret = SSL_do_handshake( ssl ) ;
   switch( ret )
   {
     case SSL_ERROR_NONE:
            break;
         case SSL_ERROR_SSL:
            break;
         case SSL_ERROR_WANT_READ:
            break;
         case SSL_ERROR_WANT_WRITE:         
            break;
         case SSL_ERROR_SYSCALL:
            break;
         case SSL_ERROR_ZERO_RETURN:
            break;
         case SSL_ERROR_WANT_CONNECT:
            break;
         case SSL_ERROR_WANT_ACCEPT:
            break;
         default:
            break ;
   }
}

Proposed change to use SSL_get_error() function:
 
void
DtlsTransport::_doHandshake( void )
{
   DtlsMessage *msg = mHandshakePending.getNext() ;
   SSL *ssl = msg->getSsl() ;
   delete msg ;
 
   ERR_clear_error();
 
   int ret = SSL_do_handshake( ssl ) ;
   if (ret <= 0)
   {
      int err = SSL_get_error(ssl, ret);
      switch (err)
      {
         case SSL_ERROR_NONE:
            break;
         case SSL_ERROR_SSL:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS handshake code SSL_ERROR_SSL"
                         << " error = " << errorString );
            }
            break;
         case SSL_ERROR_WANT_READ:
            break;
         case SSL_ERROR_WANT_WRITE:
            break;
         case SSL_ERROR_SYSCALL:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS handshake code SSL_ERROR_SYSCALL"
                         << " error = " << errorString );
            }
            break;
         case SSL_ERROR_ZERO_RETURN:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS handshake code SSL_ERROR_ZERO_RETURN"
                         << " error = " << errorString );
            }
            break;
         case SSL_ERROR_WANT_CONNECT:
            break;
         case SSL_ERROR_WANT_ACCEPT:
            break;
         default:
            break;
      }
   }
}
 
We can add diagnostic log:
 
void
DtlsTransport::_read( FdSet& fdset )
{
[...]
   if ( len <= 0 )
   {
      switch( err )
      {
         case SSL_ERROR_NONE:
            break ;
         case SSL_ERROR_SSL:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS read condition SSL_ERROR_SSL on"
                         << " addr = " << inet_ntoa(((struct sockaddr_in 
*)&peer)->sin_addr)
                         << " port = " << ntohs(((struct sockaddr_in 
*)&peer)->sin_port)
                         << " error = " << errorString );
            }
            break ;
         case SSL_ERROR_WANT_READ:
            break ;
         case SSL_ERROR_WANT_WRITE:
            break ;
         case SSL_ERROR_SYSCALL:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS read condition SSL_ERROR_SYSCALL on"
                         << " addr = " << inet_ntoa(((struct sockaddr_in 
*)&peer)->sin_addr)
                         << " port = " << ntohs(((struct sockaddr_in 
*)&peer)->sin_port)
                         << " error = " << errorString );
            }
            break ;
            /* connection closed */
         case SSL_ERROR_ZERO_RETURN:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS read condition SSL_ERROR_ZERO_RETURN on"
                         << " addr = " << inet_ntoa(((struct sockaddr_in 
*)&peer)->sin_addr)
                         << " port = " << ntohs(((struct sockaddr_in 
*)&peer)->sin_port)
                         << " error = " << errorString );
            }
            _cleanupConnectionState( ssl, *((struct sockaddr_in *)&peer) ) ;
            break ;
         case SSL_ERROR_WANT_CONNECT:
            break ;
         case SSL_ERROR_WANT_ACCEPT:
            break ;
         default:
            break ;
      }
   }
[...]
}
 
void DtlsTransport::_write( FdSet& fdset )
{
[...]
   if ( count <= 0 )
   {
      /* cache unqueued data */
      mSendData = sendData ; 
      int err = SSL_get_error( ssl, count ) ;
      switch( err )
      {
         case SSL_ERROR_NONE:
            break;
         case SSL_ERROR_SSL:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS write condition SSL_ERROR_SSL on " << 
sendData->destination
                         << " error = " << errorString );
            }
            break;
         case SSL_ERROR_WANT_READ:
            retry = 1 ;
            break;
         case SSL_ERROR_WANT_WRITE:
             retry = 1 ;
             fdset.setWrite(mFd);
            break;
         case SSL_ERROR_SYSCALL:
            {
               {
                  char errorString[1024];
                  ERR_error_string_n(ERR_get_error(), errorString, 1024);
                  DebugLog( << "Got DTLS write condition SSL_ERROR_SYSCALL on " 
<< sendData->destination
                            << " error = " << errorString );
               }
               int e = getErrno();
               error(e);
               InfoLog (<< "Failed (" << e << ") sending to " 
                        << sendData->destination);
               fail(sendData->transactionId);
               break;
            }
         case SSL_ERROR_ZERO_RETURN:
            {
               char errorString[1024];
               ERR_error_string_n(ERR_get_error(), errorString, 1024);
               DebugLog( << "Got DTLS write condition SSL_ERROR_ZERO_RETURN on 
" << sendData->destination
                         << " error = " << errorString );
            }
            _cleanupConnectionState( ssl, *((struct sockaddr_in *)&peer) ) ;
            break ;
         case SSL_ERROR_WANT_CONNECT:
            break;
         case SSL_ERROR_WANT_ACCEPT:
            break;
         default:
            break ;
      }
   }
   else
   {
      mSendData = NULL ;
   }
[...]
}
 
 
Thank you and best regards,
Dario Bozzali.