check/milter: apply fail_open to milter session-dial failures - #865
Open
marcominetti wants to merge 1 commit into
Open
check/milter: apply fail_open to milter session-dial failures#865marcominetti wants to merge 1 commit into
marcominetti wants to merge 1 commit into
Conversation
CheckStateForMsg dials the milter backend directly and returned any dial error unconditionally, before a *state existed to route the failure through ioError()'s fail_open handling. This made fail_open silently ineffective for the specific case of the milter being unreachable: the original SMTP sender got a hard reject regardless of the directive. Handle a dial failure the same way ioError() handles a later I/O failure: skip checks and let the message through when fail_open is set, otherwise keep today's behavior and reject. Guard CheckConnection and Close() for the resulting sessionless state.
Author
|
Filed #866 to track this properly — should have opened it before the PR per CONTRIBUTING.md, doing it now for the record. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
check.milter'sfail_opendirective is documented to let a message through when the milter can't be consulted, but it was silently ineffective for one specific failure mode: the milter being completely unreachable.CheckStateForMsg(internal/check/milter/milter.go) dials the milter backend directly viac.cl.Session()and returns any error unconditionally — this happens before a*stateexists, so the failure never reachesioError(), which is the only placefail_openis actually checked. The result: withfail_openset, a message is still hard-rejected (SMTP-level error straight to the sender) if the milter is simply down, exactly the casefail_openexists to guard against.This PR handles a dial failure the same way
ioError()handles a later I/O failure: whenfail_openis true, return a sessionless check state withskipChecks: trueinstead of propagating the raw error.CheckConnectionandClose()are guarded for the resulting nil session;CheckSender/CheckRcpt/CheckBodyalready short-circuit onskipChecksvia existing checks, so no change was needed there.Includes a regression test (
TestCheckStateForMsg_DialFailure_FailOpenTrue) that dials a closed loopback port to simulate an unreachable milter without a timeout wait, and confirms it fails against the pre-fix code (verified via mutation check: reverted the fix locally, confirmed this test fails with the exact predicted symptom, reapplied, confirmed it passes).