From: Amos Jeffries Date: Sun, 27 Oct 2013 05:08:49 +0000 (-0700) Subject: Receive annotations from authentication helpers X-Git-Tag: SQUID_3_5_0_1~576 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=71e7400cd8484df1ab2369cd12e1868fe8a90674;p=thirdparty%2Fsquid.git Receive annotations from authentication helpers This saves the kv-pair from authentication helper responses as annotations on the HttpRequest which was authenticated and pass on from there to logging. Added a method appendNewOnly() to ensure duplicate-free addition to a NotePairs list. Also, fixes a bug in hasPair() accessor which was returning true if either the key OR the value matched. ie. hasPair("a","1") would match true for notes "a=2 b=1" --- diff --git a/src/Notes.cc b/src/Notes.cc index b482f4eb20..9100e6cbfa 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -205,7 +205,7 @@ bool NotePairs::hasPair(const char *key, const char *value) const { for (Vector::const_iterator i = entries.begin(); i != entries.end(); ++i) { - if ((*i)->name.cmp(key) == 0 || (*i)->value.cmp(value) == 0) + if ((*i)->name.cmp(key) == 0 && (*i)->value.cmp(value) == 0) return true; } return false; @@ -219,12 +219,25 @@ NotePairs::append(const NotePairs *src) } } +void +NotePairs::appendNewOnly(const NotePairs *src) +{ + for (Vector::const_iterator i = src->entries.begin(); i != src->entries.end(); ++i) { + if (!hasPair((*i)->name.termedBuf(), (*i)->value.termedBuf())) + entries.push_back(new NotePairs::Entry((*i)->name.termedBuf(), (*i)->value.termedBuf())); + } +} + NotePairs & SyncNotes(AccessLogEntry &ale, HttpRequest &request) { + // XXX: auth code only has access to HttpRequest being authenticated + // so we must handle the case where HttpRequest is set without ALE being set. + if (!ale.notes) { - assert(!request.notes); - ale.notes = request.notes = new NotePairs; + if (!request.notes) + request.notes = new NotePairs; + ale.notes = request.notes; } else { assert(ale.notes == request.notes); } diff --git a/src/Notes.h b/src/Notes.h index 29bac331ee..8583da4e93 100644 --- a/src/Notes.h +++ b/src/Notes.h @@ -130,6 +130,12 @@ public: */ void append(const NotePairs *src); + /** + * Append any new entries of the src NotePairs list to our list. + * Entries which already exist in the destination set are ignored. + */ + void appendNewOnly(const NotePairs *src); + /** * Returns a comma separated list of notes with key 'noteKey'. * Use findFirst instead when a unique kv-pair is needed. diff --git a/src/auth/User.cc b/src/auth/User.cc index 83526f966e..782196c7b7 100644 --- a/src/auth/User.cc +++ b/src/auth/User.cc @@ -58,6 +58,7 @@ Auth::User::User(Auth::Config *aConfig) : config(aConfig), ipcount(0), expiretime(0), + notes(), credentials_state(Auth::Unchecked), username_(NULL) { @@ -99,6 +100,9 @@ Auth::User::absorb(Auth::User::Pointer from) debugs(29, 5, HERE << "auth_user '" << from << "' into auth_user '" << this << "'."); + // combine the helper response annotations. Ensuring no duplicates are copied. + notes.appendNewOnly(&from->notes); + /* absorb the list of IP address sources (for max_user_ip controls) */ AuthUserIP *new_ipdata; while (from->ip_list.head != NULL) { diff --git a/src/auth/User.h b/src/auth/User.h index 11d2e15078..4e680c37c1 100644 --- a/src/auth/User.h +++ b/src/auth/User.h @@ -39,6 +39,7 @@ #include "base/RefCount.h" #include "dlink.h" #include "ip/Address.h" +#include "Notes.h" class AuthUserHashPointer; class StoreEntry; @@ -75,6 +76,9 @@ public: size_t ipcount; long expiretime; + /// list of key=value pairs the helper produced + NotePairs notes; + public: static void cacheInit(); static void CachedACLsReset(); diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc index 3655db1516..d6b6bf1cab 100644 --- a/src/auth/UserRequest.cc +++ b/src/auth/UserRequest.cc @@ -248,14 +248,27 @@ authenticateAuthenticateUser(Auth::UserRequest::Pointer auth_user_request, HttpR static Auth::UserRequest::Pointer authTryGetUser(Auth::UserRequest::Pointer auth_user_request, ConnStateData * conn, HttpRequest * request) { + Auth::UserRequest::Pointer res; + if (auth_user_request != NULL) - return auth_user_request; + res = auth_user_request; else if (request != NULL && request->auth_user_request != NULL) - return request->auth_user_request; + res = request->auth_user_request; else if (conn != NULL) - return conn->getAuth(); - else - return NULL; + res = conn->getAuth(); + + // attach the credential notes from helper to the transaction + if (res != NULL && res->user() != NULL) { + // XXX: we have no access to the transaction / AccessLogEntry so cant SyncNotes(). + // workaround by using anything already set in HttpRequest + // OR use new and rely on a later Sync copying these to AccessLogEntry + if (!request->notes) + request->notes = new NotePairs; + + request->notes->appendNewOnly(&res->user()->notes); + } + + return res; } /* returns one of diff --git a/src/auth/basic/UserRequest.cc b/src/auth/basic/UserRequest.cc index c85830fc3d..ef9dc6fa81 100644 --- a/src/auth/basic/UserRequest.cc +++ b/src/auth/basic/UserRequest.cc @@ -142,6 +142,10 @@ Auth::Basic::UserRequest::HandleReply(void *data, const HelperReply &reply) assert(r->auth_user_request != NULL); assert(r->auth_user_request->user()->auth_type == Auth::AUTH_BASIC); + // add new helper kv-pair notes to the credentials object + // so that any transaction using those credentials can access them + r->auth_user_request->user()->notes.appendNewOnly(&reply.notes); + /* this is okay since we only play with the Auth::Basic::User child fields below * and dont pass the pointer itself anywhere */ Auth::Basic::User *basic_auth = dynamic_cast(r->auth_user_request->user().getRaw()); diff --git a/src/auth/digest/UserRequest.cc b/src/auth/digest/UserRequest.cc index e9fc63012b..af26ac532f 100644 --- a/src/auth/digest/UserRequest.cc +++ b/src/auth/digest/UserRequest.cc @@ -282,6 +282,10 @@ Auth::Digest::UserRequest::HandleReply(void *data, const HelperReply &reply) assert(replyData->auth_user_request != NULL); Auth::UserRequest::Pointer auth_user_request = replyData->auth_user_request; + // add new helper kv-pair notes to the credentials object + // so that any transaction using those credentials can access them + auth_user_request->user()->notes.appendNewOnly(&reply.notes); + static bool oldHelperWarningDone = false; switch (reply.result) { case HelperReply::Unknown: { diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc index 2115841ade..086da9f7b9 100644 --- a/src/auth/negotiate/UserRequest.cc +++ b/src/auth/negotiate/UserRequest.cc @@ -226,6 +226,10 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) Auth::UserRequest::Pointer auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); + // add new helper kv-pair notes to the credentials object + // so that any transaction using those credentials can access them + auth_user_request->user()->notes.appendNewOnly(&reply.notes); + Auth::Negotiate::UserRequest *lm_request = dynamic_cast(auth_user_request.getRaw()); assert(lm_request != NULL); assert(lm_request->waiting); diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc index 01300609ec..4ce04ebf41 100644 --- a/src/auth/ntlm/UserRequest.cc +++ b/src/auth/ntlm/UserRequest.cc @@ -220,6 +220,10 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const HelperReply &reply) Auth::UserRequest::Pointer auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); + // add new helper kv-pair notes to the credentials object + // so that any transaction using those credentials can access them + auth_user_request->user()->notes.appendNewOnly(&reply.notes); + Auth::Ntlm::UserRequest *lm_request = dynamic_cast(auth_user_request.getRaw()); assert(lm_request != NULL); assert(lm_request->waiting); diff --git a/src/client_side.cc b/src/client_side.cc index 24c8147e37..7277aca978 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -670,8 +670,7 @@ ClientHttpRequest::logRequest() /*Add notes*/ // The al->notes and request->notes must point to the same object. - // Enable the following assertion to check for possible bugs. - // assert(request->notes == al->notes); + (void)SyncNotes(*al, *request); typedef Notes::iterator ACAMLI; for (ACAMLI i = Config.notes.begin(); i != Config.notes.end(); ++i) { if (const char *value = (*i)->match(request, al->reply)) {