felixauringer commented on code in PR #2773:
URL: https://github.com/apache/james-project/pull/2773#discussion_r2509746751
##########
protocols/managesieve/src/main/java/org/apache/james/managesieve/core/PlainAuthenticationProcessor.java:
##########
@@ -50,7 +50,7 @@ public PlainAuthenticationProcessor(UsersRepository
usersRepository) {
@Override
public String initialServerResponse(Session session) {
- return "+ \"\"";
+ return "\"\"\r\nOK";
Review Comment:
I do not understand where this `+ ""` comes from but I didn't find it in any
of the RFCs relating to ManageSieve SASL authentication.
There is one mention in the [SASL
RFC](https://www.rfc-editor.org/rfc/rfc4422#appendix-A) but only for the
`EXTERNAL` mechanism which we do not use here.
The in my opinion relevant RFC here is [RFC 5804: A Protocol for Remotely
Managing Sieve
Scripts](https://datatracker.ietf.org/doc/html/rfc5804#section-4). The formal
syntax there defines the response to an authenticate command as follows:
```
response-authenticate = *(string CRLF)
((response-ok [response-capability]) /
response-nobye)
;; <response-capability> is REQUIRED if a
;; SASL security layer was negotiated and
;; MUST be omitted otherwise.
```
If we want continuation, we want to have a `response-ok` and because we do
not negotiate a SASL security layer, we must omit `response-capability`.
It then simplifies to `response-authenticate = *(string CRLF) response-ok`.
Assuming we do not want to have a response code or description, it further
simplifies to `response-authenticate = *(string CRLF) "OK" CRLF`.
The [description](https://datatracker.ietf.org/doc/html/rfc5804#section-2.1)
says:
> Note that an empty challenge/response is sent as an empty string.
With all our supported authentication mechanisms, the challenge sent by the
server is empty, so I would argue that the server should send the empty string.
Sadly, the examples in the RFC do not show this case. Using the formal syntax
from above, this leads to `response-authenticate = DQUOTE DQUOTE CRLF "OK"
CRLF`. (One could probably also use the literal string variant without quotes
but by supplying the exact string length. However, I would argue that it does
not make much sense with an empty string.)
That's the reason why I changed it. In the RFC, I do not see any possibility
how an (unquoted!) `+` sign could be the response to an authenticate command. I
also didn't find why this server response shouldn't end with an `OK`, but I'm
less sure about that (the examples seem to omit it in the challenge-response
exchanges but I don't think the formal syntax allows it). What's the reason why
you use the `+` here? Is there another RFC that I do not know?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
