eclipse-ee4j / angus-mail

Angus Mail
https://eclipse-ee4j.github.io/angus-mail
Other
58 stars 16 forks source link

Authenticator does not use credentials. user=root, password=<null> #109

Closed Osiris-Team closed 1 year ago

Osiris-Team commented 1 year ago

Describe the bug / To Reproduce

            Properties props = new Properties();
            props.put("mail.smtp.auth", true);
            props.put("mail.smtp.starttls.enable", "true");
            props.put("mail.smtp.host", Config.master.mailHost);
            props.put("mail.smtp.port", Config.master.mailPort);
            props.put("mail.smtp.ssl.trust", Config.master.mailHost);
            AL.info(props.toString());

            session = Session.getInstance(props, new Authenticator() {
                @Override
                protected PasswordAuthentication getPasswordAuthentication() {
                    return new PasswordAuthentication(Config.master.mailUser, Config.master.mailPassword);
                }
            });
            session.setDebug(true);
DEBUG: getProvider() returning jakarta.mail.Provider[TRANSPORT,smtp,org.eclipse.angus.mail.smtp.SMTPTransport,Oracle]
DEBUG SMTP: need username and password for authentication
DEBUG SMTP: protocolConnect returning false, host=localhost, user=root, password=<null>
DEBUG SMTP: useEhlo true, useAuth true
DEBUG SMTP: trying to connect to host "localhost", port 465, isSSL false

The user is not root and the password is not null, no clue where it's getting that info from. I already checked the values I pass over, they are correct, something seems to go wrong on your end, or the code above is not right, for connecting to an SMTP server using SSL/TLS?

Note that I create one session and send all mails using that session. The mails are also sent from another thread if thats an issue?


Desktop (please complete the following information):

Mail server:

@lukasj @jbescos @jmehrens

jmehrens commented 1 year ago

Per the output on the logs your code executed this condition:

Which shows that the user name an password were null.

Add the following for debugging

session = Session.getInstance(props, new Authenticator() {
                @Override
                protected PasswordAuthentication getPasswordAuthentication() {
                    PasswordAuthentication pa = new PasswordAuthentication(Config.master.mailUser, Config.master.mailPassword);
                    AL.info(getRequestingSite()  + " - " + getDefaultUserName() + " - " + pa.getUserName() 
                    + " - " + pa.getPassword());  //<<----this is going to leak the password in log.
                    return pa;
                }
            });

and retest. Either:

  1. The Config.master.mailUser, Config.master.mailPassword were null at runtime.
  2. The message that was sent has not been assigned the correct session.
  3. The caller is using the Transport::(Message,String, String) method or Transport::(Message, Address[], String, String) method.
  4. A smtp provider is being used directly and calling one of the connect methods that takes a user/pass.
  5. Missing session parameters or not using smtps vs. smtp. This is to be determined via testing.
  6. Maybe a bug in Service. However, I think this code is correct at this time. As long as the password is not given it will recompute the user name and password from the Authenticator.
Osiris-Team commented 1 year ago

@jmehrens

The Config.master.mailUser, Config.master.mailPassword were null at runtime. The caller is using the Transport::(Message,String, String) method or Transport::(Message, Address[], String, String) method.

Can't be because: When sending the email I used Transport.send(mail), which didnt work, now I use Transport.send(message, Config.master.mailUser, Config.master.mailPassword), which works. At least the authentication now succedes, however, it still doesn't use SMTP with SSL/TLS, even though props.put("mail.smtp.starttls.enable", "true"); is set and passed over to the session.

The message that was sent has not been assigned the correct session.

Can't be because I use a single session to send all emails.

A smtp provider is being used directly and calling one of the connect methods that takes a user/pass.

I'm not setting the provider, or using another provider.

Here is the full code:

```java /* * Copyright Osiris Team * All rights reserved. * * This software is copyrighted work licensed under the terms of the * AutoPlug License. Please consult the file "LICENSE" for details. */ package com.osiris.autoplug.webserver.services.mail; import com.osiris.autoplug.webserver.Config; import com.osiris.autoplug.webserver.services.mail.templates.MailTemplate; import com.osiris.autoplug.webserver.utils.Exec; import com.osiris.jlib.logger.AL; import jakarta.mail.*; import jakarta.mail.internet.MimeMessage; import org.eclipse.angus.mail.smtp.SMTPProvider; import org.eclipse.angus.mail.smtp.SMTPSSLProvider; import java.util.Properties; import java.util.concurrent.atomic.AtomicBoolean; public class ServiceEmail { private static Session session; public static synchronized void sendAsync(MailTemplate mail) { AtomicBoolean isFinished = new AtomicBoolean(); Exec.later(() -> { try { MimeMessage message = new MimeMessage(session); message.setFrom("no-reply@" + Config.master.linkRawWebsite); message.setRecipients(Message.RecipientType.TO, mail.getTo()); message.setSubject(mail.getSubject()); message.setContent(mail.getContent(), "text/html; charset=utf-8"); AL.info("Sending verification mail to: " + mail.getTo() + "..."); Transport.send(message, Config.master.mailUser, Config.master.mailPassword); AL.info("Verification mail was sent to: " + mail.getTo()); isFinished.set(true); } catch (Exception e) { AL.warn(e); } }); Exec.later(() -> { try { Thread.sleep(60000); // Wait 1 minute before throwing error if (isFinished.get()) { } else throw new Exception("Failed to deliver mail within 1 minute!"); } catch (Exception e) { AL.warn(e); } }); } public void init() { try { // Note: SSL is not needed, because we are not connecting from outside Properties props = new Properties(); props.put("mail.smtp.auth", true); props.put("mail.smtp.starttls.enable", "true"); props.put("mail.smtp.host", Config.master.mailHost); props.put("mail.smtp.port", Config.master.mailPort); props.put("mail.smtp.ssl.trust", Config.master.mailHost); AL.info(props.toString()); session = Session.getInstance(props, new Authenticator() { @Override protected PasswordAuthentication getPasswordAuthentication() { return new PasswordAuthentication(Config.master.mailUser, Config.master.mailPassword); } }); session.setDebug(true); AL.info("ServiceMail was initialised! Encoding: auto"); } catch (Exception e) { AL.warn("FAILED TO START THE MAIL SERVICE!!!", e); } } } ```
jmehrens commented 1 year ago

Can't be because: When sending the email I used Transport.send(mail), which didnt work, now I use Transport.send(message, Config.master.mailUser, Config.master.mailPassword), which works. At least the authentication now succedes, however, it still doesn't use SMTP with SSL/TLS, even though props.put("mail.smtp.starttls.enable", "true"); is set and passed over to the session.

When I say that the user/pass was null I mean that it was null when it reached protocolConnect because your debug output proves that. So to troubleshoot, we have to work backwards from there to determine why mail witnessed null.

In my original reply I talked about adding a log statement to your anonymous Authenticator class. If you add that debug line and re-run the code you'll see why we are witnessing null.

Looking at the full code I would assume what is happening is that null is being passed to: MimeMessage message = new MimeMessage(session); This would align with the behavior of not using SSL/TLS.

Change that line to:

MimeMessage message = new MimeMessage(java.util.Objects.requireNonNull(session, "Session was null"));

Here are your next steps:

  1. Add the debug line to authenticator.
  2. Add null check to ensure you are passing a session to the message as described.
  3. That static session should have modifiers private static volatile to ensure the thread hand-off is visible to the other thread.
  4. Make sure you are using the Transport.send(message) single argument static method.
  5. Run the new code.
  6. If you don't see the new debug line then we know the session was not passed to the message.
  7. If you see a NullPointerException then you have to determine if init was ever invoked.
  8. If you see the debug line but the values are different from expected then we need to troubleshoot that.
Osiris-Team commented 1 year ago

@jmehrens This is the output:

INFO Sending verification mail to: nehakin891@docwl.com...
DEBUG: getProvider() returning jakarta.mail.Provider[TRANSPORT,smtp,org.eclipse.angus.mail.smtp.SMTPTransport,Oracle]
DEBUG SMTP: need username and password for authentication
DEBUG SMTP: protocolConnect returning false, host=localhost, user=root, password=<null>
INFO localhost/127.0.0.1 - root - autoplug - ACTUAL_PASSWORD
DEBUG SMTP: useEhlo true, useAuth true
DEBUG SMTP: trying to connect to host "localhost", port 465, isSSL false

I think this means something goes wrong in Transport.send(message) or inside new MimeMessage(session), the username and password are not get correctly somehow (idk at which point the credentials are get).

Note that the SSL/TLS issue also still exists. I also tried setting the provider explicitly with session.setProvider(new SMTPSSLProvider()); but this seems to have no effect.

Doing some research: Transport.send(msg):

    public static void send(Message msg) throws MessagingException {
        msg.saveChanges(); // do this first
        send0(msg, msg.getAllRecipients(), null, null);
    }

Why are username and password passed over as null? This results in transport.connect(); inside send0 instead of transport.connect(user, password);. Shouldnt it get the username and password from the session inside the message? Oh I just saw that inside package jakarta.mail; so you guys have nothing to do with it I guess.

I see your SMTPTransport class only overrides sendMessage. Shouldn't it override send0? Or at least the send(msg) function from above, and set the credentials? Ah nevermind, thats a static method.

Why even use the Transport class? It seems like a Jakarta implementation and the source code shows that it doesn't use any of your provided implementations??? Ah I see it uses your transport class in sendMessage#227, however, this is too late? Since it already connects without the credentials?

        if (dsize == 1) {
            transport = s.getTransport(addresses[0]);
            try {
                if (user != null) // <--- function arg from above, not from our transport, thus its null
                    transport.connect(user, password);
                else
                    transport.connect();
                transport.sendMessage(msg, addresses);
            } finally {
                transport.close();
            }
            return;
        }
jmehrens commented 1 year ago

@Osiris-Team

Oh I just saw that inside package jakarta.mail; so you guys have nothing to do with it I guess.

Everyone you tagged in this issue maintains jakarta.mail so no worries there.

Thanks for the debug output this is really helpful.

I'm looking at the Service code and it makes two connection attempts with a comment explaining the behavior:

// try connecting, if the protocol needs some missing // information (user, password) it will not connect. // if it tries to connect and fails, remember why for later.

I think the reason you are seeing the user=root, password=<null> is the result of the first connection attempt before the Authenticator callback is invoked. You see root as the user because there are fallbacks to lookup the user.name JVM system property. Setting the mail.user session property can override the fallback behavior on the user but there is no property for the password.

Are you able to send emails with your code?

Osiris-Team commented 1 year ago

@jmehrens No, sadly no emails get sent no matter what I try.

Osiris-Team commented 1 year ago

@jmehrens This is the exception btw:

[07-09-2023 18:46:26][AP-Web][WARN] Message: null
[07-09-2023 18:46:26][AP-Web][WARN] Details: Got bad greeting from SMTP host: localhost, port: 465, response: [EOF]
[07-09-2023 18:46:26][AP-Web][WARN] Type: jakarta.mail.MessagingException
[07-09-2023 18:46:26][AP-Web][WARN] Stacktrace: 
[07-09-2023 18:46:26][AP-Web][WARN] org.eclipse.angus.mail.smtp.SMTPTransport.openServer(SMTPTransport.java:2231)
[07-09-2023 18:46:26][AP-Web][WARN] org.eclipse.angus.mail.smtp.SMTPTransport.protocolConnect(SMTPTransport.java:729)
[07-09-2023 18:46:26][AP-Web][WARN] jakarta.mail.Service.connect(Service.java:345)
[07-09-2023 18:46:26][AP-Web][WARN] jakarta.mail.Service.connect(Service.java:225)
[07-09-2023 18:46:26][AP-Web][WARN] jakarta.mail.Service.connect(Service.java:246)
[07-09-2023 18:46:26][AP-Web][WARN] jakarta.mail.Transport.send0(Transport.java:230)
[07-09-2023 18:46:26][AP-Web][WARN] jakarta.mail.Transport.send(Transport.java:152)
[07-09-2023 18:46:26][AP-Web][WARN] com.osiris.autoplug.webserver.services.mail.ServiceEmail.lambda$sendAsync$0(ServiceEmail.java:40)
[07-09-2023 18:46:26][AP-Web][WARN] java.base/java.util.concurrent.ThreadPerTaskExecutor$TaskRunner.run(ThreadPerTaskExecutor.java:314)
[07-09-2023 18:46:26][AP-Web][WARN] java.base/java.lang.VirtualThread.run(VirtualThread.java:311)

I also tried a regular thread executor to make sure its not because of the virtual thread.

jmehrens commented 1 year ago

@Osiris-Team Thanks for feedback the output exception is helpful.

I already checked the values I pass over, they are correct, something seems to go wrong on your end, or the code above is not right, for connecting to an SMTP server using SSL/TLS?

I think your code is correct and the Mail code is correct but I think you just might have the wrong properties to connect to your server.

Does your SMTP server running on localhost support SSL or TLS? Are you able to connect an email client to this server? Sometimes you can use that client to determine what the correct mail settings should be. Are you able to connect to yahoo, outlook, or gmail with this test program?

It might be that your server is expecting SSL instead of starttls. If you are using SSL then the easiest way to enable SSL support in current versions of Jakarta Mail is to set the property "mail.smtp.ssl.enable" to "true". (Replace "smtp" with "imap" or "pop3" as appropriate.)

Also I would recommend that you change mail.smtp.host to mail.host. This has the benefit of removing some additional reverse host lookups. You should set the mail.from or mail.user as some SMTP servers require that username to be the same as the username passed to connect.

You'll want to read up on

  1. Mail properties
  2. SMTP and SMTPS properties
  3. SSLNOTES.txt
Osiris-Team commented 1 year ago

@jmehrens This is the error message I get on my mail server:

2023-09-06 18:13:04 TLS error on connection from localhost [127.0.0.1] (SSL_accept): timed out

These are its relevant config sections, where I have TLS enabled:

# If Exim is compiled with support for TLS, you may want to enable the
# following options so that Exim allows clients to make encrypted
# connections. In the authenticators section below, there are template
# configurations for plaintext username/password authentication. This kind
# of authentication is only safe when used within a TLS connection, so the
# authenticators will only work if the following TLS settings are turned on
# as well.

# Allow any client to use TLS.

tls_advertise_hosts = *

# Specify the location of the Exim server's TLS certificate and private key.
# The private key must not be encrypted (password protected). You can put
# the certificate and private key in the same file, in which case you only
# need the first setting, or in separate files, in which case you need both
# options.

tls_certificate = /etc/pki/tls/certs/exim.pem
tls_privatekey = /etc/pki/tls/private/exim.pem

# For OpenSSL, prefer EC- over RSA-authenticated ciphers
# tls_require_ciphers = ECDSA:RSA:!COMPLEMENTOFDEFAULT

# In order to support roaming users who wish to send email from anywhere,
# you may want to make Exim listen on other ports as well as port 25, in
# case these users need to send email from a network that blocks port 25.
# The standard port for this purpose is port 587, the "message submission"
# port. See RFC 4409 for details. Microsoft MUAs cannot be configured to
# talk the message submission protocol correctly, so if you need to support
# them you should also allow TLS-on-connect on the traditional but
# non-standard port 465.

daemon_smtp_ports = 25 : 465 : 587
tls_on_connect_ports = 465
Osiris-Team commented 1 year ago

Even though the config mentions TLS all the time, I don't think it actually supports it. I switched to SSL and it works now, thanks for all the help!

jmehrens commented 1 year ago

Awesome. For my own notes, going back to the original issue of "user=root, password=" I think the only minor patches that could come out of this issue would be to:

  1. Consider updating the wording of DEBUG SMTP: need username and password for authentication to indicate that the Service is attempting a connection using computed default session properties and or incomplete cached properties and may fail. (API ticket)
  2. Consider updating DEBUG SMTP: trying to connect to host "localhost", port 465, isSSL false to include authenticator identity toString or authenticator toString if the correct mail.debug.auth.* properties are set. This would indicate that user supplied authenticator is being used. (Angus ticket)
  3. Update only the static Transport.send to log when default session. The problem using the correct logger to see that message as it will go to the default session which may not be in debug mode. A better approach might be to include information in any exception thrown to indicate that the given message didn't have an assigned session. (API ticket)
  4. In Service class consider using getLocalAddress and parsing result to get user name as it looks like there is some duplicated logic. (API ticket)
  5. Add an entry in the FAQ to cover Got bad greeting from SMTP host: localhost, port: 465, response: [EOF]