You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2022/03/22 02:01:40 UTC
[james-project] 09/29: JAMES-3715 IMAP: Avoid a deadlock upon STARTTLS
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 67f3e11af82f9e13b0fbcdbc41771789ec572742
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Mar 7 15:36:28 2022 +0700
JAMES-3715 IMAP: Avoid a deadlock upon STARTTLS
The initial STARTTLS OK response was never sent because it was sent out of
the event loop.
We explicitly write it from the event loop so that the client is guarantied to receive it.
We wrap the answer inside the "disabled-auto-read" to guaranty that a
"too quick client" do not end up with a race condition...
---
.../apache/james/imap/api/process/ImapSession.java | 3 +-
.../apache/james/imap/encode/FakeImapSession.java | 3 +-
.../james/imap/processor/StartTLSProcessor.java | 6 ++--
.../james/imapserver/netty/NettyImapSession.java | 39 +++++++++++++++++++++-
4 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java b/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
index 6c3ac4a..4444ed4 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/api/process/ImapSession.java
@@ -25,6 +25,7 @@ import java.util.Optional;
import org.apache.commons.text.RandomStringGenerator;
import org.apache.james.core.Username;
import org.apache.james.imap.api.ImapSessionState;
+import org.apache.james.imap.message.response.ImmutableStatusResponse;
import org.apache.james.mailbox.MailboxSession;
import org.apache.james.protocols.api.CommandDetectionSession;
import org.apache.james.protocols.api.OidcSASLConfiguration;
@@ -156,7 +157,7 @@ public interface ImapSession extends CommandDetectionSession {
*
* @return true if the encryption of the session was successfully
*/
- boolean startTLS();
+ boolean startTLS(ImmutableStatusResponse startTlsResponse);
/**
* Return true if the session is bound to a TLS encrypted socket.
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java b/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
index 153f615..cb1de55 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/encode/FakeImapSession.java
@@ -26,6 +26,7 @@ import org.apache.james.imap.api.ImapSessionState;
import org.apache.james.imap.api.process.ImapLineHandler;
import org.apache.james.imap.api.process.ImapSession;
import org.apache.james.imap.api.process.SelectedMailbox;
+import org.apache.james.imap.message.response.ImmutableStatusResponse;
import org.apache.james.protocols.api.OidcSASLConfiguration;
public class FakeImapSession implements ImapSession {
@@ -118,7 +119,7 @@ public class FakeImapSession implements ImapSession {
}
@Override
- public boolean startTLS() {
+ public boolean startTLS(ImmutableStatusResponse response) {
return false;
}
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/StartTLSProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/StartTLSProcessor.java
index 028ac98..e5d63de 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/StartTLSProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/StartTLSProcessor.java
@@ -29,6 +29,7 @@ import org.apache.james.imap.api.message.response.StatusResponseFactory;
import org.apache.james.imap.api.process.ImapProcessor;
import org.apache.james.imap.api.process.ImapSession;
import org.apache.james.imap.message.request.StartTLSRequest;
+import org.apache.james.imap.message.response.ImmutableStatusResponse;
import org.apache.james.imap.processor.base.AbstractChainedProcessor;
import org.apache.james.util.MDCBuilder;
@@ -49,13 +50,10 @@ public class StartTLSProcessor extends AbstractChainedProcessor<StartTLSRequest>
@Override
protected void doProcess(StartTLSRequest request, Responder responder, ImapSession session) {
if (session.supportStartTLS()) {
- responder.respond(factory.taggedOk(request.getTag(), request.getCommand(), HumanReadableText.STARTTLS));
- session.startTLS();
-
+ session.startTLS((ImmutableStatusResponse) factory.taggedOk(request.getTag(), request.getCommand(), HumanReadableText.STARTTLS));
} else {
responder.respond(factory.taggedBad(request.getTag(), request.getCommand(), HumanReadableText.UNKNOWN_COMMAND));
}
-
}
@Override
diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
index 11223ef..2e6c109 100644
--- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
+++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/NettyImapSession.java
@@ -18,18 +18,27 @@
****************************************************************/
package org.apache.james.imapserver.netty;
+import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
+import org.apache.commons.lang3.NotImplementedException;
import org.apache.james.imap.api.ImapSessionState;
import org.apache.james.imap.api.process.ImapLineHandler;
import org.apache.james.imap.api.process.ImapSession;
import org.apache.james.imap.api.process.SelectedMailbox;
+import org.apache.james.imap.encode.ImapResponseWriter;
+import org.apache.james.imap.encode.StatusResponseEncoder;
+import org.apache.james.imap.encode.base.ImapResponseComposerImpl;
+import org.apache.james.imap.encode.main.DefaultLocalizer;
+import org.apache.james.imap.message.Literal;
+import org.apache.james.imap.message.response.ImmutableStatusResponse;
import org.apache.james.protocols.api.Encryption;
import org.apache.james.protocols.api.OidcSASLConfiguration;
import org.apache.james.protocols.netty.LineHandlerAware;
+import io.netty.buffer.Unpooled;
import io.netty.channel.Channel;
import io.netty.handler.codec.compression.JZlibDecoder;
import io.netty.handler.codec.compression.JZlibEncoder;
@@ -141,13 +150,20 @@ public class NettyImapSession implements ImapSession, NettyConstants {
}
@Override
- public boolean startTLS() {
+ public boolean startTLS(ImmutableStatusResponse statusResponse) {
if (!supportStartTLS()) {
return false;
}
channel.config().setAutoRead(false);
+ try {
+ new StatusResponseEncoder(new DefaultLocalizer()).encode(statusResponse,
+ new ImapResponseComposerImpl(new EventLoopImapResponseWriter(channel), 2048));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
SslHandler filter = new SslHandler(secure.createSSLEngine(), false);
+
filter.engine().setUseClientMode(false);
channel.pipeline().addFirst(SSL_HANDLER, filter);
@@ -156,6 +172,27 @@ public class NettyImapSession implements ImapSession, NettyConstants {
return true;
}
+ public static class EventLoopImapResponseWriter implements ImapResponseWriter {
+ private final Channel channel;
+
+ public EventLoopImapResponseWriter(Channel channel) {
+ this.channel = channel;
+ }
+
+ @Override
+ public void write(byte[] buffer) {
+ if (channel.isActive()) {
+ channel.pipeline().firstContext().writeAndFlush(Unpooled.wrappedBuffer(buffer));
+ }
+ }
+
+ @Override
+ public void write(Literal literal) {
+ throw new NotImplementedException();
+ }
+ }
+
+
@Override
public boolean supportStartTLS() {
return secure != null && secure.getContext() != null;
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org