]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
SourceLayout: Add enum Direction for AuthUserRequests state logics
authorAmos Jeffries <squid3@treenet.co.nz>
Wed, 20 Apr 2011 05:08:16 +0000 (17:08 +1200)
committerAmos Jeffries <squid3@treenet.co.nz>
Wed, 20 Apr 2011 05:08:16 +0000 (17:08 +1200)
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

TODO:
 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 why it is not a CHALLENGE in all auth
schemes.

 Also there is a little obfuscation of the cases around the generalized
fixHeader() calls. This will be handled in a followup patch.

src/auth/UserRequest.cc
src/auth/UserRequest.h
src/auth/basic/UserRequest.cc
src/auth/basic/UserRequest.h
src/auth/digest/UserRequest.cc
src/auth/digest/UserRequest.h
src/auth/negotiate/UserRequest.cc
src/auth/negotiate/UserRequest.h
src/auth/ntlm/UserRequest.cc
src/auth/ntlm/UserRequest.h

index 94b2fcaea7c919fb83b92ecaa3a25cae4edf589d..6e1cd1c8f32533f2c9d49e81d1eb0a112c4560e8 100644 (file)
@@ -208,11 +208,14 @@ authenticateUserAuthenticated(AuthUserRequest::Pointer auth_user_request)
     return auth_user_request->authenticated();
 }
 
-int
+Auth::Direction
 AuthUserRequest::direction()
 {
+    if (user() == NULL)
+        return Auth::CRED_ERROR; // No credentials. Should this be a CHALLENGE instead?
+
     if (authenticateUserAuthenticated(this))
-        return 0;
+        return Auth::CRED_VALID;
 
     return module_direction();
 }
@@ -397,42 +400,44 @@ AuthUserRequest::authenticate(AuthUserRequest::Pointer * auth_user_request, http
     }
 
     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 Auth::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 Auth::CRED_ERROR:
             /* this ACL check is finished. */
             *auth_user_request = NULL;
             return AUTH_ACL_CHALLENGE;
 
-        case -1:
+        case Auth::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 Auth::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 @@ AuthUserRequest::tryToAuthenticateAndSetAuthUser(AuthUserRequest::Pointer * auth
     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 @@ AuthUserRequest::addReplyAuthHeader(HttpReply * rep, AuthUserRequest::Pointer au
         /* 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() == Auth::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 */
index c2db0b434b6e85d604314cc92a4f1e5549f4daa0..b585217a12d63ac79b1b3b370236b720cd5dbe25 100644 (file)
@@ -60,6 +60,19 @@ public:
     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 @@ public:
     /**
      *  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 @@ public:
 
     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 authenticateAuthUserRequestRemoveIp(AuthUserRequest::Pointer, Ip::Ad
 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()
index dd784fe862bbf6536e7cdfcd3e7d915864257628..bb2d5bedf70424a3466a66deab81c1f0c53084e9 100644 (file)
@@ -42,29 +42,29 @@ AuthBasicUserRequest::authenticate(HttpRequest * request, ConnStateData * conn,
     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;
     }
 }
 
index fabd599247b1606118fcb4608ef8d2ab880b8f8c..ed9f797e0d495c9855f94bdd88585883c9df5620 100644 (file)
@@ -20,7 +20,7 @@ public:
 
     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 *);
 };
 
index efca45c56795dcd86874997dc334fba41cfd7a33..245072b731142228694bc5533ab327f345b619bd 100644 (file)
@@ -166,27 +166,27 @@ AuthDigestUserRequest::authenticate(HttpRequest * request, ConnStateData * conn,
     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;
     }
 }
 
index 6c706bf19509106d3268e0b1fb804b1dfda10f94..c2416728bd63b989965fdad6e6f59b57f00d7c5d 100644 (file)
@@ -23,7 +23,7 @@ public:
 
     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
 
index d08355d932b89550082ad3933224c6f2c28c784a..b50df9eaf9bd5264ce1135deb6bba3ad55d53ec0 100644 (file)
@@ -58,33 +58,32 @@ AuthNegotiateUserRequest::authenticated() const
     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::CRED_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;
     }
 }
 
index 657f4b5cd0dec8c79199523ca93fb08bdc39277f..f2d372df2f6e8452b9ec24886fa135f0537dde13 100644 (file)
@@ -21,7 +21,7 @@ public:
     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 *);
 
index 59655df62fdcd278fb4a6c0fe7a634450d31584a..b811357c9528043c23a216fff50078210fa6f32d 100644 (file)
@@ -37,33 +37,32 @@ AuthNTLMUserRequest::connLastHeader()
     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;
     }
 }
 
index 189cdcce3bbf805c705227add9701d41569e255d..21561e85d079ee958ef2ec1b9b2df910d5e481cf 100644 (file)
@@ -20,7 +20,7 @@ public:
     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 *);