Permalink
Browse files

Update to latest Csocket

Fixes:

- A possible crash bug for empty DNS replies with c-ares. E.g. a AAAA lookup for
  google.com doesn't give any reply but is still successful. This might be a
  c-ares bug (there is ARES_ENODATA) or c-ares just changed its behavior?
  (No bug report, just noticed accidentally)
- Connecting to ipv4-only hosts with a v6 bindhost caused weird errors:
  #47
- There was a pull request for some DSA server certificate thingy:
  #46
- Busy loop waiting for an SSL handshake to finish:
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=631590
- Some other stuff? No idea what some of the changes in here are actually doing.

Signed-off-by: Uli Schlachter <psychon@znc.in>
  • Loading branch information...
1 parent 554429a commit 88e7f093a1024a411bc8518d091b6196bfcd0e3c @psychon psychon committed Jun 26, 2011
Showing with 97 additions and 62 deletions.
  1. +56 −18 Csocket.cpp
  2. +41 −44 Csocket.h
View
@@ -36,6 +36,7 @@
#endif /* __NetBSD__ */
#ifdef HAVE_LIBSSL
+#include <stdio.h>
#include <openssl/conf.h>
#include <openssl/engine.h>
#endif /* HAVE_LIBSSL */
@@ -241,7 +242,7 @@ void Csock::FreeAres()
static void AresHostCallback( void *pArg, int status, int timeouts, struct hostent *hent )
{
Csock *pSock = (Csock *)pArg;
- if( status == ARES_SUCCESS && hent )
+ if( status == ARES_SUCCESS && hent && hent->h_addr_list[0] != NULL )
{
CSSockAddr *pSockAddr = pSock->GetCurrentAddr();
if( hent->h_addrtype == AF_INET )
@@ -264,6 +265,11 @@ static void AresHostCallback( void *pArg, int status, int timeouts, struct hoste
else
{
CS_DEBUG( ares_strerror( status ) );
+ if( status == ARES_SUCCESS )
+ {
+ CS_DEBUG("Received ARES_SUCCESS without any useful reply, using NODATA instead");
+ status = ARES_ENODATA;
+ }
}
pSock->SetAresFinished( status );
}
@@ -301,7 +307,7 @@ int GetAddrInfo( const CS_STRING & sHostname, Csock *pSock, CSSockAddr & csSockA
if( iRet == EAI_AGAIN )
return( EAGAIN ); // need to return telling the user to try again
else if( ( iRet == 0 ) && ( res ) )
- {
+ {
std::list<struct addrinfo *> lpTryAddrs;
bool bFound = false;
for( struct addrinfo *pRes = res; pRes; pRes = pRes->ai_next )
@@ -312,7 +318,7 @@ int GetAddrInfo( const CS_STRING & sHostname, Csock *pSock, CSSockAddr & csSockA
if( ( pRes->ai_socktype != SOCK_STREAM ) || ( pRes->ai_protocol != IPPROTO_TCP ) )
#endif /* __sun work around broken impl of getaddrinfo */
continue;
-
+
if( ( csSockAddr.GetAFRequire() != CSSockAddr::RAF_ANY ) && ( pRes->ai_family != csSockAddr.GetAFRequire() ) )
continue; // they requested a special type, so be certain we woop past anything unwanted
lpTryAddrs.push_back( pRes );
@@ -486,7 +492,7 @@ void SSLErrors( const char *filename, u_int iLineNum )
void __Perror( const CS_STRING & s, const char *pszFile, unsigned int iLineNo )
{
-#if defined(__sun) || defined(_WIN32) || (defined(__NetBSD_Version__) && __NetBSD_Version__ < 4000000000)
+#if defined( sgi ) || defined(__sun) || defined(_WIN32) || (defined(__NetBSD_Version__) && __NetBSD_Version__ < 4000000000)
std::cerr << s << "(" << pszFile << ":" << iLineNo << "): " << strerror( GetSockError() ) << endl;
#else
char buff[512];
@@ -1050,7 +1056,7 @@ cs_sock_t Csock::Accept( CS_STRING & sHost, u_short & iRPort )
if ( !m_bBLOCK )
{
- // make it none blocking
+ // make it none blocking
set_non_blocking( iSock );
}
@@ -1259,29 +1265,57 @@ bool Csock::SSLServerSetup()
//
// set up the CTX
- if ( SSL_CTX_use_certificate_chain_file( m_ssl_ctx, m_sPemFile.c_str() ) <= 0 )
+ if( SSL_CTX_use_certificate_chain_file( m_ssl_ctx, m_sPemFile.c_str() ) <= 0 )
{
CS_DEBUG( "Error with PEM file [" << m_sPemFile << "]" );
SSLErrors( __FILE__, __LINE__ );
return( false );
}
- if ( SSL_CTX_use_PrivateKey_file( m_ssl_ctx, m_sPemFile.c_str(), SSL_FILETYPE_PEM ) <= 0 )
+ if( SSL_CTX_use_PrivateKey_file( m_ssl_ctx, m_sPemFile.c_str(), SSL_FILETYPE_PEM ) <= 0 )
{
CS_DEBUG( "Error with PEM file [" << m_sPemFile << "]" );
SSLErrors( __FILE__, __LINE__ );
return( false );
}
- if ( SSL_CTX_set_cipher_list( m_ssl_ctx, m_sCipherType.c_str() ) <= 0 )
+ // check to see if this pem file contains a DH structure for use with DH key exchange
+ // https://github.com/znc/znc/pull/46
+ FILE *dhParamsFile = fopen( m_sPemFile.c_str(), "r" );
+ if( !dhParamsFile )
+ {
+ CS_DEBUG( "There is a problem with [" << m_sPemFile << "]" );
+ return( false );
+ }
+
+ DH * dhParams = PEM_read_DHparams( dhParamsFile, NULL, NULL, NULL );
+ fclose( dhParamsFile );
+ if( dhParams )
+ {
+ SSL_CTX_set_options( m_ssl_ctx, SSL_OP_SINGLE_DH_USE );
+ if( !SSL_CTX_set_tmp_dh( m_ssl_ctx, dhParams ) )
+ {
+ CS_DEBUG( "Error setting ephemeral DH parameters from [" << m_sPemFile << "]" );
+ SSLErrors( __FILE__, __LINE__ );
+ DH_free( dhParams );
+ return( false );
+ }
+ DH_free( dhParams );
+ }
+ else
+ { // Presumably PEM_read_DHparams failed, as there was no DH structure. Clearing those errors here so they are removed off the stack
+ ERR_clear_error();
+ }
+
+ if( SSL_CTX_set_cipher_list( m_ssl_ctx, m_sCipherType.c_str() ) <= 0 )
{
CS_DEBUG( "Could not assign cipher [" << m_sCipherType << "]" );
return( false );
}
//
// setup the SSL
- m_ssl = SSL_new ( m_ssl_ctx );
+ m_ssl = SSL_new( m_ssl_ctx );
if ( !m_ssl )
return( false );
@@ -1328,7 +1362,7 @@ bool Csock::ConnectSSL( const CS_STRING & sBindhost )
bPass = true;
#ifdef _WIN32
else if( sslErr == SSL_ERROR_SYSCALL && iErr < 0 && GetLastError() == WSAENOTCONN )
- {
+ {
// this seems to be an issue with win32 only. I've seen it happen on slow connections
// the issue is calling this before select(), which isn't a problem on unix. Allowing this
// to pass in this case is fine because subsequent ssl transactions will occur and the handshake
@@ -1523,7 +1557,7 @@ cs_ssize_t Csock::Read( char *data, size_t len )
{
cs_ssize_t bytes = 0;
- if ( ( IsReadPaused() ) && ( SslIsEstablished() ) )
+ if ( IsReadPaused() && SslIsEstablished() )
return( READ_EAGAIN ); // allow the handshake to complete first
if ( m_bBLOCK )
@@ -1541,7 +1575,11 @@ cs_ssize_t Csock::Read( char *data, size_t len )
#ifdef HAVE_LIBSSL
if ( m_bssl )
+ {
bytes = SSL_read( m_ssl, data, (int)len );
+ if( bytes >= 0 )
+ m_bsslEstablished = true; // this means all is good in the realm of ssl
+ }
else
#endif /* HAVE_LIBSSL */
#ifdef _WIN32
@@ -1657,12 +1695,12 @@ CS_STRING Csock::ConvertAddress( void *addr, bool bIPv6 )
{
CS_STRING sRet;
- if( !bIPv6 )
+ if( !bIPv6 )
{
in_addr *p = (in_addr*) addr;
sRet = inet_ntoa(*p);
- }
- else
+ }
+ else
{
char straddr[INET6_ADDRSTRLEN];
if( inet_ntop( AF_INET6, addr, straddr, sizeof(straddr) ) > 0 )
@@ -2180,7 +2218,7 @@ int Csock::GetPending()
int iBytes = SSL_pending( m_ssl );
ERR_pop_to_mark();
return( iBytes );
-#else
+#else
int iBytes = SSL_pending( m_ssl );
ERR_clear_error(); // to get safer handling, upgrade your openssl version!
return( iBytes );
@@ -2394,18 +2432,18 @@ cs_sock_t Csock::CreateSocket( bool bListen )
cs_sock_t iRet = socket( PF_INET, SOCK_STREAM, IPPROTO_TCP );
#endif /* HAVE_IPV6 */
- if ( iRet != CS_INVALID_SOCK )
+ if ( iRet != CS_INVALID_SOCK )
{
set_close_on_exec( iRet );
- if ( bListen )
+ if ( bListen )
{
const int on = 1;
if ( setsockopt( iRet, SOL_SOCKET, SO_REUSEADDR, (char *)&on, sizeof( on ) ) != 0 )
PERROR( "SO_REUSEADDR" );
}
- }
+ }
else
PERROR( "socket" );
View
@@ -188,22 +188,14 @@ class CSSockAddr
void SinFamily()
{
#ifdef HAVE_IPV6
- if( m_bIsIPv6 )
- {
- m_saddr6.sin6_family = PF_INET6;
- return;
- }
+ m_saddr6.sin6_family = PF_INET6;
#endif /* HAVE_IPV6 */
m_saddr.sin_family = PF_INET;
}
void SinPort( u_short iPort )
{
#ifdef HAVE_IPV6
- if( m_bIsIPv6 )
- {
- m_saddr6.sin6_port = htons( iPort );
- return;
- }
+ m_saddr6.sin6_port = htons( iPort );
#endif /* HAVE_IPV6 */
m_saddr.sin_port = htons( iPort );
}
@@ -775,7 +767,7 @@ class Csock
unsigned int GetRequireClientCertFlags();
//! legacy, deprecated @see SetRequireClientCertFlags
void SetRequiresClientCert( bool bRequiresCert );
- //! bitwise flags, 0 means don't require cert, SSL_VERIFY_PEER verifies peers, SSL_VERIFY_FAIL_IF_NO_PEER_CERT will cause the connection to fail if no cert
+ //! bitwise flags, 0 means don't require cert, SSL_VERIFY_PEER verifies peers, SSL_VERIFY_FAIL_IF_NO_PEER_CERT will cause the connection to fail if no cert
void SetRequireClientCertFlags( unsigned int iRequireClientCertFlags ) { m_iRequireClientCertFlags = iRequireClientCertFlags; }
#endif /* HAVE_LIBSSL */
@@ -919,7 +911,7 @@ class Csock
time_t GetLastCheckTimeout() { return( m_iLastCheckTimeoutTime ); }
//! Returns the time when CheckTimeout() should be called next
- time_t GetNextCheckTimeout( time_t iNow = 0 )
+ time_t GetNextCheckTimeout( time_t iNow = 0 )
{
if( iNow == 0 )
iNow = time( NULL );
@@ -1222,7 +1214,7 @@ class CSListener
void SetPemPass( const CS_STRING & s ) { m_sPemPass = s; }
//! set to true if require a client certificate (deprecated @see SetRequireClientCertFlags)
void SetRequiresClientCert( bool b ) { m_iRequireCertFlags = ( b ? SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT : 0 ); }
- //! bitwise flags, 0 means don't require cert, SSL_VERIFY_PEER verifies peers, SSL_VERIFY_FAIL_IF_NO_PEER_CERT will cause the connection to fail if no cert
+ //! bitwise flags, 0 means don't require cert, SSL_VERIFY_PEER verifies peers, SSL_VERIFY_FAIL_IF_NO_PEER_CERT will cause the connection to fail if no cert
void SetRequireClientCertFlags( unsigned int iRequireCertFlags ) { m_iRequireCertFlags = iRequireCertFlags; }
#endif /* HAVE_LIBSSL */
private:
@@ -2029,7 +2021,7 @@ class TSocketManager : public std::vector<T *>
continue;
}
#endif /* CSOCK_USE_POLL */
-
+
#ifdef HAVE_C_ARES
ares_channel pChannel = pcSock->GetAresChannel();
if( pChannel )
@@ -2063,45 +2055,50 @@ class TSocketManager : public std::vector<T *>
continue; // invalid sock fd
}
- if ( pcSock->GetType() != T::LISTENER )
+ if( pcSock->GetType() != T::LISTENER )
{
- if ( ( pcSock->IsConnected() ) && ( pcSock->GetWriteBuffer().empty() ) )
- {
- if ( !bIsReadPaused )
- FDSetCheck( iRSock, miiReadyFds, eCheckRead );
+ bool bHasWriteBuffer = !pcSock->GetWriteBuffer().empty();
- } else if ( ( pcSock->GetSSL() ) && ( !pcSock->SslIsEstablished() ) && ( !pcSock->GetWriteBuffer().empty() ) )
- {
- // do this here, cause otherwise ssl will cause a small
- // cpu spike waiting for the handshake to finish
+ if ( !bIsReadPaused )
FDSetCheck( iRSock, miiReadyFds, eCheckRead );
- // resend this data
- if ( !pcSock->Write( "" ) )
- {
- pcSock->Close();
+
+ if( pcSock->AllowWrite( iNOW ) && ( !pcSock->IsConnected() || bHasWriteBuffer ) )
+ {
+ if( !pcSock->IsConnected() )
+ { // set the write bit if not connected yet
+ FDSetCheck( iWSock, miiReadyFds, eCheckWrite );
}
- if( !pcSock->GetWriteBuffer().empty() )
- { // this means we need to write again, not everything got knocked out
+ else if( bHasWriteBuffer && !pcSock->GetSSL() )
+ { // always set the write bit if there is data to send when NOT ssl
FDSetCheck( iWSock, miiReadyFds, eCheckWrite );
}
-
- } else
- {
- if ( !bIsReadPaused )
- FDSetCheck( iRSock, miiReadyFds, eCheckRead );
-
- if( pcSock->AllowWrite( iNOW ) )
- {
+ else if( bHasWriteBuffer && pcSock->GetSSL() && pcSock->SslIsEstablished() )
+ { // ONLY set the write bit if there is data to send and the SSL handshake is finished
FDSetCheck( iWSock, miiReadyFds, eCheckWrite );
}
}
- }
+ if( pcSock->GetSSL() && !pcSock->SslIsEstablished() && bHasWriteBuffer )
+ { // if this is an unestabled SSL session with data to send ... try sending it
+ // do this here, cause otherwise ssl will cause a small
+ // cpu spike waiting for the handshake to finish
+ // resend this data
+ if ( !pcSock->Write( "" ) )
+ {
+ pcSock->Close();
+ }
+ // warning ... setting write bit in here causes massive CPU spinning on invalid SSL servers
+ // http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=631590
+ // however, we can set the select WAY down and it will retry quickly, but keep it from spinning at 100%
+ tv.tv_usec = iQuickReset;
+ tv.tv_sec = 0;
+ }
+ }
else
{
FDSetCheck( iRSock, miiReadyFds, eCheckRead );
}
-
+
if( pcSock->GetSSL() && pcSock->GetType() != Csock::LISTENER )
{
if ( ( pcSock->GetPending() > 0 ) && ( !pcSock->IsReadPaused() ) )
@@ -2112,12 +2109,12 @@ class TSocketManager : public std::vector<T *>
// old fashion select, go fer it
int iSel;
- if ( !mpeSocks.empty() ) // .1 ms pause to see if anything else is ready (IE if there is SSL data pending, don't wait too long)
+ if( !mpeSocks.empty() ) // .1 ms pause to see if anything else is ready (IE if there is SSL data pending, don't wait too long)
{
tv.tv_usec = iQuickReset;
tv.tv_sec = 0;
}
- else if ( ( !this->empty() ) && ( !bHasAvailSocks ) )
+ else if ( !this->empty() && !bHasAvailSocks )
{
tv.tv_usec = iQuickReset;
tv.tv_sec = 0;
@@ -2152,7 +2149,7 @@ class TSocketManager : public std::vector<T *>
m_errno = SUCCESS;
return;
- }
+ }
else if ( iSel == -1 )
{
if ( mpeSocks.empty() )
@@ -2161,7 +2158,7 @@ class TSocketManager : public std::vector<T *>
m_errno = SUCCESS;
return;
- }
+ }
else
{
m_errno = SUCCESS;
@@ -2217,7 +2214,7 @@ class TSocketManager : public std::vector<T *>
SelectSock( mpeSocks, iErrno, pcSock );
- }
+ }
else if ( FDHasCheck( iRSock, miiReadyFds, eCheckRead ) )
{
if ( iSel > 0 )

0 comments on commit 88e7f09

Please sign in to comment.