Ncat HTTP proxy Digest: honor "algorithm" param

David Fifield <[email protected]> Fri, 1 Feb 2019 13:06:23 -0700

Newsgroups	gmane.comp.security.nmap.devel
Message-ID	<[email protected]>

I noticed a copy-and-paste error in the Ncat HTTP proxy Digest
authentication. An unknown algorithm in credentials like
algorithm="foobar" was still being treated as ALGORITHM_MD5.

That led me to find that the server was not even checking the algorithm
param, and always acting as if it were ALGORITHM_MD5.

This patch fixes the copy-and-paste error and makes it so that
Proxy-Authenticate and Proxy-Authorization headers that have an unknown
algorithm are ignored.

_______________________________________________
Sent through the dev mailing list
https://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/

0001-Ncat-HTTP-proxy-Digest-check-the-algorithm-auth-para.patch (text/x-diff, 8.2 KB)

From b4f2467272127bacd5a156f967458c89e45f796c Mon Sep 17 00:00:00 2001
From: David Fifield <[email protected]>
Date: Fri, 1 Feb 2019 11:47:24 -0700
Subject: [PATCH] Ncat HTTP proxy Digest: check the "algorithm" auth-param.

There were two bugs here: a copy-paste error caused an unknown algorithm
in credentials to be treated as "MD5"; and a missing check that
algorithm=="MD5" before doing MD5 digests.
---
 ncat/http.c            | 33 +++++++++++++++------
 ncat/http_digest.c     |  3 +-
 ncat/test/ncat-test.pl | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 10 deletions(-)

diff --git a/ncat/http.c b/ncat/http.c
index a57b8f155..1729cba0d 100644
--- a/ncat/http.c
+++ b/ncat/http.c
@@ -1524,7 +1524,7 @@ static const char *http_read_credentials(const char *s,
                 if (str_equal_i(value, "MD5"))
                     credentials->u.digest.algorithm = ALGORITHM_MD5;
                 else
-                    credentials->u.digest.algorithm = ALGORITHM_MD5;
+                    credentials->u.digest.algorithm = ALGORITHM_UNKNOWN;
             } else if (str_equal_i(name, "qop")) {
                 if (str_equal_i(value, "auth"))
                     credentials->u.digest.qop = QOP_AUTH;
@@ -1605,12 +1605,21 @@ struct http_challenge *http_header_get_proxy_challenge(const struct http_header
                 http_challenge_free(challenge);
                 return NULL;
             }
-            if (auth_scheme_is_better(tmp_info.scheme, challenge->scheme)) {
-                http_challenge_free(challenge);
-                *challenge = tmp_info;
-            } else {
+#if HAVE_HTTP_DIGEST
+            // RFC 2617, section 3.2.1: "If the algorithm is not understood, the
+            // challenge should be ignored (and a different one used, if there
+            // is more than one)."
+            if (tmp_info.scheme == AUTH_DIGEST && tmp_info.digest.algorithm == ALGORITHM_UNKNOWN) {
+                http_challenge_free(&tmp_info);
+                continue;
+            }
+#endif
+            if (!auth_scheme_is_better(tmp_info.scheme, challenge->scheme)) {
                 http_challenge_free(&tmp_info);
+		continue;
             }
+            http_challenge_free(challenge);
+            *challenge = tmp_info;
         }
     }
 
@@ -1636,12 +1645,18 @@ struct http_credentials *http_header_get_proxy_credentials(const struct http_hea
                 http_credentials_free(credentials);
                 return NULL;
             }
-            if (auth_scheme_is_better(tmp_info.scheme, credentials->scheme)) {
-                http_credentials_free(credentials);
-                *credentials = tmp_info;
-            } else {
+#if HAVE_HTTP_DIGEST
+            if (tmp_info.scheme == AUTH_DIGEST && tmp_info.u.digest.algorithm == ALGORITHM_UNKNOWN) {
+                http_credentials_free(&tmp_info);
+                continue;
+            }
+#endif
+            if (!auth_scheme_is_better(tmp_info.scheme, credentials->scheme)) {
                 http_credentials_free(&tmp_info);
+                continue;
             }
+            http_credentials_free(credentials);
+            *credentials = tmp_info;
         }
     }
 
diff --git a/ncat/http_digest.c b/ncat/http_digest.c
index eb54f2796..f8f43f744 100644
--- a/ncat/http_digest.c
+++ b/ncat/http_digest.c
@@ -398,7 +398,8 @@ int http_digest_check_credentials(const char *username, const char *realm,
         || credentials->u.digest.realm == NULL
         || credentials->u.digest.nonce == NULL
         || credentials->u.digest.uri == NULL
-        || credentials->u.digest.response == NULL) {
+        || credentials->u.digest.response == NULL
+        || credentials->u.digest.algorithm != ALGORITHM_MD5) {
         return 0;
     }
     if (credentials->u.digest.qop != QOP_NONE && credentials->u.digest.qop != QOP_AUTH)
diff --git a/ncat/test/ncat-test.pl b/ncat/test/ncat-test.pl
index 904478dee..15551b458 100755
--- a/ncat/test/ncat-test.pl
+++ b/ncat/test/ncat-test.pl
@@ -1889,6 +1889,28 @@ Proxy-Authenticate: Basic realm=\"$realm\", Digest realm=\"$realm\", nonce=\"$no
 	return 1;
 };
 
+server_client_test "HTTP proxy client ignores Digest with unknown algorithm",
+["-k"], ["--proxy", "$HOST:$PORT", "--proxy-auth", "user:pass", "--proxy-type", "http"],
+sub {
+	my $nonce = "0123456789abcdef";
+	my $realm = "realm";
+	my $req = timeout_read($s_out);
+	$req or die "No initial request from client";
+	syswrite($s_in, "HTTP/1.0 407 Authentication Required\r\
+Proxy-Authenticate: Basic realm=\"$realm\"\r\
+Proxy-Authenticate: Digest algorithm=\"foobar\" realm=\"$realm\", nonce=\"$nonce\", qop=\"auth\"\r\n\r\n");
+	$req = timeout_read($s_out);
+	$req or die "No followup request from client";
+	$req = HTTP::Request->parse($req);
+	foreach my $hdr ($req->header("Proxy-Authorization")) {
+		my ($scheme, %attrs) = parse_proxy_header($hdr);
+		if ($scheme eq "Digest") {
+			die "Client used Digtest auth with an unknown algorithm";
+		}
+	}
+	return 1;
+};
+
 server_client_test "HTTP proxy Digest client auth",
 ["-k"], ["--proxy", "$HOST:$PORT", "--proxy-auth", "user:pass", "--proxy-type", "http"],
 sub {
@@ -2457,6 +2479,63 @@ Proxy-Authorization: Digest username=\"user\", realm=\"$attrs{realm}\", nonce=\"
 kill_children;
 
 ($s_pid, $s_out, $s_in) = ncat_server("--proxy-type", "http", "--proxy-auth", "user:pass");
+test "HTTP proxy Digest credentials unknown algorithm",
+sub {
+	my ($c_pid, $c_out, $c_in) = ncat_client();
+	syswrite($c_in, "CONNECT $HOST:$PORT HTTP/1.0\r\n\r\n");
+	my $resp = timeout_read($c_out);
+	$resp or die "No response from server";
+	$resp = HTTP::Response->parse($resp);
+	foreach my $hdr ($resp->header("Proxy-Authenticate")) {
+		my ($scheme, %attrs) = parse_proxy_header($hdr);
+		next if $scheme ne "Digest";
+		die "no nonce" if not $attrs{"nonce"};
+		die "no realm" if not $attrs{"realm"};
+		my ($c_pid, $c_out, $c_in) = ncat_client();
+		my $response = digest_response("user", "pass", $attrs{"realm"}, "CONNECT", "$HOST:$PORT", $attrs{"nonce"}, undef, undef, undef);
+		syswrite($c_in, "CONNECT $HOST:$PORT HTTP/1.0\r\
+Proxy-Authorization: Digest algorithm=\"foobar\", username=\"user\", realm=\"$attrs{realm}\", nonce=\"$attrs{nonce}\", uri=\"$HOST:$PORT\", response=\"$response\"\r\n\r\n");
+		$resp = timeout_read($c_out);
+		$resp or die "No response from server";
+		$resp = HTTP::Response->parse($resp);
+		my $code = $resp->code;
+		$resp->code == 407 or die "Expected response code 407, got $code";
+		return 1;
+	}
+	die "No Proxy-Authenticate: Digest in server response";
+};
+kill_children;
+
+($s_pid, $s_out, $s_in) = ncat_server("--proxy-type", "http", "--proxy-auth", "user:pass");
+test "HTTP proxy Digest credentials unknown algorithm followed by known algorithm",
+sub {
+	my ($c_pid, $c_out, $c_in) = ncat_client();
+	syswrite($c_in, "CONNECT $HOST:$PORT HTTP/1.0\r\n\r\n");
+	my $resp = timeout_read($c_out);
+	$resp or die "No response from server";
+	$resp = HTTP::Response->parse($resp);
+	foreach my $hdr ($resp->header("Proxy-Authenticate")) {
+		my ($scheme, %attrs) = parse_proxy_header($hdr);
+		next if $scheme ne "Digest";
+		die "no nonce" if not $attrs{"nonce"};
+		die "no realm" if not $attrs{"realm"};
+		my ($c_pid, $c_out, $c_in) = ncat_client();
+		my $response = digest_response("user", "pass", $attrs{"realm"}, "CONNECT", "$HOST:$PORT", $attrs{"nonce"}, undef, undef, undef);
+		syswrite($c_in, "CONNECT $HOST:$PORT HTTP/1.0\r\
+Proxy-Authorization: Digest algorithm=\"foobar\", username=\"user\", realm=\"$attrs{realm}\", nonce=\"$attrs{nonce}\", uri=\"$HOST:$PORT\", response=\"$response\"\r\
+Proxy-Authorization: Digest algorithm=\"MD5\", username=\"user\", realm=\"$attrs{realm}\", nonce=\"$attrs{nonce}\", uri=\"$HOST:$PORT\", response=\"$response\"\r\n\r\n");
+		$resp = timeout_read($c_out);
+		$resp or die "No response from server";
+		$resp = HTTP::Response->parse($resp);
+		my $code = $resp->code;
+		$resp->code == 200 or die "Expected response code 200, got $code";
+		return 1;
+	}
+	die "No Proxy-Authenticate: Digest in server response";
+};
+kill_children;
+
+($s_pid, $s_out, $s_in) = ncat_server("--proxy-type", "http", "--proxy-auth", "user:pass");
 test "HTTP proxy Digest missing fields",
 sub {
 	my ($c_pid, $c_out, $c_in) = ncat_client();
-- 
2.11.0