From 14c48153aaaf609d55a842328ff24a29f2243c56 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Sat, 22 Oct 2016 18:24:31 +0100 Subject: [PATCH] smtpsrv: If the hook exits with code 20, it's a permanent failure It is can be convenient for hooks to indicate that an error is permanent; for example if the anti-virus found something. This patch makes it so that if the hook exits with code 20, then it's considered permanent. Otherwise it is considered transient, to help prevent accidental errors cause final delivery issues. --- hooks/post-data | 7 +++++-- internal/smtpsrv/conn.go | 27 ++++++++++++++++++--------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/hooks/post-data b/hooks/post-data index e0e9621..6035253 100755 --- a/hooks/post-data +++ b/hooks/post-data @@ -5,6 +5,9 @@ # # - spamc (from Spamassassin) to filter spam. # - clamdscan (from ClamAV) to filter virus. +# +# If it exits with code 20, it will be considered a permanent error. +# Otherwise, temporary. set -e @@ -19,7 +22,7 @@ cat > "$TF" if command -v spamc >/dev/null; then if ! SL=$(spamc -c - < "$TF") ; then echo "spam detected" - exit 1 + exit 20 # permanent fi echo "X-Spam-Score: $SL" fi @@ -28,7 +31,7 @@ fi if command -v clamdscan >/dev/null; then if ! clamdscan --no-summary --infected - < "$TF" 1>&2 ; then echo "virus detected" - exit 1 + exit 20 # permanent fi echo "X-Virus-Scanned: pass" fi diff --git a/internal/smtpsrv/conn.go b/internal/smtpsrv/conn.go index 23e6b08..0c167f1 100644 --- a/internal/smtpsrv/conn.go +++ b/internal/smtpsrv/conn.go @@ -16,6 +16,7 @@ import ( "os/exec" "strconv" "strings" + "syscall" "time" "blitiri.com.ar/go/chasquid/internal/aliases" @@ -530,10 +531,13 @@ func (c *Conn) DATA(params string) (code int, msg string) { c.addReceivedHeader() - hookOut, err := c.runPostDataHook(c.data) + hookOut, permanent, err := c.runPostDataHook(c.data) if err != nil { maillog.Rejected(c.conn.RemoteAddr(), c.mailFrom, c.rcptTo, err.Error()) - return 554, err.Error() + if permanent { + return 554, err.Error() + } + return 451, err.Error() } c.data = append(hookOut, c.data...) @@ -623,13 +627,13 @@ func checkData(data []byte) error { return nil } -// runPostDataHook and return the new headers to add, an error (if any), and -// true if the error is permanent or false if transient. -func (c *Conn) runPostDataHook(data []byte) ([]byte, error) { +// runPostDataHook and return the new headers to add, and on error a boolean +// indicating if it's permanent, and the error itself. +func (c *Conn) runPostDataHook(data []byte) ([]byte, bool, error) { // TODO: check if the file is executable. if _, err := os.Stat(c.postDataHook); os.IsNotExist(err) { hookResults.Add("post-data:skip", 1) - return nil, nil + return nil, false, nil } tr := trace.New("Hook.Post-DATA", c.conn.RemoteAddr().String()) defer tr.Finish() @@ -662,14 +666,19 @@ func (c *Conn) runPostDataHook(data []byte) ([]byte, error) { hookResults.Add("post-data:fail", 1) tr.Error(err) tr.Debugf("stdout: %s", out) + + permanent := false if ee, ok := err.(*exec.ExitError); ok { tr.Printf("stderr: %s", string(ee.Stderr)) + if status, ok := ee.Sys().(syscall.WaitStatus); ok { + permanent = status.ExitStatus() == 20 + } } // The error contains the last line of stdout, so filters can pass // some rejection information back to the sender. err = fmt.Errorf(lastLine(string(out))) - return nil, err + return nil, permanent, err } // Check that output looks like headers, to avoid breaking the email @@ -677,13 +686,13 @@ func (c *Conn) runPostDataHook(data []byte) ([]byte, error) { if !isHeader(out) { hookResults.Add("post-data:badoutput", 1) tr.Errorf("error parsing post-data output: '%s'", out) - return nil, nil + return nil, false, nil } tr.Debugf("success") tr.Debugf("stdout: %s", out) hookResults.Add("post-data:success", 1) - return out, nil + return out, false, nil } // isHeader checks if the given buffer is a valid MIME header.