Skip to content

Allow SMTP over unix socket (& rework mailer settings) #18901

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.

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

Closed
clarfonthey opened this issue Feb 25, 2022 · 0 comments · Fixed by #18982
Closed

Allow SMTP over unix socket (& rework mailer settings) #18901

clarfonthey opened this issue Feb 25, 2022 · 0 comments · Fixed by #18982
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Feb 25, 2022

Motivation

Essentially, systems like OpenSMTPD allow listening for SMTP connections on a unix socket directly instead of over localhost. This allows the connections to be faster (unix domain sockets have less overhead than local TCP) and also lets you restrict who can access the socket.

It would be nice if we could provide a path for the mailer.HOST setting, so that it will submit via a unix socket connection instead of TCP. If we were to avoid reworking the mailer settings, we could simply accept a path here, but IMHO we should rework the mailer settings to match the existing server settings, since it would get rid of a lot of weird edge cases like TLS over unix socket.

Proposed settings

The relevant settings right now:

  • HOST: accepts something of form addr:port or just addr to imply port (addr can be domain or IP)
  • DISABLE_HELO, HELO_HOSTNAME: describe host used in HELO operation; has some weird settings
  • SKIP_VERIFY: whether to skip verifying server certificate
  • USE_CERTIFICATE, CERT_FILE, KEY_FILE: client certificates
  • IS_TLS_ENABLED: confusingly, doesn't determine whether TLS is enabled, but the usage of STARTTLS or plain TLS
  • MAILER_TYPE: set to SMTP to use the SMTP mailer

My proposal:

  • PROTOCOL: only has effect for SMTP mailer. can be smtp, smtps, smtp+startls, smtp+unix
  • SMTP_ADDR: domain for SMTP, or path to unix socket
  • SMTP_PORT: port for SMTP; defaults to 25 for smtp, 465 for smtps, and 587 for smtp+startls
  • ENABLE_HELO, HELO_HOSTNAME: reverse DISABLE_HELO to ENABLE_HELO; default to false + system hostname
  • FORCE_TRUST_SERVER_CERT: scarier version of SKIP_VERIFY
  • CLIENT_CERT_FILE, CLIENT_KEY_FILE, USE_CLIENT_CERT: clarify client certificates here
  • MAILER_TYPE: still allows smtp, dummy, sendmail

Note: we don't have to do all these renames, but I strongly encourage replacing HOST + IS_TLS_ENABLED with PROTOCOL, SMTP_ADDR, and SMTP_PORT.

After looking at the code, it seems like these changes would be relatively easy to make, and I'm willing to provide those, but I figured that making an issue to discuss this first would be a good idea.

Alternatives

Easy option: just keep the settings as-is and make HOST = /path work for unix sockets. IS_TLS_ENABLED = true will error for unix sockets.

Compromise option: just deprecate HOST and IS_TLS_ENABLED and replace them with PROTOCOL, SMTP_ADDR, and SMTP_PORT. Keep the other options as-is.

Other notes

Having PROTOCOL = smtp should probably be highly not recommended unless SMTP_ADDR = localhost. Not sure where to fit this in the docs for these. I do know that it has some recommendations about only using IS_TLS_ENABLED = false when HOST = localhost.

One big reason why I propose changing all of these settings at once is that every option except modifying HOST to accept paths will break people's configs anyway, and we might as well clean things up while we do that.

@Gusted Gusted added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. and removed type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Feb 25, 2022
clarfonthey added a commit to clarfonthey/gitea that referenced this issue Apr 27, 2022
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants