emersion / go-imap

📥 An IMAP library for clients and servers
MIT License
2.07k stars 291 forks source link

feature: don't request CAPABILITY after auth/login if they are returned in auth response #561

Open Rikkuru opened 10 months ago

Rikkuru commented 10 months ago

imapclients invalidates capabilities on login/auth/starttls as it should but login and auth often send capabilities in response, those can be used but they are cleaned too.

From rfc - https://datatracker.ietf.org/doc/rfc9051/

A server MAY include a CAPABILITY response code in the tagged OK response to a successful LOGIN command in order to send capabilities automatically. It is unnecessary for a client to send a separate CAPABILITY command if it recognizes these automatic capabilities.

Here is debug log that demonstrates the problem (with creds removed) - T3 request can be avoided T1 CAPABILITY\r\n* CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID AUTH=PLAIN AUTH=XOAUTH2 IDLE MOVE\r\nT1 OK CAPABILITY Completed.\r\nT2 LOGIN "diana@example.com" "password"\r\n* CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID IDLE MOVE\r\nT2 OK LOGIN Completed.\r\nT3 CAPABILITY\r\n* CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID IDLE MOVE\r\nT3 OK CAPABILITY Completed.

emersion commented 10 months ago

This is v2 right?

v2 already tries to avoid requesting CAPABILITY if unnecessary, the issue is when something requires capabilities before the server sends them on its own. For instance, if a fresh client is created and then Authenticate is used. Authenticate checks whether the server supports SASL-IR, however a fresh client hasn't waited for the greeting, and we have no guarantee that the server will send the capabilities in its greeting. If we don't request capabilities at this point, then we'll potentially incur a cost of two roundtrips before sending the AUTHENTICATE command.

Which specific commands are you using that trigger the CAPABILITIES request? AFAIU LOGIN shouldn't trigger it.

Rikkuru commented 10 months ago

This is v2.

I think capabilities in response are parsed and then invalideted in the next line from server (when auth/login command ends ) At least i don't see what would prevent it from happening

T2 LOGIN "diana@example.com" "password"
* CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID IDLE MOVE # here handleCapability from readResponseData
T2 OK LOGIN Completed. // here invalidate the capabilities from readResponseTagged
T3 CAPABILITY
* CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID IDLE MOVE

Here is what i was doing on login , later i call Caps to deside on options for List call

    if client.Caps().Has(goimap.CapStartTLS) {
        // good servers dont allow login and other plain auth methods without tls
        err = client.StartTLS(tlsConfig())
        if err != nil {
            return fmt.Errorf("StartTLS: %w", err)
        }
    }

    err = client.WaitGreeting()
    if err != nil {
        return fmt.Errorf("WaitGreeting: %w", err)
    }

    if client.Caps().Has(goimap.CapLoginDisabled) {
        return ErrLoginDisabled
        return client.Login(b.connSettings.Login, b.connSettings.Secret).Wait()
emersion commented 10 months ago

You need to WaitGreeting before the STARTTLS stuff. That should help, I think?

Rikkuru commented 10 months ago

oh sorry this was a tls conn so starttls branch should not be here

updated flow is just create client , waitgreeting, check logindisabled (this code is common for tls and plain conn ) , login

LOGINDISABLED capability check is what initiated T1 capability command then we login T3 is initiated by next capability check after login wait

        client, err = dialTLS(
            host, port,
            timeout,
            tlsConfig(),
        )
        if err != nil {
            return fmt.Errorf("dialTLS: %w", err)
        }
        err = client.WaitGreeting()
        if err != nil {
            return fmt.Errorf("WaitGreeting: %w", err)
        }

        if client.Caps().Has(goimap.CapLoginDisabled) { // T1
            return ErrLoginDisabled
        return client.Login(b.connSettings.Login, b.connSettings.Secret).Wait() // T2

next list calls Caps

    if client.Caps().Has(goimap.CapSpecialUse) { // T3
        listOptions.ReturnSpecialUse = true
    }

I can write a complite example that reproduces the problem later ( gmail returns capabilities in login for example )

emersion commented 10 months ago

Ah, OK, so the problem is not the CapLoginDisabled check. GMail doesn't return the capabilities in its greeting.

The problem is that the capabilities are sent in a response separate from the OK from LOGIN.

Section 7.2.2 says:

A server MAY send capabilities automatically, by using the CAPABILITY response code in the initial PREAUTH or OK responses and by sending an updated CAPABILITY response code in the tagged OK response as part of a successful authentication.

However GMail doesn't comply with this.

Rikkuru commented 10 months ago

so they mean server should send CAPABILITY response code in LOGIN response ? like so ?

C: T1 LOGIN foo bar
S: T1 OK CAPABILITY IMAP4rev2 IDLE

seems wrong

emersion commented 10 months ago

Like this:

T2 OK [CAPABILITY IMAP4rev1 CHILDREN UNSELECT LITERAL+ NAMESPACE XLIST UIDPLUS ENABLE ID IDLE MOVE] LOGIN Completed.
Rikkuru commented 10 months ago

I found an example of this

1 LOGIN user pass
1 OK [CAPABILITY IMAP4REV1 UIDPLUS] Logged in.

Ok this seems like what they mean by this If you know where in rfc they explain those square brackets pls let me know too )

emersion commented 10 months ago

Section 7.1 describes these.

strarsis commented 6 days ago
1 LOGIN user pass
1 OK [CAPABILITY IMAP4REV1 UIDPLUS] Logged in.

This is typical for Dovecot IMAP, which is one of the most used IMAP servers. https://github.com/dovecot/core/blob/2.3.21/src/imap/main.c#L214