On 2017-12-17 19:32:52 [+0100], Hilko Bengen wrote: > Control: tag -1 patch > > Hi, > > here's a patch that fixes the OpenSSL-1.1-related FTBFS for sslsniff. > > I'd appreciate a review of the patch.
It is not back compatible with openssl 1.0.2 >Index: sslsniff/SessionCache.cpp >=================================================================== >--- sslsniff.orig/SessionCache.cpp >+++ sslsniff/SessionCache.cpp >@@ -47,7 +47,9 @@ void SessionCache::removeSessionId(unsig > } > > int SessionCache::setNewSessionId(SSL *s, SSL_SESSION *session) { >- return setNewSessionId(s, session, session->session_id, >session->session_id_length); >+ unsigned int id_length; >+ const unsigned char *id = SSL_SESSION_get_id(session, &id_length); >+ return setNewSessionId(s, session, (unsigned char*)id, id_length); > } > > int SessionCache::setNewSessionId(SSL *s, SSL_SESSION *session, >@@ -117,8 +119,8 @@ SSL_SESSION * SessionCache::getSessionId > > // Trampoline Functions. Yay C. > >-SSL_SESSION * SessionCache::getSessionIdTramp(SSL *s, unsigned char *id, int >idLength, int *ref) { >- return SessionCache::getInstance()->getSessionId(s, id, idLength, ref); >+SSL_SESSION * SessionCache::getSessionIdTramp(SSL *s, const unsigned char >*id, int idLength, int *ref) { >+ return SessionCache::getInstance()->getSessionId(s, (unsigned char*)id, >idLength, ref); since you propage that `const' to getSessionIdTramp(), you could propage it further and drop that cast. > } > > int SessionCache::setNewSessionIdTramp(SSL *s, SSL_SESSION *session) { >Index: sslsniff/SessionCache.hpp >=================================================================== >--- sslsniff.orig/SessionCache.hpp >+++ sslsniff/SessionCache.hpp >@@ -49,7 +49,7 @@ class SessionCache { > > public: > static SessionCache* getInstance(); >- static SSL_SESSION * getSessionIdTramp(SSL *s, unsigned char *id, int >idLength, int *ref); >+ static SSL_SESSION * getSessionIdTramp(SSL *s, const unsigned char *id, int >idLength, int *ref); > static int setNewSessionIdTramp(SSL *s, SSL_SESSION *session); > > int setNewSessionId(SSL *s, SSL_SESSION *session); >Index: sslsniff/certificate/Certificate.hpp >=================================================================== >--- sslsniff.orig/certificate/Certificate.hpp >+++ sslsniff/certificate/Certificate.hpp >@@ -92,7 +92,7 @@ private: > } > > void parseCommonName(X509 *cert) { >- std::string distinguishedName(cert->name); >+ std::string >distinguishedName(X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0)); X509_NAME_oneline() allocates memory which you leak. > std::string::size_type cnIndex = distinguishedName.find("CN="); That part grabs the hostname from the CN= part of the subject. I haven't look *why* this is done but usually one wants to check the "subject alternative name" extension and the content may conttain an asterisk. > if (cnIndex == std::string::npos) throw BadCertificateException(); >Index: sslsniff/certificate/TargetedCertificateManager.cpp >=================================================================== >--- sslsniff.orig/certificate/TargetedCertificateManager.cpp >+++ sslsniff/certificate/TargetedCertificateManager.cpp >@@ -117,6 +117,6 @@ void TargetedCertificateManager::dump() > std::list<Certificate*>::iterator i; > > for(i=certificates.begin(); i != certificates.end(); ++i) >- std::cout << "Certificate: " << (*i)->getCert()->name << std::endl; >+ std::cout << "Certificate: " << >X509_NAME_oneline(X509_get_subject_name((*i)->getCert()), NULL, 0) << >std::endl; also a leak. > > } > Cheers, > -Hilko Sebastian