Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client anonymous authentication not working when sending empty trace string #258

Closed
albertony opened this issue Apr 10, 2024 · 9 comments · Fixed by #259
Closed

Client anonymous authentication not working when sending empty trace string #258

albertony opened this issue Apr 10, 2024 · 9 comments · Fixed by #259

Comments

@albertony
Copy link
Contributor

Example code:

client.Auth(sasl.NewAnonymousClient(""))

Output from a test server:

220 localhost ESMTP Service Ready
EHLO localhost
250-Hello localhost
250-PIPELINING
250-8BITMIME
250-ENHANCEDSTATUSCODES
250-CHUNKING
250-AUTH ANONYMOUS
250-SIZE 1024
250 LIMITS RCPTMAX=10
AUTH ANONYMOUS
334
*
501 5.0.0 Negotiation cancelled
smtp/server 2024/04/10 19:25:09 handler error: read tcp 127.0.0.1:10025->127.0.0.1:58986: wsarecv: An existing connection was forcibly closed by the remote host.

Output from a test client:

220 localhost ESMTP Service Ready
EHLO localhost
250-Hello localhost
250-PIPELINING
250-8BITMIME
250-ENHANCEDSTATUSCODES
250-CHUNKING
250-AUTH ANONYMOUS
250-SIZE 1024
250 LIMITS RCPTMAX=10
AUTH ANONYMOUS
334
*
501 5.0.0 Negotiation cancelled
2024/04/10 19:25:09 sasl: unexpected server challenge
@emersion
Copy link
Owner

An empty trace (argument passed to sasl.NewAnonymousClient) is invalid per the RFC.

@albertony
Copy link
Contributor Author

Right. Thanks for the pointer.

If I'm not mistaken: With plain authentication, username and password are also required to be non-empty according to the RFC, however it is possible with the client to send all empty strings (sasl.NewPlainClient("", "", "")) and a (non-conforming?) server may choose to accept it. So I found it strange that the trace string with anonymous authentication, which bears no semantical value is not used for authentication, cannot be empty as well...

@emersion
Copy link
Owner

The SASL PLAIN RFC says that the identity is optional but username/password are required.

Oh, hang on, I missed the square brackets in the ANONYMOUS RFC: it seems like the trace is optional after all!

@emersion emersion reopened this Apr 11, 2024
@albertony
Copy link
Contributor Author

The SASL PLAIN RFC says that the identity is optional but username/password are required.

Yes, that was what I was referring to. I read it correct then. Your client api does not enforce it, which is fine, leaving it up to implementation of client and server to conform.

Oh, hang on, I missed the square brackets in the ANONYMOUS RFC: it seems like the trace is optional after all!

Aha. I missed that too after studying the link you referred to!

@emersion
Copy link
Owner

So I think the issue is that we don't implement this bit of the SMTP AUTH RFC:

If the client is transmitting an initial response of zero
length, it MUST instead transmit the response as a single
equals sign ("="). This indicates that the response is
present, but contains no data.

Would you be willing to try that out and send a patch?

@albertony
Copy link
Contributor Author

Sure. I will do that.

albertony added a commit to albertony/go-smtp that referenced this issue Apr 11, 2024
With anonymous authentication according to RFC4505 the trace information string is
optional, and SMTP authentication extension described in RFC4954 states that:

    If the client is transmitting an initial response of zero
    length, it MUST instead transmit the response as a single
    equals sign ("=").  This indicates that the response is
    present, but contains no data.

Fixes emersion#258
@albertony
Copy link
Contributor Author

I've reached sort of a road block here, or at least a speed bump...

I have implemented what I think will make the client and server more conformant; client sending AUTH ANONYMOUS = if initial response is empty, and server handling that as an empty initial response from client.

By the way; This have effect for anonymous authentication, when sending empty trace message, but it does not affect plain authentication even if empty string is supplied for identity, username and password. That is because they are joined with a NUL character as separator before encoded to base64 and used in the auth command, and therefore when all are empty the result is a string of two NULs which encodes to base64 string "AAA=", so it does not end up in the same case as originally described in this issue.

However, my changes do not solve my original case. I'm able to fix that by changing the go-sasl anonymous implementation, as suggested here:

That makes my case with a go-smtp based client and server work with anonymous authentication with empty trace string - with or without my patch #259 in fact.

@albertony
Copy link
Contributor Author

Progress. Based on the following explanation from the go-sasl PR I think I will be able to solve this with the go-sasl change only, after an update to the go-sasl PR:

There is a subtle detail in the go-sasl API here: response = nil means no initial response, and response = []byte{} (a non-nil empty slice) means that the initial response was sent but was empty. (See the doc comment for Server.)

(emersion/go-sasl#27 (comment))

@albertony
Copy link
Contributor Author

I've updated #259, force pushed a change that seem to fix this issue on its own (without the go-sasl change). Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants