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 82fa8fce18c9ee11e36fbdc0c4a571606dfdcb16 Author: Felix Auringer <[email protected]> AuthorDate: Fri Aug 15 07:57:19 2025 +0200 fix(managesieve): small fixes in authentication logic - Authentication is now aborted when session is already authenticated - Authentication is now aborted when client sends "*" - Arguments by the client are now consistently unquoted - If unquotation fails, authentication is aborted to prevent later NullPointerExceptions - Fixed NullPointerException on AuthenticationException - Server now sends correctly formatted response (including code) when waiting for additional authentication arguments - When client sends authentication credentials in second message, the first argument (command) is now interpreted as argument - When choosing mechanism failed, a proper error is returned to the client - When argument is not quoted, authentication is aborted I think with these changes, the server now behaves more like described in the RFC: https://datatracker.ietf.org/doc/html/rfc5804#section-4 --- .../org/apache/james/managesieve/core/CoreProcessor.java | 14 ++++++++++++-- .../managesieve/core/OAUTHAuthenticationProcessor.java | 2 +- .../managesieve/core/PlainAuthenticationProcessor.java | 2 +- .../james/managesieve/transcode/ManageSieveProcessor.java | 13 ++++++++++--- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java index 3417fff3cd..ebb8b7e36d 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/CoreProcessor.java @@ -210,6 +210,9 @@ public class CoreProcessor implements CoreCommands { @Override public String chooseMechanism(Session session, String mechanism) { + if (session.isAuthenticated()) { + return "NO \"already authenticated\""; + } try { if (Strings.isNullOrEmpty(mechanism)) { return "NO ManageSieve syntax is incorrect : You must specify a SASL mechanism as an argument of AUTHENTICATE command"; @@ -234,7 +237,14 @@ public class CoreProcessor implements CoreCommands { try { SupportedMechanism currentAuthenticationMechanism = session.getChoosedAuthenticationMechanism(); AuthenticationProcessor authenticationProcessor = authenticationProcessorMap.get(currentAuthenticationMechanism); - Username authenticatedUsername = authenticationProcessor.isAuthenticationSuccesfull(session, suppliedData); + String unquotedSuppliedData = ParserUtils.unquoteFirst(suppliedData); + if (unquotedSuppliedData == null) { + return "NO \"authentication failed\""; + } + if (unquotedSuppliedData.equals("*")) { + return "NO \"authentication aborted\""; + } + Username authenticatedUsername = authenticationProcessor.isAuthenticationSuccesfull(session, unquotedSuppliedData); if (authenticatedUsername != null) { session.setUser(authenticatedUsername); session.setState(Session.State.AUTHENTICATED); @@ -245,7 +255,7 @@ public class CoreProcessor implements CoreCommands { return "NO authentication failed"; } } catch (AuthenticationException e) { - return "NO Authentication failed with " + e.getCause().getClass() + " : " + e.getMessage(); + return "NO Authentication failed with: " + e.getMessage(); } catch (SyntaxException e) { return "NO ManageSieve syntax is incorrect : " + e.getMessage(); } diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/OAUTHAuthenticationProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/OAUTHAuthenticationProcessor.java index ba925141a8..e424d2759e 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/OAUTHAuthenticationProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/OAUTHAuthenticationProcessor.java @@ -45,7 +45,7 @@ public class OAUTHAuthenticationProcessor implements AuthenticationProcessor { @Override public String initialServerResponse(Session session) { - return "+ \"\""; + return "\"\"\r\nOK"; } @Override diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java index f59daedb9a..72837b37e6 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java @@ -50,7 +50,7 @@ public class PlainAuthenticationProcessor implements AuthenticationProcessor { @Override public String initialServerResponse(Session session) { - return "+ \"\""; + return "\"\"\r\nOK"; } diff --git a/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java b/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java index 94708c1761..d07b323c4d 100644 --- a/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java +++ b/protocols/managesieve/src/main/java/org/apache/james/managesieve/transcode/ManageSieveProcessor.java @@ -86,17 +86,24 @@ public class ManageSieveProcessor { private String matchCommandWithImplementation(Session session, String arguments, String command) throws SessionTerminatedException { if (session.getState() == Session.State.AUTHENTICATION_IN_PROGRESS) { - return argumentParser.authenticate(session, arguments); + return argumentParser.authenticate(session, command); } if (command.equalsIgnoreCase(AUTHENTICATE)) { if (StringUtils.countMatches(arguments, "\"") == 4) { - argumentParser.chooseMechanism(session, arguments); + String result = argumentParser.chooseMechanism(session, arguments); + if (session.getState() != Session.State.AUTHENTICATION_IN_PROGRESS) { + return result; + } int bracket1 = arguments.indexOf('\"'); int bracket2 = arguments.indexOf('\"', bracket1 + 1); int bracket3 = arguments.indexOf('\"', bracket2 + 1); int bracket4 = arguments.indexOf('\"', bracket3 + 1); - return argumentParser.authenticate(session, arguments.substring(bracket3 + 1, bracket4)); + return argumentParser.authenticate(session, arguments.substring(bracket3, bracket4 + 1)); + } else if (arguments.split(" ").length != 1) { + // The client send additional arguments but didn't quote them. It probably thinks that it does not need + // to send more, but the server expects more. Reject this authentication now to solve this conflict. + return "NO \"unquoted argument found\""; } return argumentParser.chooseMechanism(session, arguments); } else if (command.equalsIgnoreCase(CAPABILITY)) { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
