Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c06e1a9d authored by Vincent Breitmoser's avatar Vincent Breitmoser
Browse files

always just send IP instead of hostname in SMTP EHLO message 

Clients are very often behind NATs, which makes the hostname in
HELO/EHLO messages virtually useless these days. Attempting to figure
out a hostname we could use also led to issues with some strict Postfix
configurations (see https://github.com/k9mail/k-9/issues/3387). This
commit changes our behavior to simply send the local IP always, getting
rid of this metadata.

Fixes #3387
parent 8b3d5168

app/k9mail/src/main/java/com/fsck/k9/backends/ImapBackendFactory.kt

+1 −2
Original line number Diff line number Diff line
@@ -11,7 +11,6 @@ import com.fsck.k9.backend.imap.ImapStoreUriDecoder 
import com.fsck.k9.mail.ServerSettings

import com.fsck.k9.mail.oauth.OAuth2TokenProvider

import com.fsck.k9.mail.power.PowerManager

import com.fsck.k9.mail.ssl.DefaultTrustedSocketFactory

import com.fsck.k9.mail.ssl.TrustedSocketFactory

import com.fsck.k9.mail.store.imap.ImapStore

import com.fsck.k9.mail.transport.smtp.SmtpTransport
@@ -50,7 +49,7 @@ class ImapBackendFactory( 
    private fun createSmtpTransport(account: Account): SmtpTransport {

        val serverSettings = decodeTransportUri(account.transportUri)

        val oauth2TokenProvider: OAuth2TokenProvider? = null

        return SmtpTransport(serverSettings, account, trustedSocketFactory, oauth2TokenProvider)

        return SmtpTransport(serverSettings, trustedSocketFactory, oauth2TokenProvider)

    }



    override fun decodeStoreUri(storeUri: String): ServerSettings {

app/k9mail/src/main/java/com/fsck/k9/backends/Pop3BackendFactory.kt

+1 −1
Original line number Diff line number Diff line
@@ -37,7 +37,7 @@ class Pop3BackendFactory( 
    private fun createSmtpTransport(account: Account): SmtpTransport {

        val serverSettings = decodeTransportUri(account.transportUri)

        val oauth2TokenProvider: OAuth2TokenProvider? = null

        return SmtpTransport(serverSettings, account, trustedSocketFactory, oauth2TokenProvider)

        return SmtpTransport(serverSettings, trustedSocketFactory, oauth2TokenProvider)

    }



    override fun decodeStoreUri(storeUri: String): ServerSettings {

mail/protocols/smtp/src/main/java/com/fsck/k9/mail/transport/smtp/SmtpTransport.java

+8 −35
Original line number Diff line number Diff line
@@ -46,7 +46,6 @@ import com.fsck.k9.mail.internet.CharsetSupport; 
import com.fsck.k9.mail.oauth.OAuth2TokenProvider;

import com.fsck.k9.mail.oauth.XOAuth2ChallengeParser;

import com.fsck.k9.mail.ssl.TrustedSocketFactory;

import com.fsck.k9.mail.store.StoreConfig;

import javax.net.ssl.SSLException;

import org.apache.commons.io.IOUtils;

import timber.log.Timber;
@@ -79,10 +78,9 @@ public class SmtpTransport extends Transport { 
    private int largestAcceptableMessage;

    private boolean retryXoauthWithNewToken;

    private boolean isPipeliningSupported;

    private boolean shouldHideHostname;





    public SmtpTransport(ServerSettings serverSettings, StoreConfig storeConfig,

    public SmtpTransport(ServerSettings serverSettings,

            TrustedSocketFactory trustedSocketFactory, OAuth2TokenProvider oauthTokenProvider) {

        if (!serverSettings.type.equals("smtp")) {

            throw new IllegalArgumentException("Expected SMTP StoreConfig!");
@@ -100,7 +98,6 @@ public class SmtpTransport extends Transport { 


        this.trustedSocketFactory = trustedSocketFactory;

        this.oauthTokenProvider = oauthTokenProvider;

        this.shouldHideHostname = storeConfig.shouldHideHostname();

    }



    @Override
@@ -304,30 +301,16 @@ public class SmtpTransport extends Transport { 
        }

    }



    private String buildHostnameToReport() {

        if (shouldHideHostname) {

            return "localhost";

        }

    @VisibleForTesting

    protected String buildHostnameToReport() {

        InetAddress localAddress = socket.getLocalAddress();

        String localHostname = getCanonicalHostName(localAddress);

        String ipAddr = getHostAddress(localAddress);

        String ipAddr = localAddress.getHostAddress();



        if (localHostname.equals("") || localHostname.equals(ipAddr) || localHostname.contains("_")) {

            // We don't have a FQDN or the hostname contains invalid

            // characters (see issue 2143), so use IP address.

            if (!ipAddr.equals("")) {

        if (localAddress instanceof Inet6Address) {

            return "[IPv6:" + ipAddr + "]";

        } else {

            return "[" + ipAddr + "]";

        }

            } else {

                // If the IP address is no good, set a sane default

                return "android";

            }

        } else {

            return localHostname;

        }

    }



    private void parseOptionalSizeValue(Map<String, String> extensions) {
@@ -826,16 +809,6 @@ public class SmtpTransport extends Transport { 
        executeCommand("AUTH EXTERNAL %s", Base64.encode(username));

    }



    @VisibleForTesting

    protected String getCanonicalHostName(InetAddress localAddress) {

        return localAddress.getCanonicalHostName();

    }



    @VisibleForTesting

    protected String getHostAddress(InetAddress localAddress) {

        return localAddress.getHostAddress();

    }



    public void checkSettings() throws MessagingException {

        close();

        try {

mail/protocols/smtp/src/test/java/com/fsck/k9/mail/transport/smtp/SmtpTransportTest.java

+35 −93
Original line number Diff line number Diff line 
package com.fsck.k9.mail.transport.smtp;





import java.net.InetAddress;



import com.fsck.k9.mail.AuthType;

import com.fsck.k9.mail.AuthenticationFailedException;

import com.fsck.k9.mail.CertificateValidationException;
@@ -57,63 +55,15 @@ public class SmtpTransportTest { 
    }



    @Test

    public void open_withShouldHideHostnameTrue_shouldProvideLocalhost() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.output("250-localhost Hello client.localhost");

        server.output("250 OK");

        when(storeConfig.shouldHideHostname()).thenReturn(true);

        SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE, null,

                "private.host.org", "127.0.0.1");



        transport.open();



        server.verifyConnectionStillOpen();

        server.verifyInteractionCompleted();

    }



    @Test

    public void open_withShouldHideHostnameFalse_shouldProvideHostname() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO visible.host.org");

        server.output("250-localhost Hello client.localhost");

        server.output("250 OK");

        SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE, null,

                "visible.host.org", "127.0.0.1");



        transport.open();



        server.verifyConnectionStillOpen();

        server.verifyInteractionCompleted();

    }



    @Test

    public void open_withEmptyHostname_shouldProvideIPAddress() throws Exception {

    public void open__shouldProvideHostname() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 OK");

        SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE, null,

                "", "127.0.0.1");



        transport.open();



        server.verifyConnectionStillOpen();

        server.verifyInteractionCompleted();

    }



    @Test

    public void open_withEmptyHostnameAndIP_shouldProvideSensibleDefault() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO android");

        server.output("250-localhost Hello client.localhost");

        server.output("250 OK");

        SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE, null,

                "", "");

        when(storeConfig.shouldHideHostname()).thenReturn(true);

        SmtpTransport transport = startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE,

                null, "[127.0.0.1]");



        transport.open();
@@ -125,7 +75,7 @@ public class SmtpTransportTest { 
    public void open_withoutAuthLoginExtension_shouldConnectWithoutAuthentication() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 OK");

        SmtpTransport transport = startServerAndCreateSmtpTransportWithoutPassword(server);
@@ -140,7 +90,7 @@ public class SmtpTransportTest { 
    public void open_withAuthPlainExtension() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH PLAIN LOGIN");

        server.expect("AUTH PLAIN AHVzZXIAcGFzc3dvcmQ=");
@@ -157,7 +107,7 @@ public class SmtpTransportTest { 
    public void open_withAuthLoginExtension() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH LOGIN");

        server.expect("AUTH LOGIN");
@@ -178,7 +128,7 @@ public class SmtpTransportTest { 
    public void open_withoutLoginAndPlainAuthExtensions_shouldThrow() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH");

        server.expect("QUIT");
@@ -200,7 +150,7 @@ public class SmtpTransportTest { 
    public void open_withCramMd5AuthExtension() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH CRAM-MD5");

        server.expect("AUTH CRAM-MD5");
@@ -219,7 +169,7 @@ public class SmtpTransportTest { 
    public void open_withoutCramMd5AuthExtension_shouldThrow() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH PLAIN LOGIN");

        server.expect("QUIT");
@@ -241,7 +191,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -258,7 +208,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldThrowOn401Response() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -291,7 +241,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldInvalidateAndRetryOn400Response() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -317,7 +267,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldInvalidateAndRetryOnInvalidJsonResponse() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -343,7 +293,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldInvalidateAndRetryOnMissingStatusJsonResponse() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -369,7 +319,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldThrowOnMultipleFailure() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("AUTH XOAUTH2 dXNlcj11c2VyAWF1dGg9QmVhcmVyIG9sZFRva2VuAQE=");
@@ -405,7 +355,7 @@ public class SmtpTransportTest { 
    public void open_withXoauth2Extension_shouldThrowOnFailure_fetchingToken() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH XOAUTH2");

        server.expect("QUIT");
@@ -429,7 +379,7 @@ public class SmtpTransportTest { 
    public void open_withoutXoauth2Extension_shouldThrow() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH PLAIN LOGIN");

        server.expect("QUIT");
@@ -451,7 +401,7 @@ public class SmtpTransportTest { 
    public void open_withAuthExternalExtension() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH EXTERNAL");

        server.expect("AUTH EXTERNAL dXNlcg==");
@@ -468,7 +418,7 @@ public class SmtpTransportTest { 
    public void open_withoutAuthExternalExtension_shouldThrow() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH");

        server.expect("QUIT");
@@ -491,7 +441,7 @@ public class SmtpTransportTest { 
            throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH CRAM-MD5");

        server.expect("AUTH CRAM-MD5");
@@ -511,7 +461,7 @@ public class SmtpTransportTest { 
    public void open_withAutomaticAuthAndNoTransportSecurityAndAuthPlainExtension_shouldThrow() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250 AUTH PLAIN LOGIN");

        server.expect("QUIT");
@@ -535,9 +485,9 @@ public class SmtpTransportTest { 
    public void open_withEhloFailing_shouldTryHelo() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("502 5.5.1, Unrecognized command.");

        server.expect("HELO localhost");

        server.expect("HELO [127.0.0.1]");

        server.output("250 localhost");

        SmtpTransport transport = startServerAndCreateSmtpTransportWithoutPassword(server);
@@ -552,7 +502,7 @@ public class SmtpTransportTest { 
            throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        server.output("250-ENHANCEDSTATUSCODES");

        server.output("250 AUTH XOAUTH2");
@@ -586,7 +536,7 @@ public class SmtpTransportTest { 
    public void open_withManyExtensions_shouldParseAll() throws Exception {

        MockSmtpServer server = new MockSmtpServer();

        server.output("220 smtp.gmail.com ESMTP x25sm19117693wrx.27 - gsmtp");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-smtp.gmail.com at your service, [86.147.34.216]");

        server.output("250-SIZE 35882577");

        server.output("250-8BITMIME");
@@ -913,17 +863,17 @@ public class SmtpTransportTest { 


    private SmtpTransport startServerAndCreateSmtpTransportWithoutPassword(MockSmtpServer server) throws Exception {

        return startServerAndCreateSmtpTransport(server, AuthType.PLAIN, ConnectionSecurity.NONE, null,

                "localhost", "127.0.0.1");

                "[127.0.0.1]");

    }



    private SmtpTransport startServerAndCreateSmtpTransport(MockSmtpServer server, AuthType authenticationType,

            ConnectionSecurity connectionSecurity) throws Exception {

        return startServerAndCreateSmtpTransport(server, authenticationType, connectionSecurity, PASSWORD,

                "localhost", "127.0.0.1");

                "[127.0.0.1]");

    }



    private SmtpTransport startServerAndCreateSmtpTransport(MockSmtpServer server, AuthType authenticationType,

            ConnectionSecurity connectionSecurity, String password, String injectedHostname, String injectedIP)

            ConnectionSecurity connectionSecurity, String password, String injectedHostname)

            throws Exception {

        server.start();
@@ -939,8 +889,7 @@ public class SmtpTransportTest { 
                password,

                CLIENT_CERTIFICATE_ALIAS);



        return new TestSmtpTransport(serverSettings, storeConfig, socketFactory, oAuth2TokenProvider, injectedHostname,

                injectedIP);

        return new TestSmtpTransport(serverSettings, socketFactory, oAuth2TokenProvider, injectedHostname);

    }



    private TestMessageBuilder getDefaultMessageBuilder() {
@@ -964,7 +913,7 @@ public class SmtpTransportTest { 
        MockSmtpServer server = new MockSmtpServer();

        

        server.output("220 localhost Simple Mail Transfer Service Ready");

        server.expect("EHLO localhost");

        server.expect("EHLO [127.0.0.1]");

        server.output("250-localhost Hello client.localhost");

        

        for (String extension : extensions) {
@@ -981,24 +930,17 @@ public class SmtpTransportTest { 
    

    static class TestSmtpTransport extends SmtpTransport {

        private final String injectedHostname;

        private final String injectedIP;



        TestSmtpTransport(ServerSettings serverSettings, StoreConfig storeConfig,

        TestSmtpTransport(ServerSettings serverSettings,

                TrustedSocketFactory trustedSocketFactory, OAuth2TokenProvider oAuth2TokenProvider,

                String injectedHostname, String injectedIP)  {

            super(serverSettings, storeConfig, trustedSocketFactory, oAuth2TokenProvider);

                String injectedHostname)  {

            super(serverSettings, trustedSocketFactory, oAuth2TokenProvider);

            this.injectedHostname = injectedHostname;

            this.injectedIP = injectedIP;

        }



        @Override

        protected String getCanonicalHostName(InetAddress localAddress) {

        protected String buildHostnameToReport() {

            return injectedHostname;

        }



        @Override

        protected String getHostAddress(InetAddress localAddress) {

            return injectedIP;

        }

    }

}