Some checks failed
Three defects in the SMTP backend, all predating the FMG-2 dependency upgrade.
## Authentication bypass
The credential check joined its two comparisons with `&&`, so it only rejected a client that got *both* halves wrong — a correct username with any password authenticated, as did a correct password with any username. Both comparisons now have to pass, and both are evaluated before either is acted on so the timing does not distinguish a wrong username from a wrong password. They compare SHA-256 digests through `subtle.ConstantTimeCompare`, which keeps the length of the configured credential out of the timing as well.
Fixing the comparison alone does not close the hole. go-smtp advertises `AUTH` but leaves enforcement to the backend, so a client could skip `AUTH` entirely and still have its message forwarded. The session now tracks authentication and refuses `MAIL`, `RCPT` and `DATA` without it.
Nor does the gate mean anything while the credentials it checks can be guessed. `SetDefaults` invented `Username = "username"` and `Password = "password"` when the config omitted them, with only a `slog.Warn` — so a `config.toml` naming nothing but a port and its recipients relayed to anyone who tried the obvious pair, for exactly the deployments that had configured the least. `Validate` now rejects unset credentials the way it already rejects an unusable target, and the invented defaults are gone.
**Behavior change:** a client that used to send without issuing `AUTH` now has to be configured with the credentials from `config.toml`, and a deployment that never set them will not start until it does. Both documented in the README.
## Remote denial of service
A malformed `Content-Type` reached `log.Fatalf`, which calls `os.Exit(1)`. One message with an attacker-controlled header terminated the daemon and took every other in-flight message with it. `Body()` now returns the parse error like the rest of the function does.
## Silent notification loss
`Data` logged forwarding errors and returned `250`, so a client whose notification reached no target at all treated the message as delivered and never retried. The reply now reflects what happened:
- `250` — at least one target accepted it. A *partial* failure is logged but still accepted, since a retry redelivers to every target and would duplicate the notification on the ones that already have it. This keeps the existing `TestPartialTargetFailure` contract.
- `451` — no target accepted it. Worth retrying.
- `550 5.6.0` — the message cannot be parsed. No retry can fix the same bytes.
- `550 5.3.5` — the recipient has no notification targets configured. A configuration fault; no retry can add targets. `LoadConfig` blocks such a config, so this is only reachable from one built in code.
## Session transaction state
`Session.Reset` was empty while go-smtp calls it after every message, so recipients accumulated across a connection and the second message on it was also forwarded to the first one's targets.
Fixing that introduced a data race, caught in review: `Reset` runs on the connection goroutine, but go-smtp runs `Data` on its own goroutine for a `BDAT` transfer and does not block `RSET` during one. A mutex now guards the recipients and the authentication flag across `Rcpt`, `Data` and `Reset`, and `Data` snapshots the recipients up front instead of reading the slice while forwarding.
## Validation
`make format`, `make ci-lint` (0 issues), `make test` (`-race`, 94.2% coverage), `make check` and `make build` all pass locally.
Every regression test was confirmed to fail against the code it guards:
- the six original ones against `origin/main` — the malformed `Content-Type` one by killing the test binary outright, which is the reported DoS;
- `TestResetDuringChunkedTransferIsSynchronized` against `da8e8e4`, where `-race` reports `Session.Reset` racing `Session.Data` on all three runs attempted;
- `TestLoadConfigRejectsUnsetCredentials` against `da8e8e4`'s `config.go`.
## Known gaps, not addressed here
- `Body()` returns `("", nil)` for a `multipart/*` with an unusable boundary and for any media type that is neither `multipart/` nor `text/`, so an empty notification is pushed to every target and answered `250`.
- `forwardEmail` breaks after the first matching `[[Recipients]]` entry, so a message addressed to two matching entries notifies only the first and still answers `250`. Pre-existing.
- A message for an address matching no entry, with no `[CatchAll]`, is accepted with `250` and dropped. Rejecting at `RCPT` would be more honest.
- `smtp.ErrAuthRequired` is go-smtp's `502 5.7.0` where RFC 4954 §6 specifies `530 5.7.0`. Left on the library's own constant.
Closes FMG-3
Co-authored-by: Full-Stack Developer <me@fmartingr.com>
Reviewed-on: #9
Reviewed-by: Felipe M <me@fmartingr.com>
103 lines
3.4 KiB
Markdown
103 lines
3.4 KiB
Markdown
# smtp2Shoutrrr
|
|
|
|
A simple SMTP server that forwards incoming emails to a [Shoutrrr supported service](https://containrrr.dev/shoutrrr/).
|
|
|
|
## Installing
|
|
|
|
First generate a new configuration file following this example:
|
|
|
|
```toml
|
|
# config.toml
|
|
# Port the SMTP server will listen on
|
|
Port = 11025
|
|
|
|
# Credentials for the SMTP server (required — the server refuses to start without them)
|
|
Username = "user"
|
|
Password = "nometokens"
|
|
|
|
# Configure recipients to forward emails to
|
|
|
|
# Multiple Targets (Recommended)
|
|
# Use the Targets array to send notifications to multiple services
|
|
[[Recipients]]
|
|
# Email addresses to forward emails from
|
|
Addresses = ["user@example.com"]
|
|
# Shoutrrr services to forward emails to
|
|
# See shoutrrr documentation: https://containrrr.dev/shoutrrr/
|
|
Targets = [
|
|
"ntfy://ntfy.sh/my-ntfy-topic?tags=email",
|
|
"discord://token@id",
|
|
"slack://token@channel"
|
|
]
|
|
|
|
# Single Target (Deprecated)
|
|
# The Target field is still supported for backward compatibility
|
|
[[Recipients]]
|
|
Addresses = ["legacy@example.com"]
|
|
Target = "ntfy://ntfy.sh/legacy-topic?tags=thing" # Will show deprecation warning
|
|
|
|
# Note: Repeat [[Recipients]] as needed
|
|
|
|
# Optional: Configure a catch-all recipient for unmatched email addresses
|
|
[CatchAll]
|
|
# Shoutrrr services to forward unmatched emails to
|
|
Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"]
|
|
```
|
|
|
|
The server refuses to start if the configuration cannot forward anything — no
|
|
recipients and no catch-all, a recipient without addresses or usable targets, or
|
|
a mistyped table name such as `[[Recipient]]`, which TOML would otherwise accept
|
|
silently. It also refuses to start without a `Username` and a `Password`: these
|
|
used to fall back to `username`/`password`, which left the authentication gate
|
|
open to the most obvious guess there is.
|
|
|
|
Clients must authenticate with the configured `Username` and `Password` before
|
|
starting a mail transaction; an unauthenticated client is refused rather than
|
|
relayed. A client that used to send without issuing `AUTH` has to be configured
|
|
with the credentials above.
|
|
|
|
The reply the sending client gets reflects what happened to the notification:
|
|
`250` once at least one target has accepted it, `451` when none did so the
|
|
sender retries, and `550` for a message that cannot be parsed at all — a
|
|
malformed `Content-Type`, for instance — which no retry could fix. A partial
|
|
failure is logged but still accepted, since a retry would redeliver to every
|
|
target and duplicate the notification on the ones that already have it.
|
|
|
|
### From releases
|
|
|
|
- Grab the latest release from the [releases page](https://git.nakama.town/fmartingr/smtp2shoutrrr/releases)
|
|
- Put the configuration file in the same directory as the binary
|
|
- Run the binary for your appropriate platform
|
|
|
|
### From source (development)
|
|
|
|
- Clone [this repository](https://git.nakama.town/fmartingr/smtp2shoutrrr)
|
|
- Put the configuration file in the repository folder
|
|
- Run `make quick-run`
|
|
|
|
### Using docker
|
|
|
|
- Create a `config.toml` file as described above
|
|
- Run the docker image mounting the `Config.toml` file as `/config.toml` and exposing the configured port:
|
|
|
|
```bash
|
|
docker run -v /path/to/config.toml:/config.toml \
|
|
-p 11025:11025 \
|
|
git.nakama.town/fmartingr/smtp2shoutrrr:latest
|
|
```
|
|
|
|
## Development
|
|
|
|
Run the server with:
|
|
|
|
```
|
|
make quick-run
|
|
```
|
|
|
|
Send a test email with:
|
|
|
|
> This will read the `config.toml` in the current directory to set the appropriate SMTP client configuration.
|
|
|
|
```
|
|
go run ./cmd/sendmail/.
|
|
```
|