This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit ce11441019fd91d172d2ce36b0f5cdf91d9846ce Author: Quan Tran <[email protected]> AuthorDate: Tue Dec 26 16:56:06 2023 +0700 JAMES 3897: Integration test for CrowdsecImapConnectionCheck - Better handle Crowdsec timeout: wrong CrowdSec endpoint could lead to hanging CrowdSec connection check - Better log when James refuse connection from an IP - Better reuse CrowdsecHttpClient object in CrowdsecImapConnectionCheck. Do not create a new client every check - Stay away from evil regression `remoteAddress.getHostName` - Only perform connection check if needed. Most of the case it would be empty connection check therefore no need Flux manipulation. - Set up Integration test for CrowdSec: Plug James log file to CrowdSec, then let CrowdSec and James do their work. --- .../james/imap/api/ConnectionCheckFactory.java | 5 +- .../protocols/ConnectionCheckFactoryImpl.java | 11 +-- .../james/modules/protocols/ImapGuiceProbe.java | 2 +- .../imapserver/netty/HAProxyMessageHandler.java | 12 ++- .../james/imapserver/netty/IMAPServerFactory.java | 10 +-- .../netty/ImapChannelUpstreamHandler.java | 15 +++- .../james/imapserver/netty/IpConnectionCheck.java | 7 +- .../apache/james/CrowdsecImapConnectionCheck.java | 21 ++++- .../org/apache/james/model/CrowdsecHttpClient.java | 4 +- .../java/org/apache/james/CrowdsecContract.java | 38 --------- .../org/apache/james/CrowdsecEhloHookTest.java | 7 -- .../org/apache/james/CrowdsecHttpClientTest.java | 6 -- .../james/CrowdsecImapConnectionCheckTest.java | 6 -- .../org/apache/james/CrowdsecIntegrationTest.java | 90 +++++++++++++++++++++- .../java/org/apache/james/HAProxyExtension.java | 7 +- .../java/org/apache/james/MemoryCrowdsecTest.java | 58 -------------- .../crowdsec/src/test/resources/haproxy.cfg | 21 +++++ .../crowdsec/src/test/resources/imapserver.xml | 4 + .../crowdsec/src/test/resources/logback.xml | 48 ------------ 19 files changed, 177 insertions(+), 195 deletions(-) diff --git a/protocols/imap/src/main/java/org/apache/james/imap/api/ConnectionCheckFactory.java b/protocols/imap/src/main/java/org/apache/james/imap/api/ConnectionCheckFactory.java index aa5dcded5c..8c7c2945a0 100644 --- a/protocols/imap/src/main/java/org/apache/james/imap/api/ConnectionCheckFactory.java +++ b/protocols/imap/src/main/java/org/apache/james/imap/api/ConnectionCheckFactory.java @@ -21,7 +21,10 @@ package org.apache.james.imap.api; import java.util.Set; +import org.apache.commons.configuration2.HierarchicalConfiguration; +import org.apache.commons.configuration2.tree.ImmutableNode; + @FunctionalInterface public interface ConnectionCheckFactory { - Set<ConnectionCheck> create(ImapConfiguration imapConfiguration); + Set<ConnectionCheck> create(HierarchicalConfiguration<ImmutableNode> config); } diff --git a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ConnectionCheckFactoryImpl.java b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ConnectionCheckFactoryImpl.java index 99cecbd97c..8ceec84019 100644 --- a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ConnectionCheckFactoryImpl.java +++ b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ConnectionCheckFactoryImpl.java @@ -19,14 +19,15 @@ package org.apache.james.modules.protocols; -import java.util.Optional; +import java.util.Arrays; import java.util.Set; import javax.inject.Inject; +import org.apache.commons.configuration2.HierarchicalConfiguration; +import org.apache.commons.configuration2.tree.ImmutableNode; import org.apache.james.imap.api.ConnectionCheck; import org.apache.james.imap.api.ConnectionCheckFactory; -import org.apache.james.imap.api.ImapConfiguration; import org.apache.james.utils.ClassName; import org.apache.james.utils.GuiceGenericLoader; @@ -42,9 +43,9 @@ public class ConnectionCheckFactoryImpl implements ConnectionCheckFactory { } @Override - public Set<ConnectionCheck> create(ImapConfiguration imapConfiguration) { - return Optional.ofNullable(imapConfiguration.getAdditionalConnectionChecks()) - .orElse(ImmutableSet.of()) + public Set<ConnectionCheck> create(HierarchicalConfiguration<ImmutableNode> config) { + return Arrays.stream(config.getStringArray("additionalConnectionChecks")) + .collect(ImmutableSet.toImmutableSet()) .stream() .map(ClassName::new) .map(Throwing.function(loader::instantiate)) diff --git a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java index 5be2ebcae8..bbfcd71d34 100644 --- a/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java +++ b/server/container/guice/protocols/imap/src/main/java/org/apache/james/modules/protocols/ImapGuiceProbe.java @@ -59,7 +59,7 @@ public class ImapGuiceProbe implements GuiceProbe { .orElseThrow(() -> new IllegalStateException("IMAPS server not defined")); } - private Optional<Integer> getPort(Predicate<? super AbstractConfigurableAsyncServer> filter) { + public Optional<Integer> getPort(Predicate<? super AbstractConfigurableAsyncServer> filter) { return imapServerFactory.getServers().stream() .filter(filter) .findFirst() diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java index 3308f6caf0..318ba5429b 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/HAProxyMessageHandler.java @@ -69,7 +69,8 @@ public class HAProxyMessageHandler extends ChannelInboundHandlerAdapter { LOGGER.info("Connection from {} runs through {} proxy", haproxyMsg.sourceAddress(), haproxyMsg.destinationAddress()); // Refresh MDC info to account for proxying MDCBuilder boundMDC = IMAPMDCContext.boundMDC(ctx); - Flux.fromIterable(connectionChecks).concatMap(connectionCheck -> connectionCheck.validate(sourceIP)).then().block(); + + performConnectionCheck(sourceIP); if (imapSession != null) { imapSession.setAttribute(MDC_KEY, boundMDC); @@ -84,4 +85,13 @@ public class HAProxyMessageHandler extends ChannelInboundHandlerAdapter { super.channelRead(ctx, msg); } } + + private void performConnectionCheck(InetSocketAddress realClientIp) { + if (!connectionChecks.isEmpty()) { + Flux.fromIterable(connectionChecks) + .concatMap(connectionCheck -> connectionCheck.validate(realClientIp)) + .then() + .block(); + } + } } diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServerFactory.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServerFactory.java index 3d0a2ad488..18a87b5aae 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServerFactory.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServerFactory.java @@ -19,7 +19,6 @@ package org.apache.james.imapserver.netty; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import javax.inject.Inject; @@ -29,7 +28,6 @@ import org.apache.commons.configuration2.tree.ImmutableNode; import org.apache.james.filesystem.api.FileSystem; import org.apache.james.imap.ImapSuite; import org.apache.james.imap.api.ConnectionCheckFactory; -import org.apache.james.imap.api.ImapConfiguration; import org.apache.james.imap.api.process.ImapProcessor; import org.apache.james.imap.decode.ImapDecoder; import org.apache.james.imap.encode.ImapEncoder; @@ -39,7 +37,6 @@ import org.apache.james.protocols.lib.netty.AbstractConfigurableAsyncServer; import org.apache.james.protocols.lib.netty.AbstractServerFactory; import com.github.fge.lambdas.functions.ThrowingFunction; -import com.google.common.collect.ImmutableSet; public class IMAPServerFactory extends AbstractServerFactory { @@ -71,11 +68,8 @@ public class IMAPServerFactory extends AbstractServerFactory { protected IMAPServer createServer(HierarchicalConfiguration<ImmutableNode> config) { ImapSuite imapSuite = imapSuiteProvider.apply(config); - ImmutableSet<String> connectionChecks = Arrays.stream(config.getStringArray("additionalConnectionChecks")).collect(ImmutableSet.toImmutableSet()); - return new IMAPServer(imapSuite.getDecoder(), imapSuite.getEncoder(), imapSuite.getProcessor(), imapMetrics, gaugeRegistry, connectionCheckFactory.create(ImapConfiguration.builder() - .connectionChecks(connectionChecks) - .build())); + return new IMAPServer(imapSuite.getDecoder(), imapSuite.getEncoder(), imapSuite.getProcessor(), imapMetrics, gaugeRegistry, connectionCheckFactory.create(config)); } @Override @@ -91,7 +85,7 @@ public class IMAPServerFactory extends AbstractServerFactory { } return servers; - + } } diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java index 32e66a7f6b..40a3aab57e 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/ImapChannelUpstreamHandler.java @@ -197,10 +197,10 @@ public class ImapChannelUpstreamHandler extends ChannelInboundHandlerAdapter imp ctx.channel().attr(LINEARALIZER_ATTRIBUTE_KEY).set(new Linearalizer()); MDCBuilder boundMDC = IMAPMDCContext.boundMDC(ctx) .addToContext(MDCBuilder.SESSION_ID, sessionId.asString()); - if (!proxyRequired) { - Flux.fromIterable(connectionChecks).concatMap(connectionCheck -> connectionCheck.validate(imapsession.getRemoteAddress())).then().block(); - } imapsession.setAttribute(MDC_KEY, boundMDC); + + performConnectionCheck(imapsession.getRemoteAddress()); + try (Closeable closeable = mdc(imapsession).build()) { InetSocketAddress address = (InetSocketAddress) ctx.channel().remoteAddress(); LOGGER.info("Connection established from {}", address.getAddress().getHostAddress()); @@ -216,6 +216,15 @@ public class ImapChannelUpstreamHandler extends ChannelInboundHandlerAdapter imp } + private void performConnectionCheck(InetSocketAddress clientIp) { + if (!connectionChecks.isEmpty() && !proxyRequired) { + Flux.fromIterable(connectionChecks) + .concatMap(connectionCheck -> connectionCheck.validate(clientIp)) + .then() + .block(); + } + } + private MDCBuilder mdc(ChannelHandlerContext ctx) { ImapSession maybeSession = ctx.channel().attr(IMAP_SESSION_ATTRIBUTE_KEY).get(); diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IpConnectionCheck.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IpConnectionCheck.java index 23e61a0432..c717097bd1 100644 --- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IpConnectionCheck.java +++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IpConnectionCheck.java @@ -25,16 +25,19 @@ import java.util.Set; import org.apache.james.imap.api.ConnectionCheck; import org.reactivestreams.Publisher; +import com.google.common.annotations.VisibleForTesting; + import reactor.core.publisher.Mono; public class IpConnectionCheck implements ConnectionCheck { private Set<String> bannedIps = Set.of(); @Override + @VisibleForTesting public Publisher<Void> validate(InetSocketAddress remoteAddress) { - String ip = remoteAddress.getHostName(); + String ip = remoteAddress.getAddress().getHostAddress(); // check against bannedIps - if (bannedIps.stream().anyMatch(bannedIp -> bannedIp.equals(ip))) { + if (bannedIps.contains(ip)) { return Mono.error(() -> new RuntimeException("Banned")); } else { return Mono.empty(); diff --git a/third-party/crowdsec/src/main/java/org/apache/james/CrowdsecImapConnectionCheck.java b/third-party/crowdsec/src/main/java/org/apache/james/CrowdsecImapConnectionCheck.java index 270445645e..3b7f038f55 100644 --- a/third-party/crowdsec/src/main/java/org/apache/james/CrowdsecImapConnectionCheck.java +++ b/third-party/crowdsec/src/main/java/org/apache/james/CrowdsecImapConnectionCheck.java @@ -19,7 +19,10 @@ package org.apache.james; +import static org.apache.james.model.CrowdsecClientConfiguration.DEFAULT_TIMEOUT; + import java.net.InetSocketAddress; +import java.util.concurrent.TimeoutException; import javax.inject.Inject; @@ -30,29 +33,39 @@ import org.apache.james.model.CrowdsecClientConfiguration; import org.apache.james.model.CrowdsecDecision; import org.apache.james.model.CrowdsecHttpClient; import org.reactivestreams.Publisher; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import reactor.core.publisher.Mono; public class CrowdsecImapConnectionCheck implements ConnectionCheck { - private final CrowdsecClientConfiguration crowdsecClientConfiguration; + private static final Logger LOGGER = LoggerFactory.getLogger(CrowdsecImapConnectionCheck.class); + + private final CrowdsecHttpClient client; @Inject public CrowdsecImapConnectionCheck(CrowdsecClientConfiguration crowdsecClientConfiguration) { - this.crowdsecClientConfiguration = crowdsecClientConfiguration; + this.client = new CrowdsecHttpClient(crowdsecClientConfiguration); } @Override public Publisher<Void> validate(InetSocketAddress remoteAddress) { - String ip = remoteAddress.getHostName(); - CrowdsecHttpClient client = new CrowdsecHttpClient(crowdsecClientConfiguration); + String ip = remoteAddress.getAddress().getHostAddress(); + return client.getCrowdsecDecisions() + .timeout(DEFAULT_TIMEOUT) + .onErrorResume(TimeoutException.class, e -> Mono.fromRunnable(() -> LOGGER.warn("Timeout while questioning to CrowdSec. May need to check the CrowdSec configuration."))) .filter(decisions -> decisions.stream().anyMatch(decision -> isBanned(decision, ip))) .handle((crowdsecDecisions, synchronousSink) -> synchronousSink.error(new CrowdsecException("Ip " + ip + " is not allowed to connect to IMAP server by Crowdsec"))); } private boolean isBanned(CrowdsecDecision decision, String ip) { if (decision.getScope().equals("Ip") && ip.contains(decision.getValue())) { + LOGGER.warn("Connection from IP {} has been blocked by CrowdSec for duration {}", ip, decision.getDuration()); return true; } if (decision.getScope().equals("Range") && belongToNetwork(decision.getValue(), ip)) { + LOGGER.warn("Connection from IP {} has been blocked by CrowdSec for duration {}", ip, decision.getDuration()); return true; } return false; diff --git a/third-party/crowdsec/src/main/java/org/apache/james/model/CrowdsecHttpClient.java b/third-party/crowdsec/src/main/java/org/apache/james/model/CrowdsecHttpClient.java index e537d158d1..e1956baca5 100644 --- a/third-party/crowdsec/src/main/java/org/apache/james/model/CrowdsecHttpClient.java +++ b/third-party/crowdsec/src/main/java/org/apache/james/model/CrowdsecHttpClient.java @@ -27,8 +27,6 @@ import javax.inject.Inject; import org.apache.http.HttpStatus; import org.apache.http.entity.ContentType; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; @@ -41,7 +39,6 @@ import reactor.core.publisher.Mono; import reactor.netty.http.client.HttpClient; public class CrowdsecHttpClient { - private static final Logger LOGGER = LoggerFactory.getLogger(CrowdsecHttpClient.class); private static final String GET_DECISION = "/decisions"; private final HttpClient httpClient; @@ -88,6 +85,7 @@ public class CrowdsecHttpClient { private HttpClient buildReactorNettyHttpClient(CrowdsecClientConfiguration configuration) { return HttpClient.create() + .disableRetry(true) .responseTimeout(DEFAULT_TIMEOUT) .baseUrl(configuration.getUrl().toString()) .headers(headers -> headers.add("X-Api-Key", configuration.getApiKey())) diff --git a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecContract.java b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecContract.java deleted file mode 100644 index 2ca5d6a2ae..0000000000 --- a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecContract.java +++ /dev/null @@ -1,38 +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.james; - - -import org.apache.james.utils.DataProbeImpl; -import org.junit.jupiter.api.BeforeEach; - -public interface CrowdsecContract { - - String DOMAIN = "domain.tld"; - String BOB = "bob@" + DOMAIN; - String BOB_PASSWORD = "bobPassword"; - - @BeforeEach - default void setup(GuiceJamesServer server) throws Exception { - server.getProbe(DataProbeImpl.class).fluent() - .addDomain(DOMAIN) - .addUser(BOB, BOB_PASSWORD); - } -} diff --git a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecEhloHookTest.java b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecEhloHookTest.java index b358c57ec6..396a858f1d 100644 --- a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecEhloHookTest.java +++ b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecEhloHookTest.java @@ -20,7 +20,6 @@ package org.apache.james; import static org.apache.james.CrowdsecExtension.CROWDSEC_PORT; -import static org.apache.james.model.CrowdsecClientConfiguration.DEFAULT_API_KEY; import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; @@ -30,7 +29,6 @@ import org.apache.james.model.CrowdsecClientConfiguration; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.hook.HookResult; import org.apache.james.protocols.smtp.utils.BaseFakeSMTPSession; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -41,11 +39,6 @@ class CrowdsecEhloHookTest { @RegisterExtension static CrowdsecExtension crowdsecExtension = new CrowdsecExtension(); - @BeforeAll - static void setUp() throws IOException, InterruptedException { - crowdsecExtension.getCrowdsecContainer().execInContainer("cscli", "bouncer", "add", "bouncer", "-k", DEFAULT_API_KEY); - } - @BeforeEach void setUpEach() throws IOException { int port = crowdsecExtension.getCrowdsecContainer().getMappedPort(CROWDSEC_PORT); diff --git a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecHttpClientTest.java b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecHttpClientTest.java index e6ee7c57f1..d731d64a27 100644 --- a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecHttpClientTest.java +++ b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecHttpClientTest.java @@ -33,7 +33,6 @@ import org.apache.james.model.CrowdsecClientConfiguration; import org.apache.james.model.CrowdsecDecision; import org.apache.james.model.CrowdsecHttpClient; import org.assertj.core.api.SoftAssertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -41,11 +40,6 @@ class CrowdsecHttpClientTest { @RegisterExtension static CrowdsecExtension crowdsecExtension = new CrowdsecExtension(); - @BeforeAll - static void setUp() throws IOException, InterruptedException { - crowdsecExtension.getCrowdsecContainer().execInContainer("cscli", "bouncer", "add", "bouncer", "-k", DEFAULT_API_KEY); - } - @Test void getDecisionsWhenBanningAnIP() throws IOException, InterruptedException { banIP("--ip", "192.168.0.4"); diff --git a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecImapConnectionCheckTest.java b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecImapConnectionCheckTest.java index df8287cb94..51498e98f5 100644 --- a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecImapConnectionCheckTest.java +++ b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecImapConnectionCheckTest.java @@ -28,7 +28,6 @@ import java.net.InetSocketAddress; import java.net.URL; import org.apache.james.model.CrowdsecClientConfiguration; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -38,11 +37,6 @@ public class CrowdsecImapConnectionCheckTest { @RegisterExtension static CrowdsecExtension crowdsecExtension = new CrowdsecExtension(); - @BeforeAll - static void setUp() throws IOException, InterruptedException { - crowdsecExtension.getCrowdsecContainer().execInContainer("cscli", "bouncer", "add", "bouncer", "-k", DEFAULT_API_KEY); - } - @Test void givenIPBannedByCrowdsecDecisionIp() throws IOException, InterruptedException { banIP("--ip", "127.0.0.3"); diff --git a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecIntegrationTest.java b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecIntegrationTest.java index 24a9dc5bd6..60e134a617 100644 --- a/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecIntegrationTest.java +++ b/third-party/crowdsec/src/test/java/org/apache/james/CrowdsecIntegrationTest.java @@ -23,8 +23,10 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.james.data.UsersRepositoryModuleChooser.Implementation.DEFAULT; import static org.apache.james.model.CrowdsecClientConfiguration.DEFAULT_API_KEY; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.awaitility.Durations.ONE_HUNDRED_MILLISECONDS; +import java.io.EOFException; import java.io.IOException; import java.net.InetAddress; import java.nio.file.Files; @@ -37,6 +39,7 @@ import org.apache.commons.net.smtp.SMTPClient; import org.apache.james.model.CrowdsecClientConfiguration; import org.apache.james.model.CrowdsecDecision; import org.apache.james.model.CrowdsecHttpClient; +import org.apache.james.modules.protocols.ImapGuiceProbe; import org.apache.james.modules.protocols.SmtpGuiceProbe; import org.apache.james.utils.DataProbeImpl; import org.apache.james.utils.TestIMAPClient; @@ -101,9 +104,12 @@ class CrowdsecIntegrationTest { } private Path createHaProxyConfigFile(GuiceJamesServer server, Path tempDir) throws IOException { - String smtpServerWithProxySupportIp = crowdsecExtension.getCrowdsecContainer().getContainerInfo().getNetworkSettings() + String jamesServerWithProxySupportIp = crowdsecExtension.getCrowdsecContainer().getContainerInfo().getNetworkSettings() .getGateway(); // James server listens at the docker bridge network's gateway IP int smtpWithProxySupportPort = server.getProbe(SmtpGuiceProbe.class).getSmtpAuthRequiredPort().getValue(); + int imapWithProxySupportPort = server.getProbe(ImapGuiceProbe.class) + .getPort(asyncServer -> asyncServer.getHelloName().equals("imapServerWithProxyEnabled")) + .get(); String haproxyConfigContent = String.format("global\n" + " log stdout format raw local0 info\n" + "\n" + @@ -120,8 +126,16 @@ class CrowdsecIntegrationTest { " default_backend james-server-smtp\n" + "\n" + "backend james-server-smtp\n" + - " server james1 %s:%d send-proxy\n", - smtpServerWithProxySupportIp, smtpWithProxySupportPort); + " server james1 %s:%d send-proxy\n" + + "\n" + + "frontend imap-frontend\n" + + " bind :143\n" + + " default_backend james-server-imap\n" + + "\n" + + "backend james-server-imap\n" + + " server james2 %s:%d send-proxy\n", + jamesServerWithProxySupportIp, smtpWithProxySupportPort, + jamesServerWithProxySupportIp, imapWithProxySupportPort); Path haProxyConfigFile = tempDir.resolve("haproxy.cfg"); Files.write(haProxyConfigFile, haproxyConfigContent.getBytes()); @@ -133,6 +147,76 @@ class CrowdsecIntegrationTest { haProxyExtension.stop(); } + @Nested + class IMAP { + @Test + void ipShouldBeBannedByCrowdSecWhenFailingToImapLoginThreeTimes(GuiceJamesServer server) { + // GIVEN an IP failed to log in 3 consecutive times in a short period + IntStream.range(0, 3) + .forEach(any -> + assertThatThrownBy(() -> testIMAPClient.connect("127.0.0.1", server.getProbe(ImapGuiceProbe.class).getImapPort()) + .login(BOB, BAD_PASSWORD)) + .isInstanceOf(IOException.class) + .hasMessage("Login failed")); + + // THEN connection from the IP would be blocked. CrowdSec takes time to processing the ban decision therefore the await below. + CALMLY_AWAIT.atMost(Durations.TEN_SECONDS) + .untilAsserted(() -> assertThatThrownBy(() -> testIMAPClient.connect("127.0.0.1", server.getProbe(ImapGuiceProbe.class).getImapPort())) + .isInstanceOf(EOFException.class) + .hasMessage("Connection closed without indication.")); + } + + @Test + void imapConnectionShouldRejectedAfterThreeFailedIMAPAuthenticationsViaProxy() { + // GIVEN a client connected via proxy failed to log in 3 consecutive times in a short period + IntStream.range(0, 3) + .forEach(Throwing.intConsumer(any -> { + assertThatThrownBy(() -> testIMAPClient.connect(LOCALHOST_IP, haProxyExtension.getProxiedImapPort()) + .login(BOB, BAD_PASSWORD)) + .isInstanceOf(IOException.class) + .hasMessage("Login failed"); + })); + + // THEN IMAP connection from the client would be blocked. + CALMLY_AWAIT.atMost(Durations.TEN_SECONDS) + .untilAsserted(() -> assertThatThrownBy(() -> testIMAPClient.connect(LOCALHOST_IP, haProxyExtension.getProxiedImapPort()) + .login(BOB, BOB_PASSWORD)) + .isInstanceOf(EOFException.class) + .hasMessage("Connection closed without indication.")); + } + + @Test + void shouldBanRealClientIpAndNotProxyIp() { + // GIVEN a client connected via proxy failed to log in 3 consecutive times in a short period + IntStream.range(0, 3) + .forEach(Throwing.intConsumer(any -> { + assertThatThrownBy(() -> testIMAPClient.connect(LOCALHOST_IP, haProxyExtension.getProxiedImapPort()) + .login(BOB, BAD_PASSWORD)) + .isInstanceOf(IOException.class) + .hasMessage("Login failed"); + })); + + CALMLY_AWAIT.atMost(Durations.TEN_SECONDS) + .untilAsserted(() -> assertThatThrownBy(() -> testIMAPClient.connect(LOCALHOST_IP, haProxyExtension.getProxiedImapPort()) + .login(BOB, BOB_PASSWORD)) + .isInstanceOf(EOFException.class) + .hasMessage("Connection closed without indication.")); + + // THEN real client IP must be banned, not proxy IP + String realClientIp = haProxyExtension.getHaproxyContainer().getContainerInfo().getNetworkSettings() + .getGateway(); // client connect to HAProxy container via the docker bridge network's gateway IP + String haProxyIp = haProxyExtension.getHaproxyContainer().getContainerInfo().getNetworkSettings() + .getIpAddress(); + + List<CrowdsecDecision> decisions = crowdsecClient.getCrowdsecDecisions().block(); + SoftAssertions.assertSoftly(softly -> { + softly.assertThat(decisions).hasSize(1); + softly.assertThat(decisions.get(0).getValue()).isEqualTo(realClientIp); + softly.assertThat(decisions.get(0).getValue()).isNotEqualTo(haProxyIp); + }); + } + } + @Nested class SMTP { @Test diff --git a/third-party/crowdsec/src/test/java/org/apache/james/HAProxyExtension.java b/third-party/crowdsec/src/test/java/org/apache/james/HAProxyExtension.java index 31f556cfe7..3ee562f309 100644 --- a/third-party/crowdsec/src/test/java/org/apache/james/HAProxyExtension.java +++ b/third-party/crowdsec/src/test/java/org/apache/james/HAProxyExtension.java @@ -29,13 +29,14 @@ import org.testcontainers.utility.MountableFile; public class HAProxyExtension { private static final String HAPROXY_IMAGE = "haproxytech/haproxy-alpine:2.9.1"; private static final int SMTP_PORT = 25; + private static final int IMAP_PORT = 143; private final GenericContainer<?> haproxyContainer; public HAProxyExtension(MountableFile haProxyConfigFile) { this.haproxyContainer = new GenericContainer<>(HAPROXY_IMAGE) .withCreateContainerCmdModifier(cmd -> cmd.withName("james-haproxy-test-" + UUID.randomUUID())) - .withExposedPorts(SMTP_PORT) + .withExposedPorts(SMTP_PORT, IMAP_PORT) .withCopyFileToContainer(haProxyConfigFile, "/usr/local/etc/haproxy/") .waitingFor(new HostPortWaitStrategy().withRateLimiter(RateLimiters.TWENTIES_PER_SECOND)); } @@ -56,6 +57,10 @@ public class HAProxyExtension { return haproxyContainer.getMappedPort(SMTP_PORT); } + public int getProxiedImapPort() { + return haproxyContainer.getMappedPort(IMAP_PORT); + } + public GenericContainer<?> getHaproxyContainer() { return this.haproxyContainer; } diff --git a/third-party/crowdsec/src/test/java/org/apache/james/MemoryCrowdsecTest.java b/third-party/crowdsec/src/test/java/org/apache/james/MemoryCrowdsecTest.java deleted file mode 100644 index 3155bbbf07..0000000000 --- a/third-party/crowdsec/src/test/java/org/apache/james/MemoryCrowdsecTest.java +++ /dev/null @@ -1,58 +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.james; - -import static org.apache.james.data.UsersRepositoryModuleChooser.Implementation.DEFAULT; -import static org.apache.james.model.CrowdsecClientConfiguration.DEFAULT_API_KEY; - -import java.io.IOException; - -import org.apache.james.utils.TestIMAPClient; -import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -public class MemoryCrowdsecTest implements CrowdsecContract { - @RegisterExtension - static JamesServerExtension testExtension = new JamesServerBuilder<MemoryJamesConfiguration>(tmpDir -> - MemoryJamesConfiguration.builder() - .workingDirectory(tmpDir) - .configurationFromClasspath() - .usersRepository(DEFAULT) - .build()) - .server(MemoryJamesServerMain::createServer) - .build(); - - @RegisterExtension - static CrowdsecExtension crowdsecExtension = new CrowdsecExtension(); - - @RegisterExtension - public TestIMAPClient testIMAPClient = new TestIMAPClient(); - - @BeforeAll - static void setUp() throws IOException, InterruptedException { - crowdsecExtension.getCrowdsecContainer().execInContainer("cscli", "bouncer", "add", "bouncer", "-k", DEFAULT_API_KEY); - } - - @Test - void IPShouldBeBannedByCrowdsecWhenFailingToLoginThreeTimes() { - // TODO client login failed 3 times in short period, James will ban this IP based on Crowdsec decision - } -} diff --git a/third-party/crowdsec/src/test/resources/haproxy.cfg b/third-party/crowdsec/src/test/resources/haproxy.cfg new file mode 100644 index 0000000000..322446fffc --- /dev/null +++ b/third-party/crowdsec/src/test/resources/haproxy.cfg @@ -0,0 +1,21 @@ +global + log stdout format raw local0 info + +defaults + mode tcp + timeout client 1800s + timeout connect 5s + timeout server 1800s + log global + option tcplog +frontend imap1-frontend + bind :143 + default_backend james-servers-imaps +frontend imaps-frontend + bind :993 + default_backend james-servers-imaps + +backend james-servers-imap + server james1 172.17.0.1:39847 send-proxy +backend james-servers-imaps + server james1 172.17.0.1:39347 send-proxy \ No newline at end of file diff --git a/third-party/crowdsec/src/test/resources/imapserver.xml b/third-party/crowdsec/src/test/resources/imapserver.xml index a2ee96c8c9..bbeb65e5c4 100644 --- a/third-party/crowdsec/src/test/resources/imapserver.xml +++ b/third-party/crowdsec/src/test/resources/imapserver.xml @@ -40,6 +40,7 @@ under the License. <idleTimeIntervalUnit>SECONDS</idleTimeIntervalUnit> <enableIdle>true</enableIdle> <plainAuthDisallowed>false</plainAuthDisallowed> + <additionalConnectionChecks>org.apache.james.CrowdsecImapConnectionCheck</additionalConnectionChecks> </imapserver> <imapserver enabled="true"> <jmxName>imapserver-ssl</jmxName> @@ -57,8 +58,11 @@ under the License. <connectionLimitPerIP>0</connectionLimitPerIP> <gracefulShutdown>false</gracefulShutdown> <idleTimeInterval>120</idleTimeInterval> + <helloName autodetect="false">imapServerWithProxyEnabled</helloName> <idleTimeIntervalUnit>SECONDS</idleTimeIntervalUnit> <enableIdle>true</enableIdle> <plainAuthDisallowed>false</plainAuthDisallowed> + <additionalConnectionChecks>org.apache.james.CrowdsecImapConnectionCheck</additionalConnectionChecks> + <proxyRequired>true</proxyRequired> </imapserver> </imapservers> diff --git a/third-party/crowdsec/src/test/resources/logback.xml b/third-party/crowdsec/src/test/resources/logback.xml deleted file mode 100644 index 3d56ea2e15..0000000000 --- a/third-party/crowdsec/src/test/resources/logback.xml +++ /dev/null @@ -1,48 +0,0 @@ -<?xml version="1.0" encoding="UTF-8"?> -<!-- - 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. - --> - -<configuration> - - <contextListener class="ch.qos.logback.classic.jul.LevelChangePropagator"> - <resetJUL>true</resetJUL> - </contextListener> - - <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender"> - <encoder class="ch.qos.logback.core.encoder.LayoutWrappingEncoder"> - <layout class="ch.qos.logback.contrib.json.classic.JsonLayout"> - <timestampFormat>yyyy-MM-dd'T'HH:mm:ss.SSSX</timestampFormat> - <timestampFormatTimezoneId>Etc/UTC</timestampFormatTimezoneId> - - <!-- Importance for handling multiple lines log --> - <appendLineSeparator>true</appendLineSeparator> - - <jsonFormatter class="ch.qos.logback.contrib.jackson.JacksonJsonFormatter"> - <prettyPrint>false</prettyPrint> - </jsonFormatter> - </layout> - </encoder> - <immediateFlush>false</immediateFlush> - </appender> - <root level="WARN"> - <appender-ref ref="CONSOLE" /> - </root> - - <logger name="org.apache.james" level="INFO" /> -</configuration> \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
