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

[reSIProcate] TlsConnection.cxx: checkState() changes



We made changes in this code, and since they are in sensitive security-related code, someone smart should look them over. I'm not sure of the circumstances that led us to make these changes, but they may have involved use of self-generated, self-signed certificates. Both changes are in resip/stack/ssl/TlsConnection.cxx, in the routine TlsConnection::checkState(), just below the comment that says:

//post-connection verification: check that certificate name matches domain name

First, a sanity check: if we don't have any peerNames, fail immediately. In the old code, the body of the loop would never be executed, and we would still fall into an error case, so the only real differences between this code and the old code are the log message and the mFailureReason being set more accurately.

Second, as the comment in the modified code says:

// If the calling app never bothered to fill out the domain, don't demand that it match the CM from the cert.

The attached diff file is against 1.9.6.

-John Gregg

Index: TlsConnection.cxx
===================================================================
--- TlsConnection.cxx	(revision 27529)
+++ TlsConnection.cxx	(working copy)
@@ -307,6 +307,25 @@
    if (!mServer)
    {
       bool matches = false;
+
+       // AYLUS_CHANGE BEGIN
+
+       // no mPeerNames means we never got a good certificate from the server
+       if (mPeerNames.empty())
+       {
+           mTlsState = Broken;
+           mBio = 0;
+           ErrLog (<< "Certificate validation failed: no common name(s)");
+           mFailureReason = TransportFailure::CertValidationFailure;
+           return mTlsState;
+       }
+
+       // If the calling app never bothered to fill out the domain, don't demand that it match the CM from the cert.
+       if (!who().getTargetDomain().empty())
+       {
+
+       // AYLUS_CHANGE END
+
       for(std::list<BaseSecurity::PeerName>::iterator it = mPeerNames.begin(); it != mPeerNames.end(); it++)
       {
          if(BaseSecurity::matchHostName(it->mName, who().getTargetDomain()))
@@ -328,6 +347,8 @@
       }
    }
 
+   } // AYLUS_CHANGE
+
    InfoLog( << "TLS handshake done for peer " << getPeerNamesData()); 
    mTlsState = Up;
    if (!mOutstandingSends.empty())