Skip to content

[Mailer] [Smtp] Add DSN param 'auto_tls' to disable automatic STARTTLS #53621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 30, 2024
Merged

[Mailer] [Smtp] Add DSN param 'auto_tls' to disable automatic STARTTLS #53621

merged 1 commit into from
Jan 30, 2024

Conversation

srsbiz
Copy link
Contributor

@srsbiz srsbiz commented Jan 23, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #49114
License MIT

Many times we don't have any jurisdiction over configuration of SMTP server we are trying to connect to. If such server claims to be capable of STARTTLS, but drops connection after sending this command, there was no way to prevent mailer from sending it - despite defining protocol as stmp and port 25.

Now adding auto_tls=false to DSN we can prevent transport to automatically send STARTTLS when we do not intend to use TLS.

@carsonbot carsonbot added this to the 7.1 milestone Jan 23, 2024
@carsonbot carsonbot changed the title [Mailer][Smtp] Add DSN param 'auto_tls' to disable automatic STARTTLS [Mailer] [Smtp] Add DSN param 'auto_tls' to disable automatic STARTTLS Jan 23, 2024
@nicolas-grekas
Copy link
Member

I think this is a valid and working approach.
I'm just wondering about the naming :)
What about just tls instead of auto_tls?

@srsbiz
Copy link
Contributor Author

srsbiz commented Jan 25, 2024

Using simple tls might create confusion when using smptps or port 465 that should enable TLS by default. So it would require more checks and warnings about param having no effect, but adding auto_ IMHO suggest that it appliest to automatic TLS detection.

But I'm not attached to this name so it can be changed.

@derrabus
Copy link
Member

Using simple tls might create confusion when using smptps or port 465 that should enable TLS by default.

Indeed. If there were a setting called tls, my expectation would be that I either get a TLS connection or no connection at all. However, the STARTTLS feature is about plaintext connections that might be upgraded to TLS (opportunistic TLS). auto_tls is a better fit.

@nicolas-grekas nicolas-grekas mentioned this pull request Jan 29, 2024
Closed
@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jan 29, 2024 •
edited
Loading

In #52944, there is a need to be able to disable TLS while using smtp on port 465.
What about supporting tls=auto|bool here to support this use case and yours?

@srsbiz srsbiz force-pushed the mailer-disable-autotls branch from 2c60e1c to 59a6796 Compare January 29, 2024 23:30
@nicolas-grekas
Copy link
Member

Thank you @srsbiz.

@nicolas-grekas
Copy link
Member

/cc @aslangery
Symfony 7.1 will allow using stmp on port 465 by adding auto_tls=false to the DSN.

@nicolas-grekas nicolas-grekas merged commit 5091cd5 into symfony:7.1 Jan 30, 2024
This was referenced Jan 30, 2024
Closed
Closed
@aslangery
Copy link

What about versions >5.4?

@nicolas-grekas
Copy link
Member

That's a new feature so you should upgrade.

@xabbuh xabbuh mentioned this pull request Jan 31, 2024
Closed
@fabpot fabpot mentioned this pull request May 2, 2024
Merged
@kesselb kesselb mentioned this pull request Aug 5, 2024
Open
@ssddanbrown ssddanbrown mentioned this pull request Jan 11, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Feb 11, 2025
@nicolas-grekas
feature #59479 [Mailer] [Smtp] Add DSN param to enforce TLS/STARTTLS …
af2df09 
…(ssddanbrown)

This PR was merged into the 7.3 branch.

Discussion
----------

[Mailer] [Smtp] Add DSN param to enforce TLS/STARTTLS

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | Fix #48297
| License       | MIT

Adds 'require_tls' param which can be set to true to enforce the use of TLS/STARTTLS within the ESMTP transport.
This was discussed in #48297.
These changes are based upon patches [I've been maintaining](ssddanbrown/symfony-mailer@e9de8dc) for my own projects.

This is my first PR to Symfony, I've tried to follow the guide as best as possible, and I was also using #53621 as a general guide. There are some other ways I could have gone about things, but I've tried to avoid touching as much existing Symfony code as possible.

In #48297, nicolas-grekas mentioned unifying such an option with `auto_tls` under a `tls` option, but I think these are distinct options which may not be as clear combined (in addition to any expectations of such an option disabling/enabling TLS in general).

Commits
-------

a93d5f6 [Mailer] [Smtp] Add DSN param to enforce TLS/STARTTLS
@alliek-mti alliek-mti mentioned this pull request Apr 2, 2025
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Feature Mailer Status: Reviewed
Projects
None yet
Milestone
7.1
Development

Successfully merging this pull request may close these issues.

[Mailer] Force disable STARTTLS
5 participants
@srsbiz @nicolas-grekas @derrabus @aslangery @carsonbot