=== modified file 'helpers/digest_auth/LDAP/digest_pw_auth.cc' --- helpers/digest_auth/LDAP/digest_pw_auth.cc 2012-08-28 13:00:30 +0000 +++ helpers/digest_auth/LDAP/digest_pw_auth.cc 2012-11-09 04:01:31 +0000 @@ -64,10 +64,10 @@ requestData->error = 0; GetHHA1(requestData); if (requestData->error) { - SEND_ERR("No such user"); + SEND_ERR("message=\"No such user\""); return; } - printf("%s\n", requestData->HHA1); + printf("OK ha1=\"%s\"\n", requestData->HHA1); } static void @@ -76,7 +76,7 @@ RequestData requestData; ParseBuffer(buf, &requestData); if (!requestData.parsed) { - SEND_ERR(""); + SEND_BH("message=\"Invalid line received\""); return; } OutputHHA1(&requestData); === modified file 'helpers/digest_auth/eDirectory/digest_pw_auth.cc' --- helpers/digest_auth/eDirectory/digest_pw_auth.cc 2012-01-20 18:55:04 +0000 +++ helpers/digest_auth/eDirectory/digest_pw_auth.cc 2012-11-09 04:03:57 +0000 @@ -64,10 +64,10 @@ requestData->error = 0; GetHHA1(requestData); if (requestData->error) { - SEND_ERR("No such user"); + SEND_ERR("message=\"No such user\""); return; } - printf("%s\n", requestData->HHA1); + printf("OK ha1=\"%s\"\n", requestData->HHA1); } static void @@ -76,7 +76,7 @@ RequestData requestData; ParseBuffer(buf, &requestData); if (!requestData.parsed) { - SEND_ERR(""); + SEND_BH("message=\"Invalid line received\""); return; } OutputHHA1(&requestData); === modified file 'helpers/digest_auth/file/digest_file_auth.cc' --- helpers/digest_auth/file/digest_file_auth.cc 2012-01-20 18:55:04 +0000 +++ helpers/digest_auth/file/digest_file_auth.cc 2012-11-09 04:01:03 +0000 @@ -38,8 +38,6 @@ #include "helpers/defines.h" #include "text_backend.h" -#define PROGRAM_NAME "digest_file_auth" - static void GetHHA1(RequestData * requestData) { @@ -68,10 +66,10 @@ requestData->error = 0; GetHHA1(requestData); if (requestData->error) { - SEND_ERR("No such user"); + SEND_ERR("message=\"No such user\""); return; } - printf("%s\n", requestData->HHA1); + printf("OK ha1=\"%s\"\n", requestData->HHA1); } static void @@ -80,7 +78,7 @@ RequestData requestData; ParseBuffer(buf, &requestData); if (!requestData.parsed) { - SEND_ERR(""); + SEND_BH("message=\"Invalid line received\""); return; } OutputHHA1(&requestData); === modified file 'helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in' --- helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in 2012-06-12 08:50:53 +0000 +++ helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in 2012-09-22 09:04:27 +0000 @@ -151,10 +151,10 @@ print(stderr "Received: Channel=".$cid.", UID='".$uid."'\n") if ($debug); - $status = $cid . " ERR database error"; + $status = $cid . " ERR message=\"database error\""; my $sth = query_db($uid) || next; print(stderr "Rows: ". $sth->rows()."\n") if ($debug); - $status = $cid . " ERR unknown UID '".$uid."'"; + $status = $cid . " ERR message=\"unknown UID '".$uid."'\""; my $row = $sth->fetchrow_hashref() || next; $status = $cid . " OK" . ($row->{'user'} ne "" ? " user=" . $row->{'user'} : "" ) . ($row->{'tag'} ne "" ? " tag=" . $row->{'tag'} : "" ); $sth->finish(); === modified file 'helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc' --- helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc 2012-07-02 15:31:03 +0000 +++ helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc 2012-09-23 01:18:51 +0000 @@ -1776,7 +1776,7 @@ /* No space given, but group string is required --> ERR */ if ((edui_conf.mode & EDUI_MODE_GROUP) && (p == NULL)) { debug("while() -> Search group is missing. (required)\n"); - local_printfx("ERR (Search Group Required)\n"); + local_printfx("ERR message=\"(Search Group Required)\"\n"); continue; } x = 0; @@ -1819,7 +1819,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, %s, %s, (LDAP_AUTH_TLS)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x)); @@ -1830,7 +1830,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, %s, %s, (LDAP_AUTH_SIMPLE)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x)); @@ -1840,7 +1840,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (BindLDAP: %s - %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, -, -, (LDAP_AUTH_NONE)) -> %s\n", ErrLDAP(x)); @@ -1859,7 +1859,7 @@ /* Everything failed --> ERR */ debug("while() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); CloseLDAP(&edui_ldap); - local_printfx("ERR (General Failure: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(General Failure: %s)\"\n", ErrLDAP(x)); continue; } edui_ldap.err = -1; @@ -1874,14 +1874,14 @@ x = ConvertIP(&edui_ldap, bufb); if (x < 0) { debug("ConvertIP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR (ConvertIP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufb, x, edui_ldap.search_ip); x = SearchFilterLDAP(&edui_ldap, bufa); if (x < 0) { debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR (SearchFilterLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); } else { /* Do Search */ edui_ldap.err = -1; @@ -1889,14 +1889,14 @@ x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib); if (x != LDAP_ERR_SUCCESS) { debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (SearchLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x)); x = SearchIPLDAP(&edui_ldap); if (x != LDAP_ERR_SUCCESS) { debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (SearchIPLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); } else { debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x)); local_printfx("OK user=%s\n", edui_ldap.userid); /* Got userid --> OK user= */ @@ -1908,35 +1908,35 @@ } } else { debug("StringSplit() -> Error: %" PRIuSIZE "\n", i); - local_printfx("ERR (StringSplit Error %" PRIuSIZE ")\n", i); + local_printfx("ERR message=\"(StringSplit Error %" PRIuSIZE ")\"\n", i); } } else { /* No group to match against, only an IP */ x = ConvertIP(&edui_ldap, bufa); if (x < 0) { debug("ConvertIP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR (ConvertIP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); } else { debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufa, x, edui_ldap.search_ip); /* Do search */ x = SearchFilterLDAP(&edui_ldap, NULL); if (x < 0) { debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR (SearchFilterLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchFilterLDAP(-, NULL) -> Length: %u\n", x); x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib); if (x != LDAP_ERR_SUCCESS) { debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(x)); - local_printfx("ERR (SearchLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x)); x = SearchIPLDAP(&edui_ldap); if (x != LDAP_ERR_SUCCESS) { debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR (SearchIPLDAP: %s)\n", ErrLDAP(x)); + local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); } else { debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x)); local_printfx("OK user=%s\n", edui_ldap.userid); /* Got a userid --> OK user= */ === modified file 'src/HelperReply.cc' --- src/HelperReply.cc 2012-10-30 00:13:18 +0000 +++ src/HelperReply.cc 2012-11-07 12:58:13 +0000 @@ -1,11 +1,24 @@ +/* + * DEBUG: section 84 Helper process maintenance + * 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) : +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) { // for now ensure that legacy handlers are not presented with NULL strings. @@ -14,7 +27,7 @@ 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 @@ -35,13 +48,44 @@ // NTLM challenge token result = HelperReply::TT; p+=3; + // followed by an auth token + char *w1 = strwordtok(NULL, &p); + MemBuf authToken; + authToken.init(); + authToken.append(w1, strlen(w1)); + responseKeys.add("token",authToken.content()); + } else if (!strncmp(p,"AF ",3)) { - // NTLM OK response - result = HelperReply::AF; + // NTLM/Negotate OK response + result = HelperReply::Okay; p+=3; + // followed by: + // an optional auth token and user field + // or, an optional username field + char *w1 = strwordtok(NULL, &p); + char *w2 = strwordtok(NULL, &p); + if (w2 != NULL) { + // Negotiate "token user" + MemBuf authToken; + authToken.init(); + authToken.append(w1, strlen(w1)); + responseKeys.add("token",authToken.content()); + + MemBuf user; + user.init(); + user.append(w2,strlen(w2)); + responseKeys.add("user",user.content()); + + } else if (w1 != NULL) { + // NTLM "user" + MemBuf user; + user.init(); + 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; } @@ -54,13 +98,51 @@ // NULL-terminate so the helper callback handlers do not buffer-overrun other_.terminate(); + + 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(); + } +} + +void +HelperReply::parseResponseKeys(bool urlQuotingValues) +{ + // 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(); + } } std::ostream & operator <<(std::ostream &os, const HelperReply &r) { os << "{result="; - switch (r.result) { + switch(r.result) { case HelperReply::Okay: os << "OK"; break; @@ -73,17 +155,22 @@ case HelperReply::TT: os << "TT"; break; - case HelperReply::AF: - os << "AF"; - 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() << '\"'; === modified file 'src/HelperReply.h' --- src/HelperReply.h 2012-10-29 23:12:04 +0000 +++ src/HelperReply.h 2012-11-08 01:59:02 +0000 @@ -3,6 +3,7 @@ #include "base/CbcPointer.h" #include "MemBuf.h" +#include "Notes.h" #if HAVE_OSTREAM #include @@ -23,8 +24,14 @@ HelperReply &operator =(const HelperReply &r); public: + HelperReply() : result(HelperReply::Unknown), responseKeys(), whichServer(NULL) { + other_.init(1,1); + other_.terminate(); + } + // create/parse details from the msg buffer provided - HelperReply(const char *buf, size_t len); + // 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,28 +41,37 @@ /// and by token blob/arg parsing in Negotiate auth handler MemBuf &modifiableOther() const { return *const_cast(&other_); } + /** 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, - AF, - NA + // result codes for backward compatibility with NTLM/Negotiate + // TODO: migrate to a variant of the above results with key-pair parameters + TT } result; -// TODO other 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_; }; === modified file 'src/MemBuf.cc' --- src/MemBuf.cc 2012-09-01 14:38:36 +0000 +++ src/MemBuf.cc 2012-09-19 06:05:15 +0000 @@ -220,6 +220,18 @@ PROF_stop(MemBuf_consume); } +/// removes all whitespace prefix bytes and "packs" by moving content left +void MemBuf::consumeWhitespace() +{ + PROF_start(MemBuf_consumeWhitespace); + const char *end = buf + contentSize(); + const char *p = buf; + for(; p<=end && xisspace(*p); ++p); + if (p-buf > 0) + consume(p-buf); + PROF_stop(MemBuf_consumeWhitespace); +} + // removes last tailSize bytes void MemBuf::truncate(mb_size_t tailSize) { === modified file 'src/MemBuf.h' --- src/MemBuf.h 2012-10-03 17:32:57 +0000 +++ src/MemBuf.h 2012-11-04 11:47:44 +0000 @@ -81,6 +81,8 @@ /// \note there is currently no stretch() method to grow without appending void consume(mb_size_t sz); // removes sz bytes, moving content left + void consumeWhitespace(); // removes all prefix whitespace, moving content left + void append(const char *c, mb_size_t sz); // grows if needed and possible void appended(mb_size_t sz); // updates content size after external append void truncate(mb_size_t sz); // removes sz last bytes === modified file 'src/Notes.cc' --- src/Notes.cc 2012-10-26 19:43:58 +0000 +++ src/Notes.cc 2012-11-09 09:45:48 +0000 @@ -73,16 +73,32 @@ } Note::Pointer -Notes::add(const String ¬eKey) +Notes::findByName(const String ¬eKey) const { - typedef Notes::NotesList::iterator AMLI; + typedef Notes::NotesList::const_iterator AMLI; for (AMLI i = notes.begin(); i != notes.end(); ++i) { if ((*i)->key == noteKey) return (*i); } - Note::Pointer note = new Note(noteKey); - notes.push_back(note); + return Note::Pointer(); +} + +void +Notes::add(const String ¬eKey, const String ¬eValue) +{ + Note::Pointer key = add(noteKey); + key->addValue(noteValue); +} + +Note::Pointer +Notes::add(const String ¬eKey) +{ + Note::Pointer note = findByName(noteKey); + if (note == NULL) { + note = new Note(noteKey); + notes.push_back(note); + } return note; } === modified file 'src/Notes.h' --- src/Notes.h 2012-10-27 00:13:19 +0000 +++ src/Notes.h 2012-11-09 09:45:48 +0000 @@ -61,6 +61,7 @@ public: typedef Vector NotesList; typedef NotesList::iterator iterator; ///< iterates over the notes list + typedef NotesList::const_iterator const_iterator; ///< iterates over the notes list Notes(const char *aDescr, const char **metasBlacklist): descr(aDescr), blacklisted(metasBlacklist) {} Notes(): descr(NULL), blacklisted(NULL) {} @@ -93,6 +94,18 @@ * returns a pointer to the existing object. */ Note::Pointer add(const String ¬eKey); + +public: + /** + * Adds a note key and value to the notes list. + * If the key name already exists in list, add the given value to its set of values. + */ + void add(const String ¬eKey, const String ¬eValue); + + /** + * Looks up a note by key name and returns a Note::Pointer to it + */ + Note::Pointer findByName(const String ¬eKey) const; }; class NotePairs : public HttpHeader === modified file 'src/auth/digest/UserRequest.cc' --- src/auth/digest/UserRequest.cc 2012-10-30 00:13:18 +0000 +++ src/auth/digest/UserRequest.cc 2012-11-09 04:15:32 +0000 @@ -280,7 +280,48 @@ assert(replyData->auth_user_request != NULL); Auth::UserRequest::Pointer auth_user_request = replyData->auth_user_request; + static bool oldHelperWarningDone = false; switch (reply.result) { + case HelperReply::Unknown: { + // Squid 3.3 and older the digest helper only returns a HA1 hash (no "OK") + // the HA1 will be found in content() for these responses. + if (!oldHelperWarningDone) { + debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format HA1 response. It needs to be upgraded."); + oldHelperWarningDone=true; + } + + /* allow this because the digest_request pointer is purely local */ + Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); + assert(digest_user != NULL); + + CvtBin(reply.other().content(), digest_user->HA1); + digest_user->HA1created = 1; + } + break; + + case HelperReply::Okay: { + /* allow this because the digest_request pointer is purely local */ + Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); + assert(digest_user != NULL); + + Note::Pointer ha1Note = reply.responseKeys.findByName("ha1"); + if (ha1Note != NULL) { + CvtBin(ha1Note->values[0]->value.termedBuf(), digest_user->HA1); + digest_user->HA1created = 1; + } else { + debugs(29, DBG_IMPORTANT, "ERROR: Digest auth helper did not produce a HA1. Using the wrong helper program? received: " << reply); + } + } + break; + + case HelperReply::TT: + debugs(29, DBG_IMPORTANT, "ERROR: Digest auth does not support the result code received. Using the wrong helper program? received: " << reply); + // fall through to next case. Handle this as an ERR response. + + case HelperReply::BrokenHelper: + // TODO retry the broken lookup on another helper? + // fall through to next case for now. Handle this as an ERR response silently. + case HelperReply::Error: { /* allow this because the digest_request pointer is purely local */ Auth::Digest::UserRequest *digest_request = dynamic_cast(auth_user_request.getRaw()); @@ -289,24 +330,19 @@ digest_request->user()->credentials(Auth::Failed); digest_request->flags.invalid_password = 1; - if (reply.other().hasContent()) + Note::Pointer msgNote = reply.responseKeys.findByName("message"); + if (msgNote != NULL) { + digest_request->setDenyMessage(msgNote->values[0]->value.termedBuf()); + } else if (reply.other().hasContent()) { + // old helpers did send ERR result but a bare message string instead of message= key name. digest_request->setDenyMessage(reply.other().content()); - } - break; - - case HelperReply::Unknown: // Squid 3.2 and older the digest helper only returns a HA1 hash (no "OK") - case HelperReply::Okay: { - /* allow this because the digest_request pointer is purely local */ - Auth::Digest::User *digest_user = dynamic_cast(auth_user_request->user().getRaw()); - assert(digest_user != NULL); - - CvtBin(reply.other().content(), digest_user->HA1); - digest_user->HA1created = 1; - } - break; - - default: - ; // XXX: handle other states properly. + if (!oldHelperWarningDone) { + debugs(29, DBG_IMPORTANT, "WARNING: Digest auth helper returned old format ERR response. It needs to be upgraded."); + oldHelperWarningDone=true; + } + } + } + break; } void *cbdata = NULL; === modified file 'src/auth/negotiate/UserRequest.cc' --- src/auth/negotiate/UserRequest.cc 2012-10-30 00:13:18 +0000 +++ src/auth/negotiate/UserRequest.cc 2012-11-06 09:14:41 +0000 @@ -262,37 +262,27 @@ 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(blob); + 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 blob '" << blob << "'"); + 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("NTLM authentication requires a persistent connection"); + auth_user_request->denyMessage("Negotiate authentication requires a persistent connection"); } break; - case HelperReply::AF: case HelperReply::Okay: { - if (arg == NULL) { + 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()); @@ -300,10 +290,10 @@ } /* we're finished, release the helper */ - auth_user_request->user()->username(arg); + 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(blob); + lm_request->server_blob = xstrdup(tokenNote->values[0]->value.termedBuf()); lm_request->releaseAuthServer(); /* connection is authenticated */ @@ -332,45 +322,53 @@ * 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 '" << arg << "'"); + 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; === modified file 'src/auth/ntlm/UserRequest.cc' --- src/auth/ntlm/UserRequest.cc 2012-10-30 00:13:18 +0000 +++ src/auth/ntlm/UserRequest.cc 2012-11-06 08:21:31 +0000 @@ -255,34 +255,32 @@ 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(blob); + 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 blob '" << blob << "'"); + 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(blob); + 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 '" << blob << "'"); + 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 @@ * 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 '" << blob << "'"); + 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) { === modified file 'src/client_side.cc' --- src/client_side.cc 2012-11-07 19:26:45 +0000 +++ src/client_side.cc 2012-11-09 09:45:48 +0000 @@ -3698,8 +3698,10 @@ void ConnStateData::sslCrtdHandleReply(const HelperReply &reply) { - if (!reply.other().hasContent()) { - debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper return reply"); + if (reply.result == HelperReply::BrokenHelper) { + debugs(33, 5, HERE << "Certificate for " << sslConnectHostOrIp << " cannot be generated. ssl_crtd response: " << reply); + } else if (!reply.other().hasContent()) { + debugs(1, DBG_IMPORTANT, HERE << "\"ssl_crtd\" helper returned reply."); } else { Ssl::CrtdMessage reply_message(Ssl::CrtdMessage::REPLY); if (reply_message.parse(reply.other().content(), reply.other().contentSize()) != Ssl::CrtdMessage::OK) { === modified file 'src/client_side_request.cc' --- src/client_side_request.cc 2012-10-30 00:36:45 +0000 +++ src/client_side_request.cc 2012-11-09 09:45:48 +0000 @@ -902,7 +902,7 @@ if (answer == ACCESS_ALLOWED) redirectStart(http, clientRedirectDoneWrapper, context); else { - HelperReply nilReply(NULL,0); + HelperReply nilReply; context->clientRedirectDone(nilReply); } } === modified file 'src/dns.cc' --- src/dns.cc 2012-10-30 09:16:33 +0000 +++ src/dns.cc 2012-11-08 06:00:57 +0000 @@ -131,7 +131,13 @@ debugs(34, DBG_IMPORTANT, "dnsSubmit: queue overload, rejecting " << lookup); const char *t = "$fail Temporary network problem, please retry later"; - HelperReply failReply(t, strlen(t)); + HelperReply failReply; + /* XXX: upgrade the ipcache and fqdn cache handlers to new syntax + failReply.result= HelperReply::BrokenHelper; + failReply.responseKeys.add("message","Temporary network problem, please retry later"); + failReply.responseKeys.add("message","DNS lookup queue overloaded"); + */ + failReply.modifiableOther().append(t, strlen(t)); callback(data, failReply); return; } === modified file 'src/external_acl.cc' --- src/external_acl.cc 2012-10-29 23:12:04 +0000 +++ src/external_acl.cc 2012-11-06 10:08:45 +0000 @@ -1311,7 +1311,6 @@ { externalAclState *state = static_cast(data); externalAclState *next; - char *t = NULL; ExternalACLEntryData entryData; entryData.result = ACCESS_DENIED; external_acl_entry *entry = NULL; @@ -1322,41 +1321,29 @@ entryData.result = ACCESS_ALLOWED; // XXX: handle other non-DENIED results better - if (reply.other().hasContent()) { - char *temp = reply.modifiableOther().content(); - char *token = strwordtok(temp, &t); - - while ((token = strwordtok(NULL, &t))) { - char *value = strchr(token, '='); - - if (value) { - *value = '\0'; /* terminate the token, and move up to the value */ - ++value; - - if (state->def->quote == external_acl::QUOTE_METHOD_URL) - rfc1738_unescape(value); - - if (strcmp(token, "message") == 0) - entryData.message = value; - else if (strcmp(token, "error") == 0) - entryData.message = value; - else if (strcmp(token, "tag") == 0) - entryData.tag = value; - else if (strcmp(token, "log") == 0) - entryData.log = value; + // XXX: make entryData store a proper helperReply object. + + Note::Pointer label = reply.responseKeys.findByName("tag"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.tag = label->values[0]->value; + + label = reply.responseKeys.findByName("message"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.message = label->values[0]->value; + + label = reply.responseKeys.findByName("log"); + if (label != NULL && label->values[0]->value.size() > 0) + entryData.log = label->values[0]->value; + #if USE_AUTH - else if (strcmp(token, "user") == 0) - entryData.user = value; - else if (strcmp(token, "password") == 0) - entryData.password = value; - 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); @@ -1513,6 +1500,9 @@ p->theHelper->addr = p->local_addr; + if (p->quote == external_acl::QUOTE_METHOD_URL) + p->theHelper->url_quoting = true; + helperOpenServers(p->theHelper); } === modified file 'src/helper.cc' --- src/helper.cc 2012-10-30 09:16:33 +0000 +++ src/helper.cc 2012-11-08 02:25:17 +0000 @@ -398,7 +398,7 @@ { if (hlp == NULL) { debugs(84, 3, "helperSubmit: hlp == NULL"); - HelperReply nilReply(NULL, 0); + HelperReply nilReply; callback(data, nilReply); return; } @@ -424,7 +424,7 @@ { if (hlp == NULL) { debugs(84, 3, "helperStatefulSubmit: hlp == NULL"); - HelperReply nilReply(NULL, 0); + HelperReply nilReply; callback(data, nilReply); return; } @@ -758,7 +758,7 @@ void *cbdata; if (cbdataReferenceValidDone(r->data, &cbdata)) { - HelperReply nilReply(NULL, 0); + HelperReply nilReply; r->callback(cbdata, nilReply); } @@ -824,7 +824,7 @@ void *cbdata; if (cbdataReferenceValidDone(r->data, &cbdata)) { - HelperReply nilReply(NULL,0); + HelperReply nilReply; nilReply.whichServer = srv; r->callback(cbdata, nilReply); } @@ -1366,7 +1366,7 @@ /* a callback is needed before this request can _use_ a helper. */ /* we don't care about releasing this helper. The request NEVER * gets to the helper. So we throw away the return code */ - HelperReply nilReply(NULL,0); + HelperReply nilReply; nilReply.whichServer = srv; r->callback(r->data, nilReply); /* throw away the placeholder */ === modified file 'src/helper.h' --- src/helper.h 2012-10-29 23:12:04 +0000 +++ src/helper.h 2012-11-04 12:00:41 +0000 @@ -48,7 +48,7 @@ class helper { public: - inline helper(const char *name) : cmdline(NULL), id_name(name), eom('\n') {} + inline helper(const char *name) : cmdline(NULL), id_name(name), eom('\n'), url_quoting(false) {} ~helper(); public: @@ -62,6 +62,7 @@ time_t last_queue_warn; time_t last_restart; char eom; ///< The char which marks the end of (response) message, normally '\n' + bool url_quoting; struct _stats { int requests; === modified file 'src/redirect.cc' --- src/redirect.cc 2012-10-30 09:16:33 +0000 +++ src/redirect.cc 2012-11-09 09:45:48 +0000 @@ -151,8 +151,10 @@ if (Config.onoff.redirector_bypass && redirectors->stats.queue_size) { /* Skip redirector if there is one request queued */ ++n_bypassed; - HelperReply nilReply(NULL,0); - handler(data, nilReply); + HelperReply bypassReply; + bypassReply.result = HelperReply::Okay; + bypassReply.responseKeys.add("message","URL rewrite/redirect queue too long. Bypassed."); + handler(data, bypassReply); return; } === modified file 'src/ssl/helper.cc' --- src/ssl/helper.cc 2012-10-30 09:16:33 +0000 +++ src/ssl/helper.cc 2012-11-08 08:49:10 +0000 @@ -93,8 +93,9 @@ if (squid_curtime - first_warn > 3 * 60) fatal("SSL servers not responding for 3 minutes"); debugs(34, DBG_IMPORTANT, HERE << "Queue overload, rejecting"); - const char *errMsg = "BH error 45 Temporary network problem, please retry later"; // XXX: upgrade to message="" - HelperReply failReply(errMsg,strlen(errMsg)); + HelperReply failReply; + failReply.result = HelperReply::BrokenHelper; + failReply.responseKeys.add("message", "error 45 Temporary network problem, please retry later"); callback(data, failReply); return; }