diff --git a/internal/check/milter/milter.go b/internal/check/milter/milter.go
index 2a6d6e9e..02923be5 100644
--- a/internal/check/milter/milter.go
+++ b/internal/check/milter/milter.go
@@ -119,6 +119,22 @@ type state struct {
func (c *Check) CheckStateForMsg(ctx context.Context, msgMeta *module.MsgMetadata) (module.CheckState, error) {
session, err := c.cl.Session()
if err != nil {
+ // fail_open is meant to apply to any I/O failure talking to the
+ // milter, but a failure to even establish the session (e.g. the
+ // milter is down or unreachable) happens before a *state exists to
+ // route through ioError, so it has to be handled here explicitly -
+ // otherwise fail_open is silently ineffective for this specific
+ // failure mode and the message is hard-rejected regardless of the
+ // directive.
+ if c.failOpen {
+ c.log.Error("I/O error, skipping checks", err)
+ return &state{
+ c: c,
+ msgMeta: msgMeta,
+ skipChecks: true,
+ log: target.DeliveryLogger(c.log, msgMeta),
+ }, nil
+ }
return nil, err
}
return &state{
@@ -226,6 +242,10 @@ func (s *state) apply(modifyActs []milter.ModifyAction, res module.CheckResult)
}
func (s *state) CheckConnection(ctx context.Context) module.CheckResult {
+ if s.skipChecks {
+ return module.CheckResult{}
+ }
+
if s.msgMeta.Conn == nil {
// Submit some dummy values as the message is likely generated locally.
@@ -437,6 +457,9 @@ func (s *state) CheckBody(ctx context.Context, header textproto.Header, body buf
}
func (s *state) Close() error {
+ if s.session == nil {
+ return nil
+ }
return s.session.Close()
}
diff --git a/internal/check/milter/milter_test.go b/internal/check/milter/milter_test.go
index 50e3954e..8c4df018 100644
--- a/internal/check/milter/milter_test.go
+++ b/internal/check/milter/milter_test.go
@@ -19,9 +19,15 @@ along with this program. If not, see .
package milter
import (
+ "context"
+ "net"
"testing"
+ "time"
+ "github.com/emersion/go-milter"
"github.com/foxcpp/maddy/framework/config"
+ "github.com/foxcpp/maddy/framework/log"
+ "github.com/foxcpp/maddy/framework/module"
)
func TestAcceptValidEndpoints(t *testing.T) {
@@ -59,3 +65,77 @@ func TestRejectInvalidEndpoints(t *testing.T) {
}
}
}
+
+// unreachableAddr returns a loopback TCP address that is guaranteed to
+// refuse connections: it binds a listener to get a free port, then closes
+// it immediately, so dialing it fails fast with ECONNREFUSED instead of
+// waiting out a dial timeout.
+func unreachableAddr(t *testing.T) string {
+ t.Helper()
+ l, err := net.Listen("tcp", "127.0.0.1:0")
+ if err != nil {
+ t.Fatalf("failed to allocate a port to make unreachable: %v", err)
+ }
+ addr := l.Addr().String()
+ if err := l.Close(); err != nil {
+ t.Fatalf("failed to close listener: %v", err)
+ }
+ return addr
+}
+
+func newUnreachableCheck(t *testing.T, failOpen bool) *Check {
+ t.Helper()
+ return &Check{
+ failOpen: failOpen,
+ log: &log.Logger{Out: log.NopOutput{}},
+ cl: milter.NewClientWithOptions("tcp", unreachableAddr(t), milter.ClientOptions{
+ Dialer: &net.Dialer{Timeout: 2 * time.Second},
+ ReadTimeout: 2 * time.Second,
+ WriteTimeout: 2 * time.Second,
+ }),
+ }
+}
+
+// TestCheckStateForMsg_DialFailure_FailOpenFalse locks in the existing,
+// correct behavior: without fail_open, an unreachable milter must still
+// hard-reject at state-creation time.
+func TestCheckStateForMsg_DialFailure_FailOpenFalse(t *testing.T) {
+ c := newUnreachableCheck(t, false)
+ _, err := c.CheckStateForMsg(context.Background(), &module.MsgMetadata{ID: "test"})
+ if err == nil {
+ t.Fatal("expected an error when the milter is unreachable and fail_open is false, got nil")
+ }
+}
+
+// TestCheckStateForMsg_DialFailure_FailOpenTrue reproduces the fail_open
+// gap: a dial failure while establishing the milter session used to bypass
+// fail_open entirely (CheckStateForMsg returned the raw dial error
+// unconditionally), hard-rejecting the message despite fail_open being set.
+// It must instead behave like ioError() does for a later I/O failure: let
+// the message through unchecked.
+func TestCheckStateForMsg_DialFailure_FailOpenTrue(t *testing.T) {
+ c := newUnreachableCheck(t, true)
+
+ st, err := c.CheckStateForMsg(context.Background(), &module.MsgMetadata{ID: "test"})
+ if err != nil {
+ t.Fatalf("expected fail_open to let the message through despite the unreachable milter, got error: %v", err)
+ }
+ if st == nil {
+ t.Fatal("expected a non-nil check state with fail_open, got nil")
+ }
+ defer func() {
+ if err := st.Close(); err != nil {
+ t.Errorf("Close: expected nil error on a sessionless (dial-failed) state, got: %v", err)
+ }
+ }()
+
+ if res := st.CheckConnection(context.Background()); res.Reject {
+ t.Errorf("CheckConnection: expected no rejection with fail_open after a dial failure, got Reject=true, reason=%v", res.Reason)
+ }
+ if res := st.CheckSender(context.Background(), "sender@example.org"); res.Reject {
+ t.Errorf("CheckSender: expected no rejection with fail_open after a dial failure, got Reject=true, reason=%v", res.Reason)
+ }
+ if res := st.CheckRcpt(context.Background(), "rcpt@example.org"); res.Reject {
+ t.Errorf("CheckRcpt: expected no rejection with fail_open after a dial failure, got Reject=true, reason=%v", res.Reason)
+ }
+}