alertmanager icon indicating copy to clipboard operation
alertmanager copied to clipboard

chore: add comment about SMTP PLAIN authentication

Open TheMeier opened this issue 1 month ago • 3 comments

Ref: #1236

TheMeier avatar Nov 14 '25 16:11 TheMeier

So I have added a commit that implements validation if TLS is disabled and SMTP PLAIN auth config params are set. But I would consider this somewhat breaking since it will prevent AM starting with some (broken) configs which it previously did accept. I can also break out that commit into another PR.

TheMeier avatar Nov 17 '25 17:11 TheMeier

So about that validation. I think a more correct implementation would be to use something like this:

			if !*ec.RequireTLS && (ec.AuthUsername != "" || ec.AuthPassword != "" || ec.AuthPasswordFile != "") {
				if ip := net.ParseIP(ec.Smarthost.Host); ip != nil && !ip.IsLoopback() {
					return errors.New("PLAIN SMTP authentication without TLS can only be used with loopback (aka localhost) addresses")
				}
			}

But that of cause is different than https://cs.opensource.google/go/go/+/refs/tags/go1.25.4:src/net/smtp/auth.go;l=67 The issue I see here is, whatever validation we implement is prone to become different at some point if the implementation in net/smtp changes in the future. So I also don't feel very happy to just copy their isLocalhost implementation.

TheMeier avatar Nov 18 '25 17:11 TheMeier

The validation also seems ok to me. Shame that there is no public function to check that SMTP is safe.

SoloJacobs avatar Nov 18 '25 17:11 SoloJacobs