From 46b1e6bf1f18262e17293814d6d1c8f4ee1aa07d Mon Sep 17 00:00:00 2001 From: Francesco Chemolli Date: Fri, 4 Sep 2015 15:02:54 +0200 Subject: [PATCH] Audit round 1 --- src/auth/Gadgets.cc | 16 ++++++++++------ src/auth/Gadgets.h | 1 - src/auth/User.h | 6 ++++-- src/auth/UserNameCache.cc | 12 ++++++------ src/auth/UserNameCache.h | 16 +++++++++------- src/auth/basic/Config.cc | 2 +- src/auth/basic/User.cc | 3 +-- src/auth/basic/User.h | 2 +- src/auth/digest/User.h | 2 +- src/auth/negotiate/User.h | 2 +- src/auth/negotiate/UserRequest.cc | 2 +- src/auth/ntlm/User.cc | 2 +- src/auth/ntlm/User.h | 3 +-- src/auth/ntlm/UserRequest.cc | 2 +- 14 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/auth/Gadgets.cc b/src/auth/Gadgets.cc index 3eddb8f285..402aae7fff 100644 --- a/src/auth/Gadgets.cc +++ b/src/auth/Gadgets.cc @@ -109,16 +109,20 @@ std::vector authenticateCachedUsersList() { auto aucp_compare = [=](const Auth::User::Pointer lhs, const Auth::User::Pointer rhs) { - return lhs->SBUserKey() < rhs->SBUserKey(); + return lhs->SBufUserKey() < rhs->SBufUserKey(); }; - std::vector v1, v2, rv; - auto u1 = Auth::Basic::User::Cache()->sortedUsersList(); - auto u2 = Auth::Digest::User::Cache()->sortedUsersList(); + std::vector v1, v2, rv, u1, u2; + if (Auth::Config::Find("basic") != nullptr) + u1 = Auth::Basic::User::Cache()->sortedUsersList(); + if (Auth::Config::Find("digest") != nullptr) + u2 = Auth::Digest::User::Cache()->sortedUsersList(); v1.reserve(u1.size()+u2.size()); std::merge(u1.begin(), u1.end(),u2.begin(), u2.end(), std::back_inserter(v1), aucp_compare); - u1 = Auth::Negotiate::User::Cache()->sortedUsersList(); - u2 = Auth::Ntlm::User::Cache()->sortedUsersList(); + if (Auth::Config::Find("negotiate") != nullptr) + u1 = Auth::Negotiate::User::Cache()->sortedUsersList(); + if (Auth::Config::Find("ntlm") != nullptr) + u2 = Auth::Ntlm::User::Cache()->sortedUsersList(); v2.reserve(u1.size()+u2.size()); std::merge(u1.begin(), u1.end(),u2.begin(), u2.end(), std::back_inserter(v2), aucp_compare); diff --git a/src/auth/Gadgets.h b/src/auth/Gadgets.h index 3a312522b6..5b2ff5dca9 100644 --- a/src/auth/Gadgets.h +++ b/src/auth/Gadgets.h @@ -55,7 +55,6 @@ int authenticateSchemeCount(void); /// \ingroup AuthAPI void authenticateOnCloseConnection(ConnStateData * conn); -/// \ingroup AuthAPI std::vector authenticateCachedUsersList(); #endif /* USE_AUTH */ diff --git a/src/auth/User.h b/src/auth/User.h index 923a5c9208..579048a42c 100644 --- a/src/auth/User.h +++ b/src/auth/User.h @@ -67,7 +67,7 @@ public: // NP: key is set at the same time as username_. Until then both are empty/NULL. const char *userKey() {return !userKey_.isEmpty() ? userKey_.c_str() : NULL;} // user key as a SBuf - const SBuf SBUserKey() { return userKey_;} + const SBuf SBufUserKey() const {return userKey_;} /** * How long these credentials are still valid for. @@ -84,7 +84,9 @@ public: virtual void addToNameCache() = 0; static void UsernameCacheStats(StoreEntry * output); - static CbcPointer Cache(); //must be implemented in subclasses + // userKey ->Auth::User::Pointer cache + // must be reimplemented in subclasses + static CbcPointer Cache(); CredentialState credentials() const; void credentials(CredentialState); diff --git a/src/auth/UserNameCache.cc b/src/auth/UserNameCache.cc index a08b1dcb13..6ae6f05d3e 100644 --- a/src/auth/UserNameCache.cc +++ b/src/auth/UserNameCache.cc @@ -6,11 +6,11 @@ * Please see the COPYING and CONTRIBUTORS files for details. */ -/* debug section 29 */ +/* DEBUG: section 29 Authenticator */ #include "squid.h" #include "auth/UserNameCache.h" -#include "acl/Gadgets.h" //for aclCacheMatchFlush +#include "acl/Gadgets.h" #include "Debug.h" #include "event.h" #include "SquidConfig.h" @@ -69,8 +69,8 @@ UserNameCache::Cleanup(void *data) void UserNameCache::insert(Auth::User::Pointer anAuth_user) { - debugs(29, 6, "adding " << anAuth_user->SBUserKey()); - store_[anAuth_user->SBUserKey()] = anAuth_user; + debugs(29, 6, "adding " << anAuth_user->SBufUserKey()); + store_[anAuth_user->SBufUserKey()] = anAuth_user; } // generates the list of cached usernames in a format that is convenient @@ -82,7 +82,7 @@ UserNameCache::sortedUsersList() const std::transform(store_.begin(), store_.end(), rv.begin(), [](StoreType::value_type v) { return v.second; } ); - sort(rv.begin(), rv.end(), + std::sort(rv.begin(), rv.end(), [](const Auth::User::Pointer &lhs, const Auth::User::Pointer &rhs) { return strcmp(lhs->username(), rhs->username()) < 0; } @@ -103,7 +103,7 @@ UserNameCache::syncConfig() { debugs(29, 5, "Reconfiguring username cache " << cachename); for (auto i : store_) { - aclCacheMatchFlush(&i.second->proxy_match_cache); //flush + aclCacheMatchFlush(&i.second->proxy_match_cache); } } diff --git a/src/auth/UserNameCache.h b/src/auth/UserNameCache.h index 63e12b6a49..bf31866e9c 100644 --- a/src/auth/UserNameCache.h +++ b/src/auth/UserNameCache.h @@ -6,8 +6,8 @@ * Please see the COPYING and CONTRIBUTORS files for details. */ -#ifndef SQUID_USERNAMECACHE_H_ -#define SQUID_USERNAMECACHE_H_ +#ifndef SQUID_SRC_AUTH_UERNAMECACHE_H +#define SQUID_SRC_AUTH_UERNAMECACHE_H #include "SBufAlgos.h" #include "auth/User.h" @@ -20,8 +20,7 @@ namespace Auth { /** Cache of Auth::User::Pointer, keyed by Auth::User::userKey * - * It's meant to be used as a per-authentication protocol cache, - * cleaning up objects which are past authenticate_ttl life + * \returns a pointer to cached credentials, or nullptr if none found */ class UserNameCache : public RegisteredRunner { @@ -62,7 +61,7 @@ public: */ std::vector sortedUsersList() const; - /// RegisteredRunner API + /* RegisteredRunner API */ virtual void endingShutdown() override; virtual void syncConfig() override; @@ -72,9 +71,12 @@ private: // for logs, events etc. const char *cachename; - // must be unique to the cache and valid for the object's lifetime + // c_str() raw pointer is used in event. std::string must not reallocate + // after ctor and until shutdown + // must be unique std::string cacheCleanupEventName; }; } /* namespace Auth */ -#endif /* SQUID_USERNAMECACHE_H_ */ + +#endif /* SQUID_SRC_AUTH_UERNAMECACHE_H */ diff --git a/src/auth/basic/Config.cc b/src/auth/basic/Config.cc index c409417f54..41876a2593 100644 --- a/src/auth/basic/Config.cc +++ b/src/auth/basic/Config.cc @@ -247,7 +247,7 @@ Auth::Basic::Config::decode(char const *proxy_auth, const char *aRequestRealm) /* now lookup and see if we have a matching auth_user structure in memory. */ Auth::User::Pointer auth_user; - if (!(auth_user = Auth::Basic::User::Cache()->lookup(lb->SBUserKey()))) { + if (!(auth_user = Auth::Basic::User::Cache()->lookup(lb->SBufUserKey()))) { /* the user doesn't exist in the username cache yet */ /* save the credentials */ debugs(29, 9, HERE << "Creating new user '" << lb->username() << "'"); diff --git a/src/auth/basic/User.cc b/src/auth/basic/User.cc index 8c31744c6b..11bffb10cf 100644 --- a/src/auth/basic/User.cc +++ b/src/auth/basic/User.cc @@ -19,8 +19,7 @@ Auth::Basic::User::User(Auth::Config *aConfig, const char *aRequestRealm) : passwd(NULL), queue(NULL), currentRequest(NULL) -{ -} +{} Auth::Basic::User::~User() { diff --git a/src/auth/basic/User.h b/src/auth/basic/User.h index ec366df485..97dc193258 100644 --- a/src/auth/basic/User.h +++ b/src/auth/basic/User.h @@ -36,7 +36,7 @@ public: void updateCached(User *from); virtual int32_t ttl() const; - // userKey ->Auth::User::Pointer cache + /* Auth::User API */ static CbcPointer Cache(); virtual void addToNameCache() override; diff --git a/src/auth/digest/User.h b/src/auth/digest/User.h index f6e0182575..0ba69db2f8 100644 --- a/src/auth/digest/User.h +++ b/src/auth/digest/User.h @@ -27,9 +27,9 @@ public: User(Auth::Config *, const char *requestRealm); ~User(); int authenticated() const; - virtual int32_t ttl() const; + /* Auth::User API */ static CbcPointer Cache(); virtual void addToNameCache() override; diff --git a/src/auth/negotiate/User.h b/src/auth/negotiate/User.h index 7566af1cde..91ae262710 100644 --- a/src/auth/negotiate/User.h +++ b/src/auth/negotiate/User.h @@ -29,10 +29,10 @@ public: ~User(); virtual int32_t ttl() const; + /* Auth::User API */ static CbcPointer Cache(); virtual void addToNameCache() override; - dlink_list proxy_auth_list; }; diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc index 2aa42f1f2c..eb9b736b30 100644 --- a/src/auth/negotiate/UserRequest.cc +++ b/src/auth/negotiate/UserRequest.cc @@ -334,7 +334,7 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const Helper::Reply &reply /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); auto local_auth_user = lm_request->user(); - auto cached_user = Auth::Negotiate::User::Cache()->lookup(auth_user_request->user()->SBUserKey()); + auto cached_user = Auth::Negotiate::User::Cache()->lookup(auth_user_request->user()->SBufUserKey()); if (!cached_user) { local_auth_user->addToNameCache(); } else { diff --git a/src/auth/ntlm/User.cc b/src/auth/ntlm/User.cc index 6e218b3c20..de3ef4cf4e 100644 --- a/src/auth/ntlm/User.cc +++ b/src/auth/ntlm/User.cc @@ -31,7 +31,7 @@ Auth::Ntlm::User::ttl() const CbcPointer Auth::Ntlm::User::Cache() { - static CbcPointer p(new Auth::UserNameCache("basic")); + static CbcPointer p(new Auth::UserNameCache("ntlm")); return p; } diff --git a/src/auth/ntlm/User.h b/src/auth/ntlm/User.h index 3e63df0ab2..74b4c45509 100644 --- a/src/auth/ntlm/User.h +++ b/src/auth/ntlm/User.h @@ -27,9 +27,9 @@ class User : public Auth::User public: User(Auth::Config *, const char *requestRealm); ~User(); - virtual int32_t ttl() const; + /* Auth::User API */ static CbcPointer Cache(); virtual void addToNameCache() override; @@ -40,4 +40,3 @@ public: } // namespace Auth #endif /* _SQUID_AUTH_NTLM_USER_H */ - diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc index caf9e65921..9d9f380435 100644 --- a/src/auth/ntlm/UserRequest.cc +++ b/src/auth/ntlm/UserRequest.cc @@ -329,7 +329,7 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const Helper::Reply &reply) debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); /* see if this is an existing user */ auto local_auth_user = lm_request->user(); - auto cached_user = Auth::Ntlm::User::Cache()->lookup(auth_user_request->user()->SBUserKey()); + auto cached_user = Auth::Ntlm::User::Cache()->lookup(auth_user_request->user()->SBufUserKey()); if (!cached_user) { local_auth_user->addToNameCache(); } else { -- 2.47.3