SoupAuthManagerNTLM: fix don't-fallback-to-Basic code
authorDan Winship <danw@gnome.org>
Fri, 24 Feb 2012 19:50:52 +0000 (14:50 -0500)
committerDan Winship <danw@gnome.org>
Fri, 24 Feb 2012 20:01:03 +0000 (15:01 -0500)
Sessions using NTLM should never fall back to using Basic auth to
access a resource which advertises NTLM auth. But ntlm-test wasn't
actually testing this, and it broke at some point. Fix that, and
add tests to ntlm-test.

libsoup/soup-auth-manager-ntlm.c
libsoup/soup-auth-manager.c
libsoup/soup-auth-manager.h
tests/ntlm-test.c

index 33043e7..cf5218b 100644 (file)
@@ -388,6 +388,7 @@ ntlm_authorize_pre (SoupMessage *msg, gpointer ntlm)
                SOUP_AUTH_MANAGER_NTLM_GET_PRIVATE (ntlm);
        SoupNTLMConnection *conn;
        const char *val;
+       char *challenge = NULL;
        SoupURI *uri;
 
        conn = get_connection_for_msg (priv, msg);
@@ -396,10 +397,11 @@ ntlm_authorize_pre (SoupMessage *msg, gpointer ntlm)
 
        val = soup_message_headers_get_list (msg->response_headers,
                                             "WWW-Authenticate");
-       if (val)
-               val = strstr (val, "NTLM ");
        if (!val)
                return;
+       challenge = soup_auth_manager_extract_challenge (val, "NTLM");
+       if (!challenge)
+               return;
 
        if (conn->state > SOUP_NTLM_SENT_REQUEST) {
                /* We already authenticated, but then got another 401.
@@ -410,7 +412,7 @@ ntlm_authorize_pre (SoupMessage *msg, gpointer ntlm)
                goto done;
        }
 
-       if (!soup_ntlm_parse_challenge (val, &conn->nonce, &conn->domain)) {
+       if (!soup_ntlm_parse_challenge (challenge, &conn->nonce, &conn->domain)) {
                conn->state = SOUP_NTLM_FAILED;
                goto done;
        }
@@ -418,7 +420,7 @@ ntlm_authorize_pre (SoupMessage *msg, gpointer ntlm)
        conn->auth = soup_auth_ntlm_new (conn->domain,
                                         soup_message_get_uri (msg)->host);
 #ifdef USE_NTLM_AUTH
-       conn->challenge_header = g_strdup (val + 5);
+       conn->challenge_header = g_strdup (challenge + 5);
        if (conn->state == SOUP_NTLM_SENT_SSO_REQUEST) {
                conn->state = SOUP_NTLM_RECEIVED_SSO_CHALLENGE;
                goto done;
@@ -435,6 +437,8 @@ ntlm_authorize_pre (SoupMessage *msg, gpointer ntlm)
        }
 
  done:
+       g_free (challenge);
+
        /* Remove the WWW-Authenticate headers so the session won't try
         * to do Basic auth too.
         */
index 12e3f4e..1aacf48 100644 (file)
@@ -241,37 +241,84 @@ auth_header_for_message (SoupMessage *msg)
        }
 }
 
-static char *
-extract_challenge (const char *challenges, const char *scheme)
+static GSList *
+next_challenge_start (GSList *items)
 {
-       GSList *items, *i;
-       int schemelen = strlen (scheme);
-       char *item, *space, *equals;
-       GString *challenge;
-
-       /* The relevant grammar:
+       /* The relevant grammar (from httpbis):
         *
         * WWW-Authenticate   = 1#challenge
         * Proxy-Authenticate = 1#challenge
-        * challenge          = auth-scheme 1#auth-param
+        * challenge          = auth-scheme [ 1*SP ( b64token / #auth-param ) ]
         * auth-scheme        = token
-        * auth-param         = token "=" ( token | quoted-string )
+        * auth-param         = token BWS "=" BWS ( token / quoted-string )
+        * b64token           = 1*( ALPHA / DIGIT /
+        *                          "-" / "." / "_" / "~" / "+" / "/" ) *"="
         *
         * The fact that quoted-strings can contain commas, equals
         * signs, and auth scheme names makes it tricky to "cheat" on
-        * the parsing. We just use soup_header_parse_list(), and then
-        * reassemble the pieces after we find the one we want.
+        * the parsing. So soup_auth_manager_extract_challenge() will
+        * have used soup_header_parse_list() to split the header into
+        * items. Given the grammar above, the possible items are:
+        *
+        *   auth-scheme
+        *   auth-scheme 1*SP b64token
+        *   auth-scheme 1*SP auth-param
+        *   auth-param
+        *
+        * where the first three represent the start of a new challenge and
+        * the last one does not.
         */
 
+       for (; items; items = items->next) {
+               const char *item = items->data;
+               const char *sp = strpbrk (item, "\t\r\n ");
+               const char *eq = strchr (item, '=');
+
+               if (!eq) {
+                       /* No "=", so it can't be an auth-param */
+                       return items;
+               }
+               if (!sp || sp > eq) {
+                       /* No space, or first space appears after the "=",
+                        * so it must be an auth-param.
+                        */
+                       continue;
+               }
+               while (g_ascii_isspace (*++sp))
+                       ;
+               if (sp == eq) {
+                       /* First "=" appears immediately after the first
+                        * space, so this must be an auth-param with
+                        * space around the "=".
+                        */
+                       continue;
+               }
+
+               /* "auth-scheme auth-param" or "auth-scheme b64token" */
+               return items;
+       }
+
+       return NULL;
+}
+
+char *
+soup_auth_manager_extract_challenge (const char *challenges, const char *scheme)
+{
+       GSList *items, *i, *next;
+       int schemelen = strlen (scheme);
+       char *item;
+       GString *challenge;
+
        items = soup_header_parse_list (challenges);
 
-       /* First item will start with the scheme name, followed by a
-        * space and then the first auth-param.
+       /* First item will start with the scheme name, followed by
+        * either nothing, or else a space and then the first
+        * auth-param.
         */
-       for (i = items; i; i = i->next) {
+       for (i = items; i; i = next_challenge_start (i->next)) {
                item = i->data;
                if (!g_ascii_strncasecmp (item, scheme, schemelen) &&
-                   g_ascii_isspace (item[schemelen]))
+                   (!item[schemelen] || g_ascii_isspace (item[schemelen])))
                        break;
        }
        if (!i) {
@@ -279,17 +326,10 @@ extract_challenge (const char *challenges, const char *scheme)
                return NULL;
        }
 
-       /* The challenge extends from this item until the end, or until
-        * the next item that has a space before an equals sign.
-        */
+       next = next_challenge_start (i->next);
        challenge = g_string_new (item);
-       for (i = i->next; i; i = i->next) {
+       for (i = i->next; i != next; i = i->next) {
                item = i->data;
-               space = strpbrk (item, " \t");
-               equals = strchr (item, '=');
-               if (!equals || (space && equals > space))
-                       break;
-
                g_string_append (challenge, ", ");
                g_string_append (challenge, item);
        }
@@ -313,7 +353,7 @@ create_auth (SoupAuthManagerPrivate *priv, SoupMessage *msg)
 
        for (i = priv->auth_types->len - 1; i >= 0; i--) {
                auth_class = priv->auth_types->pdata[i];
-               challenge = extract_challenge (header, auth_class->scheme_name);
+               challenge = soup_auth_manager_extract_challenge (header, auth_class->scheme_name);
                if (challenge)
                        break;
        }
@@ -336,7 +376,7 @@ check_auth (SoupMessage *msg, SoupAuth *auth)
        if (!header)
                return FALSE;
 
-       challenge = extract_challenge (header, soup_auth_get_scheme_name (auth));
+       challenge = soup_auth_manager_extract_challenge (header, soup_auth_get_scheme_name (auth));
        if (!challenge)
                return FALSE;
 
index 493960a..d82fbb1 100644 (file)
@@ -32,10 +32,13 @@ typedef struct {
 
 GType soup_auth_manager_get_type (void);
 
-void soup_auth_manager_emit_authenticate (SoupAuthManager *manager,
-                                         SoupMessage     *msg,
-                                         SoupAuth        *auth,
-                                         gboolean         retrying);
+void  soup_auth_manager_emit_authenticate (SoupAuthManager *manager,
+                                          SoupMessage     *msg,
+                                          SoupAuth        *auth,
+                                          gboolean         retrying);
+
+char *soup_auth_manager_extract_challenge (const char      *challenges,
+                                          const char      *scheme);
 
 G_END_DECLS
 
index 46ac46e..f5462c6 100644 (file)
@@ -58,29 +58,25 @@ server_callback (SoupServer *server, SoupMessage *msg,
        SoupSocket *socket;
        const char *auth;
        NTLMServerState state, required_user = 0;
-       gboolean auth_required = FALSE, not_found = FALSE;
-       gboolean basic_allowed = FALSE, ntlm_allowed = FALSE;
+       gboolean auth_required, not_found = FALSE;
+       gboolean basic_allowed = TRUE, ntlm_allowed = TRUE;
 
        if (msg->method != SOUP_METHOD_GET) {
                soup_message_set_status (msg, SOUP_STATUS_NOT_IMPLEMENTED);
                return;
        }
 
-       if (!strncmp (path, "/alice", 6)) {
-               auth_required = TRUE;
-               ntlm_allowed = TRUE;
+       if (!strncmp (path, "/alice", 6))
                required_user = NTLM_AUTHENTICATED_ALICE;
-       } else if (!strncmp (path, "/bob", 4)) {
-               auth_required = TRUE;
-               ntlm_allowed = TRUE;
+       else if (!strncmp (path, "/bob", 4))
                required_user = NTLM_AUTHENTICATED_BOB;
-       } else if (!strncmp (path, "/either", 7)) {
-               auth_required = TRUE;
-               ntlm_allowed = basic_allowed = TRUE;
-       } else if (!strncmp (path, "/basic", 6)) {
-               auth_required = TRUE;
-               basic_allowed = TRUE;
-       }
+       else if (!strncmp (path, "/either", 7))
+               ;
+       else if (!strncmp (path, "/basic", 6))
+               ntlm_allowed = FALSE;
+       else if (!strncmp (path, "/noauth", 7))
+               basic_allowed = ntlm_allowed = FALSE;
+       auth_required = ntlm_allowed || basic_allowed;
 
        if (strstr (path, "/404"))
                not_found = TRUE;
@@ -95,7 +91,9 @@ server_callback (SoupServer *server, SoupMessage *msg,
                        if (!strncmp (auth + 5, NTLM_REQUEST_START,
                                      strlen (NTLM_REQUEST_START))) {
                                state = NTLM_RECEIVED_REQUEST;
-                               /* If they start, they must finish */
+                               /* If they start, they must finish, even if
+                                * it was unnecessary.
+                                */
                                auth_required = ntlm_allowed = TRUE;
                                basic_allowed = FALSE;
                        } else if (state == NTLM_SENT_CHALLENGE &&
@@ -104,12 +102,15 @@ server_callback (SoupServer *server, SoupMessage *msg,
                                state = NTLM_RESPONSE_USER (auth + 5);
                        } else
                                state = NTLM_UNAUTHENTICATED;
-               } else if (!strncmp (auth, "Basic ", 6) && basic_allowed) {
+               } else if (basic_allowed && !strncmp (auth, "Basic ", 6)) {
                        gsize len;
                        char *decoded = (char *)g_base64_decode (auth + 6, &len);
 
-                       if (!strncmp (decoded, "alice:password", len) ||
-                           !strncmp (decoded, "bob:password", len))
+                       if (!strncmp (decoded, "alice:password", len) &&
+                           required_user != NTLM_AUTHENTICATED_BOB)
+                               auth_required = FALSE;
+                       else if (!strncmp (decoded, "bob:password", len) &&
+                                required_user != NTLM_AUTHENTICATED_ALICE)
                                auth_required = FALSE;
                        g_free (decoded);
                }
@@ -122,13 +123,13 @@ server_callback (SoupServer *server, SoupMessage *msg,
        if (auth_required) {
                soup_message_set_status (msg, SOUP_STATUS_UNAUTHORIZED);
 
-               if (basic_allowed) {
+               if (basic_allowed && state != NTLM_RECEIVED_REQUEST) {
                        soup_message_headers_append (msg->response_headers,
                                                     "WWW-Authenticate",
                                                     "Basic realm=\"ntlm-test\"");
                }
 
-               if (state == NTLM_RECEIVED_REQUEST) {
+               if (ntlm_allowed && state == NTLM_RECEIVED_REQUEST) {
                        soup_message_headers_append (msg->response_headers,
                                                     "WWW-Authenticate",
                                                     "NTLM " NTLM_CHALLENGE);
@@ -158,7 +159,8 @@ static void
 authenticate (SoupSession *session, SoupMessage *msg,
              SoupAuth *auth, gboolean retrying, gpointer user)
 {
-       soup_auth_authenticate (auth, user, "password");
+       if (!retrying)
+               soup_auth_authenticate (auth, user, "password");
 }
 
 typedef struct {
@@ -180,11 +182,9 @@ prompt_check (SoupMessage *msg, gpointer user_data)
                                                "WWW-Authenticate");
        if (header && strstr (header, "Basic "))
                state->got_basic_prompt = TRUE;
-       if (!state->sent_ntlm_request) {
-               if (header && strstr (header, "NTLM") &&
-                   !strstr (header, NTLM_CHALLENGE))
-                       state->got_ntlm_prompt = TRUE;
-       }
+       if (header && strstr (header, "NTLM") &&
+           !strstr (header, NTLM_CHALLENGE))
+               state->got_ntlm_prompt = TRUE;
 }
 
 static void
@@ -333,10 +333,11 @@ static void
 do_ntlm_round (SoupURI *base_uri, gboolean use_ntlm, const char *user)
 {
        SoupSession *session;
-       gboolean alice = use_ntlm && !strcmp (user, "alice");
-       gboolean bob = use_ntlm && !strcmp (user, "bob");
-
-       g_return_if_fail (use_ntlm || !alice);
+       gboolean alice = !g_strcmp0 (user, "alice");
+       gboolean bob = !g_strcmp0 (user, "bob");
+       gboolean alice_via_ntlm = use_ntlm && alice;
+       gboolean bob_via_ntlm = use_ntlm && bob;
+       gboolean alice_via_basic = !use_ntlm && alice;
 
        session = soup_test_session_new (SOUP_TYPE_SESSION_ASYNC, NULL);
        if (use_ntlm)
@@ -347,40 +348,87 @@ do_ntlm_round (SoupURI *base_uri, gboolean use_ntlm, const char *user)
                                  G_CALLBACK (authenticate), (char *)user);
        }
 
+       /* 1. Server doesn't request auth, so both get_ntlm_prompt and
+        * get_basic_prompt are both FALSE, and likewise do_basic. But
+        * if we're using NTLM we'll try that even without the server
+        * asking.
+        */
        do_message (session, base_uri, "/noauth",
                    FALSE, use_ntlm,
                    FALSE, FALSE,
                    SOUP_STATUS_OK);
+
+       /* 2. Server requires auth as Alice, so it will request that
+        * if we didn't already authenticate the connection to her in
+        * the previous step. If we authenticated as Bob in the
+        * previous step, then we'll just immediately get a 401 here.
+        * So in no case will we see the client try to do_ntlm.
+        */
        do_message (session, base_uri, "/alice",
-                   !use_ntlm || bob, FALSE,
-                   FALSE, FALSE,
+                   !alice_via_ntlm, FALSE,
+                   !alice_via_ntlm, alice_via_basic,
                    alice ? SOUP_STATUS_OK :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 3. Server still requires auth as Alice, but this URI
+        * doesn't exist, so Alice should get a 404, but others still
+        * get 401. Alice-via-NTLM is still authenticated, and so
+        * won't get prompts, and Alice-via-Basic knows at this point
+        * to send auth without it being requested, so also won't get
+        * prompts. But Bob/nobody will.
+        */
        do_message (session, base_uri, "/alice/404",
-                   !use_ntlm, bob,
-                   FALSE, FALSE,
+                   !alice, bob_via_ntlm,
+                   !alice, alice_via_basic,
                    alice ? SOUP_STATUS_NOT_FOUND :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 4. Should be exactly the same as #3, except the status code */
        do_message (session, base_uri, "/alice",
-                   !use_ntlm, bob,
-                   FALSE, FALSE,
+                   !alice, bob_via_ntlm,
+                   !alice, alice_via_basic,
                    alice ? SOUP_STATUS_OK :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 5. This path requires auth as Bob; Alice-via-NTLM will get
+        * an immediate 401 and not try to reauthenticate.
+        * Alice-via-Basic will get a 401 and then try to do Basic
+        * (and fail). Bob-via-NTLM will try to do NTLM right away and
+        * succeed.
+        */
        do_message (session, base_uri, "/bob",
-                   !use_ntlm || alice, bob,
-                   FALSE, FALSE,
+                   !bob_via_ntlm, bob_via_ntlm,
+                   !bob_via_ntlm, alice_via_basic,
                    bob ? SOUP_STATUS_OK :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 6. Back to /alice. Somewhat the inverse of #5; Bob-via-NTLM
+        * will get an immediate 401 and not try again, Alice-via-NTLM
+        * will try to do NTLM right away and succeed. Alice-via-Basic
+        * still knows about this path, so will try Basic right away
+        * and succeed.
+        */
        do_message (session, base_uri, "/alice",
-                   !use_ntlm || bob, alice,
-                   FALSE, FALSE,
+                   !alice_via_ntlm, alice_via_ntlm,
+                   !alice_via_ntlm, alice_via_basic,
                    alice ? SOUP_STATUS_OK :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 7. Server accepts Basic auth from either user, but not NTLM.
+        * Since Bob-via-NTLM is unauthenticated at this point, he'll try
+        * NTLM before realizing that the server doesn't support it.
+        */
        do_message (session, base_uri, "/basic",
-                   FALSE, bob,
+                   FALSE, bob_via_ntlm,
                    TRUE, user != NULL,
                    user != NULL ? SOUP_STATUS_OK :
                    SOUP_STATUS_UNAUTHORIZED);
+
+       /* 8. Server accepts Basic or NTLM from either user.
+        * Alice-via-NTLM is still authenticated at this point from #6,
+        * and Bob-via-NTLM is authenticated from #7, so neither
+        * of them will do anything.
+        */
        do_message (session, base_uri, "/either",
                    !use_ntlm, FALSE,
                    !use_ntlm, !use_ntlm && user != NULL,