Polishing updates from audit

diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc
index 16f20845ed4ad202edfe84cd0b8c5dbca0645bf0..cb0778f31bc6e1e35fbc2ed10383a4c34d027b90 100644 (file)
--- a/src/security/PeerOptions.cc
+++ b/src/security/PeerOptions.cc
@@ -731,8 +731,9 @@ Security::PeerOptions::updateContextCrl(Security::ContextPointer &ctx)
 void
 Security::PeerOptions::updateSessionOptions(Security::SessionPointer &s)
 {
+#if USE_OPENSSL
     // 'options=' value being set to session is a GnuTLS specific thing.
-#if !USE_OPENSSL && USE_GNUTLS
+#elif USE_GNUTLS
     int x;
     SBuf errMsg;
     if (!parsedOptions) {

diff --git a/src/security/Session.cc b/src/security/Session.cc
index b5f2a6d9055fa976227cfc7820e706794bde4344..f9083f6b92eb1b6f288eaddfc86091a8e6387484 100644 (file)
--- a/src/security/Session.cc
+++ b/src/security/Session.cc
@@ -87,6 +87,19 @@ tls_write_method(int fd, const char *buf, int len)
 }
 #endif
 
+#if USE_OPENSSL
+Security::SessionPointer
+Security::NewSessionObject(const Security::ContextPointer &ctx)
+{
+    Security::SessionPointer session(SSL_new(ctx.get()), [](SSL *p) {
+            debugs(83, 5, "SSL_free session=" << (void*)p);
+            SSL_free(p);
+        });
+    debugs(83, 5, "SSL_new session=" << (void*)session.get());
+    return session;
+}
+#endif
+
 static bool
 CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer &conn, Security::Io::Type type, const char *squidCtx)
 {
@@ -100,11 +113,7 @@ CreateSession(const Security::ContextPointer &ctx, const Comm::ConnectionPointer
     const char *errAction = "with no TLS/SSL library";
     int errCode = 0;
 #if USE_OPENSSL
-    Security::SessionPointer session(SSL_new(ctx.get()), [](SSL *p) {
-            debugs(83, 5, "SSL_free session=" << (void*)p);
-            SSL_free(p);
-        });
-    debugs(83, 5, "SSL_new session=" << (void*)session.get());
+    Security::SessionPointer session(Security::NewSessionObject(ctx));
     if (!session) {
         errCode = ERR_get_error();
         errAction = "failed to allocate handle";

diff --git a/src/security/Session.h b/src/security/Session.h
index 6574743679044014657b115f7a516bc666230eaa..8c40efdb488ef101203b3aa3f7715b1e9c6ec923 100644 (file)
--- a/src/security/Session.h
+++ b/src/security/Session.h
@@ -77,6 +77,13 @@ void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::Sessi
 /// Needs to be done before using the SessionPointer for a handshake.
 void SetSessionResumeData(const Security::SessionPointer &, const Security::SessionStatePointer &);
 
+#if USE_OPENSSL
+/// \deprecated use the PeerOptions/ServerOptions API methods instead.
+/// Wraps SessionPointer value creation to reduce risk of
+/// a nasty hack in ssl/support.cc.
+Security::SessionPointer NewSessionObject(const Security::ContextPointer &);
+#endif
+
 } // namespace Security
 
 #endif /* SQUID_SRC_SECURITY_SESSION_H */

diff --git a/src/security/forward.h b/src/security/forward.h
index 844611a2d20085825478403c498b120c43420b58..e9866f6e9c2056bd52ae1cd9b7b6b86250234efe 100644 (file)
--- a/src/security/forward.h
+++ b/src/security/forward.h
@@ -111,7 +111,10 @@ typedef std::unordered_set<Security::ErrorCode> Errors;
 namespace Io
 {
     enum Type {
-#if USE_GNUTLS
+#if USE_OPENSSL
+        BIO_TO_CLIENT = 6000,
+        BIO_TO_SERVER
+#elif USE_GNUTLS
         // NP: this is odd looking but correct.
         // 'to-client' means we are a server, and vice versa.
         BIO_TO_CLIENT = GNUTLS_SERVER,
@@ -126,10 +129,12 @@ namespace Io
 
 class KeyData;
 
-#if !USE_OPENSSL && USE_GNUTLS
+#if USE_OPENSSL
+typedef long ParsedOptions;
+#elif USE_GNUTLS
 typedef std::shared_ptr<struct gnutls_priority_st> ParsedOptions;
 #else
-typedef long ParsedOptions;
+class ParsedOptions {}; // we never parse/use TLS options in this case
 #endif
 
 class PeerConnector;

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index d726c4f30eb6c3af6c7a5eb90a391b8ef8c51de9..dea8df5ab07f69763c0dc342d6787311701e1ad6 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -1020,11 +1020,7 @@ Ssl::verifySslCertificate(Security::ContextPointer &ctx, CertificateProperties c
     assert(0);
 #else
     // Temporary ssl for getting X509 certificate from SSL_CTX.
-    Security::SessionPointer ssl(SSL_new(ctx.get()), [](SSL *p) {
-            debugs(83, 5, "SSL_free session=" << (void*)p);
-            SSL_free(p);
-        });
-    debugs(83, 5, "SSL_new session=" << (void*)ssl.get());
+    Security::SessionPointer ssl(Security::NewSessionObject(ctx));
     X509 * cert = SSL_get_certificate(ssl.get());
 #endif
     if (!cert)

diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc
index a9f8f4f1e2bcab9ef17746e8ff9c6734e5c92470..28ee94ca8336c4827ebf9c72daba600882fd64dd 100644 (file)
--- a/src/tests/stub_libsecurity.cc
+++ b/src/tests/stub_libsecurity.cc
@@ -69,7 +69,7 @@ void PeerConnector::recordNegotiationDetails() STUB
 #include "security/PeerOptions.h"
 Security::PeerOptions Security::ProxyOutgoingConfig;
 Security::PeerOptions::PeerOptions() {
-#if !USE_GNUTLS
+#if USE_OPENSSL
     parsedOptions = 0;
 #endif
      STUB_NOP
@@ -103,5 +103,8 @@ void SessionSendGoodbye(const Security::SessionPointer &) STUB
 bool SessionIsResumed(const Security::SessionPointer &) STUB_RETVAL(false)
 void MaybeGetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &) STUB
 void SetSessionResumeData(const Security::SessionPointer &, const Security::SessionStatePointer &) STUB
+#if USE_OPENSSL
+Security::SessionPointer NewSessionObject(const Security::ContextPointer &) STUB_RETVAL(nullptr)
+#endif
 } // namespace Security