Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#36125 closed enhancement (invalid)

Disable automatic TLS encryption in PHPMailer

Reported by: scara's profile scara Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.6
Component: External Libraries Keywords: has-patch
Focuses: Cc:

Description

Since 5.2.10, PHPMailer has added a new feature to automatically use TLS encryption when available at the SMTP server side, via SMTPAutoTLS.
This setting is true by default which is kind of security feature but in real life this is causing headaches due to wrongly configured SMTP server with regards to TLS support (see e.g.: https://github.com/PHPMailer/PHPMailer/issues/642, https://github.com/PHPMailer/PHPMailer/issues/643).

That setting should be set to false to allow WP admin people to select TLS support only when it is actually supported by their SMTP server.

Attachments (2)

wp45_PHPMailer_Disable_Automatic_TLS_Encryption.36125.diff (572 bytes) - added by scara 8 years ago.
PHPMailer: disabled automatic TLS encryption.
wp45_PHPMailer_Disable_Automatic_TLS_Encryption.36125.git.patch (904 bytes) - added by scara 8 years ago.
$ git format-patch HEAD~1 --stdout > wp45_PHPMailer_Disable_Automatic_TLS_Encryption.36125.git.patch

Download all attachments as: .zip

Change History (11)

@scara
8 years ago

PHPMailer: disabled automatic TLS encryption.

#1 @scara
8 years ago

  • Keywords has-patch added

@scara
8 years ago

$ git format-patch HEAD~1 --stdout > wp45_PHPMailer_Disable_Automatic_TLS_Encryption.36125.git.patch

#2 @Synchro
8 years ago

Just to note that this represents a security downgrade; the correct solution would be to fix the mail server - it's not as if you would live with a web server that had a broken SSL certificate, and mail is no different. A halfway solution would be to downgrade certificate verification which would usually allow TLS to work even on a misconfigured server, but it's still not ideal. Downgrading everybody's security because a few can't be bothered to fix theirs doesn't seem sensible.

#3 @scara
8 years ago

Before the PHPMailer 5.2.10 release, a WP instance - generally, a PHP based App - worked with those SMTP server with a broken TLS support because of WP admins - generally PHP based App admins - were not using TLS when configuring the SMTP support in the App.

While I concur that the new PHPMailer feature contributes to secure the communication between the App and the SMTP server - otherwise, I'd open an issue in PHPMailer ;) -, I disagree that a PHP based App which could be "able to configure" that kind of security should be forced to use a behavior not directly managed by its Admin.

That's my point and the reason why I raised this improvement: IMHO it is actually kind of regression in WP due to the security enhancements adopted in one of its external libraries.

Forgive me if the arguments above are not strong enough at the App side - not at the library side - or if it is enough to clearly document it in the release notes.

HTH,
Matteo

Version 0, edited 8 years ago by scara (next)

#4 @Synchro
8 years ago

It depends on your definition of "working". Many would consider failing to report broken security as "not working". If the server is not meant to be supporting TLS, it should not be advertising that it does.

#5 @scara
8 years ago

You're right, if you can fix or change the SMTP server you rel(a)y on ;).
But if you are/were happy with a plain SMTP connection why not keeping on using it, at the App side?

BTW I think that now there are enough info and points of view to let this ticket be happily reviewed.

TNX,
Matteo

#6 @BinaryKitten
8 years ago

Hi @scara

Welcome to Trac and thank you for submitting this ticket.

I'm not sure turning off security by default should be the way to go. Especially since the version you've mentioned has been in Core since July of last year and "turning off" would leave users that are in need of or use TLS, without that security.

Would a documentation update be a better choice here, since there is a hook for phpmailer_init which will allow this to be achieved for those with the issue.

<?php
add_action('phpmailer_init', 'disableTLS');

/**
 * @param \PHPMailer $phpmailer
 */
function disableTLS($phpmailer){
    // Disable the automatic TLS encryption added in PHPMailer v5.2.10 (9da56fc1328a72aa124b35b738966315c41ef5c6) 
    $phpmailer->SMTPAutoTLS = false; 
}

This is a simple plugin file that you could drop into mu-plugins to get the same result without compromising security that is already there.

#7 @dd32
8 years ago

It appears, from first glance, that this specifically requires you to be using the SMTP functionality of PHPMailer, whereas WordPress specifically specifies to use the PHP mail() command.

Are you perhaps running a plugin to enable the usage of SMTP? I have a feeling that this should be an option for that plugin, if it's indeed why that code branch is being hit.

#8 @scara
8 years ago

TNX for your prompt replies:

  • @BinaryKitten: yes that could be the way, documentation update. Everyone here will be happy with that, including a mention for the hook.
  • @dd32: yes, the issue comes from the plug-ins.

Have a nice Sunday,
Matteo

#9 @dd32
8 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

Hi @scara

As this specifically requires a plugin to trigger, this should be reported to the plugin author directly instead.

Right this moment, it doesn't make sense to globally disable this functionality for all WordPress users, and it should instead be something a site administrator has to explicitly turn on within a plugin on their site.

Note: See TracTickets for help on using tickets.