Skip to content

Commit

Permalink
[8.x] Fix lingering license warning header in IP filter (elastic#115510
Browse files Browse the repository at this point in the history
…) (elastic#115838)

* Fix lingering license warning header in IP filter (elastic#115510)

Fixes another place where we do not stash thread context
that causes the license warning header to persist in
the thread context across Netty worker threads.

Resolves elastic#114865

Relates to elastic#107573
  • Loading branch information
slobodanadamovic authored Oct 29, 2024
1 parent a1bfc99 commit a988c03
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 8 deletions.
6 changes: 6 additions & 0 deletions docs/changelog/115510.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 115510
summary: Fix lingering license warning header in IP filter
area: License
type: bug
issues:
- 114865
2 changes: 0 additions & 2 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,6 @@ tests:
- class: org.elasticsearch.xpack.remotecluster.RemoteClusterSecurityWithApmTracingRestIT
method: testTracingCrossCluster
issue: https://github.com/elastic/elasticsearch/issues/112731
- class: org.elasticsearch.license.LicensingTests
issue: https://github.com/elastic/elasticsearch/issues/114865
- class: org.elasticsearch.xpack.inference.TextEmbeddingCrudIT
method: testPutE5Small_withPlatformSpecificVariant
issue: https://github.com/elastic/elasticsearch/issues/113950
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.elasticsearch.xpack.security.LocalStateSecurity;
import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;

import java.nio.file.Files;
Expand Down Expand Up @@ -241,6 +242,7 @@ public void testNoWarningHeaderWhenAuthenticationFailed() throws Exception {
Header[] headers = null;
try {
getRestClient().performRequest(request);
Assert.fail("expected response exception");
} catch (ResponseException e) {
headers = e.getResponse().getHeaders();
List<String> afterWarningHeaders = getWarningHeaders(headers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.netty.channel.ChannelHandlerContext;
import io.netty.handler.ipfilter.AbstractRemoteAddressFilter;

import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.xpack.security.transport.filter.IPFilter;

import java.net.InetSocketAddress;
Expand All @@ -19,16 +20,21 @@ class IpFilterRemoteAddressFilter extends AbstractRemoteAddressFilter<InetSocket

private final IPFilter filter;
private final String profile;
private final ThreadContext threadContext;

IpFilterRemoteAddressFilter(final IPFilter filter, final String profile) {
IpFilterRemoteAddressFilter(final IPFilter filter, final String profile, final ThreadContext threadContext) {
this.filter = filter;
this.profile = profile;
this.threadContext = threadContext;
}

@Override
protected boolean accept(final ChannelHandlerContext ctx, final InetSocketAddress remoteAddress) throws Exception {
// at this stage no auth has happened, so we do not have any principal anyway
return filter.accept(profile, remoteAddress);
// this prevents thread-context changes to propagate beyond the channel accept test, as netty worker threads are reused
try (ThreadContext.StoredContext ignore = threadContext.newStoredContext()) {
return filter.accept(profile, remoteAddress);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ protected void initChannel(final Channel ch) throws Exception {

private void maybeAddIPFilter(final Channel ch, final String name) {
if (authenticator != null) {
ch.pipeline().addFirst("ipfilter", new IpFilterRemoteAddressFilter(authenticator, name));
ch.pipeline().addFirst("ipfilter", new IpFilterRemoteAddressFilter(authenticator, name, getThreadPool().getThreadContext()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.transport.BoundTransportAddress;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.http.HttpServerTransport;
import org.elasticsearch.license.MockLicenseState;
import org.elasticsearch.license.TestUtils;
Expand Down Expand Up @@ -90,10 +91,11 @@ public void init() throws Exception {
ipFilter.setBoundHttpTransportAddress(httpTransport.boundAddress());
}

ThreadContext threadContext = new ThreadContext(Settings.EMPTY);
if (isHttpEnabled) {
handler = new IpFilterRemoteAddressFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME);
handler = new IpFilterRemoteAddressFilter(ipFilter, IPFilter.HTTP_PROFILE_NAME, threadContext);
} else {
handler = new IpFilterRemoteAddressFilter(ipFilter, "default");
handler = new IpFilterRemoteAddressFilter(ipFilter, "default", threadContext);
}
}

Expand All @@ -106,7 +108,11 @@ public void testThatFilteringWorksByIp() throws Exception {
}

public void testFilteringWorksForRemoteClusterPort() throws Exception {
handler = new IpFilterRemoteAddressFilter(ipFilter, RemoteClusterPortSettings.REMOTE_CLUSTER_PROFILE);
handler = new IpFilterRemoteAddressFilter(
ipFilter,
RemoteClusterPortSettings.REMOTE_CLUSTER_PROFILE,
new ThreadContext(Settings.EMPTY)
);
InetSocketAddress localhostAddr = new InetSocketAddress(InetAddresses.forString("127.0.0.1"), 12345);
assertThat(handler.accept(mock(ChannelHandlerContext.class), localhostAddr), is(true));

Expand Down

0 comments on commit a988c03

Please sign in to comment.