The state of credentials lookup and handling is recorded by
authenticateDirection / AuthUserRequest::direction() and its per-scheme
helper methods AuthUserRequest::module_direction().
This formalizes and coordinates the state being returned by using a
shared enum.
The states can generally be considerd as:
- LOOKUP with a helper still needs to validate the credentials
- CHALLENGE if the helepr needs more info from the client
- VALID if everything is fine and the credentials are known Good/Bad
- ERROR if there is any problem with the state or credentials
This combination has highlighted a few strange things in the NTLM and
Negotiate states. Where known but Failed credentials are marked as
ERROR. This needs closer investigation since it should logically be a
re-CHALLENGE in all auth types.
Also there is a little obfuscation of the cases around the generalized
fixHeader() calls. This will be handled in a followup patch.
On the side any ideas what to call this state management besides
"Direction"? I'm at a loss for terminology.
NP: labels above are relatively arbitrary. If they need changing to
fit some terminology scope of the function name so be it.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.12
Beta testers wanted for 3.2.0.6
=== modified file 'src/auth/UserRequest.cc'
--- src/auth/UserRequest.cc 2011-04-11 14:08:54 +0000
+++ src/auth/UserRequest.cc 2011-04-17 04:59:08 +0000
@@ -208,11 +208,14 @@
return auth_user_request->authenticated();
}
-int
+Auth::Direction
AuthUserRequest::direction()
{
+ if (user() == NULL)
+ return CRED_ERROR; // No credentials. Should this be a CHALLENGE instead?
+
if (authenticateUserAuthenticated(this))
- return 0;
+ return CRED_VALID;
return module_direction();
}
@@ -397,42 +400,44 @@
}
if (!authenticateUserAuthenticated(*auth_user_request)) {
- /* User not logged in. Log them in */
+ /* User not logged in. Try to log them in */
authenticateAuthenticateUser(*auth_user_request, request, conn, headertype);
- switch (authenticateDirection(*auth_user_request)) {
+ switch ((*auth_user_request)->direction()) {
- case 1:
+ case CRED_CHALLENGE:
if (request->auth_user_request == NULL) {
request->auth_user_request = *auth_user_request;
}
- /* fallthrough to -2 */
+ /* fallthrough to ERROR case and do the challenge */
- case -2:
+ case CRED_ERROR:
/* this ACL check is finished. */
*auth_user_request = NULL;
return AUTH_ACL_CHALLENGE;
- case -1:
+ case CRED_LOOKUP:
/* we are partway through authentication within squid,
* the *auth_user_request variables stores the auth_user_request
* for the callback to here - Do not Unlock */
return AUTH_ACL_HELPER;
- }
- /* on 0 the authentication is finished - fallthrough */
- /* See if user authentication failed for some reason */
- if (!authenticateUserAuthenticated(*auth_user_request)) {
- if ((*auth_user_request)->username()) {
- if (!request->auth_user_request) {
- request->auth_user_request = *auth_user_request;
+ case CRED_VALID:
+ /* authentication is finished */
+ /* See if user authentication failed for some reason */
+ if (!authenticateUserAuthenticated(*auth_user_request)) {
+ if ((*auth_user_request)->username()) {
+ if (!request->auth_user_request) {
+ request->auth_user_request = *auth_user_request;
+ }
}
+
+ *auth_user_request = NULL;
+ return AUTH_ACL_CHALLENGE;
}
-
- *auth_user_request = NULL;
- return AUTH_ACL_CHALLENGE;
+ // otherwise fallthrough to acceptance.
}
}
@@ -473,21 +478,6 @@
return result;
}
-/* returns
- * 0: no output needed
- * 1: send to client
- * -1: send to helper
- * -2: authenticate broken in some fashion
- */
-int
-authenticateDirection(AuthUserRequest::Pointer auth_user_request)
-{
- if (auth_user_request == NULL || auth_user_request->user() == NULL)
- return -2;
-
- return auth_user_request->direction();
-}
-
void
AuthUserRequest::addReplyAuthHeader(HttpReply * rep, AuthUserRequest::Pointer auth_user_request, HttpRequest * request, int accelerated, int internal)
/* send the auth types we are configured to support (and have compiled in!) */
@@ -520,8 +510,8 @@
/* this is a authenticate-needed response */
{
- if ((auth_user_request != NULL) && authenticateDirection(auth_user_request) == 1)
- /* scheme specific */
+ if (auth_user_request != NULL && auth_user_request->direction() == CRED_CHALLENGE)
+ /* add the scheme specific challenge header to the response */
auth_user_request->user()->config->fixHeader(auth_user_request, rep, type, request);
else {
/* call each configured & running authscheme */
=== modified file 'src/auth/UserRequest.h'
--- src/auth/UserRequest.h 2011-04-11 14:08:54 +0000
+++ src/auth/UserRequest.h 2011-04-17 04:51:07 +0000
@@ -60,6 +60,19 @@
time_t ip_expiretime;
};
+namespace Auth
+{
+
+// NP: numeric values specified for old code backward compatibility.
+// remove after transition is complete
+enum Direction {
+ CRED_CHALLENGE = 1, ///< Client needs to be challenged. secure token.
+ CRED_VALID = 0, ///< Credentials are valid and a up to date. The OK/Failed state is accurate.
+ CRED_LOOKUP = -1, ///< Credentials need to be validated with the backend helper
+ CRED_ERROR = -2 ///< ERROR in the auth module. Cannot determine the state of this request.
+};
+} // namespace Auth
+
/**
\ingroup AuthAPI
* This is a short lived structure is the visible aspect of the authentication framework.
@@ -83,14 +96,16 @@
/**
* Used by squid to determine what the next step in performing authentication for a given scheme is.
*
- \retval -2 ERROR in the auth module. Cannot determine request direction.
- \retval -1 The auth module needs to send data to an external helper.
- * Squid will prepare for a callback on the request and call the AUTHSSTART function.
- \retval 0 The auth module has all the information it needs to perform the authentication and provide a succeed/fail result.
- \retval 1 The auth module needs to send a new challenge to the request originator.
- * Squid will return the appropriate status code (401 or 407) and call the registered FixError function to allow the auth module to insert it's challenge.
+ * \retval CRED_ERROR ERROR in the auth module. Cannot determine request direction.
+ * \retval CRED_LOOKUP The auth module needs to send data to an external helper.
+ * Squid will prepare for a callback on the request and call the AUTHSSTART function.
+ * \retval CRED_VALID The auth module has all the information it needs to perform the authentication
+ * and provide a succeed/fail result.
+ * \retval CRED_CHALLENGE The auth module needs to send a new challenge to the request originator.
+ * Squid will return the appropriate status code (401 or 407) and call the registered
+ * FixError function to allow the auth module to insert it's challenge.
*/
- int direction();
+ Auth::Direction direction();
/**
* Used by squid to determine whether the auth scheme has successfully authenticated the user request.
@@ -114,7 +129,7 @@
virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type) = 0;
/* template method */
- virtual int module_direction() = 0;
+ virtual Auth::Direction module_direction() = 0;
virtual void addHeader(HttpReply * rep, int accel);
virtual void addTrailer(HttpReply * rep, int accel);
virtual void onConnectionClose(ConnStateData *);
@@ -194,9 +209,6 @@
extern void authenticateAuthUserRequestClearIp(AuthUserRequest::Pointer);
/// \ingroup AuthAPI
extern int authenticateAuthUserRequestIPCount(AuthUserRequest::Pointer);
-/// \ingroup AuthAPI
-/// \deprecated Use AuthUserRequest::direction() instead.
-extern int authenticateDirection(AuthUserRequest::Pointer);
/// \ingroup AuthAPI
/// See AuthUserRequest::authenticated()
=== modified file 'src/auth/basic/UserRequest.cc'
--- src/auth/basic/UserRequest.cc 2011-04-14 02:40:59 +0000
+++ src/auth/basic/UserRequest.cc 2011-04-17 05:08:49 +0000
@@ -42,29 +42,29 @@
return;
}
-int
+Auth::Direction
AuthBasicUserRequest::module_direction()
{
- /* null auth_user is checked for by authenticateDirection */
+ /* null auth_user is checked for by AuthUserRequest::direction() */
if (user()->auth_type != Auth::AUTH_BASIC)
- return -2;
+ return Auth::CRED_ERROR;
switch (user()->credentials()) {
case Auth::Unchecked:
case Auth::Pending:
- return -1;
+ return Auth::CRED_LOOKUP;
case Auth::Ok:
if (user()->expiretime + static_cast<Auth::Basic::Config*>(Auth::Config::Find("basic"))->credentialsTTL <= squid_curtime)
- return -1;
- return 0;
+ return Auth::CRED_LOOKUP;
+ return Auth::CRED_VALID;
case Auth::Failed:
- return 0;
+ return Auth::CRED_VALID;
default:
- return -2;
+ return Auth::CRED_ERROR;
}
}
=== modified file 'src/auth/basic/UserRequest.h'
--- src/auth/basic/UserRequest.h 2011-03-28 04:02:03 +0000
+++ src/auth/basic/UserRequest.h 2011-04-17 04:39:06 +0000
@@ -20,7 +20,7 @@
virtual int authenticated() const;
virtual void authenticate(HttpRequest * request, ConnStateData *conn, http_hdr_type type);
- virtual int module_direction();
+ virtual Auth::Direction module_direction();
virtual void module_start(RH *, void *);
};
=== modified file 'src/auth/digest/UserRequest.cc'
--- src/auth/digest/UserRequest.cc 2011-04-14 02:40:59 +0000
+++ src/auth/digest/UserRequest.cc 2011-04-17 04:43:41 +0000
@@ -166,27 +166,27 @@
return;
}
-int
+Auth::Direction
AuthDigestUserRequest::module_direction()
{
if (user()->auth_type != Auth::AUTH_DIGEST)
- return -2;
+ return Auth::CRED_ERROR;
switch (user()->credentials()) {
case Auth::Ok:
- return 0;
+ return Auth::CRED_VALID;
case Auth::Failed:
/* send new challenge */
- return 1;
+ return Auth::CRED_CHALLENGE;
case Auth::Unchecked:
case Auth::Pending:
- return -1;
+ return Auth::CRED_LOOKUP;
default:
- return -2;
+ return Auth::CRED_ERROR;
}
}
=== modified file 'src/auth/digest/UserRequest.h'
--- src/auth/digest/UserRequest.h 2011-03-28 04:02:03 +0000
+++ src/auth/digest/UserRequest.h 2011-04-17 04:42:39 +0000
@@ -23,7 +23,7 @@
virtual int authenticated() const;
virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type);
- virtual int module_direction();
+ virtual Auth::Direction module_direction();
virtual void addHeader(HttpReply * rep, int accel);
#if WAITING_FOR_TE
=== modified file 'src/auth/negotiate/UserRequest.cc'
--- src/auth/negotiate/UserRequest.cc 2011-04-14 02:40:59 +0000
+++ src/auth/negotiate/UserRequest.cc 2011-04-17 04:51:50 +0000
@@ -58,33 +58,32 @@
return 0;
}
-/* See AuthUserRequest.cc::authenticateDirection for return values */
-int
+Auth::Direction
AuthNegotiateUserRequest::module_direction()
{
- /* null auth_user is checked for by authenticateDirection */
+ /* null auth_user is checked for by AuthUserRequest::direction() */
if (waiting || client_blob)
- return -1; /* need helper response to continue */
+ return Auth::CRED_LOOKUP; /* need helper response to continue */
if (user()->auth_type != Auth::AUTH_NEGOTIATE)
- return -2;
+ return Auth::CRED_ERROR;
switch (user()->credentials()) {
case Auth::Handshake:
assert(server_blob);
- return 1; /* send to client */
+ return Auth::CHALLENGE;
case Auth::Ok:
- return 0; /* do nothing */
+ return Auth::CRED_VALID;
case Auth::Failed:
- return -2;
+ return Auth::CRED_ERROR; // XXX: really? not VALID or CHALLENGE?
default:
debugs(29, DBG_IMPORTANT, "WARNING: Negotiate Authentication in unexpected state: " << user()->credentials());
- return -2;
+ return Auth::CRED_ERROR;
}
}
=== modified file 'src/auth/negotiate/UserRequest.h'
--- src/auth/negotiate/UserRequest.h 2011-03-28 04:02:03 +0000
+++ src/auth/negotiate/UserRequest.h 2011-04-17 04:48:30 +0000
@@ -21,7 +21,7 @@
virtual ~AuthNegotiateUserRequest();
virtual int authenticated() const;
virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type);
- virtual int module_direction();
+ virtual Auth::Direction module_direction();
virtual void onConnectionClose(ConnStateData *);
virtual void module_start(RH *, void *);
=== modified file 'src/auth/ntlm/UserRequest.cc'
--- src/auth/ntlm/UserRequest.cc 2011-04-11 14:08:54 +0000
+++ src/auth/ntlm/UserRequest.cc 2011-04-17 04:50:32 +0000
@@ -37,33 +37,32 @@
return NULL;
}
-/* See AuthUserRequest.cc::authenticateDirection for return values */
-int
+Auth::Direction
AuthNTLMUserRequest::module_direction()
{
- /* null auth_user is checked for by authenticateDirection */
+ /* null auth_user is checked for by AuthUserRequest::direction() */
if (waiting || client_blob)
- return -1; /* need helper response to continue */
+ return Auth::CRED_LOOKUP; /* need helper response to continue */
if (user()->auth_type != Auth::AUTH_NTLM)
- return -2;
+ return Auth::CRED_ERROR;
switch (user()->credentials()) {
case Auth::Handshake:
assert(server_blob);
- return 1; /* send to client */
+ return Auth::CRED_CHALLENGE; /* send to client */
case Auth::Ok:
- return 0; /* do nothing */
+ return Auth::CRED_VALID;
case Auth::Failed:
- return -2;
+ return Auth::CRED_ERROR; // XXX really? not VALID or CHALLENGE?
default:
debugs(29, DBG_IMPORTANT, "WARNING: NTLM Authentication in unexpected state: " << user()->credentials());
- return -2;
+ return Auth::CRED_ERROR;
}
}
=== modified file 'src/auth/ntlm/UserRequest.h'
--- src/auth/ntlm/UserRequest.h 2011-03-28 04:02:03 +0000
+++ src/auth/ntlm/UserRequest.h 2011-04-17 04:50:42 +0000
@@ -20,7 +20,7 @@
virtual ~AuthNTLMUserRequest();
virtual int authenticated() const;
virtual void authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type);
- virtual int module_direction();
+ virtual Auth::Direction module_direction();
virtual void onConnectionClose(ConnStateData *);
virtual void module_start(RH *, void *);
