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 2026/10/20 16:42:59 UTC

(james-project) branch sec-20-10-2026 created (now 28fcd255c5)

This is an automated email from the ASF dual-hosted git repository.

chibenwa pushed a change to branch sec-20-10-2026
in repository https://gitbox.apache.org/repos/asf/james-project.git


      at 28fcd255c5 [FIX] MDNReportParser: bound comment nesting depth

This branch includes the following new commits:

     new 5612555dfd JAMES-4210 SMTP PLAIN/LOGIN authentication should honor auth.requireSSL
     new 28fcd255c5 [FIX] MDNReportParser: bound comment nesting depth

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


(james-project) 01/02: JAMES-4210 SMTP PLAIN/LOGIN authentication should honor auth.requireSSL

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

chibenwa pushed a commit to branch sec-20-10-2026
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 5612555dfdabb48b52c4e573c2c7a677194ba611
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue Oct 20 18:37:24 2026 +0200

    JAMES-4210 SMTP PLAIN/LOGIN authentication should honor auth.requireSSL
    
    auth.requireSSL only governed AUTH capability announcement: PLAIN and LOGIN
    were still accepted when explicitly sent over clear-text connections.
    Reject them over insecure connections when auth.requireSSL is true.
    
    Credit: Morgan <29...@qq.com> Kangcheng Cui
---
 docs/modules/servers/partials/configure/smtp.adoc  | 10 ++++----
 .../protocols/sasl/PlainSaslMechanismFactory.java  | 10 --------
 .../james/modules/protocols/SMTPServerModule.java  |  5 +---
 .../src/main/resources/smtpserver.xml              |  2 +-
 .../james/smtpserver/netty/SMTPServerFactory.java  |  5 +---
 .../apache/james/smtpserver/AuthAnnounceTest.java  | 28 ++++++++++++++++++++++
 upgrade-instructions.md                            | 21 ++++++++++++++++
 7 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/docs/modules/servers/partials/configure/smtp.adoc b/docs/modules/servers/partials/configure/smtp.adoc
index e1cb27b62e..e434e6204b 100644
--- a/docs/modules/servers/partials/configure/smtp.adoc
+++ b/docs/modules/servers/partials/configure/smtp.adoc
@@ -144,22 +144,22 @@ Note that this configures only the **announcement** of authentication capabiliti
 `auth.requireSSL` also has an influence on whether authentication capabilities are announced.
 
 Even if authentication capabilities are not announced, clients can still use the SMTP AUTH command.
-However, if authentication capabilities are not announced, only the mechanisms `PLAIN` and `LOGIN` are available.
+However, if authentication capabilities are not announced, only the mechanisms `PLAIN` and `LOGIN` are available
+(and only over a secure connection when `auth.requireSSL` is `true`).
 `XOAUTH2` and `OAUTHBEARER` are disabled if authentication capabilities are not announced.
 
 | `auth.requireSSL`
-| Configures whether announcing authentication capabilities requires a secure connection (TLS or STARTTLS).
+| Configures whether authentication requires a secure connection (TLS or STARTTLS).
 If this value is `true` and the connection is not secure, authentication capabilities are not announced.
 This always holds, even if `auth.announce` is set to `always`.
+The `PLAIN` and `LOGIN` mechanisms also use this value to reject authentication attempts over an insecure connection.
 ifndef::no-kerberos[]
 The optional `GSSAPI` mechanism also uses this value to reject authentication attempts over an insecure connection.
 endif::[]
 
 It is an optional boolean that defaults to `true`.
 
-For the built-in mechanisms, this configures only the **announcement** of authentication capabilities.
-Even if authentication capabilities are not announced, clients can still use the SMTP AUTH command with `PLAIN` and `LOGIN`.
-However, as the mechanisms `XOAUTH2` and `OAUTHBEARER` are only available if authentication mechanisms are announced, those mechanisms can only be used in secure connections if `auth.requireSSL` is set to `true`.
+As the mechanisms `XOAUTH2` and `OAUTHBEARER` are only available if authentication mechanisms are announced, those mechanisms can only be used in secure connections if `auth.requireSSL` is set to `true`.
 ifndef::no-kerberos[]
 When `GSSAPI` is configured and `auth.requireSSL` is `true`, it is both unannounced and unavailable on insecure connections.
 endif::[]
diff --git a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java
index 4543c3c30d..7964454d43 100644
--- a/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java
+++ b/protocols/sasl/src/main/java/org/apache/james/protocols/sasl/PlainSaslMechanismFactory.java
@@ -26,24 +26,17 @@ import org.apache.james.protocols.api.sasl.SaslMechanismFactory;
 import org.apache.james.protocols.sasl.plain.PlainSaslMechanism;
 
 public class PlainSaslMechanismFactory implements SaslMechanismFactory {
-    public static final boolean IGNORE_REQUIRE_SSL_CONFIGURATION = false;
     private static final boolean PLAIN_AUTH_DISALLOWED_DEFAULT = true;
     private static final boolean PLAIN_AUTH_ENABLED_DEFAULT = true;
 
     private final boolean requireSslDefault;
-    private final boolean honorRequireSslConfiguration;
 
     public PlainSaslMechanismFactory() {
         this(PLAIN_AUTH_DISALLOWED_DEFAULT);
     }
 
     public PlainSaslMechanismFactory(boolean requireSslDefault) {
-        this(requireSslDefault, true);
-    }
-
-    public PlainSaslMechanismFactory(boolean requireSslDefault, boolean honorRequireSslConfiguration) {
         this.requireSslDefault = requireSslDefault;
-        this.honorRequireSslConfiguration = honorRequireSslConfiguration;
     }
 
     @Override
@@ -56,9 +49,6 @@ public class PlainSaslMechanismFactory implements SaslMechanismFactory {
     }
 
     protected boolean requiresSsl(HierarchicalConfiguration<ImmutableNode> serverConfiguration) {
-        if (!honorRequireSslConfiguration) {
-            return requireSslDefault;
-        }
         return serverConfiguration.getBoolean("auth.requireSSL",
             serverConfiguration.getBoolean("plainAuthDisallowed", requireSslDefault));
     }
diff --git a/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SMTPServerModule.java b/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SMTPServerModule.java
index b6a1255992..a323ae9375 100644
--- a/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SMTPServerModule.java
+++ b/server/container/guice/protocols/smtp/src/main/java/org/apache/james/modules/protocols/SMTPServerModule.java
@@ -96,10 +96,7 @@ public class SMTPServerModule extends AbstractModule {
     @SmtpDefaultSaslMechanismFactories
     ImmutableList<SaslMechanismFactory> provideDefaultSmtpSaslMechanismFactories(OauthBearerSaslMechanismFactory oauthBearer,
                                                                                  XOauth2SaslMechanismFactory xoauth2) {
-        // SMTP historically used auth.requireSSL for capability announcement only: PLAIN/LOGIN remain accepted
-        // when sent explicitly by clients, even over clear-text test/configuration ports.
-        PlainSaslMechanismFactory plain = new PlainSaslMechanismFactory(AuthAnnouncementConfiguration.REQUIRE_SSL_DEFAULT,
-            PlainSaslMechanismFactory.IGNORE_REQUIRE_SSL_CONFIGURATION);
+        PlainSaslMechanismFactory plain = new PlainSaslMechanismFactory(AuthAnnouncementConfiguration.REQUIRE_SSL_DEFAULT);
         return ImmutableList.of(new LoginSaslMechanismFactory(plain), plain, oauthBearer, xoauth2);
     }
 
diff --git a/server/mailet/integration-testing/src/main/resources/smtpserver.xml b/server/mailet/integration-testing/src/main/resources/smtpserver.xml
index 4cea11c9a0..11b296e340 100644
--- a/server/mailet/integration-testing/src/main/resources/smtpserver.xml
+++ b/server/mailet/integration-testing/src/main/resources/smtpserver.xml
@@ -84,7 +84,7 @@
         {{/hasAuthorizedAddresses}}
         <auth>
             <required>{{forbidUnauthenticatedSenders}}</required>
-            <requireSSL>true</requireSSL>
+            <requireSSL>false</requireSSL>
             <plainAuthEnabled>true</plainAuthEnabled>
 {{#hasSaslMechanisms}}
             <saslMechanisms>{{saslMechanisms}}</saslMechanisms>
diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServerFactory.java b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServerFactory.java
index 99d3fa564c..30f04dec94 100644
--- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServerFactory.java
+++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServerFactory.java
@@ -58,11 +58,8 @@ public class SMTPServerFactory extends AbstractServerFactory implements Disconne
     @FunctionalInterface
     public interface SmtpSaslMechanismLoader {
         static SmtpSaslMechanismLoader defaultLoader() {
-            PlainSaslMechanismFactory plain = new PlainSaslMechanismFactory(AuthAnnouncementConfiguration.REQUIRE_SSL_DEFAULT,
-                PlainSaslMechanismFactory.IGNORE_REQUIRE_SSL_CONFIGURATION);
+            PlainSaslMechanismFactory plain = new PlainSaslMechanismFactory(AuthAnnouncementConfiguration.REQUIRE_SSL_DEFAULT);
             ImmutableList<SaslMechanismFactory> defaultFactories = ImmutableList.of(
-                // SMTP historically used auth.requireSSL for capability announcement only: PLAIN/LOGIN remain accepted
-                // when sent explicitly by clients, even over clear-text test/configuration ports.
                 new LoginSaslMechanismFactory(plain),
                 plain,
                 new OauthBearerSaslMechanismFactory(),
diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/AuthAnnounceTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/AuthAnnounceTest.java
index 24464bc596..b11393138a 100644
--- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/AuthAnnounceTest.java
+++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/AuthAnnounceTest.java
@@ -154,6 +154,34 @@ class AuthAnnounceTest {
         });
     }
 
+    @Test
+    void plainAuthShouldFailOnPlainChannelsWhenRequireSSL() throws Exception {
+        configureAndInit("smtpserver-requireSSL.xml");
+
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = testSystem.getBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), bindedAddress.getPort());
+        smtpProtocol.sendCommand("EHLO localhost");
+        smtpProtocol.sendCommand("AUTH PLAIN");
+
+        assertThat(smtpProtocol.getReplyString())
+            .isEqualTo("504 Unrecognized Authentication Type\r\n");
+    }
+
+    @Test
+    void loginAuthShouldFailOnPlainChannelsWhenRequireSSL() throws Exception {
+        configureAndInit("smtpserver-requireSSL.xml");
+
+        SMTPClient smtpProtocol = new SMTPClient();
+        InetSocketAddress bindedAddress = testSystem.getBindedAddress();
+        smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), bindedAddress.getPort());
+        smtpProtocol.sendCommand("EHLO localhost");
+        smtpProtocol.sendCommand("AUTH LOGIN");
+
+        assertThat(smtpProtocol.getReplyString())
+            .isEqualTo("504 Unrecognized Authentication Type\r\n");
+    }
+
     @Test
     void shouldStartWithPreviousConfiguration() throws Exception {
         configureAndInit("smtpserver-noauth.xml");
diff --git a/upgrade-instructions.md b/upgrade-instructions.md
index 360b7c5326..e9f0f4b9bf 100644
--- a/upgrade-instructions.md
+++ b/upgrade-instructions.md
@@ -20,6 +20,7 @@ Change list:
  - [Adding thread_id column to Postgresql email_query_view table](#adding-thread_id-column-to-postgresql-email_query_view-table)
  - [Lucene mailbox index schema update for collapseThreads support](#lucene-mailbox-index-schema-update-for-collapsethreads-support)
  - [JAMES-4210 SMTP AuthHook deprecation](#james-4210-smtp-authhook-deprecation)
+ - [JAMES-4210 SMTP PLAIN/LOGIN authentication honors auth.requireSSL](#james-4210-smtp-plainlogin-authentication-honors-authrequiressl)
  - [JAMES-4210 POP3 USER/PASS requires TLS by default](#james-4210-pop3-userpass-requires-tls-by-default)
  - [JAMES-4210 ManageSieve SASL adoption](#james-4210-managesieve-sasl-adoption)
  - [JAMES-4225 Blob ids default to 128 bits of entropy](#james-4225-blob-ids-default-to-128-bits-of-entropy)
@@ -104,6 +105,26 @@ against its shorter counterpart until it is rewritten. To keep the previous beha
 james.blobid.entropy=256
 ```
 
+### JAMES-4210 SMTP PLAIN/LOGIN authentication honors auth.requireSSL
+
+Date: 20/10/2026
+
+Concerned products: SMTP servers configured with `auth.requireSSL` set to `true` on ports accepting clear-text connections
+
+`auth.requireSSL` used to only control the announcement of SMTP authentication capabilities: clients explicitly
+sending `AUTH PLAIN` or `AUTH LOGIN` over a clear-text connection were still authenticated. These mechanisms now
+reject authentication attempts over insecure connections when `auth.requireSSL` is `true`, answering
+`504 Unrecognized Authentication Type`. Clients have to issue `STARTTLS` (or use implicit TLS) prior to authenticating.
+
+Deployments that intentionally allow clear-text SMTP authentication must add the following to each concerned
+`smtpserver.xml` entry:
+
+```xml
+<auth>
+    <requireSSL>false</requireSSL>
+</auth>
+```
+
 ### JAMES-4210 POP3 USER/PASS requires TLS by default
 
 Date: 16/07/2026


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


(james-project) 02/02: [FIX] MDNReportParser: bound comment nesting depth

Posted by bt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

chibenwa pushed a commit to branch sec-20-10-2026
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit 28fcd255c51b7f529d8c58ae4e7058e6e58956ef
Author: Benoit TELLIER <bt...@linagora.com>
AuthorDate: Tue Oct 20 18:39:41 2026 +0200

    [FIX] MDNReportParser: bound comment nesting depth
    
    Deeply nested RFC 5322 comments recursed until StackOverflowError.
    Limit nesting to MAX_COMMENT_DEPTH (10) so that such input results in a
    regular parsing failure.
    
    Credit: Morgan <29...@qq.com> Kangcheng Cui
---
 .../org/apache/james/mdn/MDNReportParser.scala     | 10 ++++++---
 .../org/apache/james/mdn/MDNReportParserTest.scala | 25 +++++++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/mdn/src/main/scala/org/apache/james/mdn/MDNReportParser.scala b/mdn/src/main/scala/org/apache/james/mdn/MDNReportParser.scala
index 8471236544..207960cc0d 100644
--- a/mdn/src/main/scala/org/apache/james/mdn/MDNReportParser.scala
+++ b/mdn/src/main/scala/org/apache/james/mdn/MDNReportParser.scala
@@ -35,6 +35,8 @@ import scala.util.{Failure, Try}
 
 object MDNReportParser {
   private val LOGGER = LoggerFactory.getLogger(classOf[MDNReportParser])
+  // Bounds comment recursion: deeply nested comments would otherwise lead to StackOverflowError
+  val MAX_COMMENT_DEPTH: Int = 10
 
   def parse(is: InputStream, charset: String): Try[MDNReport] = new MDNReportParser(IOUtils.toString(is, charset)).dispositionNotificationContent.run()
 
@@ -302,7 +304,7 @@ class MDNReportParser(val input: ParserInput) extends Parser {
   }
 
   //   CFWS            =   (1*([FWS] comment) [FWS]) / FWS
-  private def cfws: Rule0 = rule { (oneOrMore(fws.? ~ comment) ~ fws) | fws }
+  private def cfws: Rule0 = rule { (oneOrMore(fws.? ~ comment(1)) ~ fws) | fws }
 
   //   FWS             =   ([*WSP CRLF] 1*WSP) /  obs-FWS
   private def fws: Rule0 = rule { ((zeroOrMore(wsp) ~ crlf).? ~ oneOrMore(wsp)) | obsFWS }
@@ -329,10 +331,12 @@ class MDNReportParser(val input: ParserInput) extends Parser {
   private def obsFWS: Rule0 = rule { oneOrMore(wsp) ~ zeroOrMore(crlf ~ oneOrMore(wsp)) }
 
   //   comment         =   "(" *([FWS] ccontent) [FWS] ")"
-  private def comment: Rule[HNil, HNil] = rule { "(" ~ zeroOrMore(fws.? ~ ccontent) ~ fws.? ~ ")" }
+  private def comment(depth: Int): Rule[HNil, HNil] = rule {
+    test(depth <= MDNReportParser.MAX_COMMENT_DEPTH) ~ "(" ~ zeroOrMore(fws.? ~ ccontent(depth)) ~ fws.? ~ ")"
+  }
 
   //   ccontent        =   ctext / quoted-pair / comment
-  private def ccontent: Rule[HNil, HNil] = rule { ctext | quotedPair | comment }
+  private def ccontent(depth: Int): Rule[HNil, HNil] = rule { ctext | quotedPair | comment(depth + 1) }
 
   /*   ctext           =   %d33-39 /          ; Printable US-ASCII
                          %d42-91 /          ;  characters not including
diff --git a/mdn/src/test/scala/org/apache/james/mdn/MDNReportParserTest.scala b/mdn/src/test/scala/org/apache/james/mdn/MDNReportParserTest.scala
index 13cd4191a0..8397f4197d 100644
--- a/mdn/src/test/scala/org/apache/james/mdn/MDNReportParserTest.scala
+++ b/mdn/src/test/scala/org/apache/james/mdn/MDNReportParserTest.scala
@@ -284,4 +284,27 @@ class MDNReportParserTest {
     assertThat(result.isSuccess).isTrue
     assertThat(result.get).isEqualTo(ExtensionField.builder.fieldName("X-OPENPAAS-IP").rawValue(" 177.177.177.77").build)
   }
-}
\ No newline at end of file
+
+  private def mdnWithNestedComments(depth: Int): String =
+    "Final-Recipient: rfc822; final_recipient\r\n" +
+      "Original-Message-ID: " + "(" * depth + ")" * depth + " <or...@message.id>\r\n" +
+      "Disposition: automatic-action/MDN-sent-automatically;processed\r\n"
+
+  @Test
+  def parseShouldAcceptNestedCommentsUpToMaxDepth(): Unit = {
+    assertThat(MDNReportParser.parse(mdnWithNestedComments(MDNReportParser.MAX_COMMENT_DEPTH)).isSuccess)
+      .isTrue
+  }
+
+  @Test
+  def parseShouldRejectNestedCommentsAboveMaxDepth(): Unit = {
+    assertThat(MDNReportParser.parse(mdnWithNestedComments(MDNReportParser.MAX_COMMENT_DEPTH + 1)).isFailure)
+      .isTrue
+  }
+
+  @Test
+  def parseShouldRejectDeeplyNestedComments(): Unit = {
+    assertThat(MDNReportParser.parse(mdnWithNestedComments(100000)).isFailure)
+      .isTrue
+  }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org