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/12/30 02:02:54 UTC
[james-project] branch master updated (1b7b4fa96d -> 9a5075448f)
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
from 1b7b4fa96d JAMES-3869 SPF mailet: fix typos and use mockito in tests
new e72a891e85 JAMES-3868 Cannot handle IMAP PLAIN login with password longer than 255 char
new 9a5075448f JAMES-3868 Refactor AuthCmdHandler.doPlainAuthPass when parsing token && add more test cases
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.
Summary of changes:
.../imap/processor/AuthenticateProcessor.java | 22 +++---
.../protocols/smtp/core/esmtp/AuthCmdHandler.java | 85 ++++++++--------------
.../james/imapserver/netty/IMAPServerTest.java | 15 +++-
.../apache/james/smtpserver/SMTPServerTest.java | 43 +++++++++++
4 files changed, 99 insertions(+), 66 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 01/02: JAMES-3868 Cannot handle IMAP PLAIN login with password longer than 255 char
Posted by bt...@apache.org.
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 e72a891e853d8f3778a86387cbdee25de9adfd5b
Author: Tung Van TRAN <vt...@linagora.com>
AuthorDate: Tue Dec 13 11:06:36 2022 +0700
JAMES-3868 Cannot handle IMAP PLAIN login with password longer than 255 char
---
.../imap/processor/AuthenticateProcessor.java | 22 +++++++++++-----------
.../james/imapserver/netty/IMAPServerTest.java | 15 +++++++++++++--
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
index 415f25b27e..cfab921d05 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/AuthenticateProcessor.java
@@ -21,10 +21,11 @@ package org.apache.james.imap.processor;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Base64;
import java.util.List;
import java.util.Optional;
-import java.util.StringTokenizer;
+import java.util.stream.Collectors;
import javax.inject.Inject;
@@ -48,6 +49,7 @@ import org.apache.james.util.MDCBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import reactor.core.publisher.Mono;
@@ -132,15 +134,13 @@ public class AuthenticateProcessor extends AbstractAuthProcessor<AuthenticateReq
}
private AuthenticationAttempt parseDelegationAttempt(String initialClientResponse) {
- String token2;
try {
String userpass = new String(Base64.getDecoder().decode(initialClientResponse));
- StringTokenizer authTokenizer = new StringTokenizer(userpass, "\0");
- String token1 = authTokenizer.nextToken(); // Authorization Identity
- token2 = authTokenizer.nextToken(); // Authentication Identity
- try {
- return delegation(Username.of(token1), Username.of(token2), authTokenizer.nextToken());
- } catch (java.util.NoSuchElementException ignored) {
+ List<String> tokens = Arrays.stream(userpass.split("\0"))
+ .filter(token -> !token.isBlank())
+ .collect(Collectors.toList());
+ Preconditions.checkArgument(tokens.size() == 2 || tokens.size() == 3);
+ if (tokens.size() == 2) {
// If we got here, this is what happened. RFC 2595
// says that "the client may leave the authorization
// identity empty to indicate that it is the same as
@@ -156,9 +156,9 @@ public class AuthenticateProcessor extends AbstractAuthProcessor<AuthenticateReq
// elements, leading to the exception we just
// caught. So we need to move the user to the
// password, and the authorize_id to the user.
- return noDelegation(Username.of(token1), token2);
- } finally {
- authTokenizer = null;
+ return noDelegation(Username.of(tokens.get(0)), tokens.get(1));
+ } else {
+ return delegation(Username.of(tokens.get(0)), Username.of(tokens.get(1)), tokens.get(2));
}
} catch (Exception e) {
// Ignored - this exception in parsing will be dealt
diff --git a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
index 3b4461e2cf..a0c0ace750 100644
--- a/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
+++ b/server/protocols/protocols-imap4/src/test/java/org/apache/james/imapserver/netty/IMAPServerTest.java
@@ -71,7 +71,6 @@ import org.apache.commons.net.imap.IMAPSClient;
import org.apache.james.core.Username;
import org.apache.james.imap.encode.main.DefaultImapEncoderFactory;
import org.apache.james.imap.main.DefaultImapDecoderFactory;
-import org.apache.james.imap.processor.AppendProcessor;
import org.apache.james.imap.processor.base.AbstractProcessor;
import org.apache.james.imap.processor.main.DefaultImapProcessorFactory;
import org.apache.james.jwt.OidcTokenFixture;
@@ -140,6 +139,7 @@ class IMAPServerTest {
private static final String USER_PASS = "pass";
public static final String SMALL_MESSAGE = "header: value\r\n\r\nBODY";
private InMemoryIntegrationResources memoryIntegrationResources;
+ private FakeAuthenticator authenticator;
@RegisterExtension
public TestIMAPClient testIMAPClient = new TestIMAPClient();
@@ -179,7 +179,7 @@ class IMAPServerTest {
}
private IMAPServer createImapServer(HierarchicalConfiguration<ImmutableNode> config) throws Exception {
- FakeAuthenticator authenticator = new FakeAuthenticator();
+ authenticator = new FakeAuthenticator();
authenticator.addUser(USER, USER_PASS);
authenticator.addUser(USER2, USER_PASS);
authenticator.addUser(USER3, USER_PASS);
@@ -993,6 +993,17 @@ class IMAPServerTest {
.doesNotContain("LOGINDISABLED")
.contains("AUTH=PLAIN");
}
+
+ @Test
+ void authenticatePlainShouldSucceedWhenPasswordHasMoreThan255Characters() {
+ Username user1 = Username.of("user1@domain.org");
+ String user1Password = "1".repeat(300);
+ authenticator.addUser(user1, user1Password);
+ assertThatCode(() ->
+ testIMAPClient.connect("127.0.0.1", port)
+ .authenticatePlain(user1.asString(), user1Password))
+ .doesNotThrowAnyException();
+ }
}
@Nested
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[james-project] 02/02: JAMES-3868 Refactor AuthCmdHandler.doPlainAuthPass when parsing token && add more test cases
Posted by bt...@apache.org.
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 9a5075448fcbad3243b097f84c42ba39b232c490
Author: Tung Van TRAN <vt...@linagora.com>
AuthorDate: Wed Dec 14 17:22:02 2022 +0700
JAMES-3868 Refactor AuthCmdHandler.doPlainAuthPass when parsing token && add more test cases
---
.../protocols/smtp/core/esmtp/AuthCmdHandler.java | 85 ++++++++--------------
.../apache/james/smtpserver/SMTPServerTest.java | 43 +++++++++++
2 files changed, 75 insertions(+), 53 deletions(-)
diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/AuthCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/AuthCmdHandler.java
index c476c95139..0a39893cbe 100644
--- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/AuthCmdHandler.java
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/esmtp/AuthCmdHandler.java
@@ -23,14 +23,15 @@ package org.apache.james.protocols.smtp.core.esmtp;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Base64;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
-import java.util.StringTokenizer;
import java.util.function.Function;
+import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.james.core.Username;
@@ -54,6 +55,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -241,62 +243,39 @@ public class AuthCmdHandler
* @param line the initial response line passed in with the AUTH command
*/
private Response doPlainAuthPass(SMTPSession session, String line) {
- String user = null;
- String pass = null;
try {
- String userpass = decodeBase64(line);
- if (userpass != null) {
- /* See: RFC 2595, Section 6
- The mechanism consists of a single message from the client to the
- server. The client sends the authorization identity (identity to
- login as), followed by a US-ASCII NUL character, followed by the
- authentication identity (identity whose password will be used),
- followed by a US-ASCII NUL character, followed by the clear-text
- password. The client may leave the authorization identity empty to
- indicate that it is the same as the authentication identity.
-
- The server will verify the authentication identity and password with
- the system authentication database and verify that the authentication
- credentials permit the client to login as the authorization identity.
- If both steps succeed, the user is logged in.
- */
- StringTokenizer authTokenizer = new StringTokenizer(userpass, "\0");
- String authorizeId = authTokenizer.nextToken(); // Authorization Identity
- user = authTokenizer.nextToken(); // Authentication Identity
- try {
- pass = authTokenizer.nextToken(); // Password
- } catch (java.util.NoSuchElementException ignored) {
- // If we got here, this is what happened. RFC 2595
- // says that "the client may leave the authorization
- // identity empty to indicate that it is the same as
- // the authentication identity." As noted above,
- // that would be represented as a decoded string of
- // the form: "\0authenticate-id\0password". The
- // first call to nextToken will skip the empty
- // authorize-id, and give us the authenticate-id,
- // which we would store as the authorize-id. The
- // second call will give us the password, which we
- // think is the authenticate-id (user). Then when
- // we ask for the password, there are no more
- // elements, leading to the exception we just
- // caught. So we need to move the user to the
- // password, and the authorize_id to the user.
- pass = user;
- user = authorizeId;
- }
-
- authTokenizer = null;
+ List<String> tokens = Optional.ofNullable(decodeBase64(line))
+ .map(userpass1 -> Arrays.stream(userpass1.split("\0"))
+ .filter(token -> !token.isBlank())
+ .collect(Collectors.toList()))
+ .orElse(List.of());
+ Preconditions.checkArgument(tokens.size() == 2 || tokens.size() == 3);
+ Response response = null;
+ if (tokens.size() == 2) {
+ // If we got here, this is what happened. RFC 2595
+ // says that "the client may leave the authorization
+ // identity empty to indicate that it is the same as
+ // the authentication identity." As noted above,
+ // that would be represented as a decoded string of
+ // the form: "\0authenticate-id\0password". The
+ // first call to nextToken will skip the empty
+ // authorize-id, and give us the authenticate-id,
+ // which we would store as the authorize-id. The
+ // second call will give us the password, which we
+ // think is the authenticate-id (user). Then when
+ // we ask for the password, there are no more
+ // elements, leading to the exception we just
+ // caught. So we need to move the user to the
+ // password, and the authorize_id to the user.
+ response = doAuthTest(session, Username.of(tokens.get(0)), tokens.get(1), AUTH_TYPE_PLAIN);
+ } else {
+ response = doAuthTest(session, Username.of(tokens.get(1)), tokens.get(2), AUTH_TYPE_PLAIN);
}
+ session.popLineHandler();
+ return response;
} catch (Exception e) {
- // Ignored - this exception in parsing will be dealt
- // with in the if clause below
+ return new SMTPResponse(SMTPRetCode.SYNTAX_ERROR_ARGUMENTS,"Could not decode parameters for AUTH PLAIN");
}
- // Authenticate user
- Response response = doAuthTest(session, Username.of(user), pass, "PLAIN");
-
- session.popLineHandler();
-
- return response;
}
private String decodeBase64(String line) {
diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
index 01a880a147..fbde0a3f89 100644
--- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
+++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
@@ -1380,6 +1380,49 @@ public class SMTPServerTest {
.isNotNull();
}
+ @Test
+ public void testAuthShouldSucceedWhenPasswordHasMoreThan255Characters() throws Exception {
+ smtpConfiguration.setAuthorizedAddresses("128.0.0.1/8");
+ smtpConfiguration.setAuthorizingAnnounce();
+ init(smtpConfiguration);
+
+ SMTPClient smtpProtocol = new SMTPClient();
+ InetSocketAddress bindedAddress = new ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+ smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), bindedAddress.getPort());
+
+ smtpProtocol.sendCommand("ehlo", InetAddress.getLocalHost().toString());
+ String userName = USER_LOCALHOST;
+ String userPassword = "1".repeat(300);
+ usersRepository.addUser(Username.of(userName), userPassword);
+
+ smtpProtocol.sendCommand("AUTH PLAIN");
+ smtpProtocol.sendCommand(Base64.getEncoder().encodeToString(("\0" + userName + "\0" + userPassword + "\0").getBytes(UTF_8)));
+ assertThat(smtpProtocol.getReplyCode())
+ .as("authenticated")
+ .isEqualTo(235);
+ }
+
+ @Test
+ public void testAuthShouldFailedWhenUserPassIsNotBase64Decoded() throws Exception {
+ smtpConfiguration.setAuthorizedAddresses("128.0.0.1/8");
+ smtpConfiguration.setAuthorizingAnnounce();
+ init(smtpConfiguration);
+
+ SMTPClient smtpProtocol = new SMTPClient();
+ InetSocketAddress bindedAddress = new ProtocolServerUtils(smtpServer).retrieveBindedAddress();
+ smtpProtocol.connect(bindedAddress.getAddress().getHostAddress(), bindedAddress.getPort());
+
+ smtpProtocol.sendCommand("ehlo", InetAddress.getLocalHost().toString());
+ String userName = USER_LOCALHOST;
+ String userPassword = "1".repeat(300);
+ usersRepository.addUser(Username.of(userName), userPassword);
+
+ smtpProtocol.sendCommand("AUTH PLAIN");
+ smtpProtocol.sendCommand("canNotDecode");
+ assertThat(smtpProtocol.getReplyString())
+ .contains("501 Could not decode parameters for AUTH PLAIN");
+ }
+
@Test
public void testAuthSendMailFromAlias() throws Exception {
smtpConfiguration.setAuthorizedAddresses("128.0.0.1/8");
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org