postalsys / imapflow

IMAP Client library for EmailEngine Email API (https://emailengine.app)
https://imapflow.com
Other
350 stars 59 forks source link

Update starttls.js #197

Closed narsingh-pal closed 4 months ago

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

andris9 commented 4 months ago

Could you elaborate what and why you are doing? This change does not seem correct

narsingh-pal commented 4 months ago

Could you elaborate what and why you are doing? This change does not seem correct

If we need to make non-secure connection to the server, then we should not be executing 'STARTTLS' flow. I have below configuration for my client

const client = new ImapFlow({ host: 'xxxxxx', port: 143, secure : false, secureConnection : false, auth: { user: 'xxxxxx', pass: 'xxxxxx' }, });

For the if statement in starttls.js

if (!connection.capabilities.has('STARTTLS') || connection.secureConnection) {

Because connection.capabilities map contains 'STARTTLS' key, it tries to check the next part of the if condition, connection.secureConnection, which is false in my case, hence, code never goes inside if condition.

So we need to negate connection.secureConnection in the above if condition, so that if the client wants to make non-secure connection to the server, it doesnot execute 'STARTTLS' flow

andris9 commented 4 months ago

connection.secureConnection means that the connection already uses TLS. If it is false then the condition should not match which is the correct behavior.

narsingh-pal commented 4 months ago

connection.secureConnection means that the connection already uses TLS. If it is false then the condition should not match which is the correct behavior.

Hi @andris9 -

Could you please help how we can skip 'STARTTLS' execution ?

What is wrong with this config

const client = new ImapFlow({ host: 'xxxxxx', port: 143, secure : false, secureConnection : false, auth: { user: 'xxxxxx', pass: 'xxxxxx' }, });

narsingh-pal commented 4 months ago

connection.secureConnection means that the connection already uses TLS. If it is false then the condition should not match which is the correct behavior.

Hi @andris9 -

Could you please help how we can skip 'STARTTLS' execution ?

What is wrong with this config

const client = new ImapFlow({ host: 'xxxxxx', port: 143, secure : false, secureConnection : false, auth: { user: 'xxxxxx', pass: 'xxxxxx' }, });

in imap-flow.js

        // try to use STARTTLS is possible
        if (!this.secureConnection) {
            await this.upgradeConnection();
        }

Due to which it tries to use STARTTLS even though we don't want to use it

andris9 commented 4 months ago

Yes. This is a feature, not a bug. ImapFlow always tries to use TLS if it is possible.

benbucksch commented 1 month ago

@andris9 There are servers which have a broken STARTTLS implementation. It simply doesn't work. E.g. too old TLS version, or other reasons. I have such a server here, and IMAPFlow completely fails to connect. Yes, the server is broken, but that doesn't help the user. He cannot fix it. With this bug, he cannot get his email at all. There needs to be a way to say that I want to connect without STARTTLS.

Basically, connections can have 3 possible settings:

  1. Use TLS (e.g. IMAP port 993 with TLS) (preferred)
  2. Always use STARTTLS (e.g. IMAP port 143 with STARTTLS). Fail to connect, if STARTTLS is not offered or doesn't work. (only if 1 is not available)
  3. Don't use TLS nor STARTTLS - If the server doesn't support either of those, or doesn't implement them correctly (i.e. only if 1 and 2 doesn't work)

These are the 3 options that Thunderbird offers as well.

What you do is essentially "opportunistic STARTTLS". Thunderbird used to offer "STARTTLS, if available" in the past, but explicitly removed it, because it's not reliable: It works, until an active attacker prevents it from working, and then the attacker gets your password in the clear. That's a known problem since 15 years, and that's why "STARTTLS, if available" or "opportunistic STARTTLS" is a bad idea. There have been scientific papers on the subject, including by the authors of efail, and others. The secure way is: if TLS or STARTTLS are known to work, nail them down, i.e. insist on them or fail.

For servers that have known broken STARTTLS, there needs to be a way to circumvent STARTTLS, after informing the user of the dangers.