Audit round 1

diff --git a/src/auth/Gadgets.cc b/src/auth/Gadgets.cc
index 3eddb8f28589bbd4b59df9e7867d97395f76c1a3..402aae7fffdd3f23f57c666b6d26956cf741b96d 100644 (file)
--- a/src/auth/Gadgets.cc
+++ b/src/auth/Gadgets.cc
@@ -109,16 +109,20 @@ std::vector<Auth::User::Pointer>
 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<Auth::User::Pointer> v1, v2, rv;
-    auto u1 = Auth::Basic::User::Cache()->sortedUsersList();
-    auto u2 = Auth::Digest::User::Cache()->sortedUsersList();
+    std::vector<Auth::User::Pointer> 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 3a312522b6e36eefae97a35edce172b2aa573f19..5b2ff5dca9415cd6666eae5b259dee49306a3c8e 100644 (file)
--- 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<Auth::User::Pointer> authenticateCachedUsersList();
 
 #endif /* USE_AUTH */

diff --git a/src/auth/User.h b/src/auth/User.h
index 923a5c9208459af8f69415fdd4e7761b2b847825..579048a42c55b820d4330449570cf8aa0109a323 100644 (file)
--- 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<Auth::UserNameCache> Cache(); //must be implemented in subclasses
+    // userKey ->Auth::User::Pointer cache
+    // must be reimplemented in subclasses
+    static CbcPointer<Auth::UserNameCache> Cache();
 
     CredentialState credentials() const;
     void credentials(CredentialState);

diff --git a/src/auth/UserNameCache.cc b/src/auth/UserNameCache.cc
index a08b1dcb13ad828ddae9c13f9404ca7084fb7927..6ae6f05d3eb95ed00fea80d812ad5029d2ed0b03 100644 (file)
--- 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 63e12b6a499ae795c7f3babde0ba435cc5f4d9c2..bf31866e9c6410aa38dc34a081d310b19b62c168 100644 (file)
--- 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<Auth::User::Pointer> 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 c409417f54e5b28bc386f18c71e1af8a09f85d37..41876a2593303d03f22ef8ec8333ae3a720ee263 100644 (file)
--- 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 8c31744c6b8fb03306b20101d414d824979960d2..11bffb10cf831b304c07aa51e5ffcc1b6ce40041 100644 (file)
--- 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 ec366df48595b44afb05b002e47b8a677ffa0d8d..97dc193258cea7a237a03f64755e9baf1ac4ca5e 100644 (file)
--- 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<Auth::UserNameCache> Cache();
     virtual void addToNameCache() override;
 

diff --git a/src/auth/digest/User.h b/src/auth/digest/User.h
index f6e018257571f0d43307fd445d1148bcf10ed4b4..0ba69db2f8ff5b752eef5567a2a86cd3b9aea6ee 100644 (file)
--- 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<Auth::UserNameCache> Cache();
     virtual void addToNameCache() override;
 

diff --git a/src/auth/negotiate/User.h b/src/auth/negotiate/User.h
index 7566af1cdee090f119b74e23ec4af3916c53bb01..91ae262710804057505094ad7ea1f642e73fad7a 100644 (file)
--- 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<Auth::UserNameCache> 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 2aa42f1f2cb3faeed93a92177bb13add37c75d8a..eb9b736b308de73f8ed524ce1bad7b9abf5a07e8 100644 (file)
--- 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 6e218b3c207eccaf69bab7fe3d232c6a5eef9c48..de3ef4cf4e502159d0621a13d0c578d65c7f8b19 100644 (file)
--- a/src/auth/ntlm/User.cc
+++ b/src/auth/ntlm/User.cc
@@ -31,7 +31,7 @@ Auth::Ntlm::User::ttl() const
 CbcPointer<Auth::UserNameCache>
 Auth::Ntlm::User::Cache()
 {
-    static CbcPointer<Auth::UserNameCache> p(new Auth::UserNameCache("basic"));
+    static CbcPointer<Auth::UserNameCache> p(new Auth::UserNameCache("ntlm"));
     return p;
 }
 

diff --git a/src/auth/ntlm/User.h b/src/auth/ntlm/User.h
index 3e63df0ab2b0bd5aba06a7dc267af97ad73a75c5..74b4c455098b47f7fd5d47e727889ea64dfbe4a0 100644 (file)
--- 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<Auth::UserNameCache> 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 caf9e65921b4e08998045faf089bfd94369abc59..9d9f3804350dc092b7b739a83641509bcca452c1 100644 (file)
--- 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 {