From: Amos Jeffries Date: Wed, 7 Nov 2012 13:13:47 +0000 (+1300) Subject: Use Notes objects for key=pair handling in HelperReply X-Git-Tag: SQUID_3_4_0_1~471^2~14 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7bbefa01f63575cc8104addbbc65765c5e186e5d;p=thirdparty%2Fsquid.git Use Notes objects for key=pair handling in HelperReply * NTLM/Negotiate auth are expeted to return OK/ERR/BH/TT codes and key=pair. The old result lines are still received, but converted to the new format silently. * new key accepted: 'token=' for passing NTLM and Negotiate auth tokens * some undocumented old tags accepted by external_acl_type are now ignored: 'error=' replaced by 'message=' 'passwd=' replaced by 'password=' 'login=' replaced by 'user=' These were replaced some time ago and never formally documented. --- diff --git a/src/HelperReply.cc b/src/HelperReply.cc index 2fa277ac8f..06531cadf1 100644 --- a/src/HelperReply.cc +++ b/src/HelperReply.cc @@ -3,13 +3,21 @@ * AUTHOR: Amos Jeffries */ #include "squid.h" +#include "ConfigParser.h" #include "HelperReply.h" #include "helper.h" +#include "rfc1738.h" #include "SquidString.h" -HelperReply::HelperReply(const char *buf, size_t len, bool urlQuoting) : +HelperReply::HelperReply(char *buf, size_t len, bool urlQuoting) : result(HelperReply::Unknown), whichServer(NULL) +{ + parse(buf,len,urlQuoting); +} + +void +HelperReply::parse(char *buf, size_t len, bool urlQuoting) { // check we have something to parse if (!buf || len < 1) { @@ -19,7 +27,7 @@ HelperReply::HelperReply(const char *buf, size_t len, bool urlQuoting) : return; } - const char *p = buf; + char *p = buf; // optimization: do not consider parsing result code if the response is short. // URL-rewriter may return relative URLs or empty response for a large portion @@ -41,30 +49,43 @@ HelperReply::HelperReply(const char *buf, size_t len, bool urlQuoting) : result = HelperReply::TT; p+=3; // followed by an auth token - char *token = strwordtok(NULL, &p); + char *w1 = strwordtok(NULL, &p); + MemBuf authToken; authToken.init(); - authToken.append(token, strlen(token)); + authToken.append(w1, strlen(w1)); + responseKeys.add("token",authToken.content()); + } else if (!strncmp(p,"AF ",3)) { // NTLM/Negotate OK response - result = HelperReply::OK; + result = HelperReply::Okay; p+=3; // followed by: - // an auth token and user field + // an optional auth token and user field // or, an optional username field - char *blob = strwordtok(NULL, &p); - char *arg = strwordtok(NULL, &p); - if (arg != NULL) { + char *w1 = strwordtok(NULL, &p); + char *w2 = strwordtok(NULL, &p); + if (w2 != NULL) { + // Negotiate "token user" + MemBuf authToken; authToken.init(); - authToken.append(blob, strlen(blob)); + authToken.append(w1, strlen(w1)); + responseKeys.add("token",authToken.content()); + + MemBuf user; user.init(); - user.append(arg,strlen(arg)); - } else if (blob != NULL) { + user.append(w2,strlen(w2)); + responseKeys.add("user",user.content()); + + } else if (w1 != NULL) { + // NTLM "user" + MemBuf user; user.init(); - user.append(blob, strlen(blob)); + user.append(w1,strlen(w1)); + responseKeys.add("user",user.content()); } } else if (!strncmp(p,"NA ",3)) { // NTLM fail-closed ERR response - result = HelperReply::NA; + result = HelperReply::Error; p+=3; } @@ -78,47 +99,43 @@ HelperReply::HelperReply(const char *buf, size_t len, bool urlQuoting) : // NULL-terminate so the helper callback handlers do not buffer-overrun other_.terminate(); - bool found; - do { - found = false; - found |= parseKeyValue("tag=", 4, tag); - found |= parseKeyValue("user=", 5, user); - found |= parseKeyValue("password=", 9, password); - found |= parseKeyValue("message=", 8, message); - found |= parseKeyValue("log=", 8, log); - found |= parseKeyValue("token=", 8, authToken); - } while(found); - - if (urlQuoting) { - // unescape the reply values - if (tag.hasContent()) - rfc1738_unescape(tag.buf()); - if (user.hasContent()) - rfc1738_unescape(user.buf()); - if (password.hasContent()) - rfc1738_unescape(password.buf()); - if (message.hasContent()) - rfc1738_unescape(message.buf()); - if (log.hasContent()) - rfc1738_unescape(log.buf()); + parseResponseKeys(urlQuoting); + + // Hack for backward-compatibility: BH used to be a text message... + if (other().hasContent() && result == HelperReply::BrokenHelper) { + responseKeys.add("message",other().content()); + modifiableOther().clean(); } } -bool -HelperReply::parseKeyValue(const char *key, size_t key_len, MemBuf &value) +void +HelperReply::parseResponseKeys(bool urlQuotingValues) { - if (other().contentSize() > static_cast(key_len) && memcmp(other().content(), key, key_len) == 0) { - // parse the value out of the string. may be double-quoted - char *tmp = modifiableOther().content() + key_len; - const char *token = strwordtok(NULL, &tmp); - value.reset(); - value.append(token,strlen(token)); - const mb_size_t keyPairSize = tmp - other().content(); - modifiableOther().consume(keyPairSize); + // parse a "key=value" pair off the 'other()' buffer. + while(other().hasContent()) { + char *p = modifiableOther().content(); + while(*p && *p != '=' && *p != ' ') ++p; + if (*p != '=') + return; // done. Not a key. + + *p = '\0'; + ++p; + + String key(other().content()); + + // the value may be a quoted string or a token + // XXX: eww. update strwordtok() to be zero-copy + char *v = strwordtok(NULL, &p); + if ((p-v) > 2) // 1-octet %-escaped requires 3 bytes + rfc1738_unescape(v); + String value = v; + safe_free(v); + + responseKeys.add(key, value); + + modifiableOther().consume(p - other().content()); modifiableOther().consumeWhitespace(); - return true; } - return false; } std::ostream & @@ -138,14 +155,22 @@ operator <<(std::ostream &os, const HelperReply &r) case HelperReply::TT: os << "TT"; break; - case HelperReply::NA: - os << "NA"; - break; case HelperReply::Unknown: os << "Unknown"; break; } + // dump the helper key=pair "notes" list + if (r.responseKeys.notes.size() > 0) { + os << ", notes={"; + for (Notes::NotesList::const_iterator m = r.responseKeys.notes.begin(); m != r.responseKeys.notes.end(); ++m) { + for (Note::Values::iterator v = (*m)->values.begin(); v != (*m)->values.end(); ++v) { + os << ',' << (*m)->key << '=' << ConfigParser::QuoteString((*v)->value); + } + } + os << "}"; + } + if (r.other().hasContent()) os << ", other: \"" << r.other().content() << '\"'; diff --git a/src/HelperReply.h b/src/HelperReply.h index 8ebed059d7..1128507872 100644 --- a/src/HelperReply.h +++ b/src/HelperReply.h @@ -3,6 +3,7 @@ #include "base/CbcPointer.h" #include "MemBuf.h" +#include "Notes.h" #if HAVE_OSTREAM #include @@ -24,7 +25,8 @@ private: public: // create/parse details from the msg buffer provided - HelperReply(const char *buf, size_t len, bool urlQuoting = false); + // XXX: buf should be const but parse() needs non-const for now + HelperReply(char *buf, size_t len, bool urlQuoting = false); const MemBuf &other() const { return other_; } @@ -34,37 +36,37 @@ public: /// and by token blob/arg parsing in Negotiate auth handler MemBuf &modifiableOther() const { return *const_cast(&other_); } - bool parseKeyValue(const char *key, size_t key_len, MemBuf &); + /** parse a helper response line format: + * line := [ result ] *#( key-pair ) + * key-pair := OWS token '=' ( quoted-string | token ) + * + * \param urlQuoting decode note values using RFC 1738 decoder. (default: use quoted-string instead) + */ + // XXX: buf should be const but we may need strwordtok() and rfc1738_unescape() + void parse(char *buf, size_t len, bool urlQuoting = false); public: /// The helper response 'result' field. enum Result_ { Unknown, // no result code received, or unknown result code Okay, // "OK" indicating success/positive result - Error, // "ERR" indicating failure/negative result + Error, // "ERR" indicating success/negative result BrokenHelper, // "BH" indicating failure due to helper internal problems. - // some result codes for backward compatibility with NTLM/Negotiate - // TODO: migrate these into variants of the above results with key-pair parameters - TT, - NA + // result codes for backward compatibility with NTLM/Negotiate + // TODO: migrate to a variant of the above results with key-pair parameters + TT } result; - // some pre-determined keys - MemBuf tag; - MemBuf user; - MemBuf password; - MemBuf message; - MemBuf log; - MemBuf authToken; - -// TODO other (custom) key=pair values. when the callbacks actually use this object. -// for now they retain their own parsing routines handling other() + // list of key=value pairs the helper produced + Notes responseKeys; /// for stateful replies the responding helper 'server' needs to be preserved across callbacks CbcPointer whichServer; private: + void parseResponseKeys(bool urlQuotingValues); + /// the remainder of the line MemBuf other_; }; diff --git a/src/auth/negotiate/UserRequest.cc b/src/auth/negotiate/UserRequest.cc index 8749b72380..bf03ce21c1 100644 --- a/src/auth/negotiate/UserRequest.cc +++ b/src/auth/negotiate/UserRequest.cc @@ -262,28 +262,17 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) else assert(reply.whichServer == lm_request->authserver); - /* seperate out the useful data */ - char *modifiableBlob = reply.modifiableOther().content(); - char *arg = NULL; - if (modifiableBlob && *modifiableBlob != '\0') { - arg = strchr(modifiableBlob + 1, ' '); - if (arg) { - *arg = '\0'; - ++arg; - } - } - const char *blob = modifiableBlob; - switch (reply.result) { case HelperReply::TT: /* we have been given a blob to send to the client */ safe_free(lm_request->server_blob); lm_request->request->flags.mustKeepalive = 1; if (lm_request->request->flags.proxyKeepalive) { - lm_request->server_blob = xstrdup(reply.authToken.content()); + Note::Pointer tokenNote = reply.responseKeys.findByName("token"); + lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf()); auth_user_request->user()->credentials(Auth::Handshake); auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << reply.authToken << "'"); + debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << tokenNote->values[0]->value << "'"); } else { auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("Negotiate authentication requires a persistent connection"); @@ -291,7 +280,9 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) break; case HelperReply::Okay: { - if (!reply.user.hasContent()) { + Note::Pointer userNote = reply.responseKeys.findByName("user"); + Note::Pointer tokenNote = reply.responseKeys.findByName("token"); + if (userNote == NULL || tokenNote == NULL) { // XXX: handle a success with no username better /* protocol error */ fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content()); @@ -299,10 +290,10 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) } /* we're finished, release the helper */ - auth_user_request->user()->username(reply.user.content()); + auth_user_request->user()->username(userNote->values[0]->value.termedBuf()); auth_user_request->denyMessage("Login successful"); safe_free(lm_request->server_blob); - lm_request->server_blob = xstrdup(reply.authToken.content()); + lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf()); lm_request->releaseAuthServer(); /* connection is authenticated */ @@ -331,45 +322,53 @@ Auth::Negotiate::UserRequest::HandleReply(void *data, const HelperReply &reply) * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; auth_user_request->user()->credentials(Auth::Ok); - debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << reply.user << "'"); + debugs(29, 4, HERE << "Successfully validated user via Negotiate. Username '" << auth_user_request->user()->username() << "'"); } break; - case HelperReply::NA: - case HelperReply::Error: - if (arg == NULL) { + case HelperReply::Error: { + Note::Pointer messageNote = reply.responseKeys.findByName("message"); + Note::Pointer tokenNote = reply.responseKeys.findByName("token"); + if (tokenNote == NULL) { /* protocol error */ fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply.other().content()); break; } + /* authentication failure (wrong password, etc.) */ - auth_user_request->denyMessage(arg); + auth_user_request->denyMessage(messageNote->values[0]->value.termedBuf()); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); - lm_request->server_blob = xstrdup(blob); + lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf()); lm_request->releaseAuthServer(); debugs(29, 4, HERE << "Failed validating user via Negotiate. Error returned '" << reply << "'"); - break; + } + break; case HelperReply::Unknown: debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication Helper '" << reply.whichServer << "' crashed!."); - blob = "Internal error"; /* continue to the next case */ - case HelperReply::BrokenHelper: + case HelperReply::BrokenHelper: { /* TODO kick off a refresh process. This can occur after a YR or after * a KK. If after a YR release the helper and resubmit the request via * Authenticate Negotiate start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - auth_user_request->denyMessage(blob); + Note::Pointer errNote = reply.responseKeys.findByName("message"); + if (reply.result == HelperReply::Unknown) + auth_user_request->denyMessage("Internal Error"); + else if (errNote != NULL) + auth_user_request->denyMessage(errNote->values[0]->value.termedBuf()); + else + auth_user_request->denyMessage("Negotiate Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); debugs(29, DBG_IMPORTANT, "ERROR: Negotiate Authentication validating user. Error returned " << reply); + } // break; } - xfree(arg); if (lm_request->request) { HTTPMSGUNLOCK(lm_request->request); lm_request->request = NULL; diff --git a/src/auth/ntlm/UserRequest.cc b/src/auth/ntlm/UserRequest.cc index 4ded878fb7..0c9cd97695 100644 --- a/src/auth/ntlm/UserRequest.cc +++ b/src/auth/ntlm/UserRequest.cc @@ -255,34 +255,32 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const HelperReply &reply) else assert(reply.whichServer == lm_request->authserver); - /* seperate out the useful data */ - const char *blob = reply.other().content(); - switch (reply.result) { case HelperReply::TT: /* we have been given a blob to send to the client */ safe_free(lm_request->server_blob); lm_request->request->flags.mustKeepalive = 1; if (lm_request->request->flags.proxyKeepalive) { - lm_request->server_blob = xstrdup(reply.authToken.content()); + Note::Pointer serverBlob = reply.responseKeys.findByName("token"); + lm_request->server_blob = xstrdup(serverBlob->values[0]->value.termedBuf()); auth_user_request->user()->credentials(Auth::Handshake); auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << reply.authToken << "'"); + debugs(29, 4, HERE << "Need to challenge the client with a server token: '" << serverBlob->values[0]->value << "'"); } else { auth_user_request->user()->credentials(Auth::Failed); auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); } break; - case HelperReply::AF: case HelperReply::Okay: { /* we're finished, release the helper */ - auth_user_request->user()->username(reply.user.content()); + Note::Pointer userLabel = reply.responseKeys.findByName("user"); + auth_user_request->user()->username(userLabel->values[0]->value.termedBuf()); auth_user_request->denyMessage("Login successful"); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); - debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << reply.user << "'"); + debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << userLabel->values[0]->value << "'"); /* connection is authenticated */ debugs(29, 4, HERE << "authenticated user " << auth_user_request->user()->username()); /* see if this is an existing user with a different proxy_auth @@ -309,37 +307,47 @@ Auth::Ntlm::UserRequest::HandleReply(void *data, const HelperReply &reply) * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; auth_user_request->user()->credentials(Auth::Ok); - debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << reply.user << "'"); + debugs(29, 4, HERE << "Successfully validated user via NTLM. Username '" << auth_user_request->user()->username() << "'"); } break; - case HelperReply::NA: - case HelperReply::Error: + case HelperReply::Error: { /* authentication failure (wrong password, etc.) */ - auth_user_request->denyMessage(blob); + Note::Pointer errNote = reply.responseKeys.findByName("message"); + if (errNote != NULL) + auth_user_request->denyMessage(errNote->values[0]->value.termedBuf()); + else + auth_user_request->denyMessage("NTLM Authentication denied with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); - debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << blob << "'"); - break; + debugs(29, 4, HERE << "Failed validating user via NTLM. Error returned '" << errNote->values[0]->value << "'"); + } + break; case HelperReply::Unknown: debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication Helper '" << reply.whichServer << "' crashed!."); - blob = "Internal error"; /* continue to the next case */ - case HelperReply::BrokenHelper: + case HelperReply::BrokenHelper: { /* TODO kick off a refresh process. This can occur after a YR or after * a KK. If after a YR release the helper and resubmit the request via * Authenticate NTLM start. * If after a KK deny the user's request w/ 407 and mark the helper as * Needing YR. */ - auth_user_request->denyMessage(blob); + Note::Pointer errNote = reply.responseKeys.findByName("message"); + if (reply.result == HelperReply::Unknown) + auth_user_request->denyMessage("Internal Error"); + else if (errNote != NULL) + auth_user_request->denyMessage(errNote->values[0]->value.termedBuf()); + else + auth_user_request->denyMessage("NTLM Authentication failed with no reason given"); auth_user_request->user()->credentials(Auth::Failed); safe_free(lm_request->server_blob); lm_request->releaseAuthServer(); debugs(29, DBG_IMPORTANT, "ERROR: NTLM Authentication validating user. Error returned '" << reply << "'"); - break; + } + break; } if (lm_request->request) { diff --git a/src/external_acl.cc b/src/external_acl.cc index a211478859..de15fe488d 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -1311,7 +1311,6 @@ externalAclHandleReply(void *data, const HelperReply &reply) { externalAclState *state = static_cast(data); externalAclState *next; - char *t = NULL; ExternalACLEntryData entryData; entryData.result = ACCESS_DENIED; external_acl_entry *entry = NULL; @@ -1322,47 +1321,29 @@ externalAclHandleReply(void *data, const HelperReply &reply) entryData.result = ACCESS_ALLOWED; // XXX: handle other non-DENIED results better - if (reply.tag.hasContent()) - entryData.tag = reply.tag; - if (reply.message.hasContent()) - entryData.message = reply.message; - if (reply.log.hasContent()) - entryData.log = reply.log; -#if USE_AUTH - if (reply.user.hasContent()) - entryData.user = reply.user; - if (reply.password.hasContent()) - entryData.password = reply.password; -#endif + // XXX: make entryData store a proper helperReply object. - // legacy reply parser - if (reply.other().hasContent()) { - char *temp = reply.modifiableOther().content(); - char *token = strwordtok(temp, &t); + Note::Pointer label = reply.responseKeys.findByName("tag"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.tag = label->values[0]->value; - while ((token = strwordtok(NULL, &t))) { - debugs(82, DBG_IMPORTANT, "WARNING: key '" << token << "' is not supported by this Squid."); - char *value = strchr(token, '='); + label = reply.responseKeys.findByName("message"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.message = label->values[0]->value; - if (value) { - *value = '\0'; /* terminate the token, and move up to the value */ - ++value; + label = reply.responseKeys.findByName("log"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.log = label->values[0]->value; - if (state->def->quote == external_acl::QUOTE_METHOD_URL) - rfc1738_unescape(value); - - if (strcmp(token, "error") == 0) { - entryData.message = value; #if USE_AUTH - } else if (strcmp(token, "passwd") == 0) { - entryData.password = value; - } else if (strcmp(token, "login") == 0) { - entryData.user = value; + label = reply.responseKeys.findByName("user"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.user = label->values[0]->value; + + label = reply.responseKeys.findByName("password"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.password = label->values[0]->value; #endif - } - } - } - } dlinkDelete(&state->list, &state->def->queue);