diff --git a/src/main/java/org/opensearch/security/filter/NettyRequest.java b/src/main/java/org/opensearch/security/filter/NettyRequest.java index d26f397623..f3f2827367 100644 --- a/src/main/java/org/opensearch/security/filter/NettyRequest.java +++ b/src/main/java/org/opensearch/security/filter/NettyRequest.java @@ -12,8 +12,6 @@ package org.opensearch.security.filter; import java.net.InetSocketAddress; -import java.net.MalformedURLException; -import java.net.URL; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -64,11 +62,8 @@ public SSLEngine getSSLEngine() { @Override public String path() { - try { - return new URL(underlyingRequest.uri()).getPath(); - } catch (final MalformedURLException e) { - return ""; - } + String rawPath = SecurityRestUtils.path(underlyingRequest.uri()); + return RestUtils.decodeComponent(rawPath); } @Override diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java b/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java new file mode 100644 index 0000000000..1599346b90 --- /dev/null +++ b/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java @@ -0,0 +1,12 @@ +package org.opensearch.security.filter; + +public class SecurityRestUtils { + public static String path(final String uri) { + final int index = uri.indexOf('?'); + if (index >= 0) { + return uri.substring(0, index); + } else { + return uri; + } + } +} diff --git a/src/main/java/org/opensearch/security/http/XFFResolver.java b/src/main/java/org/opensearch/security/http/XFFResolver.java index e9ad412831..64c5f4b60c 100644 --- a/src/main/java/org/opensearch/security/http/XFFResolver.java +++ b/src/main/java/org/opensearch/security/http/XFFResolver.java @@ -34,11 +34,8 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.core.common.transport.TransportAddress; -import org.opensearch.http.netty4.Netty4HttpChannel; -import org.opensearch.rest.RestRequest; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.filter.SecurityRequest; -import org.opensearch.security.filter.OpenSearchRequest; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; @@ -61,15 +58,7 @@ public TransportAddress resolve(final SecurityRequest request) throws OpenSearch log.trace("resolve {}", request.getRemoteAddress().orElse(null)); } - boolean requestFromNetty = false; - if (request instanceof OpenSearchRequest) { - final OpenSearchRequest securityRequestChannel = (OpenSearchRequest) request; - final RestRequest restRequest = securityRequestChannel.breakEncapsulationForRequest(); - - requestFromNetty = restRequest.getHttpChannel() instanceof Netty4HttpChannel; - } - - if (enabled && request.getRemoteAddress().isPresent() && requestFromNetty) { + if (enabled && request.getRemoteAddress().isPresent()) { final InetSocketAddress remoteAddress = request.getRemoteAddress().get(); final InetSocketAddress isa = new InetSocketAddress(detector.detect(request, threadContext), remoteAddress.getPort()); diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java index b7c0aedebd..5e8900cb00 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java @@ -20,11 +20,13 @@ import io.netty.channel.ChannelHandler.Sharable; import io.netty.channel.ChannelHandlerContext; import org.opensearch.http.netty4.Netty4HttpChannel; +import org.opensearch.rest.RestUtils; import org.opensearch.security.filter.SecurityRequestChannel; import org.opensearch.security.filter.SecurityRequestChannelUnsupported; import org.opensearch.security.filter.SecurityRequestFactory; import org.opensearch.security.filter.SecurityResponse; import org.opensearch.security.filter.SecurityRestFilter; +import org.opensearch.security.filter.SecurityRestUtils; import org.opensearch.security.ssl.transport.SSLConfig; import org.opensearch.threadpool.ThreadPool; import org.opensearch.security.support.ConfigConstants; @@ -83,7 +85,9 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro // TODO: GET PROPER MAVEN BUILD // final Netty4HttpChannel httpChannel = ctx.channel().attr(Netty4HttpServerTransport.HTTP_CHANNEL_KEY).get(); final Netty4HttpChannel httpChannel = ctx.channel().attr(AttributeKey.valueOf("opensearch-http-channel")).get(); - Matcher matcher = PATTERN_PATH_PREFIX.matcher(msg.uri()); + String rawPath = SecurityRestUtils.path(msg.uri()); + String path = RestUtils.decodeComponent(rawPath); + Matcher matcher = PATTERN_PATH_PREFIX.matcher(path); final String suffix = matcher.matches() ? matcher.group(2) : null; if (API_AUTHTOKEN_SUFFIX.equals(suffix)) { // TODO: I think this is going to create problems - we should have a sensible size limit, not prevention of @@ -96,20 +100,26 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro ThreadContext threadContext = threadPool.getThreadContext(); try (ThreadContext.StoredContext ignore = threadPool.getThreadContext().stashContext()) { injectUser(msg, threadContext); - // If request channel is completed and a response is sent, then there was a failure during authentication - restFilter.checkAndAuthenticateRequest(requestChannel); + + boolean shouldSkipAuthentication = HttpMethod.OPTIONS.equals(msg.method()) + || HEALTH_SUFFIX.equals(suffix) + || WHO_AM_I_SUFFIX.equals(suffix); + + if (!shouldSkipAuthentication) { + // If request channel is completed and a response is sent, then there was a failure during authentication + restFilter.checkAndAuthenticateRequest(requestChannel); + } ThreadContext.StoredContext contextToRestore = threadPool.getThreadContext().newStoredContext(false); ctx.channel().attr(CONTEXT_TO_RESTORE).set(contextToRestore); requestChannel.getQueuedResponse().ifPresent(response -> ctx.channel().attr(EARLY_RESPONSE).set(response)); - if (requestChannel.getQueuedResponse().isEmpty() - && !HttpMethod.OPTIONS.equals(msg.method()) - && !HEALTH_SUFFIX.equals(suffix) - && !WHO_AM_I_SUFFIX.equals(suffix)) { + boolean shouldDecompress = !shouldSkipAuthentication && requestChannel.getQueuedResponse().isEmpty(); + + if (requestChannel.getQueuedResponse().isEmpty() || shouldSkipAuthentication) { // Only allow decompression on authenticated requests that also aren't one of those ^ - ctx.channel().attr(SHOULD_DECOMPRESS).set(Boolean.TRUE); + ctx.channel().attr(SHOULD_DECOMPRESS).set(Boolean.valueOf(shouldDecompress)); ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.TRUE); } } catch (final OpenSearchSecurityException e) {