From f678aba4815bbcb63102dcdcd31f0ebcfdf553b4 Mon Sep 17 00:00:00 2001
From: Joe Orton <jorton@redhat.com>
Date: Fri, 24 Sep 2021 12:25:24 +0100
Subject: [PATCH] Fix regression in authentication for request-target of "*"
 (#63):

* test/utils.c (any_2xx_request_method): Factored out of
  any_2xx_request.

* test/auth.c (serve_star_scope_checker, star_scope): New test.

* src/ne_auth.c (get_scope_path): Don't handle "*" here.
  (basic_challenge): Properly treat "*" as unscoped rather than simply
  CONNECT.
  (request_basic): Only check scope if there is a configured scope.
---
 NEWS          |  3 +++
 src/ne_auth.c | 20 ++++++++++----------
 test/auth.c   | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 test/utils.c  |  9 +++++++--
 test/utils.h  |  4 ++++
 5 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 0f4156d..43b50ad 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,6 @@
+Changes in release 0.32.2:
+* Fix auth handling for request-target of "*" (regressed since 0.31.x)
+
 Changes in release 0.32.1:
 * Fix configure CFLAGS handling in Kerberos detection.
 * Various spelling fixes.
diff --git a/src/ne_auth.c b/src/ne_auth.c
index 756461e..75ca43f 100644
--- a/src/ne_auth.c
+++ b/src/ne_auth.c
@@ -440,7 +440,7 @@ static char *get_scope_path(const char *uri)
     memset(&udot, 0, sizeof udot);
     udot.path = ".";
 
-    if (strcmp(uri, "*") == 0 || ne_uri_parse(uri, &base) != 0) {
+    if (ne_uri_parse(uri, &base) != 0) {
         /* Assume scope is whole origin. */
         return ne_strdup("/");
     }
@@ -491,18 +491,18 @@ static int basic_challenge(auth_session *sess, int attempt,
 
     ne__strzero(password, sizeof password);
 
-    if (sess->context == AUTH_CONNECT) {
-        /* For proxy auth w/TLS, auth is limited to handling CONNECT
-         * request, no need to derive the "scope" path. */
-        return 0;
-    }
+    if (sess->ndomains) free_domains(sess); /* is this really needed? */
 
-    if (sess->ndomains != 1) {
-        sess->domains = ne_realloc(sess->domains, sizeof(*sess->domains));
-        sess->ndomains = 1;
+    if (strcmp(uri, "*") == 0) {
+        /* If the request-target is "*" the auth scope is explicitly
+         * the whole server. */
+        return 0;
     }
 
+    sess->domains = ne_realloc(sess->domains, sizeof(*sess->domains));
     sess->domains[0] = get_scope_path(uri);
+    sess->ndomains = 1;
+
     NE_DEBUG(NE_DBG_HTTPAUTH, "auth: Basic auth scope is: %s\n",
              sess->domains[0]);
 
@@ -512,7 +512,7 @@ static int basic_challenge(auth_session *sess, int attempt,
 /* Add Basic authentication credentials to a request */
 static char *request_basic(auth_session *sess, struct auth_request *req) 
 {
-    if (!inside_domain(sess, req->uri)) {
+    if (sess->ndomains && !inside_domain(sess, req->uri)) {
         return NULL;
     }
 
diff --git a/test/auth.c b/test/auth.c
index 4fa0957..5aa7581 100644
--- a/test/auth.c
+++ b/test/auth.c
@@ -1576,7 +1576,7 @@ static int serve_basic_scope_checker(ne_socket *sock, void *userdata)
     send_response(sock, CHAL_WALLY, 401, 0);
 
     /* Retry of GET /fish/0 - expect Basic creds */
-    auth_failed = 0;
+    auth_failed = 1;
     got_header = auth_hdr;
     CALL(discard_request(sock));
     if (auth_failed) {
@@ -1620,6 +1620,50 @@ static int basic_scope(void)
     return await_server();
 }
 
+/* Test for scope of "*" */
+static int serve_star_scope_checker(ne_socket *sock, void *userdata)
+{
+    /* --- OPTIONS * -- first request */
+    digest_hdr = NULL;
+    got_header = dup_header;
+    want_header = "Authorization";
+    CALL(discard_request(sock));
+    if (digest_hdr) {
+        t_context("Got WWW-Auth header on initial request");
+        return error_response(sock, FAIL);
+    }
+
+    send_response(sock, CHAL_WALLY, 401, 0);
+
+    /* Retry of OPTIONS * - expect Basic creds */
+    auth_failed = 1;
+    got_header = auth_hdr;
+    CALL(discard_request(sock));
+    if (auth_failed) {
+        t_context("No Basic Auth in OPTIONS request");
+        return error_response(sock, FAIL);
+    }
+    send_response(sock, CHAL_WALLY, 200, 0);
+
+    return 0;
+}
+
+/* Test for the scope of "*". */
+static int star_scope(void)
+{
+    ne_session *sess;
+
+    CALL(make_session(&sess, serve_star_scope_checker, NULL));
+
+    ne_set_server_auth(sess, auth_cb, NULL);
+
+    CALL(any_2xx_request_method(sess, "OPTIONS", "*")); /* must use auth */
+
+    ne_session_destroy(sess);
+
+    return await_server();
+}
+
 /* proxy auth, proxy AND origin */
 
 ne_test tests[] = {
@@ -1643,5 +1687,6 @@ ne_test tests[] = {
     T(CVE_2008_3746),
     T(forget),
     T(basic_scope),
+    T(star_scope),
     T(NULL)
 };
diff --git a/test/utils.c b/test/utils.c
index ddb950c..2bf8228 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -99,9 +99,9 @@ int any_request(ne_session *sess, const char *uri)
     return ret;
 }
 
-int any_2xx_request(ne_session *sess, const char *uri)
+int any_2xx_request_method(ne_session *sess, const char *method, const char *uri)
 {
-    ne_request *req = ne_request_create(sess, "GET", uri);
+    ne_request *req = ne_request_create(sess, method, uri);
     int ret = ne_request_dispatch(req);
     int klass = ne_get_status(req)->klass;
     const char *context = ne_get_response_header(req, "X-Neon-Context");
@@ -119,6 +119,11 @@ int any_2xx_request(ne_session *sess, const char *uri)
     return ret;
 }
 
+int any_2xx_request(ne_session *sess, const char *uri)
+{
+    return any_2xx_request_method(sess, "GET", uri);
+}
+
 int any_2xx_request_body(ne_session *sess, const char *uri)
 {
     ne_request *req = ne_request_create(sess, "GET", uri);
diff --git a/test/utils.h b/test/utils.h
index 117338e..126ba03 100644
--- a/test/utils.h
+++ b/test/utils.h
@@ -50,6 +50,10 @@ int any_2xx_request(ne_session *sess, const char *uri);
 /* As above but with a request body. */
 int any_2xx_request_body(ne_session *sess, const char *uri);
 
+/* As any_2xx_request but with a specified method. */
+int any_2xx_request_method(ne_session *sess, const char *method,
+                           const char *uri);
+
 /* makes *session, spawns server which will run 'fn(userdata,
  * socket)'.  sets error context if returns non-zero, i.e use like:
  * CALL(make_session(...)); */
