You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/08/17 14:24:01 UTC

[tomcat] branch 10.0.x updated: Refactored check for preemptive authentication

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.0.x by this push:
     new 06f1f15  Refactored check for preemptive authentication
06f1f15 is described below

commit 06f1f152c28b651bd4f5a81a0b882b60f70f1f9b
Author: Robert Rodewald <r....@airitsystems.de>
AuthorDate: Tue Aug 10 19:11:24 2021 +0200

    Refactored check for preemptive authentication
    
    - new protected method isPreemptiveAuthPossible() in AuthenticatorBase
      which is overridden in some authenticators
    - moved getRequestCertificates() from AuthenticatorBase to
      SSLAuthenticator
    - Added more specific check for value of header "authorization"
    
    # Conflicts:
    #	webapps/docs/changelog.xml
---
 .../catalina/authenticator/AuthenticatorBase.java  | 56 +++++++---------------
 .../catalina/authenticator/BasicAuthenticator.java |  8 ++++
 .../authenticator/DigestAuthenticator.java         |  8 ++++
 .../catalina/authenticator/SSLAuthenticator.java   | 39 +++++++++++++++
 .../authenticator/SpnegoAuthenticator.java         |  7 +++
 webapps/docs/changelog.xml                         |  6 +++
 6 files changed, 85 insertions(+), 39 deletions(-)

diff --git a/java/org/apache/catalina/authenticator/AuthenticatorBase.java b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
index d751582..4a26b51 100644
--- a/java/org/apache/catalina/authenticator/AuthenticatorBase.java
+++ b/java/org/apache/catalina/authenticator/AuthenticatorBase.java
@@ -18,7 +18,6 @@ package org.apache.catalina.authenticator;
 
 import java.io.IOException;
 import java.security.Principal;
-import java.security.cert.X509Certificate;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Optional;
@@ -62,7 +61,6 @@ import org.apache.catalina.util.SessionIdGeneratorBase;
 import org.apache.catalina.util.StandardSessionIdGenerator;
 import org.apache.catalina.valves.RemoteIpValve;
 import org.apache.catalina.valves.ValveBase;
-import org.apache.coyote.ActionCode;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 import org.apache.tomcat.util.ExceptionUtils;
@@ -597,15 +595,9 @@ public abstract class AuthenticatorBase extends ValveBase
             authRequired = true;
         }
 
-        if (!authRequired && context.getPreemptiveAuthentication()) {
-            authRequired =
-                    request.getCoyoteRequest().getMimeHeaders().getValue("authorization") != null;
-        }
-
         if (!authRequired && context.getPreemptiveAuthentication() &&
-                HttpServletRequest.CLIENT_CERT_AUTH.equals(getAuthMethod())) {
-            X509Certificate[] certs = getRequestCertificates(request);
-            authRequired = certs != null && certs.length > 0;
+                isPreemptiveAuthPossible(request)) {
+            authRequired = true;
         }
 
         JaspicState jaspicState = null;
@@ -863,35 +855,6 @@ public abstract class AuthenticatorBase extends ValveBase
 
 
     /**
-     * Look for the X509 certificate chain in the Request under the key
-     * <code>jakarta.servlet.request.X509Certificate</code>. If not found, trigger
-     * extracting the certificate chain from the Coyote request.
-     *
-     * @param request
-     *            Request to be processed
-     *
-     * @return The X509 certificate chain if found, <code>null</code> otherwise.
-     */
-    protected X509Certificate[] getRequestCertificates(final Request request)
-            throws IllegalStateException {
-
-        X509Certificate certs[] =
-                (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
-
-        if ((certs == null) || (certs.length < 1)) {
-            try {
-                request.getCoyoteRequest().action(ActionCode.REQ_SSL_CERTIFICATE, null);
-                certs = (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
-            } catch (IllegalStateException ise) {
-                // Request body was too large for save buffer
-                // Return null which will trigger an auth failure
-            }
-        }
-
-        return certs;
-    }
-
-    /**
      * Associate the specified single sign on identifier with the specified
      * Session.
      *
@@ -1387,6 +1350,7 @@ public abstract class AuthenticatorBase extends ValveBase
         super.startInternal();
     }
 
+
     /**
      * Stop this component and implement the requirements of
      * {@link org.apache.catalina.util.LifecycleBase#stopInternal()}.
@@ -1404,6 +1368,20 @@ public abstract class AuthenticatorBase extends ValveBase
     }
 
 
+    /**
+     * Can the authenticator perform preemptive authentication for the given
+     * request?
+     *
+     * @param request
+     *
+     * @return {@code true} if preemptive authentication is possible, otherwise
+     *         {@code false}
+     */
+    protected boolean isPreemptiveAuthPossible(Request request) {
+        return false;
+    }
+
+
     private AuthConfigProvider getJaspicProvider() {
         Optional<AuthConfigProvider> provider = jaspicProvider;
         if (provider == null) {
diff --git a/java/org/apache/catalina/authenticator/BasicAuthenticator.java b/java/org/apache/catalina/authenticator/BasicAuthenticator.java
index 55a7b88..a1f9c86 100644
--- a/java/org/apache/catalina/authenticator/BasicAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/BasicAuthenticator.java
@@ -127,12 +127,20 @@ public class BasicAuthenticator extends AuthenticatorBase {
 
     }
 
+
     @Override
     protected String getAuthMethod() {
         return HttpServletRequest.BASIC_AUTH;
     }
 
 
+    @Override
+    protected boolean isPreemptiveAuthPossible(Request request) {
+        MessageBytes authorizationHeader = request.getCoyoteRequest().getMimeHeaders().getValue("authorization");
+        return authorizationHeader != null && authorizationHeader.startsWithIgnoreCase("basic ", 0);
+    }
+
+
     /**
      * Parser for an HTTP Authorization header for BASIC authentication
      * as per RFC 2617 section 2, and the Base64 encoded credentials as
diff --git a/java/org/apache/catalina/authenticator/DigestAuthenticator.java b/java/org/apache/catalina/authenticator/DigestAuthenticator.java
index ee926f7..e2c4fab 100644
--- a/java/org/apache/catalina/authenticator/DigestAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/DigestAuthenticator.java
@@ -31,6 +31,7 @@ import org.apache.catalina.Realm;
 import org.apache.catalina.connector.Request;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.buf.MessageBytes;
 import org.apache.tomcat.util.http.parser.Authorization;
 import org.apache.tomcat.util.security.ConcurrentMessageDigest;
 import org.apache.tomcat.util.security.MD5Encoder;
@@ -367,6 +368,13 @@ public class DigestAuthenticator extends AuthenticatorBase {
     }
 
 
+    @Override
+    protected boolean isPreemptiveAuthPossible(Request request) {
+        MessageBytes authorizationHeader = request.getCoyoteRequest().getMimeHeaders().getValue("authorization");
+        return authorizationHeader != null && authorizationHeader.startsWithIgnoreCase("digest ", 0);
+    }
+
+
     // ------------------------------------------------------- Lifecycle Methods
 
     @Override
diff --git a/java/org/apache/catalina/authenticator/SSLAuthenticator.java b/java/org/apache/catalina/authenticator/SSLAuthenticator.java
index 90142cc..bb5ffcd 100644
--- a/java/org/apache/catalina/authenticator/SSLAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/SSLAuthenticator.java
@@ -23,7 +23,9 @@ import java.security.cert.X509Certificate;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
 
+import org.apache.catalina.Globals;
 import org.apache.catalina.connector.Request;
+import org.apache.coyote.ActionCode;
 
 /**
  * An <b>Authenticator</b> and <b>Valve</b> implementation of authentication
@@ -100,4 +102,41 @@ public class SSLAuthenticator extends AuthenticatorBase {
     protected String getAuthMethod() {
         return HttpServletRequest.CLIENT_CERT_AUTH;
     }
+
+
+    @Override
+    protected boolean isPreemptiveAuthPossible(Request request) {
+        X509Certificate[] certs = getRequestCertificates(request);
+        return certs != null && certs.length > 0;
+    }
+
+
+    /**
+     * Look for the X509 certificate chain in the Request under the key
+     * <code>jakarta.servlet.request.X509Certificate</code>. If not found, trigger
+     * extracting the certificate chain from the Coyote request.
+     *
+     * @param request
+     *            Request to be processed
+     *
+     * @return The X509 certificate chain if found, <code>null</code> otherwise.
+     */
+    protected X509Certificate[] getRequestCertificates(final Request request)
+            throws IllegalStateException {
+
+        X509Certificate certs[] =
+                (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
+
+        if ((certs == null) || (certs.length < 1)) {
+            try {
+                request.getCoyoteRequest().action(ActionCode.REQ_SSL_CERTIFICATE, null);
+                certs = (X509Certificate[]) request.getAttribute(Globals.CERTIFICATES_ATTR);
+            } catch (IllegalStateException ise) {
+                // Request body was too large for save buffer
+                // Return null which will trigger an auth failure
+            }
+        }
+
+        return certs;
+    }
 }
diff --git a/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java b/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java
index 8bb3710..da9434d 100644
--- a/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java
+++ b/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java
@@ -300,6 +300,13 @@ public class SpnegoAuthenticator extends AuthenticatorBase {
     }
 
 
+    @Override
+    protected boolean isPreemptiveAuthPossible(Request request) {
+        MessageBytes authorizationHeader = request.getCoyoteRequest().getMimeHeaders().getValue("authorization");
+        return authorizationHeader != null && authorizationHeader.startsWithIgnoreCase("negotiate ", 0);
+    }
+
+
     /**
      * This class gets a gss credential via a privileged action.
      */
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3823930..a814ef7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -127,6 +127,12 @@
         <code>false</code>. Fixed via pull request <pr>438</pr> provided by
         Robert Rodewald. (markt)
       </fix>
+      <scode>
+        Refactor the authenticators to delegate the check for preemptive
+        authentication to the individual authenticators where an authentication
+        scheme specific check can be performed. Based on pull request
+        <pr>444</pr> by Robert Rodewald. (markt)
+      </scode>
     </changelog>
   </subsection>
   <subsection name="Coyote">

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org