diff --git a/changelog/unreleased/SOLR-18048.yml b/changelog/unreleased/SOLR-18048.yml new file mode 100644 index 000000000000..834d1905d7e9 --- /dev/null +++ b/changelog/unreleased/SOLR-18048.yml @@ -0,0 +1,8 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: Authentication checks have been moved to a Servlet Filter +type: other # added, changed, fixed, deprecated, removed, dependency_update, security, other +authors: + - name: Gus Heck +links: + - name: SOLR-18048 + url: https://issues.apache.org/jira/browse/SOLR-18048 diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 50dd516f5e39..6dce8abb90c0 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -39,6 +39,7 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.Tracer; import jakarta.inject.Singleton; +import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.nio.file.Files; @@ -147,6 +148,7 @@ import org.apache.solr.search.SolrFieldCacheBean; import org.apache.solr.search.SolrIndexSearcher; import org.apache.solr.security.AllowListUrlChecker; +import org.apache.solr.security.AuditEvent; import org.apache.solr.security.AuditLoggerPlugin; import org.apache.solr.security.AuthenticationPlugin; import org.apache.solr.security.AuthorizationPlugin; @@ -2510,4 +2512,29 @@ public void addKeyVal(Object map, Object key, Object val) throws IOException { } }); } + + /** + * Check if audit logging is enabled and should happen for given event type + * + * @param eventType the audit event + */ + public boolean shouldAudit(AuditEvent.EventType eventType) { + return this.getAuditLoggerPlugin() != null && this.getAuditLoggerPlugin().shouldLog(eventType); + } + + /** + * Audit an event + * + * @param type The type of event to audit. + * @param req The request being audited + */ + public void audit(AuditEvent.EventType type, HttpServletRequest req) { + if (shouldAudit(type)) { + getAuditLoggerPlugin().doAudit(new AuditEvent(type, req)); + } + } + + public boolean isAuthEnabled() { + return getAuthenticationPlugin() != null; + } } diff --git a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java index 99c1b091eaca..aaa24087ff8e 100644 --- a/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java +++ b/solr/core/src/java/org/apache/solr/security/AuthenticationPlugin.java @@ -69,8 +69,8 @@ public abstract class AuthenticationPlugin implements SolrInfoBean { * @param request the http request * @param response the http response * @param filterChain the servlet filter chain - * @return false if the request not be processed by Solr (not continue), i.e. the response and - * status code have already been sent. + * @return false if the request should not be processed by Solr (not continue), i.e. the response + * and status code have already been sent. * @throws Exception any exception thrown during the authentication, e.g. * PrivilegedActionException */ @@ -79,7 +79,7 @@ public abstract boolean doAuthenticate( throws Exception; /** - * This method is called by SolrDispatchFilter in order to initiate authentication. It does some + * This method is called by AuthenticationFilter in order to initiate authentication. It does some * standard metrics counting. */ public final boolean authenticate( diff --git a/solr/core/src/java/org/apache/solr/servlet/AuthenticationFilter.java b/solr/core/src/java/org/apache/solr/servlet/AuthenticationFilter.java new file mode 100644 index 000000000000..e8637ad9144b --- /dev/null +++ b/solr/core/src/java/org/apache/solr/servlet/AuthenticationFilter.java @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.solr.servlet; + +import static org.apache.solr.common.SolrException.ErrorCode.SERVER_ERROR; +import static org.apache.solr.security.AuditEvent.EventType.ANONYMOUS; +import static org.apache.solr.security.AuditEvent.EventType.AUTHENTICATED; +import static org.apache.solr.security.AuditEvent.EventType.REJECTED; + +import io.opentelemetry.api.trace.Span; +import jakarta.servlet.FilterChain; +import jakarta.servlet.FilterConfig; +import jakarta.servlet.ServletException; +import jakarta.servlet.ServletRequest; +import jakarta.servlet.ServletResponse; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import org.apache.solr.common.SolrException; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.security.AuthenticationPlugin; +import org.apache.solr.security.PKIAuthenticationPlugin; +import org.apache.solr.security.PublicKeyHandler; +import org.apache.solr.util.tracing.TraceUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class AuthenticationFilter extends CoreContainerAwareHttpFilter { + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + public static final String PLUGIN_DID_NOT_SET_ERROR_STATUS = + "{} threw SolrAuthenticationException without setting request status >= 400"; + + @Override + public void init(FilterConfig config) throws ServletException { + super.init(config); + } + + @Override + protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterChain chain) + throws IOException, ServletException { + + CoreContainer cc = getCores(); + AuthenticationPlugin authenticationPlugin = cc.getAuthenticationPlugin(); + String requestPath = ServletUtils.getPathAfterContext(req); + + ///////////////////////////////////////////////////////// + //////// Check cases where auth is not required. //////// + ///////////////////////////////////////////////////////// + + // Is authentication configured? + if (authenticationPlugin == null) { + cc.audit(ANONYMOUS, req); + chain.doFilter(req, res); + return; + } + + // /admin/info/key must be always open. see SOLR-9188 + if (PublicKeyHandler.PATH.equals(requestPath)) { + log.debug("Pass through PKI authentication endpoint"); + chain.doFilter(req, res); + return; + } + + if (isAdminUI(requestPath)) { + log.debug("Pass through Admin UI entry point"); + chain.doFilter(req, res); + return; + } + + ///////////////////////////////////////////////////////////////// + //////// if we make it here, authentication is required. //////// + ///////////////////////////////////////////////////////////////// + + // internode (internal) requests have their own PKI auth plugin + if (isInternodePKI(req, cc)) { + authenticationPlugin = cc.getPkiAuthenticationSecurityBuilder(); + } + + boolean authSuccess = false; + try { + authSuccess = + authenticate(req, res, new AuditSuccessChainWrapper(cc, chain), authenticationPlugin); + } finally { + if (!authSuccess) { + cc.audit(REJECTED, req); + res.flushBuffer(); + if (res.getStatus() < 400) { + log.error(PLUGIN_DID_NOT_SET_ERROR_STATUS, authenticationPlugin.getClass()); + res.sendError(401, "Authentication Plugin rejected credentials."); + } + } + } + } + + /** + * The plugin called by this method will call doFilter(req, res) for us (which is why we pass in + * the filter chain object as a parameter). + */ + private boolean authenticate( + HttpServletRequest req, + HttpServletResponse res, + FilterChain chain, + AuthenticationPlugin authenticationPlugin) { + try { + + // It is imperative that authentication plugins obey the contract in the javadoc for + // org.apache.solr.security.AuthenticationPlugin.doAuthenticate, and either return + // false or throw an exception if they cannot validate the user's credentials. + // Any plugin that doesn't do this is broken and should be fixed. + + logAuthAttempt(req); + return authenticationPlugin.authenticate(req, res, chain); + } catch (Exception e) { + log.info("Error authenticating", e); + throw new SolrException(SERVER_ERROR, "Error during request authentication, ", e); + } + } + + private static boolean isAdminUI(String requestPath) { + return "/solr/".equals(requestPath) || "/".equals(requestPath); + } + + private boolean isInternodePKI(HttpServletRequest req, CoreContainer cores) { + String header = req.getHeader(PKIAuthenticationPlugin.HEADER); + String headerV2 = req.getHeader(PKIAuthenticationPlugin.HEADER_V2); + return (header != null || headerV2 != null) + && cores.getPkiAuthenticationSecurityBuilder() != null; + } + + private void logAuthAttempt(HttpServletRequest req) { + // moved this to a method because spotless formatting is so horrible, and makes the log + // message look like a big deal... but it's just taking up space + if (log.isDebugEnabled()) { + log.debug( + "Request to authenticate: {}, domain: {}, port: {}", + req, + req.getLocalName(), + req.getLocalPort()); + } + } + + + + private record AuditSuccessChainWrapper(CoreContainer cc, FilterChain chain) + implements FilterChain { + + @Override + public void doFilter(ServletRequest rq, ServletResponse rsp) + throws IOException, ServletException { + // this is a hack. The authentication plugin should accept a callback + // to be executed before doFilter is called if authentication succeeds + cc.audit(AUTHENTICATED, (HttpServletRequest) rq); + Span span = TraceUtils.getSpan((HttpServletRequest) rq); + setPrincipalForTracing((HttpServletRequest) rq, span); + chain.doFilter(rq, rsp); + } + + private void setPrincipalForTracing(HttpServletRequest request, Span span) { + if (log.isDebugEnabled()) { + log.debug("User principal: {}", request.getUserPrincipal()); + } + final String principalName; + if (request.getUserPrincipal() != null) { + principalName = request.getUserPrincipal().getName(); + } else { + principalName = null; + } + TraceUtils.setUser(span, String.valueOf(principalName)); + } + } +} diff --git a/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java b/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java index 50ec9decdc91..d824c5427239 100644 --- a/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/RequiredSolrRequestFilter.java @@ -16,6 +16,8 @@ */ package org.apache.solr.servlet; +import static org.apache.solr.servlet.ServletUtils.closeShield; + import jakarta.servlet.FilterChain; import jakarta.servlet.ServletException; import jakarta.servlet.http.HttpServletRequest; @@ -72,7 +74,9 @@ protected void doFilter(HttpServletRequest req, HttpServletResponse res, FilterC // put the core container in request attribute // This is required for the LoadAdminUiServlet class. Removing it will cause 404 req.setAttribute(CORE_CONTAINER_REQUEST_ATTRIBUTE, getCores()); - chain.doFilter(req, res); + + // we want to prevent any attempts to close our request or response prematurely + chain.doFilter(closeShield(req), closeShield(res)); } finally { // cleanups for above stuff MDCLoggingContext.reset(); diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java b/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java deleted file mode 100644 index 04db031bb067..000000000000 --- a/solr/core/src/java/org/apache/solr/servlet/SolrAuthenticationException.java +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.solr.servlet; - -public class SolrAuthenticationException extends Exception {} diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index 0d0bef9b6ce1..6530183f7202 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -16,9 +16,9 @@ */ package org.apache.solr.servlet; -import static org.apache.solr.servlet.ServletUtils.closeShield; import static org.apache.solr.util.tracing.TraceUtils.getSpan; +import io.opentelemetry.api.trace.Span; import jakarta.servlet.FilterChain; import jakarta.servlet.FilterConfig; import jakarta.servlet.ServletException; @@ -27,8 +27,6 @@ import jakarta.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; import org.apache.solr.api.V2HttpCall; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; @@ -36,11 +34,6 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.core.NodeRoles; import org.apache.solr.handler.api.V2ApiUtils; -import org.apache.solr.security.AuditEvent; -import org.apache.solr.security.AuditEvent.EventType; -import org.apache.solr.security.AuthenticationPlugin; -import org.apache.solr.security.PKIAuthenticationPlugin; -import org.apache.solr.security.PublicKeyHandler; import org.apache.solr.util.tracing.TraceUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -125,7 +118,7 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F throws IOException, ServletException { // internal version of doFilter that tracks if we are in a retry - dispatch(chain, closeShield(request), closeShield(response), false); + dispatch(chain, request, response, false); } /* @@ -143,41 +136,13 @@ public void doFilter(HttpServletRequest request, HttpServletResponse response, F private void dispatch( FilterChain chain, HttpServletRequest request, HttpServletResponse response, boolean retry) throws IOException, ServletException { - - AtomicReference wrappedRequest = new AtomicReference<>(); - try { - authenticateRequest(request, response, wrappedRequest); - } catch (SolrAuthenticationException e) { - // it seems our auth system expects the plugin to set the status on the request. - // If this hasn't happened make sure it does happen now rather than throwing an - // exception, that formerly went on to be ignored in - // org.apache.solr.servlet.ServletUtils.traceHttpRequestExecution2 - if (response.getStatus() < 400) { - log.error( - "Authentication Plugin threw SolrAuthenticationException without setting request status >= 400"); - response.sendError(401, "Authentication Plugin rejected credentials."); - } - return; // Nothing more to do, chain.doFilter(req,res) doesn't get called. - } - if (wrappedRequest.get() != null) { - request = wrappedRequest.get(); - } - var span = getSpan(request); - if (getCores().getAuthenticationPlugin() != null) { - if (log.isDebugEnabled()) { - log.debug("User principal: {}", request.getUserPrincipal()); - } - final String principalName; - if (request.getUserPrincipal() != null) { - principalName = request.getUserPrincipal().getName(); - } else { - principalName = null; - } - TraceUtils.setUser(span, String.valueOf(principalName)); - } - HttpSolrCall call = getHttpSolrCall(request, response, retry); + + // this flag LOOKS like it should be in RequiredSolrRequestFilter, but + // the value set here is drives PKIAuthenticationPlugin.isSolrThread + // which gets used BEFORE this is set for some reason. + // BUG? Important timing? ExecutorUtil.setServerThreadFlag(Boolean.TRUE); try { Action result = call.call(); @@ -208,6 +173,8 @@ private void dispatch( } } + + /** * Allow a subclass to modify the HttpSolrCall. In particular, subclasses may want to add * attributes to the request and send errors differently @@ -225,118 +192,6 @@ protected HttpSolrCall getHttpSolrCall( return solrCallFactory.createInstance(this, path, cores, request, response, retry); } - // TODO: make this a servlet filter - private void authenticateRequest( - HttpServletRequest request, - HttpServletResponse response, - final AtomicReference wrappedRequest) - throws IOException, SolrAuthenticationException { - boolean requestContinues; - final AtomicBoolean isAuthenticated = new AtomicBoolean(false); - CoreContainer cores; - try { - cores = getCores(); - } catch (UnavailableException e) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); - } - AuthenticationPlugin authenticationPlugin = cores.getAuthenticationPlugin(); - if (authenticationPlugin == null) { - if (shouldAudit(EventType.ANONYMOUS)) { - cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.ANONYMOUS, request)); - } - return; - } else { - // /admin/info/key must be always open. see SOLR-9188 - String requestPath = ServletUtils.getPathAfterContext(request); - if (PublicKeyHandler.PATH.equals(requestPath)) { - log.debug("Pass through PKI authentication endpoint"); - return; - } - // /solr/ (Admin UI) must be always open to allow displaying Admin UI with login page - if ("/solr/".equals(requestPath) || "/".equals(requestPath)) { - log.debug("Pass through Admin UI entry point"); - return; - } - String header = request.getHeader(PKIAuthenticationPlugin.HEADER); - String headerV2 = request.getHeader(PKIAuthenticationPlugin.HEADER_V2); - if ((header != null || headerV2 != null) - && cores.getPkiAuthenticationSecurityBuilder() != null) - authenticationPlugin = cores.getPkiAuthenticationSecurityBuilder(); - try { - if (log.isDebugEnabled()) { - log.debug( - "Request to authenticate: {}, domain: {}, port: {}", - request, - request.getLocalName(), - request.getLocalPort()); - } - // For legacy reasons, upon successful authentication this wants to call the chain's next - // filter, which obfuscates the layout of the code since one usually expects to be able to - // find the call to doFilter() in the implementation of jakarta.servlet.Filter. Supplying a - // trivial impl here to keep existing code happy while making the flow clearer. Chain will - // be called after this method completes. Eventually auth all moves to its own filter - // (hopefully). Most auth plugins simply return true after calling this anyway, so they - // obviously don't care. - // - // The Hadoop Auth Plugin was removed in SOLR-17540, however leaving the below reference - // for future readers, as there may be an option to simplify this logic. - // - // Kerberos plugins seem to mostly use it to satisfy the api of a - // wrapped instance of javax.servlet.Filter and neither of those seem to be doing anything - // fancy with the filter chain, so this would seem to be a hack brought on by the fact that - // our auth code has been forced to be code within dispatch filter, rather than being a - // filter itself. The HadoopAuthPlugin has a suspicious amount of code after the call to - // doFilter() which seems to imply that anything in this chain can get executed before - // authentication completes, and I can't figure out how that's a good idea in the first - // place. - requestContinues = - authenticationPlugin.authenticate( - request, - response, - (req, rsp) -> { - isAuthenticated.set(true); - wrappedRequest.set((HttpServletRequest) req); - }); - } catch (Exception e) { - log.info("Error authenticating", e); - throw new SolrException(ErrorCode.SERVER_ERROR, "Error during request authentication, ", e); - } - } - // requestContinues is an optional short circuit, thus we still need to check isAuthenticated. - // This is because the AuthenticationPlugin doesn't always have enough information to determine - // if it should short circuit, e.g. the Kerberos Authentication Filter will send an error and - // not call later filters in chain, but doesn't throw an exception. We could force each Plugin - // to implement isAuthenticated to simplify the check here, but that just moves the complexity - // to multiple code paths. - if (!requestContinues || !isAuthenticated.get()) { - response.flushBuffer(); - if (shouldAudit(EventType.REJECTED)) { - cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.REJECTED, request)); - } - throw new SolrAuthenticationException(); - } - if (shouldAudit(EventType.AUTHENTICATED)) { - cores.getAuditLoggerPlugin().doAudit(new AuditEvent(EventType.AUTHENTICATED, request)); - } - // Auth Success - } - - /** - * Check if audit logging is enabled and should happen for given event type - * - * @param eventType the audit event - */ - private boolean shouldAudit(AuditEvent.EventType eventType) { - CoreContainer cores; - try { - cores = getCores(); - } catch (UnavailableException e) { - throw new SolrException(ErrorCode.SERVER_ERROR, "Core Container Unavailable"); - } - return cores.getAuditLoggerPlugin() != null - && cores.getAuditLoggerPlugin().shouldLog(eventType); - } - /** internal API */ public interface HttpSolrCallFactory { default HttpSolrCall createInstance( diff --git a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java index e4224e44e6c8..7bf21d549641 100644 --- a/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java +++ b/solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java @@ -60,6 +60,7 @@ import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; import org.apache.solr.metrics.SolrMetricManager; +import org.apache.solr.servlet.AuthenticationFilter; import org.apache.solr.servlet.CoreContainerProvider; import org.apache.solr.servlet.PathExclusionFilter; import org.apache.solr.servlet.RateLimitFilter; @@ -117,6 +118,7 @@ public class JettySolrRunner { volatile FilterHolder pathExcludeFilter; volatile FilterHolder requiredFilter; volatile FilterHolder rateLimitFilter; + volatile FilterHolder authFilter; volatile FilterHolder dispatchFilter; private FilterHolder tracingFilter; @@ -423,10 +425,14 @@ public void contextInitialized(ServletContextEvent event) { rateLimitFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); rateLimitFilter.setHeldClass(RateLimitFilter.class); - // Ratelimit Requests + // Trace Requests tracingFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); tracingFilter.setHeldClass(TracingFilter.class); + // Authenticate Requests + authFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); + authFilter.setHeldClass(AuthenticationFilter.class); + // This is our main workhorse dispatchFilter = root.getServletHandler().newFilterHolder(Source.EMBEDDED); dispatchFilter.setHeldClass(SolrDispatchFilter.class); @@ -436,6 +442,7 @@ public void contextInitialized(ServletContextEvent event) { root.addFilter(requiredFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); root.addFilter(rateLimitFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); root.addFilter(tracingFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); + root.addFilter(authFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); root.addFilter(dispatchFilter, "/*", EnumSet.of(DispatcherType.REQUEST)); // Default servlet as a fall-through diff --git a/solr/webapp/web/WEB-INF/web.xml b/solr/webapp/web/WEB-INF/web.xml index 6732a6cb58a4..55d3da4ec2f8 100644 --- a/solr/webapp/web/WEB-INF/web.xml +++ b/solr/webapp/web/WEB-INF/web.xml @@ -74,6 +74,15 @@ /* + + AuthenticationFilter + org.apache.solr.servlet.AuthenticationFilter + + + + AuthenticationFilter + /* + SolrRequestFilter org.apache.solr.servlet.SolrDispatchFilter