[squid-dev] [PATCH] TidyPointer removal
Amos Jeffries
squid3 at treenet.co.nz
Thu Jul 7 19:40:39 UTC 2016
On 4/07/2016 5:39 a.m., Alex Rousskov wrote:
> On 06/29/2016 05:45 AM, Amos Jeffries wrote:
>
>> /**
>> + * A pointer that deletes the object it points to when the pointer's owner or
>> + * context is gone. [...]
>> */
> ...
>> + explicit LockingPointer(const SelfType &o) : raw(nullptr) { resetAndLock(o.get()); }
>
>
> Something went wrong here: If both the class description and the copy
> constructor are correct, then the following pseudo code ought to crash
> Squid:
>
> LockingPointer a(OpenSSL_new(...)); // a points to a new object X
> {
> LockingPointer b(a); // a and b point to the same object X
> ...
> // b context ends so b deletes X
> }
> ...
> // a context ends so a deletes X
> // Squid crashes because the same X was deleted twice
>
>
> I hope that Squid code is actually correct, but the proposed class
> description is not. LockingPointer does _not_ delete the object it
> points to when the pointer's owner or context is gone. LockingPointer is
> actually a reference counting pointer like shared_ptr (not a unique_ptr
> or auto_ptr!). There is no concept of [a single] owner here -- the
> object is _shared_ among many "owners". That makes copy assignment safe.
Yes the description was incorrect. OpenSSL does ref-counting done in the
*_new() and *_free() functions (mostly).
>
> If I am right, then we need to fix the LockingPointer class description
> (and make sure the implementation matches it). There several ways to do
> that, including the following three:
>
>
> A. Provide two different LockingPointer classes, one for OpenSSL, and
> one for GnuTLS. The tricky shared pointer code will be duplicated (bad),
> but each class itself would be easy to comprehend because it will lack
> #ifdefs (good):
>
> #if USE_OPENSSL
> template <...>
> class LockingPointer ...
> #elif USE_GNUTLS
> template <...>
> class LockingPointer ...
> #else
>
>
> B. Provide a single LockingPointer class along with two wrappers that
> customize template parameters, one for OpenSSL, and one for GnuTLS. No
> tricky shared pointer duplication (good), but an extra level of wrapping
> would complicate comprehension (bad):
>
> template <...>
> class LockingPointer ...
>
> #if USE_OPENSSL
> template <...>
> using OpenSslLockingPointer = LockingPointer<...>
> #elif USE_GNUTLS
> template <...>
> class GnuTlsLockingPointer = LockingPointer<...>
> #else
>
> I have attached a sketch for B in case you want to see what that would
> look like.
>
>
> C. Provide a single LockingPointer class sprinkled with #ifdefs
> customizing its API and implementation for each library. No tricky
> shared pointer duplication (good), but large number of #ifdefs,
> including #ifdefs in class template parameters, will make the code
> difficult to comprehend (bad).
>
>
> We are currently doing C. I have no objections to us continuing doing C,
> at least for now. If you pick C, then the fixed LockingPointer class
> documentation may look like the following (compare that with the
> documentation for LockingPointer in the attached sketch for B):
I'm opting to do the below suggested fixes and merge when we are both
okay with it. Then attempt the LockingPointer replacement with
shared_ptr on a case by case basis in followups.
>
> -----
> A shared pointer to a reference-counting Object with library-specific
> absorption, locking, and unlocking implementations. The API largely
> follows std::shared_ptr.
>
> The constructor and the absorb() method import a raw Object pointer.
> Normally, absorb() would just lock(), but libraries like OpenSSL
> pre-lock objects before they are fed to LockingPointer, necessitating
> this customization hook.
>
> The lock() method increments Object's reference counter.
>
> The unlock() method decrements Object's reference counter and destroys
> the object when the counter reaches zero.
> -----
Done.
Though I've used resetWithoutLocking() instead of absorb() until the
below details with reset() and clear() are settled.
>
> The absorb/lock/unlock split above may help with seamless GnuTLS
> support, assuming GnuTLS does not pre-lock objects like OpenSSL does.
> However, somebody familiar with GnuTLS should check whether it has other
> special needs not addressed by the above LockingPointer sketch. For
> example, if GnuTLS does not have a concept of locking at all, then we
> may be better of with approach A where GnuTLS code just uses
> std::shared_ptr!
I believe that is the case yes.
However, there is currently still a lot of Ssl:: code switching between
the FooPointer and FooPtr types which does not let shared_ptr do its
copying properly. To avoid #ifdef all over the places reset() is used we
also need to make sure the matching OpenSSL pointer can be a shared_ptr
as well.
This is the main reason I want to update those bits that as a followup.
>
>> + /// Deallocate raw pointer. Become a nil pointer.
>> + void deletePointer() {
>> + if (raw)
>> + DeAllocator(raw);
>> + raw = nullptr;
>> + }
>
> please rename to unlock(). We are not deleting the pointer here and,
> depending on the lock count, we are not deleting the object either!
> Renaming DeAllocator to Unlock or Unlocker is also a good idea -- we
> have wasted enough time being confused by OpenSSL _free() functions not
> freeing locked objects!
>
Nod. Done.
>
>> + /// Reset raw pointer - delete last one and save new one.
>> + void reset(T *t) {
>> + deletePointer();
>> + raw = t;
>> }
>
> I do not think we can have a reset() method like this because the
> standard shared_ptr::reset() has a very different semantics. Let's call
> this absorb(). This is like a raw pointer assignment operator, but we
> want to keep it "explicit" because we think this is a dangerous operation.
>
What different semantics do you see in there?
<http://en.cppreference.com/w/cpp/memory/shared_ptr/reset> description
of reset(Y*) variant #2 seems to describe exactly what our custom
reset(T*) actually does, including the undefined behaviour case. That
similarity is a bit clearer after the 'deletePointer' is renamed.
We just dont provide nor use the other variants.
>
>> void resetAndLock(T *t) {
>> if (t != this->get()) {
>> this->reset(t);
>> #if USE_OPENSSL
>> if (t)
>> CRYPTO_add(&t->references, 1, lock);
>> #elif USE_GNUTLS
>> // XXX: GnuTLS does not provide locking ?
>> #else
>> assert(false);
>> #endif
>> }
>> }
>
> Please rewrite as a pair of public reset(t) and private lock(void)
> methods. reset(t) calls lock() for non-nil t, of course.
Replaced its internals with those. Though left the name for now.
>
> Please also add a public clear(void) method that reset(t) will call
> instead of lock() when t is nil. Alternatively, add a reset(void) method
> with the same semantics as clear(void) -- that is what std::shared_ptr does.
I do not see any clear() method in shared_ptr documentation.
<http://en.cppreference.com/mwiki/index.php?title=Special%3ASearch&search=clear%28%29>
>
> To resolve conflicts between abused reset() in the old code and the new
> reset(), I suggest the following procedure:
>
> 1. Add absorb() and clear(void)/reset(void) methods as discussed above.
>
> 2. Remove LockingPointer::reset(x). Replace all calls to
> LockingPointer::reset(x) with absorb() and clear(void)/reset(void),
> depending on whether x is nil. See earlier emails on how to find all
> those calls. IIRC, Christos has certified that all those old reset(x)
> calls are meant to be either non-locking absorption or clearance calls.
>
> 3. Rename LockingPointer::resetAndLock(x) to reset(x). Replace all calls
> to LockingPointer::resetAndLock() with reset(x).
>
> You may argue that these reset()-related changes are outside this
> project scope, but I think it is too dangerous to commit LockingPointer
> with a reset(x) method that does not work like shared_ptr::reset(x). In
> the current trunk code, we are calling TidyPointer::reset(), which is
> already very ugly, but can at least be half-justified because
> TidyPointer::reset() does what it is supposed to do. Moving that method
> from TidyPointer to LockingPointer moves the needle from "terribly ugly"
> to "unacceptably dangerous" IMO.
>
>
>> + void operator()(argument_type a) { sk_object ## _pop_free(a, freefunction); } \
>
> If you can make this operator "const", please make it "const".
>
>
> Please note that the originally proposed commit message no longer
> applies to your changes so we need a new one.
>
"
This replaces TidyPointer with std::unique_ptr and re-implements the
part of TidyPointer which LockingPointer was using. Removing the
inheritence in the process and updating methods names for clarity.
The LockingPointer still has some of the issues which led us down the
path to getting here. I've chosen to submit for review at this point to
ensure we still have a fully working Squid before going further down
this trail.
"
Amos
-------------- next part --------------
=== modified file 'src/base/Makefile.am'
--- src/base/Makefile.am 2016-01-15 06:47:59 +0000
+++ src/base/Makefile.am 2016-06-29 03:56:37 +0000
@@ -21,22 +21,21 @@
AsyncCallQueue.h \
ByteCounter.h \
CbcPointer.h \
CbDataList.h \
CharacterSet.h \
CharacterSet.cc \
EnumIterator.h \
InstanceId.h \
Lock.h \
LookupTable.h \
LruMap.h \
Packable.h \
PackableStream.h \
RegexPattern.cc \
RegexPattern.h \
RunnersRegistry.cc \
RunnersRegistry.h \
Subscription.h \
TextException.cc \
TextException.h \
- TidyPointer.h \
YesNoNone.h
=== removed file 'src/base/TidyPointer.h'
--- src/base/TidyPointer.h 2016-07-01 17:35:00 +0000
+++ src/base/TidyPointer.h 1970-01-01 00:00:00 +0000
@@ -1,71 +0,0 @@
-/*
- * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-#ifndef SQUID_BASE_TIDYPOINTER_H
-#define SQUID_BASE_TIDYPOINTER_H
-
-/**
- * A pointer that deletes the object it points to when the pointer's owner or
- * context is gone. Similar to std::unique_ptr but without confusing assignment
- * and with a customizable cleanup method. Prevents memory leaks in
- * the presence of exceptions and processing short cuts.
-*/
-template <typename T, void (*DeAllocator)(T *t)> class TidyPointer
-{
-public:
- /// Delete callback.
- typedef void DCB (T *t);
- TidyPointer(T *t = NULL)
- : raw(t) {}
-public:
- bool operator !() const { return !raw; }
- explicit operator bool() const { return raw; }
- T* operator ->() const { return get(); }
-
- /// Returns raw and possibly NULL pointer
- T *get() const { return raw; }
-
- /// Reset raw pointer - delete last one and save new one.
- void reset(T *t) {
- deletePointer();
- raw = t;
- }
-
- /// Forget the raw pointer without freeing it. Become a nil pointer.
- T *release() {
- T *ret = raw;
- raw = NULL;
- return ret;
- }
- /// Deallocate raw pointer.
- ~TidyPointer() {
- deletePointer();
- }
-private:
- /// Forbidden copy constructor.
- TidyPointer(TidyPointer<T, DeAllocator> const &);
- /// Forbidden assigment operator.
- TidyPointer <T, DeAllocator> & operator = (TidyPointer<T, DeAllocator> const &);
- /// Deallocate raw pointer. Become a nil pointer.
- void deletePointer() {
- if (raw) {
- DeAllocator(raw);
- }
- raw = NULL;
- }
- T *raw; ///< pointer to T object or NULL
-};
-
-/// DeAllocator for pointers that need free(3) from the std C library
-template<typename T> void tidyFree(T *p)
-{
- xfree(p);
-}
-
-#endif // SQUID_BASE_TIDYPOINTER_H
-
=== modified file 'src/client_side_request.cc'
--- src/client_side_request.cc 2016-06-25 14:35:41 +0000
+++ src/client_side_request.cc 2016-07-04 16:22:18 +0000
@@ -160,41 +160,41 @@
conn_(NULL)
#if USE_OPENSSL
, sslBumpNeed_(Ssl::bumpEnd)
#endif
#if USE_ADAPTATION
, request_satisfaction_mode(false)
, request_satisfaction_offset(0)
#endif
{
setConn(aConn);
al = new AccessLogEntry;
al->cache.start_time = current_time;
if (aConn) {
al->tcpClient = clientConnection = aConn->clientConnection;
al->cache.port = aConn->port;
al->cache.caddr = aConn->log_addr;
#if USE_OPENSSL
if (aConn->clientConnection != NULL && aConn->clientConnection->isOpen()) {
if (auto ssl = fd_table[aConn->clientConnection->fd].ssl.get())
- al->cache.sslClientCert.reset(SSL_get_peer_certificate(ssl));
+ al->cache.sslClientCert.resetWithoutLocking(SSL_get_peer_certificate(ssl));
}
#endif
}
dlinkAdd(this, &active, &ClientActiveRequests);
}
/*
* returns true if client specified that the object must come from the cache
* without contacting origin server
*/
bool
ClientHttpRequest::onlyIfCached()const
{
assert(request);
return request->cache_control &&
request->cache_control->onlyIfCached();
}
/**
* This function is designed to serve a fairly specific purpose.
=== modified file 'src/comm.cc'
--- src/comm.cc 2016-06-15 15:37:44 +0000
+++ src/comm.cc 2016-07-04 16:42:13 +0000
@@ -821,41 +821,41 @@
if (setsockopt(fd, SOL_SOCKET, SO_LINGER, (char *) &L, sizeof(L)) < 0) {
int xerrno = errno;
debugs(50, DBG_CRITICAL, "ERROR: Closing FD " << fd << " with TCP RST: " << xstrerr(xerrno));
}
comm_close(fd);
}
#if USE_OPENSSL
void
commStartSslClose(const FdeCbParams ¶ms)
{
assert(fd_table[params.fd].ssl);
ssl_shutdown_method(fd_table[params.fd].ssl.get());
}
#endif
void
comm_close_complete(const FdeCbParams ¶ms)
{
fde *F = &fd_table[params.fd];
- F->ssl.reset(nullptr);
+ F->ssl.resetWithoutLocking(nullptr);
#if USE_OPENSSL
if (F->dynamicSslContext) {
SSL_CTX_free(F->dynamicSslContext);
F->dynamicSslContext = NULL;
}
#endif
fd_close(params.fd); /* update fdstat */
close(params.fd);
++ statCounter.syscalls.sock.closes;
/* When one connection closes, give accept() a chance, if need be */
Comm::AcceptLimiter::Instance().kick();
}
/*
* Close the socket fd.
*
* + call write handlers with ERR_CLOSING
=== modified file 'src/fde.h'
--- src/fde.h 2016-01-26 21:02:00 +0000
+++ src/fde.h 2016-07-04 15:22:24 +0000
@@ -150,39 +150,39 @@
memset(&flags,0,sizeof(_fde_flags));
bytes_read = 0;
bytes_written = 0;
pconn.uses = 0;
#if USE_DELAY_POOLS
clientInfo = NULL;
#endif
epoll_state = 0;
read_handler = NULL;
read_data = NULL;
write_handler = NULL;
write_data = NULL;
timeoutHandler = NULL;
timeout = 0;
writeStart = 0;
lifetime_data = NULL;
closeHandler = NULL;
halfClosedReader = NULL;
read_method = NULL;
write_method = NULL;
- ssl.reset(nullptr);
+ ssl.resetWithoutLocking(nullptr);
dynamicSslContext = NULL;
#if _SQUID_WINDOWS_
win32.handle = (long)NULL;
#endif
tosFromServer = '\0';
nfmarkFromServer = 0;
}
};
#define fd_table fde::Table
int fdNFree(void);
#define FD_READ_METHOD(fd, buf, len) (*fd_table[fd].read_method)(fd, buf, len)
#define FD_WRITE_METHOD(fd, buf, len) (*fd_table[fd].write_method)(fd, buf, len)
#endif /* SQUID_FDE_H */
=== modified file 'src/security/LockingPointer.h'
--- src/security/LockingPointer.h 2016-03-31 23:33:45 +0000
+++ src/security/LockingPointer.h 2016-07-04 15:22:20 +0000
@@ -1,89 +1,136 @@
/*
* Copyright (C) 1996-2016 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#ifndef SQUID_SRC_SECURITY_LOCKINGPOINTER_H
#define SQUID_SRC_SECURITY_LOCKINGPOINTER_H
-#include "base/TidyPointer.h"
-
#if USE_OPENSSL
#if HAVE_OPENSSL_CRYPTO_H
#include <openssl/crypto.h>
#endif
// Macro to be used to define the C++ wrapper function of a sk_*_pop_free
// openssl family functions. The C++ function suffixed with the _free_wrapper
// extension
#define sk_free_wrapper(sk_object, argument, freefunction) \
extern "C++" inline void sk_object ## _free_wrapper(argument a) { \
sk_object ## _pop_free(a, freefunction); \
}
-#endif
+#endif /* USE_OPENSSL */
// Macro to be used to define the C++ equivalent function of an extern "C"
// function. The C++ function suffixed with the _cpp extension
#define CtoCpp1(function, argument) \
extern "C++" inline void function ## _cpp(argument a) { \
function(a); \
}
namespace Security
{
/**
- * Add SSL locking (a.k.a. reference counting) and assignment to TidyPointer
+ * A shared pointer to a reference-counting Object with library-specific
+ * absorption, locking, and unlocking implementations. The API largely
+ * follows std::shared_ptr.
+ *
+ * The constructor and the resetWithoutLocking() method import a raw Object pointer.
+ * Normally, reset() would lock(), but libraries like OpenSSL
+ * pre-lock objects before they are fed to LockingPointer, necessitating
+ * this resetWithoutLocking() customization hook.
+ *
+ * The lock() method increments Object's reference counter.
+ *
+ * The unlock() method decrements Object's reference counter and destroys
+ * the object when the counter reaches zero.
*/
-template <typename T, void (*DeAllocator)(T *t), int lock>
-class LockingPointer: public TidyPointer<T, DeAllocator>
+template <typename T, void (*UnLocker)(T *t), int lockId>
+class LockingPointer
{
public:
- typedef TidyPointer<T, DeAllocator> Parent;
- typedef LockingPointer<T, DeAllocator, lock> SelfType;
+ /// a helper label to simplify this objects API definitions below
+ typedef LockingPointer<T, UnLocker, lockId> SelfType;
- explicit LockingPointer(T *t = nullptr): Parent(t) {}
+ /**
+ * Construct directly from a raw pointer.
+ * This action requires that the producer of that pointer has already
+ * created one reference lock for the object pointed to.
+ * Our destructor will do the matching unlock.
+ */
+ explicit LockingPointer(T *t = nullptr): raw(t) {}
- explicit LockingPointer(const SelfType &o): Parent() {
- resetAndLock(o.get());
- }
+ /// use the custom UnLocker to unlock any value still stored.
+ ~LockingPointer() { unlock(); }
+ // copy semantics are okay only when adding a lock reference
+ explicit LockingPointer(const SelfType &o) : raw(nullptr) { resetAndLock(o.get()); }
SelfType &operator =(const SelfType & o) {
resetAndLock(o.get());
return *this;
}
-#if __cplusplus >= 201103L
- explicit LockingPointer(LockingPointer<T, DeAllocator, lock> &&o): Parent(o.release()) {
+ // move semantics are definitely okay, when possible
+ explicit LockingPointer(SelfType &&) = default;
+ SelfType &operator =(SelfType &&o) {
+ if (o.get() != raw)
+ resetWithoutLocking(o.release());
+ return *this;
}
- LockingPointer<T, DeAllocator, lock> &operator =(LockingPointer<T, DeAllocator, lock> &&o) {
- if (o.get() != this->get())
- this->reset(o.release());
- return *this;
+ bool operator !() const { return !raw; }
+ explicit operator bool() const { return raw; }
+
+ /// Returns raw and possibly nullptr pointer
+ T *get() const { return raw; }
+
+ /// Reset raw pointer - unlock any previous one and save new one without locking.
+ void resetWithoutLocking(T *t) {
+ unlock();
+ raw = t;
}
-#endif
void resetAndLock(T *t) {
- if (t != this->get()) {
- this->reset(t);
+ if (t != get()) {
+ resetWithoutLocking(t);
+ lock(t);
+ }
+ }
+
+ /// Forget the raw pointer without unlocking it. Become a nil pointer.
+ T *release() {
+ T *ret = raw;
+ raw = nullptr;
+ return ret;
+ }
+
+private:
+ void lock(T *t) {
#if USE_OPENSSL
if (t)
- CRYPTO_add(&t->references, 1, lock);
+ CRYPTO_add(&t->references, 1, lockId);
#elif USE_GNUTLS
// XXX: GnuTLS does not provide locking ?
#else
assert(false);
#endif
- }
}
+
+ /// Unlock the raw pointer. Become a nil pointer.
+ void unlock() {
+ if (raw)
+ UnLocker(raw);
+ raw = nullptr;
+ }
+
+ T *raw; ///< pointer to T object or nullptr
};
} // namespace Security
#endif /* SQUID_SRC_SECURITY_LOCKINGPOINTER_H */
=== modified file 'src/security/ServerOptions.cc'
--- src/security/ServerOptions.cc 2016-07-05 17:00:36 +0000
+++ src/security/ServerOptions.cc 2016-07-05 23:39:23 +0000
@@ -144,41 +144,41 @@
DH *dhp = nullptr;
if (FILE *in = fopen(dhParamsFile.c_str(), "r")) {
dhp = PEM_read_DHparams(in, NULL, NULL, NULL);
fclose(in);
}
if (!dhp) {
debugs(83, DBG_IMPORTANT, "WARNING: Failed to read DH parameters '" << dhParamsFile << "'");
return;
}
int codes;
if (DH_check(dhp, &codes) == 0) {
if (codes) {
debugs(83, DBG_IMPORTANT, "WARNING: Failed to verify DH parameters '" << dhParamsFile << "' (" << std::hex << codes << ")");
DH_free(dhp);
dhp = nullptr;
}
}
- parsedDhParams.reset(dhp);
+ parsedDhParams.resetWithoutLocking(dhp);
#endif
}
void
Security::ServerOptions::updateContextEecdh(Security::ContextPtr &ctx)
{
// set Elliptic Curve details into the server context
if (!eecdhCurve.isEmpty()) {
debugs(83, 9, "Setting Ephemeral ECDH curve to " << eecdhCurve << ".");
#if USE_OPENSSL && OPENSSL_VERSION_NUMBER >= 0x0090800fL && !defined(OPENSSL_NO_ECDH)
int nid = OBJ_sn2nid(eecdhCurve.c_str());
if (!nid) {
debugs(83, DBG_CRITICAL, "ERROR: Unknown EECDH curve '" << eecdhCurve << "'");
return;
}
auto ecdh = EC_KEY_new_by_curve_name(nid);
if (!ecdh) {
auto ssl_error = ERR_get_error();
=== modified file 'src/security/Session.cc'
--- src/security/Session.cc 2016-06-30 22:10:55 +0000
+++ src/security/Session.cc 2016-07-07 11:22:07 +0000
@@ -1,38 +1,45 @@
/*
* Copyright (C) 1996-2016 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#include "squid.h"
#include "anyp/PortCfg.h"
#include "base/RunnersRegistry.h"
#include "ipc/MemMap.h"
#include "security/Session.h"
#include "SquidConfig.h"
#define SSL_SESSION_ID_SIZE 32
#define SSL_SESSION_MAX_SIZE 10*1024
+#if USE_GNUTLS
+void
+squid_datum_free(gnutls_datum_t *D) {
+ gnutls_free(D);
+}
+#endif
+
bool
Security::SessionIsResumed(const Security::SessionPointer &s)
{
return
#if USE_OPENSSL
SSL_session_reused(s.get()) == 1;
#elif USE_GNUTLS
gnutls_session_is_resumed(s.get()) != 0;
#else
false;
#endif
}
void
Security::GetSessionResumeData(const Security::SessionPointer &s, Security::SessionStatePointer &data)
{
if (!SessionIsResumed(s)) {
#if USE_OPENSSL
data.reset(SSL_get1_session(s.get()));
#elif USE_GNUTLS
=== modified file 'src/security/Session.h'
--- src/security/Session.h 2016-06-30 22:10:55 +0000
+++ src/security/Session.h 2016-07-07 11:27:25 +0000
@@ -1,72 +1,72 @@
/*
* Copyright (C) 1996-2016 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#ifndef SQUID_SRC_SECURITY_SESSION_H
#define SQUID_SRC_SECURITY_SESSION_H
-// LockingPointer.h instead of TidyPointer.h for CtoCpp1()
#include "security/LockingPointer.h"
+#include <memory>
+
#if USE_OPENSSL
#if HAVE_OPENSSL_SSL_H
#include <openssl/ssl.h>
#endif
#endif
#if USE_GNUTLS
#if HAVE_GNUTLS_GNUTLS_H
#include <gnutls/gnutls.h>
#endif
#endif
namespace Security {
#if USE_OPENSSL
typedef SSL* SessionPtr;
CtoCpp1(SSL_free, SSL *);
typedef LockingPointer<SSL, Security::SSL_free_cpp, CRYPTO_LOCK_SSL> SessionPointer;
-typedef SSL_SESSION* SessionStatePtr;
-CtoCpp1(SSL_SESSION_free, SSL_SESSION *);
-typedef LockingPointer<SSL_SESSION, Security::SSL_SESSION_free_cpp, CRYPTO_LOCK_SSL_SESSION> SessionStatePointer;
+typedef std::unique_ptr<SSL_SESSION, std::function<decltype(SSL_SESSION_free)>> SessionStatePointer;
#elif USE_GNUTLS
typedef gnutls_session_t SessionPtr;
-CtoCpp1(gnutls_deinit, gnutls_session_t);
-// TODO: Convert to Locking pointer.
// Locks can be implemented attaching locks counter to gnutls_session_t
// objects using the gnutls_session_set_ptr()/gnutls_session_get_ptr ()
// library functions
-typedef TidyPointer<struct gnutls_session_int, Security::gnutls_deinit_cpp> SessionPointer;
+CtoCpp1(gnutls_deinit, gnutls_session_t);
+typedef LockingPointer<struct gnutls_session_int, gnutls_deinit_cpp, -1> SessionPointer;
-typedef gnutls_datum_t *SessionStatePtr;
-CtoCpp1(gnutls_free, gnutls_datum_t *);
-typedef TidyPointer<gnutls_datum_t, Security::gnutls_free_cpp> SessionStatePointer;
+/// wrapper function to avoid compile errors with gnutls_free() being a typedef.
+void squid_datum_free(gnutls_datum_t *D);
+typedef std::unique_ptr<gnutls_datum_t, std::function<decltype(squid_datum_free)>> SessionStatePointer;
#else
// use void* so we can check against NULL
typedef void* SessionPtr;
-typedef TidyPointer<void, nullptr> SessionPointer;
-typedef TidyPointer<void, nullptr> SessionStatePointer;
+CtoCpp1(xfree, SessionPtr);
+typedef LockingPointer<void, xfree_cpp, -1> SessionPointer;
+
+typedef std::unique_ptr<int> SessionStatePointer;
#endif
/// whether the session is a resumed one
bool SessionIsResumed(const Security::SessionPointer &);
/// Retrieve the data needed to resume this session on a later connection
void GetSessionResumeData(const Security::SessionPointer &, Security::SessionStatePointer &);
/// Set the data for resuming a previous session.
/// Needs to be done before using the SessionPointer for a handshake.
void SetSessionResumeData(const Security::SessionPtr &, const Security::SessionStatePointer &);
} // namespace Security
#endif /* SQUID_SRC_SECURITY_SESSION_H */
=== modified file 'src/security/cert_generators/file/certificate_db.cc'
--- src/security/cert_generators/file/certificate_db.cc 2016-02-13 16:10:26 +0000
+++ src/security/cert_generators/file/certificate_db.cc 2016-07-04 16:39:09 +0000
@@ -1,33 +1,34 @@
/*
* Copyright (C) 1996-2016 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#include "squid.h"
#include "security/cert_generators/file/certificate_db.h"
#include <cerrno>
#include <fstream>
+#include <memory>
#include <stdexcept>
#if HAVE_SYS_STAT_H
#include <sys/stat.h>
#endif
#if HAVE_SYS_FILE_H
#include <sys/file.h>
#endif
#if HAVE_FCNTL_H
#include <fcntl.h>
#endif
#define HERE "(security_file_certgen) " << __FILE__ << ':' << __LINE__ << ": "
Ssl::Lock::Lock(std::string const &aFilename) :
filename(aFilename),
#if _SQUID_WINDOWS_
hFile(INVALID_HANDLE_VALUE)
#else
fd(-1)
#endif
@@ -235,137 +236,136 @@
return strcmp(aa, bb);
}
unsigned long Ssl::CertificateDb::index_name_hash(const char **a) {
return(lh_strhash(a[Ssl::CertificateDb::cnlName]));
}
int Ssl::CertificateDb::index_name_cmp(const char **a, const char **b) {
return(strcmp(a[Ssl::CertificateDb::cnlName], b[CertificateDb::cnlName]));
}
const std::string Ssl::CertificateDb::db_file("index.txt");
const std::string Ssl::CertificateDb::cert_dir("certs");
const std::string Ssl::CertificateDb::size_file("size");
Ssl::CertificateDb::CertificateDb(std::string const & aDb_path, size_t aMax_db_size, size_t aFs_block_size)
: db_path(aDb_path),
db_full(aDb_path + "/" + db_file),
cert_full(aDb_path + "/" + cert_dir),
size_full(aDb_path + "/" + size_file),
- db(NULL),
max_db_size(aMax_db_size),
fs_block_size((aFs_block_size ? aFs_block_size : 2048)),
dbLock(db_full),
enabled_disk_store(true) {
if (db_path.empty() && !max_db_size)
enabled_disk_store = false;
else if ((db_path.empty() && max_db_size) || (!db_path.empty() && !max_db_size))
throw std::runtime_error("security_file_certgen is missing the required parameter. There should be -s and -M parameters together.");
}
bool Ssl::CertificateDb::find(std::string const & host_name, Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey) {
const Locker locker(dbLock, Here);
load();
return pure_find(host_name, cert, pkey);
}
bool Ssl::CertificateDb::purgeCert(std::string const & key) {
const Locker locker(dbLock, Here);
load();
if (!db)
return false;
if (!deleteByHostname(key))
return false;
save();
return true;
}
bool Ssl::CertificateDb::addCertAndPrivateKey(Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey, std::string const & useName) {
const Locker locker(dbLock, Here);
load();
if (!db || !cert || !pkey)
return false;
Row row;
ASN1_INTEGER * ai = X509_get_serialNumber(cert.get());
std::string serial_string;
Ssl::BIGNUM_Pointer serial(ASN1_INTEGER_to_BN(ai, NULL));
{
- TidyPointer<char, tidyFree> hex_bn(BN_bn2hex(serial.get()));
+ std::unique_ptr<char, std::function<decltype(xfree)>> hex_bn(BN_bn2hex(serial.get()));
serial_string = std::string(hex_bn.get());
}
row.setValue(cnlSerial, serial_string.c_str());
char ** rrow = TXT_DB_get_by_index(db.get(), cnlSerial, row.getRow());
// We are creating certificates with unique serial numbers. If the serial
// number is found in the database, the same certificate is already stored.
if (rrow != NULL) {
// TODO: check if the stored row is valid.
return true;
}
{
- TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
+ std::unique_ptr<char, std::function<decltype(xfree)>> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), nullptr, 0));
Security::CertPointer findCert;
Ssl::EVP_PKEY_Pointer findPkey;
if (pure_find(useName.empty() ? subject.get() : useName, findCert, findPkey)) {
// Replace with database certificate
- cert.reset(findCert.release());
- pkey.reset(findPkey.release());
+ cert = std::move(findCert);
+ pkey = std::move(findPkey);
return true;
}
// pure_find may fail because the entry is expired, or because the
// certs file is corrupted. Remove any entry with given hostname
deleteByHostname(useName.empty() ? subject.get() : useName);
}
// check db size while trying to minimize calls to size()
size_t dbSize = size();
if ((dbSize == 0 && hasRows()) ||
(dbSize > 0 && !hasRows()) ||
(dbSize > 10 * max_db_size)) {
// Invalid database size, rebuild
dbSize = rebuildSize();
}
while (dbSize > max_db_size && deleteInvalidCertificate()) {
dbSize = size(); // get the current database size
// and try to find another invalid certificate if needed
}
// there are no more invalid ones, but there must be valid certificates
while (dbSize > max_db_size) {
if (!deleteOldestCertificate()) {
rebuildSize(); // No certificates in database.Update the size file.
save(); // Some entries may have been removed. Update the index file.
return false; // errors prevented us from freeing enough space
}
dbSize = size(); // get the current database size
}
row.setValue(cnlType, "V");
ASN1_UTCTIME * tm = X509_get_notAfter(cert.get());
row.setValue(cnlExp_date, std::string(reinterpret_cast<char *>(tm->data), tm->length).c_str());
row.setValue(cnlFile, "unknown");
if (!useName.empty())
row.setValue(cnlName, useName.c_str());
else {
- TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
+ std::unique_ptr<char, std::function<decltype(xfree)>> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), nullptr, 0));
row.setValue(cnlName, subject.get());
}
if (!TXT_DB_insert(db.get(), row.getRow())) {
// failed to add index (???) but we may have already modified
// the database so save before exit
save();
return false;
}
rrow = row.getRow();
row.reset();
std::string filename(cert_full + "/" + serial_string + ".pem");
if (!writeCertAndPrivateKeyToFile(cert, pkey, filename.c_str())) {
//remove row from txt_db and save
sq_TXT_DB_delete(db.get(), (const char **)rrow);
save();
return false;
}
addSize(filename);
=== modified file 'src/security/cert_generators/file/security_file_certgen.cc'
--- src/security/cert_generators/file/security_file_certgen.cc 2016-05-02 03:17:18 +0000
+++ src/security/cert_generators/file/security_file_certgen.cc 2016-07-04 16:40:18 +0000
@@ -187,42 +187,42 @@
throw std::runtime_error("Error while parsing the crtd request: " + error);
Ssl::CertificateDb db(db_path, max_db_size, fs_block_size);
Security::CertPointer cert;
Ssl::EVP_PKEY_Pointer pkey;
std::string &cert_subject = certProperties.dbKey();
bool dbFailed = false;
try {
db.find(cert_subject, cert, pkey);
} catch (std::runtime_error &err) {
dbFailed = true;
error = err.what();
}
if (cert.get()) {
if (!Ssl::certificateMatchesProperties(cert.get(), certProperties)) {
// The certificate changed (renewed or other reason).
// Generete a new one with the updated fields.
- cert.reset(NULL);
- pkey.reset(NULL);
+ cert.resetWithoutLocking(nullptr);
+ pkey.resetWithoutLocking(nullptr);
db.purgeCert(cert_subject);
}
}
if (!cert || !pkey) {
if (!Ssl::generateSslCertificate(cert, pkey, certProperties))
throw std::runtime_error("Cannot create ssl certificate or private key.");
if (!dbFailed && db.IsEnabledDiskStore()) {
try {
if (!db.addCertAndPrivateKey(cert, pkey, cert_subject)) {
dbFailed = true;
error = "Cannot add certificate to db.";
}
} catch (const std::runtime_error &err) {
dbFailed = true;
error = err.what();
}
}
}
=== modified file 'src/security/forward.h'
--- src/security/forward.h 2016-01-01 00:12:18 +0000
+++ src/security/forward.h 2016-06-29 15:28:46 +0000
@@ -2,40 +2,50 @@
* Copyright (C) 1996-2016 The Squid Software Foundation and contributors
*
* Squid software is distributed under GPLv2+ license and includes
* contributions from numerous individuals and organizations.
* Please see the COPYING and CONTRIBUTORS files for details.
*/
#ifndef SQUID_SRC_SECURITY_FORWARD_H
#define SQUID_SRC_SECURITY_FORWARD_H
#include "security/Context.h"
#include "security/Session.h"
#if USE_GNUTLS
#if HAVE_GNUTLS_X509_H
#include <gnutls/x509.h>
#endif
#endif
#include <list>
+#if USE_OPENSSL
+// Macro to be used to define the C++ wrapper functor of the sk_*_pop_free
+// OpenSSL family of functions. The C++ functor is suffixed with the _free_wrapper
+// extension
+#define sk_dtor_wrapper(sk_object, argument_type, freefunction) \
+ struct sk_object ## _free_wrapper { \
+ void operator()(argument_type a) { sk_object ## _pop_free(a, freefunction); } \
+ }
+#endif /* USE_OPENSSL */
+
/* flags a SSL connection can be configured with */
#define SSL_FLAG_NO_DEFAULT_CA (1<<0)
#define SSL_FLAG_DELAYED_AUTH (1<<1)
#define SSL_FLAG_DONT_VERIFY_PEER (1<<2)
#define SSL_FLAG_DONT_VERIFY_DOMAIN (1<<3)
#define SSL_FLAG_NO_SESSION_REUSE (1<<4)
#define SSL_FLAG_VERIFY_CRL (1<<5)
#define SSL_FLAG_VERIFY_CRL_ALL (1<<6)
/// Network/connection security abstraction layer
namespace Security
{
class EncryptorAnswer;
class PeerOptions;
class ServerOptions;
#if USE_OPENSSL
CtoCpp1(X509_free, X509 *)
typedef Security::LockingPointer<X509, X509_free_cpp, CRYPTO_LOCK_X509> CertPointer;
=== modified file 'src/ssl/PeekingPeerConnector.cc'
--- src/ssl/PeekingPeerConnector.cc 2016-05-18 16:35:36 +0000
+++ src/ssl/PeekingPeerConnector.cc 2016-07-04 15:22:24 +0000
@@ -322,55 +322,55 @@
Ssl::PeerConnector::noteSslNegotiationError(result, ssl_error, ssl_lib_error);
}
void
Ssl::PeekingPeerConnector::handleServerCertificate()
{
if (serverCertificateHandled)
return;
if (ConnStateData *csd = request->clientConnectionManager.valid()) {
const int fd = serverConnection()->fd;
Security::SessionPtr ssl = fd_table[fd].ssl.get();
Security::CertPointer serverCert(SSL_get_peer_certificate(ssl));
if (!serverCert.get())
return;
serverCertificateHandled = true;
// remember the server certificate for later use
if (Ssl::ServerBump *serverBump = csd->serverBump()) {
- serverBump->serverCert.reset(serverCert.release());
+ serverBump->serverCert = std::move(serverCert);
}
}
}
void
Ssl::PeekingPeerConnector::serverCertificateVerified()
{
if (ConnStateData *csd = request->clientConnectionManager.valid()) {
Security::CertPointer serverCert;
if(Ssl::ServerBump *serverBump = csd->serverBump())
serverCert.resetAndLock(serverBump->serverCert.get());
else {
const int fd = serverConnection()->fd;
Security::SessionPtr ssl = fd_table[fd].ssl.get();
- serverCert.reset(SSL_get_peer_certificate(ssl));
+ serverCert.resetWithoutLocking(SSL_get_peer_certificate(ssl));
}
if (serverCert.get()) {
csd->resetSslCommonName(Ssl::CommonHostName(serverCert.get()));
debugs(83, 5, "HTTPS server CN: " << csd->sslCommonName() <<
" bumped: " << *serverConnection());
}
}
}
void
Ssl::PeekingPeerConnector::tunnelInsteadOfNegotiating()
{
Must(callback != NULL);
CbDialer *dialer = dynamic_cast<CbDialer*>(callback->getDialer());
Must(dialer);
dialer->answer().tunneled = true;
debugs(83, 5, "The SSL negotiation with server aborted");
}
=== modified file 'src/ssl/gadgets.cc'
--- src/ssl/gadgets.cc 2016-06-14 18:12:14 +0000
+++ src/ssl/gadgets.cc 2016-07-04 16:37:08 +0000
@@ -113,59 +113,59 @@
if (!bio)
return false;
if (!BIO_write_filename(bio.get(), const_cast<char *>(filename)))
return false;
if (!PEM_write_bio_X509(bio.get(), cert.get()))
return false;
if (!PEM_write_bio_PrivateKey(bio.get(), pkey.get(), NULL, NULL, 0, NULL, NULL))
return false;
return true;
}
bool Ssl::readCertAndPrivateKeyFromMemory(Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey, char const * bufferToRead)
{
Ssl::BIO_Pointer bio(BIO_new(BIO_s_mem()));
BIO_puts(bio.get(), bufferToRead);
X509 * certPtr = NULL;
- cert.reset(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
+ cert.resetWithoutLocking(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
if (!cert)
return false;
EVP_PKEY * pkeyPtr = NULL;
- pkey.reset(PEM_read_bio_PrivateKey(bio.get(), &pkeyPtr, 0, 0));
+ pkey.resetWithoutLocking(PEM_read_bio_PrivateKey(bio.get(), &pkeyPtr, 0, 0));
if (!pkey)
return false;
return true;
}
bool Ssl::readCertFromMemory(Security::CertPointer & cert, char const * bufferToRead)
{
Ssl::BIO_Pointer bio(BIO_new(BIO_s_mem()));
BIO_puts(bio.get(), bufferToRead);
X509 * certPtr = NULL;
- cert.reset(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
+ cert.resetWithoutLocking(PEM_read_bio_X509(bio.get(), &certPtr, 0, 0));
if (!cert)
return false;
return true;
}
// According to RFC 5280 (Section A.1), the common name length in a certificate
// can be at most 64 characters
static const size_t MaxCnLen = 64;
// Replace certs common name with the given
static bool replaceCommonName(Security::CertPointer & cert, std::string const &rawCn)
{
std::string cn = rawCn;
if (cn.length() > MaxCnLen) {
// In the case the length od CN is more than the maximum supported size
// try to use the first upper level domain.
size_t pos = 0;
do {
@@ -494,81 +494,81 @@
}
}
addedExtensions += mimicExtensions(cert, properties.mimicCert, properties.signWithX509);
// According to RFC 5280, using extensions requires v3 certificate.
if (addedExtensions)
X509_set_version(cert.get(), 2); // value 2 means v3
}
return true;
}
static bool generateFakeSslCertificate(Security::CertPointer & certToStore, Ssl::EVP_PKEY_Pointer & pkeyToStore, Ssl::CertificateProperties const &properties, Ssl::BIGNUM_Pointer const &serial)
{
Ssl::EVP_PKEY_Pointer pkey;
// Use signing certificates private key as generated certificate private key
if (properties.signWithPkey.get())
pkey.resetAndLock(properties.signWithPkey.get());
else // if not exist generate one
- pkey.reset(Ssl::createSslPrivateKey());
+ pkey.resetWithoutLocking(Ssl::createSslPrivateKey());
if (!pkey)
return false;
Security::CertPointer cert(X509_new());
if (!cert)
return false;
// Set pub key and serial given by the caller
if (!X509_set_pubkey(cert.get(), pkey.get()))
return false;
if (!setSerialNumber(X509_get_serialNumber(cert.get()), serial.get()))
return false;
// Fill the certificate with the required properties
if (!buildCertificate(cert, properties))
return false;
int ret = 0;
// Set issuer name, from CA or our subject name for self signed cert
if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithX509.get())
ret = X509_set_issuer_name(cert.get(), X509_get_subject_name(properties.signWithX509.get()));
else // Self signed certificate, set issuer to self
ret = X509_set_issuer_name(cert.get(), X509_get_subject_name(cert.get()));
if (!ret)
return false;
const EVP_MD *hash = properties.signHash ? properties.signHash : EVP_get_digestbyname(SQUID_SSL_SIGN_HASH_IF_NONE);
assert(hash);
/*Now sign the request */
if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithPkey.get())
ret = X509_sign(cert.get(), properties.signWithPkey.get(), hash);
else //else sign with self key (self signed request)
ret = X509_sign(cert.get(), pkey.get(), hash);
if (!ret)
return false;
- certToStore.reset(cert.release());
- pkeyToStore.reset(pkey.release());
+ certToStore = std::move(cert);
+ pkeyToStore = std::move(pkey);
return true;
}
static BIGNUM *createCertSerial(unsigned char *md, unsigned int n)
{
assert(n == 20); //for sha1 n is 20 (for md5 n is 16)
BIGNUM *serial = NULL;
serial = BN_bin2bn(md, n, NULL);
// if the serial is "0" set it to '1'
if (BN_is_zero(serial) == true)
BN_one(serial);
// serial size does not exceed 20 bytes
assert(BN_num_bits(serial) <= 160);
// According the RFC 5280, serial is an 20 bytes ASN.1 INTEGER (a signed big integer)
// and the maximum value for X.509 certificate serial number is 2^159-1 and
@@ -659,45 +659,45 @@
return certificate;
}
EVP_PKEY * Ssl::readSslPrivateKey(char const * keyFilename, pem_password_cb *passwd_callback)
{
if (!keyFilename)
return NULL;
Ssl::BIO_Pointer bio(BIO_new(BIO_s_file_internal()));
if (!bio)
return NULL;
if (!BIO_read_filename(bio.get(), keyFilename))
return NULL;
EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio.get(), NULL, passwd_callback, NULL);
return pkey;
}
void Ssl::readCertAndPrivateKeyFromFiles(Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey, char const * certFilename, char const * keyFilename)
{
if (keyFilename == NULL)
keyFilename = certFilename;
- pkey.reset(readSslPrivateKey(keyFilename));
- cert.reset(readSslX509Certificate(certFilename));
+ pkey.resetWithoutLocking(readSslPrivateKey(keyFilename));
+ cert.resetWithoutLocking(readSslX509Certificate(certFilename));
if (!pkey || !cert || !X509_check_private_key(cert.get(), pkey.get())) {
- pkey.reset(NULL);
- cert.reset(NULL);
+ pkey.resetWithoutLocking(nullptr);
+ cert.resetWithoutLocking(nullptr);
}
}
bool Ssl::sslDateIsInTheFuture(char const * date)
{
ASN1_UTCTIME tm;
tm.flags = 0;
tm.type = 23;
tm.data = (unsigned char *)date;
tm.length = strlen(date);
return (X509_cmp_current_time(&tm) > 0);
}
/// Print the time represented by a ASN1_TIME struct to a string using GeneralizedTime format
static bool asn1timeToGeneralizedTimeStr(ASN1_TIME *aTime, char *buf, int bufLen)
{
// ASN1_Time holds time to UTCTime or GeneralizedTime form.
// UTCTime has the form YYMMDDHHMMSS[Z | [+|-]offset]
// GeneralizedTime has the form YYYYMMDDHHMMSS[Z | [+|-] offset]
=== modified file 'src/ssl/gadgets.h'
--- src/ssl/gadgets.h 2016-06-14 15:56:12 +0000
+++ src/ssl/gadgets.h 2016-06-29 15:28:46 +0000
@@ -22,87 +22,75 @@
namespace Ssl
{
/**
\defgroup SslCrtdSslAPI SSL certificate generator API
These functions must not depend on Squid runtime code such as debug()
because they are used by security_file_certgen helper.
*/
#if SQUID_USE_CONST_SSL_METHOD
typedef const SSL_METHOD * ContextMethod;
#else
typedef SSL_METHOD * ContextMethod;
#endif
#if !defined(SQUID_SSL_SIGN_HASH_IF_NONE)
#define SQUID_SSL_SIGN_HASH_IF_NONE "sha256"
#endif
/**
- \ingroup SslCrtdSslAPI
- * TidyPointer typedefs for common SSL objects
+ * std::unique_ptr typedefs for common SSL objects
*/
-sk_free_wrapper(sk_X509, STACK_OF(X509) *, X509_free)
-typedef TidyPointer<STACK_OF(X509), sk_X509_free_wrapper> X509_STACK_Pointer;
+sk_dtor_wrapper(sk_X509, STACK_OF(X509) *, X509_free);
+typedef std::unique_ptr<STACK_OF(X509), sk_X509_free_wrapper> X509_STACK_Pointer;
CtoCpp1(EVP_PKEY_free, EVP_PKEY *)
typedef Security::LockingPointer<EVP_PKEY, EVP_PKEY_free_cpp, CRYPTO_LOCK_EVP_PKEY> EVP_PKEY_Pointer;
-CtoCpp1(BN_free, BIGNUM *)
-typedef TidyPointer<BIGNUM, BN_free_cpp> BIGNUM_Pointer;
+typedef std::unique_ptr<BIGNUM, std::function<decltype(BN_free)>> BIGNUM_Pointer;
-CtoCpp1(BIO_free, BIO *)
-typedef TidyPointer<BIO, BIO_free_cpp> BIO_Pointer;
+typedef std::unique_ptr<BIO, std::function<decltype(BIO_free)>> BIO_Pointer;
-CtoCpp1(ASN1_INTEGER_free, ASN1_INTEGER *)
-typedef TidyPointer<ASN1_INTEGER, ASN1_INTEGER_free_cpp> ASN1_INT_Pointer;
+typedef std::unique_ptr<ASN1_INTEGER, std::function<decltype(ASN1_INTEGER_free)>> ASN1_INT_Pointer;
-CtoCpp1(ASN1_OCTET_STRING_free, ASN1_OCTET_STRING *)
-typedef TidyPointer<ASN1_OCTET_STRING, ASN1_OCTET_STRING_free_cpp> ASN1_OCTET_STRING_Pointer;
+typedef std::unique_ptr<ASN1_OCTET_STRING, std::function<decltype(ASN1_OCTET_STRING_free)>> ASN1_OCTET_STRING_Pointer;
-CtoCpp1(TXT_DB_free, TXT_DB *)
-typedef TidyPointer<TXT_DB, TXT_DB_free_cpp> TXT_DB_Pointer;
+typedef std::unique_ptr<TXT_DB, std::function<decltype(TXT_DB_free)>> TXT_DB_Pointer;
-CtoCpp1(X509_NAME_free, X509_NAME *)
-typedef TidyPointer<X509_NAME, X509_NAME_free_cpp> X509_NAME_Pointer;
+typedef std::unique_ptr<X509_NAME, std::function<decltype(X509_NAME_free)>> X509_NAME_Pointer;
-CtoCpp1(RSA_free, RSA *)
-typedef TidyPointer<RSA, RSA_free_cpp> RSA_Pointer;
+typedef std::unique_ptr<RSA, std::function<decltype(RSA_free)>> RSA_Pointer;
-CtoCpp1(X509_REQ_free, X509_REQ *)
-typedef TidyPointer<X509_REQ, X509_REQ_free_cpp> X509_REQ_Pointer;
+typedef std::unique_ptr<X509_REQ, std::function<decltype(X509_REQ_free)>> X509_REQ_Pointer;
-sk_free_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free)
-typedef TidyPointer<STACK_OF(X509_NAME), sk_X509_NAME_free_wrapper> X509_NAME_STACK_Pointer;
+sk_dtor_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free);
+typedef std::unique_ptr<STACK_OF(X509_NAME), sk_X509_NAME_free_wrapper> X509_NAME_STACK_Pointer;
-CtoCpp1(AUTHORITY_KEYID_free, AUTHORITY_KEYID *)
-typedef TidyPointer<AUTHORITY_KEYID, AUTHORITY_KEYID_free_cpp> AUTHORITY_KEYID_Pointer;
+typedef std::unique_ptr<AUTHORITY_KEYID, std::function<decltype(AUTHORITY_KEYID_free)>> AUTHORITY_KEYID_Pointer;
-sk_free_wrapper(sk_GENERAL_NAME, STACK_OF(GENERAL_NAME) *, GENERAL_NAME_free)
-typedef TidyPointer<STACK_OF(GENERAL_NAME), sk_GENERAL_NAME_free_wrapper> GENERAL_NAME_STACK_Pointer;
+sk_dtor_wrapper(sk_GENERAL_NAME, STACK_OF(GENERAL_NAME) *, GENERAL_NAME_free);
+typedef std::unique_ptr<STACK_OF(GENERAL_NAME), sk_GENERAL_NAME_free_wrapper> GENERAL_NAME_STACK_Pointer;
-CtoCpp1(GENERAL_NAME_free, GENERAL_NAME *)
-typedef TidyPointer<GENERAL_NAME, GENERAL_NAME_free_cpp> GENERAL_NAME_Pointer;
+typedef std::unique_ptr<GENERAL_NAME, std::function<decltype(GENERAL_NAME_free)>> GENERAL_NAME_Pointer;
-CtoCpp1(X509_EXTENSION_free, X509_EXTENSION *)
-typedef TidyPointer<X509_EXTENSION, X509_EXTENSION_free_cpp> X509_EXTENSION_Pointer;
+typedef std::unique_ptr<X509_EXTENSION, std::function<decltype(X509_EXTENSION_free)>> X509_EXTENSION_Pointer;
/**
\ingroup SslCrtdSslAPI
* Create 1024 bits rsa key.
*/
EVP_PKEY * createSslPrivateKey();
/**
\ingroup SslCrtdSslAPI
* Write private key and SSL certificate to memory.
*/
bool writeCertAndPrivateKeyToMemory(Security::CertPointer const & cert, EVP_PKEY_Pointer const & pkey, std::string & bufferToWrite);
/**
\ingroup SslCrtdSslAPI
* Append SSL certificate to bufferToWrite.
*/
bool appendCertToMemory(Security::CertPointer const & cert, std::string & bufferToWrite);
/**
=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc 2016-07-05 17:00:36 +0000
+++ src/ssl/support.cc 2016-07-05 23:39:23 +0000
@@ -298,41 +298,41 @@
debugs(83, 5, err_descr << ": " << buffer);
else
debugs(83, DBG_IMPORTANT, "SSL unknown certificate error " << error_no << " in " << buffer);
// Check if the certificate error can be bypassed.
// Infinity validation loop errors can not bypassed.
if (error_no != SQUID_X509_V_ERR_INFINITE_VALIDATION) {
if (check) {
ACLFilledChecklist *filledCheck = Filled(check);
assert(!filledCheck->sslErrors);
filledCheck->sslErrors = new Ssl::CertErrors(Ssl::CertError(error_no, broken_cert));
filledCheck->serverCert.resetAndLock(peer_cert);
if (check->fastCheck() == ACCESS_ALLOWED) {
debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
ok = 1;
} else {
debugs(83, 5, "confirming SSL error " << error_no);
}
delete filledCheck->sslErrors;
filledCheck->sslErrors = NULL;
- filledCheck->serverCert.reset(NULL);
+ filledCheck->serverCert.resetWithoutLocking(nullptr);
}
// If the certificate validator is used then we need to allow all errors and
// pass them to certficate validator for more processing
else if (Ssl::TheConfig.ssl_crt_validator) {
ok = 1;
}
}
}
if (Ssl::TheConfig.ssl_crt_validator) {
// Check if we have stored certificates chain. Store if not.
if (!SSL_get_ex_data(ssl, ssl_ex_index_ssl_cert_chain)) {
STACK_OF(X509) *certStack = X509_STORE_CTX_get1_chain(ctx);
if (certStack && !SSL_set_ex_data(ssl, ssl_ex_index_ssl_cert_chain, certStack))
sk_X509_pop_free(certStack, X509_free);
}
}
if (!ok && !SSL_get_ex_data(ssl, ssl_ex_index_ssl_error_detail) ) {
@@ -1250,45 +1250,45 @@
return certificate;
}
void Ssl::readCertChainAndPrivateKeyFromFiles(Security::CertPointer & cert, EVP_PKEY_Pointer & pkey, X509_STACK_Pointer & chain, char const * certFilename, char const * keyFilename)
{
if (keyFilename == NULL)
keyFilename = certFilename;
if (certFilename == NULL)
certFilename = keyFilename;
debugs(83, DBG_IMPORTANT, "Using certificate in " << certFilename);
if (!chain)
chain.reset(sk_X509_new_null());
if (!chain)
debugs(83, DBG_IMPORTANT, "WARNING: unable to allocate memory for cert chain");
// XXX: ssl_ask_password_cb needs SSL_CTX_set_default_passwd_cb_userdata()
// so this may not fully work iff Config.Program.ssl_password is set.
pem_password_cb *cb = ::Config.Program.ssl_password ? &ssl_ask_password_cb : NULL;
- pkey.reset(readSslPrivateKey(keyFilename, cb));
- cert.reset(readSslX509CertificatesChain(certFilename, chain.get()));
+ pkey.resetWithoutLocking(readSslPrivateKey(keyFilename, cb));
+ cert.resetWithoutLocking(readSslX509CertificatesChain(certFilename, chain.get()));
if (!pkey || !cert || !X509_check_private_key(cert.get(), pkey.get())) {
- pkey.reset(NULL);
- cert.reset(NULL);
+ pkey.resetWithoutLocking(nullptr);
+ cert.resetWithoutLocking(nullptr);
}
}
bool Ssl::generateUntrustedCert(Security::CertPointer &untrustedCert, EVP_PKEY_Pointer &untrustedPkey, Security::CertPointer const &cert, EVP_PKEY_Pointer const & pkey)
{
// Generate the self-signed certificate, using a hard-coded subject prefix
Ssl::CertificateProperties certProperties;
if (const char *cn = CommonHostName(cert.get())) {
certProperties.commonName = "Not trusted by \"";
certProperties.commonName += cn;
certProperties.commonName += "\"";
} else if (const char *org = getOrganization(cert.get())) {
certProperties.commonName = "Not trusted by \"";
certProperties.commonName += org;
certProperties.commonName += "\"";
} else
certProperties.commonName = "Not trusted";
certProperties.setCommonName = true;
// O, OU, and other CA subject fields will be mimicked
// Expiration date and other common properties will be mimicked
@@ -1296,41 +1296,41 @@
certProperties.signWithPkey.resetAndLock(pkey.get());
certProperties.mimicCert.resetAndLock(cert.get());
return Ssl::generateSslCertificate(untrustedCert, untrustedPkey, certProperties);
}
SSL *
SslCreate(Security::ContextPtr sslContext, const int fd, Ssl::Bio::Type type, const char *squidCtx)
{
if (fd < 0) {
debugs(83, DBG_IMPORTANT, "Gone connection");
return NULL;
}
const char *errAction = NULL;
int errCode = 0;
if (auto ssl = SSL_new(sslContext)) {
// without BIO, we would call SSL_set_fd(ssl, fd) instead
if (BIO *bio = Ssl::Bio::Create(fd, type)) {
Ssl::Bio::Link(ssl, bio); // cannot fail
- fd_table[fd].ssl.reset(ssl);
+ fd_table[fd].ssl.resetWithoutLocking(ssl);
fd_table[fd].read_method = &ssl_read_method;
fd_table[fd].write_method = &ssl_write_method;
fd_note(fd, squidCtx);
return ssl;
}
errCode = ERR_get_error();
errAction = "failed to initialize I/O";
SSL_free(ssl);
} else {
errCode = ERR_get_error();
errAction = "failed to allocate handle";
}
debugs(83, DBG_IMPORTANT, "ERROR: " << squidCtx << ' ' << errAction <<
": " << ERR_error_string(errCode, NULL));
return NULL;
}
SSL *
Ssl::CreateClient(Security::ContextPtr sslContext, const int fd, const char *squidCtx)
More information about the squid-dev
mailing list