authhelper: verification detection improvements

Signed-off-by: Simon Bennetts <[email protected]>
zaproxy · Feb 26, 2025 · 2e6f15c · 2e6f15c
1 parent fb9a96e
commit 2e6f15c
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 14 deletions.
diff --git a/addOns/authhelper/CHANGELOG.md b/addOns/authhelper/CHANGELOG.md
@@ -11,6 +11,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ### Changed
 - Prefer form related fields in Browser Based Authentication for the selection of username field.
 - Tweaked the auth report summary keys.
+- Only check URLs and methods once for being good verification requests.
+- Added API support to the browser based auth method proxy.
 
 ### Fixed
 - Correctly read the API parameters when setting up Browser Based Authentication.

diff --git a/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthUtils.java b/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthUtils.java
@@ -67,6 +67,7 @@
 import org.parosproxy.paros.view.View;
 import org.zaproxy.addon.authhelper.BrowserBasedAuthenticationMethodType.BrowserBasedAuthenticationMethod;
 import org.zaproxy.addon.authhelper.internal.AuthenticationStep;
+import org.zaproxy.addon.commonlib.ResourceIdentificationUtils;
 import org.zaproxy.zap.authentication.AuthenticationCredentials;
 import org.zaproxy.zap.authentication.AuthenticationMethod;
 import org.zaproxy.zap.authentication.AuthenticationMethod.UnsupportedAuthenticationCredentialsException;
@@ -164,6 +165,12 @@ public class AuthUtils {
     private static Map<Integer, SessionManagementRequestDetails> contextSessionMgmtMap =
             new HashMap<>();
 
+    /**
+     * The URLs (and methods) we've checked for finding good verification requests. These will only
+     * be recorded if the user has set verification to auto-detect.
+     */
+    private static Map<Integer, Set<String>> contextVerificationMap = new HashMap<>();
+
     public static long getTimeToWaitMs() {
         return timeToWaitMs;
     }
@@ -986,6 +993,7 @@ public static void clean() {
         knownTokenMap.clear();
         contextVerifMap.clear();
         contextSessionMgmtMap.clear();
+        contextVerificationMap.clear();
         if (executorService != null) {
             executorService.shutdown();
         }
@@ -1051,7 +1059,18 @@ public static void processVerificationDetails(
             Context context,
             VerificationRequestDetails details,
             VerificationDetectionScanRule rule) {
-        getExecutorService().submit(new VerificationDetectionProcessor(context, details, rule));
+
+        String methodUrl =
+                details.getMsg().getRequestHeader().getMethod()
+                        + " "
+                        + details.getMsg().getRequestHeader().getURI().toString();
+
+        if (contextVerificationMap
+                .computeIfAbsent(context.getId(), c -> new HashSet<>())
+                .add(methodUrl)) {
+            // Have not already checked this method + url
+            getExecutorService().submit(new VerificationDetectionProcessor(context, details, rule));
+        }
     }
 
     static class AuthenticationBrowserHook implements BrowserHook {
@@ -1109,4 +1128,11 @@ public Thread newThread(Runnable r) {
             return t;
         }
     }
+
+    public static boolean isRelevantToAuth(HttpMessage msg) {
+        return !(ResourceIdentificationUtils.isImage(msg)
+                || ResourceIdentificationUtils.isFont(msg)
+                || ResourceIdentificationUtils.isCss(msg)
+                || ResourceIdentificationUtils.isJavaScript(msg));
+    }
 }
diff --git a/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthenticationDiagnostics.java b/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/AuthenticationDiagnostics.java
@@ -89,13 +89,9 @@ public void onHttpRequestSend(
                     @Override
                     public void onHttpResponseReceive(
                             HttpMessage msg, int initiator, HttpSender sender) {
-                        if (msg.getResponseHeader().isImage()
-                                || !(msg.getResponseHeader().isHtml()
-                                        || msg.getResponseHeader().isJson()
-                                        || msg.getResponseHeader().isXml())) {
+                        if (!AuthUtils.isRelevantToAuth(msg)) {
                             return;
                         }
-
                         addMessageToStep(msg);
                     }
 

diff --git a/...lper/src/main/java/org/zaproxy/addon/authhelper/BrowserBasedAuthenticationMethodType.java b/...lper/src/main/java/org/zaproxy/addon/authhelper/BrowserBasedAuthenticationMethodType.java
@@ -66,13 +66,13 @@
 import org.zaproxy.addon.authhelper.internal.StepsPanel;
 import org.zaproxy.addon.network.ExtensionNetwork;
 import org.zaproxy.addon.network.internal.client.apachev5.HttpSenderContextApache;
+import org.zaproxy.addon.network.server.HttpServerConfig;
 import org.zaproxy.addon.network.server.Server;
 import org.zaproxy.zap.authentication.AbstractAuthenticationMethodOptionsPanel;
 import org.zaproxy.zap.authentication.AbstractCredentialsOptionsPanel;
 import org.zaproxy.zap.authentication.AuthenticationCredentials;
 import org.zaproxy.zap.authentication.AuthenticationHelper;
 import org.zaproxy.zap.authentication.AuthenticationMethod;
-import org.zaproxy.zap.authentication.AuthenticationMethod.UnsupportedAuthenticationCredentialsException;
 import org.zaproxy.zap.authentication.AuthenticationMethodType;
 import org.zaproxy.zap.authentication.UsernamePasswordAuthenticationCredentials;
 import org.zaproxy.zap.extension.api.ApiDynamicActionImplementor;
@@ -144,7 +144,13 @@ private Server getProxy(Context context) {
         if (proxy == null) {
             ExtensionNetwork extNet = AuthUtils.getExtension(ExtensionNetwork.class);
             handler = new ClientSideHandler(context);
-            proxy = extNet.createHttpProxy(getHttpSender(), handler);
+            proxy =
+                    extNet.createHttpServer(
+                            HttpServerConfig.builder()
+                                    .setHttpMessageHandler(handler)
+                                    .setHttpSender(getHttpSender())
+                                    .setServeZapApi(true)
+                                    .build());
         }
         return proxy;
     }

diff --git a/.../authhelper/src/main/java/org/zaproxy/addon/authhelper/VerificationDetectionScanRule.java b/.../authhelper/src/main/java/org/zaproxy/addon/authhelper/VerificationDetectionScanRule.java
@@ -53,11 +53,7 @@ public boolean appliesToHistoryType(int historyType) {
     @Override
     public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
 
-        if (msg.getResponseHeader().isImage()
-                || !(msg.getResponseHeader().isHtml()
-                        || msg.getResponseHeader().isJson()
-                        || msg.getResponseHeader().isXml())) {
-            // An "image/svg+xml" will look like XML
+        if (!AuthUtils.isRelevantToAuth(msg)) {
             return;
         }
 

diff --git a/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/internal/ClientSideHandler.java b/addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/internal/ClientSideHandler.java
@@ -64,7 +64,11 @@ public void handleMessage(HttpMessageHandlerContext ctx, HttpMessage msg) {
 
         AuthenticationHelper.addAuthMessageToHistory(msg);
 
-        if (isPost(msg) && context.isIncluded(msg.getRequestHeader().getURI().toString())) {
+        if (!context.isIncluded(msg.getRequestHeader().getURI().toString())) {
+            return;
+        }
+
+        if (isPost(msg)) {
             // Record the last in scope POST as a fallback
             fallbackMsg = msg;
         }

diff --git a/addOns/authhelper/src/test/java/org/zaproxy/addon/authhelper/AuthUtilsUnitTest.java b/addOns/authhelper/src/test/java/org/zaproxy/addon/authhelper/AuthUtilsUnitTest.java
@@ -51,6 +51,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestTemplate;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.openqa.selenium.By;
 import org.openqa.selenium.Dimension;
 import org.openqa.selenium.OutputType;
@@ -760,6 +762,41 @@ void shouldRemoveSessionToken() throws Exception {
         assertThat(st2, is(nullValue()));
     }
 
+    @ParameterizedTest
+    @CsvSource({
+        "text/html; charset=utf-8, true",
+        "multipart/form-data; boundary=ExampleBoundaryString, true",
+        "application/x-www-form-urlencoded, true",
+        "application/json, true",
+        "application/xhtml+xml, true",
+        "application/xml, true",
+        "text/xml, true",
+        "application/x-font-ttf, false",
+        "text/css, false",
+        "text/javascript; charset=utf-8, false",
+        "image/gif, false",
+        "image/svg+xml, false",
+    })
+    void shouldReportIfRelevantToAuth(String contentType, String result) throws Exception {
+        // Given
+        HttpMessage msg =
+                new HttpMessage(
+                        new HttpRequestHeader(
+                                HttpRequestHeader.GET,
+                                new URI("https://www.example.com", true),
+                                HttpHeader.HTTP11),
+                        new HttpRequestBody(),
+                        new HttpResponseHeader(),
+                        new HttpResponseBody());
+        msg.getResponseHeader().setHeader(HttpResponseHeader.CONTENT_TYPE, contentType);
+
+        // When
+        boolean res = AuthUtils.isRelevantToAuth(msg);
+
+        // Then
+        assertThat(res, is(equalTo(Boolean.parseBoolean(result))));
+    }
+
     static class BrowserTest extends TestUtils {
 
         @RegisterExtension static SeleniumJupiter seleniumJupiter = new SeleniumJupiter();