From: Wietse Venema The Postfix postscreen(8) server performs triage on multiple
-inbound SMTP connections in parallel. While one postscreen(8) process
-keeps spambots away from Postfix SMTP server processes, more Postfix
-SMTP server processes remain available for legitimate clients. Introduction
By doing these checks in a single postscreen(8) process, Postfix can avoid wasting one SMTP server process per connection. A side @@ -28,18 +29,6 @@ benefit of postscreen(8)'s DNSBL lookups is that already cached before the Postfix SMTP server looks them up later.
-postscreen(8) maintains a temporary whitelist for clients that -have passed a number of tests. When an SMTP client IP address is -whitelisted, postscreen(8) hands off the connection immediately to -a Postfix SMTP server process. This minimizes the overhead for -legitimate mail.
- -By default, postscreen(8) logs statistics and hands off every -connection to a Postfix SMTP server process, while excluding clients -in mynetworks from all tests (primarily, to avoid problems with -non-standard SMTP implementations in network appliances). This mode -is useful for non-destructive testing.
-Topics in this document:
To avoid problems with poorly-implemented SMTP engines in network appliances or network testing tools, either exclude them from all tests with the postscreen_whitelist_networks feature or else specify -an empty teaser banner with:
+an empty teaser banner: + ++/etc/postfix/main.cf: + # Exclude broken clients by whitelisting. $mynetworks is default. + postscreen_whitelist_networks = $mynetworks, 192.168.254.0/24 +
/etc/postfix/main.cf:
+ # Disable the teaser banner (try whitelisting first if you can).
postscreen_greet_banner =
diff --git a/postfix/html/postconf.5.html b/postfix/html/postconf.5.html
index 48630c66b..43c79745e 100644
--- a/postfix/html/postconf.5.html
+++ b/postfix/html/postconf.5.html
@@ -6854,10 +6854,10 @@ when it rejects mail. When no mapping is found, the actual DNSBL
domain will be used.
For maximal stability it is best to use a file that is read -into memory such as pcre:, regexp: or texthash: (a format similar -to hash: except that there is no need to run postmap(1) before the -file can be used, and that it does not detect changes after the -file is read).
+into memory such as pcre:, regexp: or texthash: (texthash: is similar +to hash:, except a) there is no need to run postmap(1) before the +file can be used, and b) texthash: does not detect changes after +the file is read).Example:
@@ -6950,7 +6950,7 @@ parameter.The amount of time that postscreen(8) will cache results from a successful DNS blocklist test. During this time, the client IP address @@ -7053,10 +7053,10 @@ one-letter suffix that specifies the time unit). Time units: s
The amount of time that postscreen(8) will wait for an SMTP client to send a command before its turn, and for DNS blocklist -lookup results to arrive (default: 2 seconds under stress, 6 seconds -normally). This is done only when the SMTP client IP address is -not permanently whitelisted, and when it has no cached decision. -Specify a non-zero time value (an integral value plus an optional +lookup results to arrive (default: up to 2 seconds under stress, +up to 6 seconds otherwise).
+ +
Specify a non-zero time value (an integral value plus an optional one-letter suffix that specifies the time unit).
Time units: s (seconds), m (minutes), h (hours), d (days), w diff --git a/postfix/html/postscreen.8.html b/postfix/html/postscreen.8.html index 4463dfb53..3e13d95ce 100644 --- a/postfix/html/postscreen.8.html +++ b/postfix/html/postscreen.8.html @@ -14,10 +14,10 @@ POSTSCREEN(8) POSTSCREEN(8) DESCRIPTION The Postfix postscreen(8) server performs triage on multi- - ple inbound SMTP connections in parallel. While - postscreen(8) keeps spambots away from Postfix SMTP server - processes, more Postfix SMTP server processes remain - available for legitimate clients. + ple inbound SMTP connections in parallel. While a single + postscreen(8) process keeps spambots away from Postfix + SMTP server processes, more Postfix SMTP server processes + remain available for legitimate clients. postscreen(8) maintains a temporary whitelist for clients that have passed a number of tests. When an SMTP client @@ -156,8 +156,8 @@ POSTSCREEN(8) POSTSCREEN(8) The amount of time that postscreen(8) will wait for an SMTP client to send a command before its turn, and for DNS blocklist lookup results to arrive - (default: 2 seconds under stress, 6 seconds nor- - mally). + (default: up to 2 seconds under stress, up to 6 + seconds otherwise). postscreen_helo_required ($smtpd_helo_required) Require that a remote SMTP client sends HELO or @@ -209,7 +209,7 @@ POSTSCREEN(8) POSTSCREEN(8) results from a successful "bare newline" SMTP pro- tocol test. - postscreen_dnsbl_ttl (1d) + postscreen_dnsbl_ttl (1h) The amount of time that postscreen(8) will cache results from a successful DNS blocklist test. diff --git a/postfix/man/man5/postconf.5 b/postfix/man/man5/postconf.5 index dce9b81c5..9712179a2 100644 --- a/postfix/man/man5/postconf.5 +++ b/postfix/man/man5/postconf.5 @@ -3858,10 +3858,10 @@ when it rejects mail. When no mapping is found, the actual DNSBL domain will be used. .PP For maximal stability it is best to use a file that is read -into memory such as pcre:, regexp: or texthash: (a format similar -to hash: except that there is no need to run \fBpostmap\fR(1) before the -file can be used, and that it does not detect changes after the -file is read). +into memory such as pcre:, regexp: or texthash: (texthash: is similar +to hash:, except a) there is no need to run \fBpostmap\fR(1) before the +file can be used, and b) texthash: does not detect changes after +the file is read). .PP Example: .PP @@ -3947,7 +3947,7 @@ its combined DNSBL score as defined with the postscreen_dnsbl_sites parameter. .PP This feature is available in Postfix 2.8. -.SH postscreen_dnsbl_ttl (default: 1d) +.SH postscreen_dnsbl_ttl (default: 1h) The amount of time that \fBpostscreen\fR(8) will cache results from a successful DNS blocklist test. During this time, the client IP address is excluded from this test. The default is relatively short, because a @@ -4007,9 +4007,9 @@ This feature is available in Postfix 2.8. .SH postscreen_greet_wait (default: ${stress?2}${stress:6}s) The amount of time that \fBpostscreen\fR(8) will wait for an SMTP client to send a command before its turn, and for DNS blocklist -lookup results to arrive (default: 2 seconds under stress, 6 seconds -normally). This is done only when the SMTP client IP address is -not permanently whitelisted, and when it has no cached decision. +lookup results to arrive (default: up to 2 seconds under stress, +up to 6 seconds otherwise). +.PP Specify a non-zero time value (an integral value plus an optional one-letter suffix that specifies the time unit). .PP diff --git a/postfix/man/man8/postscreen.8 b/postfix/man/man8/postscreen.8 index 8c49e61ce..152137589 100644 --- a/postfix/man/man8/postscreen.8 +++ b/postfix/man/man8/postscreen.8 @@ -13,10 +13,10 @@ Postfix SMTP triage server .ad .fi The Postfix \fBpostscreen\fR(8) server performs triage on -multiple inbound SMTP connections in parallel. While -\fBpostscreen\fR(8) keeps spambots away from Postfix SMTP -server processes, more Postfix SMTP server processes remain -available for legitimate clients. +multiple inbound SMTP connections in parallel. While a +single \fBpostscreen\fR(8) process keeps spambots away from +Postfix SMTP server processes, more Postfix SMTP server +processes remain available for legitimate clients. \fBpostscreen\fR(8) maintains a temporary whitelist for clients that have passed a number of tests. When an SMTP @@ -151,8 +151,8 @@ that they speak before their turn (pre-greet). .IP "\fBpostscreen_greet_wait (${stress?2}${stress:6}s)\fR" The amount of time that \fBpostscreen\fR(8) will wait for an SMTP client to send a command before its turn, and for DNS blocklist -lookup results to arrive (default: 2 seconds under stress, 6 seconds -normally). +lookup results to arrive (default: up to 2 seconds under stress, +up to 6 seconds otherwise). .IP "\fBpostscreen_helo_required ($smtpd_helo_required)\fR" Require that a remote SMTP client sends HELO or EHLO before commencing a MAIL transaction. @@ -190,7 +190,7 @@ temporary whitelist entry before it is removed. .IP "\fBpostscreen_bare_newline_ttl (30d)\fR" The amount of time that \fBpostscreen\fR(8) will cache results from a successful "bare newline" SMTP protocol test. -.IP "\fBpostscreen_dnsbl_ttl (1d)\fR" +.IP "\fBpostscreen_dnsbl_ttl (1h)\fR" The amount of time that \fBpostscreen\fR(8) will cache results from a successful DNS blocklist test. .IP "\fBpostscreen_greet_ttl (1d)\fR" diff --git a/postfix/proto/DATABASE_README.html b/postfix/proto/DATABASE_README.html index 63e30ff2a..e1541a985 100644 --- a/postfix/proto/DATABASE_README.html +++ b/postfix/proto/DATABASE_README.html @@ -383,9 +383,9 @@ number.
The Postfix postscreen(8) server performs triage on multiple -inbound SMTP connections in parallel. While one postscreen(8) process -keeps spambots away from Postfix SMTP server processes, more Postfix -SMTP server processes remain available for legitimate clients.
+inbound SMTP connections in parallel. While a single postscreen(8) +process keeps spambots away from Postfix SMTP server processes, +more Postfix SMTP server processes remain available for legitimate +clients.By doing these checks in a single postscreen(8) process, Postfix can avoid wasting one SMTP server process per connection. A side @@ -28,18 +29,6 @@ benefit of postscreen(8)'s DNSBL lookups is that DNS records are already cached before the Postfix SMTP server looks them up later.
-postscreen(8) maintains a temporary whitelist for clients that -have passed a number of tests. When an SMTP client IP address is -whitelisted, postscreen(8) hands off the connection immediately to -a Postfix SMTP server process. This minimizes the overhead for -legitimate mail.
- -By default, postscreen(8) logs statistics and hands off every -connection to a Postfix SMTP server process, while excluding clients -in mynetworks from all tests (primarily, to avoid problems with -non-standard SMTP implementations in network appliances). This mode -is useful for non-destructive testing.
-Topics in this document:
To avoid problems with poorly-implemented SMTP engines in network appliances or network testing tools, either exclude them from all tests with the postscreen_whitelist_networks feature or else specify -an empty teaser banner with:
+an empty teaser banner: + ++/etc/postfix/main.cf: + # Exclude broken clients by whitelisting. $mynetworks is default. + postscreen_whitelist_networks = $mynetworks, 192.168.254.0/24 +
/etc/postfix/main.cf:
+ # Disable the teaser banner (try whitelisting first if you can).
postscreen_greet_banner =
diff --git a/postfix/proto/postconf.proto b/postfix/proto/postconf.proto
index 749f2086d..5f37e1a3e 100644
--- a/postfix/proto/postconf.proto
+++ b/postfix/proto/postconf.proto
@@ -12592,10 +12592,10 @@ seconds.
The amount of time that postscreen(8) will wait for an SMTP client to send a command before its turn, and for DNS blocklist -lookup results to arrive (default: 2 seconds under stress, 6 seconds -normally). This is done only when the SMTP client IP address is -not permanently whitelisted, and when it has no cached decision. -Specify a non-zero time value (an integral value plus an optional +lookup results to arrive (default: up to 2 seconds under stress, +up to 6 seconds otherwise).
+ +
Specify a non-zero time value (an integral value plus an optional one-letter suffix that specifies the time unit).
Time units: s (seconds), m (minutes), h (hours), d (days), w @@ -13040,7 +13040,7 @@ protocol engine.
This feature is available in Postfix 2.8.
-%PARAM postscreen_dnsbl_ttl 1d +%PARAM postscreen_dnsbl_ttl 1hThe amount of time that postscreen(8) will cache results from a successful DNS blocklist test. During this time, the client IP address @@ -13209,10 +13209,10 @@ when it rejects mail. When no mapping is found, the actual DNSBL domain will be used.
For maximal stability it is best to use a file that is read -into memory such as pcre:, regexp: or texthash: (a format similar -to hash: except that there is no need to run postmap(1) before the -file can be used, and that it does not detect changes after the -file is read).
+into memory such as pcre:, regexp: or texthash: (texthash: is similar +to hash:, except a) there is no need to run postmap(1) before the +file can be used, and b) texthash: does not detect changes after +the file is read).Example:
diff --git a/postfix/src/global/mail_params.h b/postfix/src/global/mail_params.h index 2635123d5..6ccf57553 100644 --- a/postfix/src/global/mail_params.h +++ b/postfix/src/global/mail_params.h @@ -3272,7 +3272,7 @@ extern char *var_ps_dnsbl_enable; extern char *var_ps_dnsbl_action; #define VAR_PS_DNSBL_TTL "postscreen_dnsbl_ttl" -#define DEF_PS_DNSBL_TTL "1d" +#define DEF_PS_DNSBL_TTL "1h" extern int var_ps_dnsbl_ttl; #define VAR_PS_DNSBL_REPLY "postscreen_dnsbl_reply_map" diff --git a/postfix/src/global/mail_version.h b/postfix/src/global/mail_version.h index 33fe9315e..47369cd68 100644 --- a/postfix/src/global/mail_version.h +++ b/postfix/src/global/mail_version.h @@ -20,7 +20,7 @@ * Patches change both the patchlevel and the release date. Snapshots have no * patchlevel; they change the release date only. */ -#define MAIL_RELEASE_DATE "20100913" +#define MAIL_RELEASE_DATE "20100914" #define MAIL_VERSION_NUMBER "2.8" #ifdef SNAPSHOT diff --git a/postfix/src/postscreen/postscreen.c b/postfix/src/postscreen/postscreen.c index cca2ae066..2026c0919 100644 --- a/postfix/src/postscreen/postscreen.c +++ b/postfix/src/postscreen/postscreen.c @@ -7,10 +7,10 @@ /* \fBpostscreen\fR [generic Postfix daemon options] /* DESCRIPTION /* The Postfix \fBpostscreen\fR(8) server performs triage on -/* multiple inbound SMTP connections in parallel. While -/* \fBpostscreen\fR(8) keeps spambots away from Postfix SMTP -/* server processes, more Postfix SMTP server processes remain -/* available for legitimate clients. +/* multiple inbound SMTP connections in parallel. While a +/* single \fBpostscreen\fR(8) process keeps spambots away from +/* Postfix SMTP server processes, more Postfix SMTP server +/* processes remain available for legitimate clients. /* /* \fBpostscreen\fR(8) maintains a temporary whitelist for /* clients that have passed a number of tests. When an SMTP @@ -133,8 +133,8 @@ /* .IP "\fBpostscreen_greet_wait (${stress?2}${stress:6}s)\fR" /* The amount of time that \fBpostscreen\fR(8) will wait for an SMTP /* client to send a command before its turn, and for DNS blocklist -/* lookup results to arrive (default: 2 seconds under stress, 6 seconds -/* normally). +/* lookup results to arrive (default: up to 2 seconds under stress, +/* up to 6 seconds otherwise). /* .IP "\fBpostscreen_helo_required ($smtpd_helo_required)\fR" /* Require that a remote SMTP client sends HELO or EHLO before /* commencing a MAIL transaction. @@ -170,7 +170,7 @@ /* .IP "\fBpostscreen_bare_newline_ttl (30d)\fR" /* The amount of time that \fBpostscreen\fR(8) will cache results from /* a successful "bare newline" SMTP protocol test. -/* .IP "\fBpostscreen_dnsbl_ttl (1d)\fR" +/* .IP "\fBpostscreen_dnsbl_ttl (1h)\fR" /* The amount of time that \fBpostscreen\fR(8) will cache results from /* a successful DNS blocklist test. /* .IP "\fBpostscreen_greet_ttl (1d)\fR" @@ -843,7 +843,7 @@ int main(int argc, char **argv) 0, }; static const CONFIG_NINT_TABLE nint_table[] = { - VAR_PS_POST_QLIMIT, DEF_PS_POST_QLIMIT, &var_ps_post_queue_limit, 10, 0, + VAR_PS_POST_QLIMIT, DEF_PS_POST_QLIMIT, &var_ps_post_queue_limit, 5, 0, VAR_PS_PRE_QLIMIT, DEF_PS_PRE_QLIMIT, &var_ps_pre_queue_limit, 10, 0, 0, }; diff --git a/postfix/src/postscreen/postscreen.h b/postfix/src/postscreen/postscreen.h index 2c357e737..4dc351c6a 100644 --- a/postfix/src/postscreen/postscreen.h +++ b/postfix/src/postscreen/postscreen.h @@ -78,21 +78,33 @@ typedef struct { * Important: every MUMBLE_TODO flag must have a MUMBLE_PASS flag, such that * MUMBLE_PASS == PS_STATE_FLAGS_TODO_TO_PASS(MUMBLE_TODO). * - * MUMBLE_TODO flags must not be cleared once they are raised. The code in - * ps_conclude() depends on this when it decides that all unfinished tests - * are completed. + * MUMBLE_TODO flags must not be cleared once raised. The _TODO_TO_PASS and + * _TODO_TO_DONE macros depend on this to decide that a group of tests is + * passed or completed. + * + * MUMBLE_DONE flags are used for "early" tests that have final results. + * + * MUMBLE_SKIP flags are used for "deep" tests where the client messed up. + * These flags look like MUMBLE_DONE but they are different. Deep tests can + * tentatively pass, but can still fail later in a session. The "ignore" + * action introduces an additional complication. MUMBLE_PASS indicates + * either that a deep test passed tentatively, or that the test failed but + * the result was ignored. MUMBLE_FAIL, on the other hand, is always final. + * We use MUMBLE_SKIP to indicate that a decision was either "fail" or + * forced "pass". */ #define PS_STATE_FLAGS_TODO_TO_PASS(todo_flags) ((todo_flags) >> 1) +#define PS_STATE_FLAGS_TODO_TO_DONE(todo_flags) ((todo_flags) << 1) - /* Room here for one more before-handshake test. */ - -#define PS_STATE_FLAG_PREGR_FAIL (1<<10) /* failed pregreet test */ -#define PS_STATE_FLAG_PREGR_PASS (1<<11) /* passed pregreet test */ -#define PS_STATE_FLAG_PREGR_TODO (1<<12) /* pregreet test expired */ +#define PS_STATE_FLAG_PREGR_FAIL (1<<8) /* failed pregreet test */ +#define PS_STATE_FLAG_PREGR_PASS (1<<9) /* passed pregreet test */ +#define PS_STATE_FLAG_PREGR_TODO (1<<10) /* pregreet test expired */ +#define PS_STATE_FLAG_PREGR_DONE (1<<11) /* decision is final */ -#define PS_STATE_FLAG_DNSBL_FAIL (1<<13) /* failed DNSBL test */ -#define PS_STATE_FLAG_DNSBL_PASS (1<<14) /* passed DNSBL test */ -#define PS_STATE_FLAG_DNSBL_TODO (1<<15) /* DNSBL test expired */ +#define PS_STATE_FLAG_DNSBL_FAIL (1<<12) /* failed DNSBL test */ +#define PS_STATE_FLAG_DNSBL_PASS (1<<13) /* passed DNSBL test */ +#define PS_STATE_FLAG_DNSBL_TODO (1<<14) /* DNSBL test expired */ +#define PS_STATE_FLAG_DNSBL_DONE (1<<15) /* decision is final */ /* Room here for one more after-handshake test. */ @@ -142,6 +154,8 @@ typedef struct { /* * Separate aggregates for early tests and deep tests. */ +#define PS_STATE_FLAG_EARLY_DONE \ + (PS_STATE_FLAG_PREGR_DONE | PS_STATE_FLAG_DNSBL_DONE) #define PS_STATE_FLAG_EARLY_TODO \ (PS_STATE_FLAG_PREGR_TODO | PS_STATE_FLAG_DNSBL_TODO) #define PS_STATE_FLAG_EARLY_PASS \ @@ -359,7 +373,7 @@ extern void ps_cache_update(DICT_CACHE *, const char *, const char *); */ extern void ps_dnsbl_init(void); extern int ps_dnsbl_retrieve(const char *, const char **); -extern void ps_dnsbl_request(const char *); +extern void ps_dnsbl_request(const char *, void (*) (int, char *), char *); /* * postscreen_tests.c diff --git a/postfix/src/postscreen/postscreen_dnsbl.c b/postfix/src/postscreen/postscreen_dnsbl.c index 1016ea65b..f40520f8e 100644 --- a/postfix/src/postscreen/postscreen_dnsbl.c +++ b/postfix/src/postscreen/postscreen_dnsbl.c @@ -8,8 +8,10 @@ /* /* void ps_dnsbl_init(void) /* -/* void ps_dnsbl_request(client_addr) +/* void ps_dnsbl_request(client_addr, callback, context) /* char *client_addr; +/* void (*callback)(int, char *); +/* char *context; /* /* int ps_dnsbl_retrieve(client_addr, dnsbl_name) /* char *client_addr; @@ -24,8 +26,13 @@ /* /* ps_dnsbl_request() requests a blocklist score for the /* specified client IP address and increments the reference -/* count. The request completes in the background. The -/* client IP address must be in inet_ntop(3) output format. +/* count. The request completes in the background. The client +/* IP address must be in inet_ntop(3) output format. The +/* callback argument specifies a function that is called when +/* the requested result is available. The context is passed +/* on to the callback function. The callback should ignore its +/* first argument (it exists for compatibility with Postfix +/* generic event infrastructure). /* /* ps_dnsbl_retrieve() retrieves the result score requested with /* ps_dnsbl_request() and decrements the reference count. It @@ -100,20 +107,64 @@ typedef struct PS_DNSBL_SITE { /* * Per-client DNSBL scores. * - * One remote SMTP client may make parallel connections and thereby trigger - * parallel blocklist score requests. We combine identical requests under - * the client IP address in a single reference-counted entry. The reference - * count goes up with each score request, and it goes down with each score - * retrieval. + * Some SMTP clients make parallel connections. This can trigger parallel + * blocklist score requests when the pre-handshake delays of the connections + * overlap. + * + * We combine requests for the same score under the client IP address in a + * single reference-counted entry. The reference count goes up with each + * request for a score, and it goes down with each score retrieval. Each + * score has one or more requestors that need to be notified when the result + * is ready, so that postscreen can terminate a pre-handshake delay when all + * pre-handshake tests are completed. */ static HTABLE *dnsbl_score_cache; /* indexed by client address */ +typedef struct { + void (*callback) (int, char *); /* generic call-back routine */ + char *context; /* generic call-back argument */ +} PS_CALL_BACK_ENTRY; + typedef struct { const char *dnsbl; /* one contributing DNSBL */ int total; /* combined blocklist score */ int refcount; /* score reference count */ + int pending_lookups; /* nr of DNS requests in flight */ + /* Call-back table support. */ + int index; /* next table index */ + int limit; /* last valid index */ + PS_CALL_BACK_ENTRY table[1]; /* actually a bunch */ } PS_DNSBL_SCORE; +#define PS_CALL_BACK_INIT(sp) do { \ + (sp)->limit = 0; \ + (sp)->index = 0; \ + } while (0) + +#define PS_CALL_BACK_EXTEND(hp, sp) do { \ + if ((sp)->index >= (sp)->limit) { \ + int _count_ = ((sp)->limit ? (sp)->limit * 2 : 5); \ + (hp)->value = myrealloc((char *) (sp), sizeof(*(sp)) + \ + _count_ * sizeof((sp)->table)); \ + (sp) = (PS_DNSBL_SCORE *) (hp)->value; \ + (sp)->limit = _count_; \ + } \ + } while (0) + +#define PS_CALL_BACK_ENTER(sp, fn, ctx) do { \ + PS_CALL_BACK_ENTRY *_cb_ = (sp)->table + (sp)->index++; \ + _cb_->callback = (fn); \ + _cb_->context = (ctx); \ + } while (0) + +#define PS_CALL_BACK_NOTIFY(sp, ev) do { \ + PS_CALL_BACK_ENTRY *_cb_; \ + for (_cb_ = (sp)->table; _cb_ < (sp)->table + (sp)->index; _cb_++) \ + _cb_->callback((ev), _cb_->context); \ + } while (0) + +#define PS_NULL_EVENT (0) + /* * Per-request state. * @@ -306,32 +357,68 @@ static void ps_dnsbl_receive(int event, char *context) } if (reply_argv != 0) argv_free(reply_argv); + } else { + msg_warn("%s: unexpected event: %d", myname, event); } + + /* + * We're done with this stream. Notify the requestor(s) that the result + * is ready to be picked up. If this call isn't made, clients have to sit + * out the entire pre-handshake delay. + */ + score->pending_lookups -= 1; + if (score->pending_lookups == 0) + PS_CALL_BACK_NOTIFY(score, PS_NULL_EVENT); + vstream_fclose(stream); } /* ps_dnsbl_request - send dnsbl query, increment reference count */ -void ps_dnsbl_request(const char *client_addr) +void ps_dnsbl_request(const char *client_addr, + void (*callback) (int, char *), + char *context) { const char *myname = "ps_dnsbl_request"; int fd; VSTREAM *stream; HTABLE_INFO **ht; PS_DNSBL_SCORE *score; + HTABLE_INFO *hash_node; /* - * Avoid duplicate effort when this lookup is already in progress. We - * store a reference-counted DNSBL score under its client IP address. We - * increment the reference count with each request, and decrement the - * reference count with each retrieval. + * Some spambots make several connections at nearly the same time, + * causing their pregreet delays to overlap. Such connections can share + * the efforts of DNSBL lookup. + * + * We store a reference-counted DNSBL score under its client IP address. We + * increment the reference count with each score request, and decrement + * the reference count with each score retrieval. * - * XXX Notify the requestor as soon as all DNS replies are in, to avoid - * unnecessary delay when we only need the DNSBL score. + * Do not notify the requestor NOW when the DNS replies are already in. + * Reason: we must not make a backwards call while we are still in the + * middle of executing the corresponding forward call. Instead we create + * a zero-delay timer request and call the notification function from + * there. + * + * ps_dnsbl_request() could instead return a result value to indicate that + * the DNSBL score is already available, but that would complicate the + * caller with two different notification code paths: one asynchronous + * code path via the callback invocation, and one synchronous code path + * via the ps_dnsbl_request() result value. That would be a source of + * future bugs. */ - if ((score = (PS_DNSBL_SCORE *) - htable_find(dnsbl_score_cache, client_addr)) != 0) { + if ((hash_node = htable_locate(dnsbl_score_cache, client_addr)) != 0) { + score = (PS_DNSBL_SCORE *) hash_node->value; score->refcount += 1; + PS_CALL_BACK_EXTEND(hash_node, score); + PS_CALL_BACK_ENTER(score, callback, context); + if (msg_verbose > 1) + msg_info("%s: reuse blocklist score for %s refcount=%d pending=%d", + myname, client_addr, score->refcount, + score->pending_lookups); + if (score->pending_lookups == 0) + event_request_timer(callback, context, EVENT_NULL_DELAY); return; } if (msg_verbose > 1) @@ -339,6 +426,9 @@ void ps_dnsbl_request(const char *client_addr) score = (PS_DNSBL_SCORE *) mymalloc(sizeof(*score)); score->total = 0; score->refcount = 1; + score->pending_lookups = 0; + PS_CALL_BACK_INIT(score); + PS_CALL_BACK_ENTER(score, callback, context); (void) htable_enter(dnsbl_score_cache, client_addr, (char *) score); /* @@ -350,7 +440,7 @@ void ps_dnsbl_request(const char *client_addr) for (ht = dnsbl_site_list; *ht; ht++) { if ((fd = LOCAL_CONNECT("private/" DNSBL_SERVICE, NON_BLOCKING, 1)) < 0) { msg_warn("%s: connect to " DNSBL_SERVICE " service: %m", myname); - return; + continue; } stream = vstream_fdopen(fd, O_RDWR); attr_print(stream, ATTR_FLAG_NONE, @@ -360,10 +450,11 @@ void ps_dnsbl_request(const char *client_addr) if (vstream_fflush(stream) != 0) { msg_warn("%s: error sending to " DNSBL_SERVICE " service: %m", myname); vstream_fclose(stream); - return; + continue; } PS_READ_EVENT_REQUEST(vstream_fileno(stream), ps_dnsbl_receive, (char *) stream, DNSBLOG_TIMEOUT); + score->pending_lookups += 1; } } diff --git a/postfix/src/postscreen/postscreen_early.c b/postfix/src/postscreen/postscreen_early.c index e6ed4d974..4cf380f50 100644 --- a/postfix/src/postscreen/postscreen_early.c +++ b/postfix/src/postscreen/postscreen_early.c @@ -195,14 +195,49 @@ static void ps_early_event(int event, char *context) msg_panic("%s: unknown pregreet action value %d", myname, ps_pregr_action); } - if (elapsed > PS_EFF_GREET_WAIT) - elapsed = PS_EFF_GREET_WAIT; - event_request_timer(ps_early_event, context, - PS_EFF_GREET_WAIT - elapsed); + + /* + * Terminate the greet delay if we're just waiting for the pregreet + * test to complete. It is safe to call ps_early_event directly, + * since we are already in that function. + * + * XXX After this code passes all tests, swap around the two blocks in + * this switch statement and fall through from EVENT_READ into + * EVENT_TIME, instead of calling ps_early_event recursively. + */ + state->flags |= PS_STATE_FLAG_PREGR_DONE; + if (elapsed >= PS_EFF_GREET_WAIT + || ((state->flags & PS_STATE_FLAG_EARLY_DONE) + == PS_STATE_FLAGS_TODO_TO_DONE(state->flags & PS_STATE_FLAG_EARLY_TODO))) + ps_early_event(EVENT_TIME, context); + else + event_request_timer(ps_early_event, context, + PS_EFF_GREET_WAIT - elapsed); return; } } +/* ps_early_dnsbl_event - cancel pregreet timer if waiting for DNS only */ + +static void ps_early_dnsbl_event(int unused_event, char *context) +{ + const char *myname = "ps_early_dnsbl_event"; + PS_STATE *state = (PS_STATE *) context; + + if (msg_verbose) + msg_info("%s: notify %s:%s", myname, PS_CLIENT_ADDR_PORT(state)); + + /* + * Terminate the greet delay if we're just waiting for DNSBL lookup to + * complete. Don't call ps_early_event directly, that would result in a + * dangling pointer. + */ + state->flags |= PS_STATE_FLAG_DNSBL_DONE; + if ((state->flags & PS_STATE_FLAG_EARLY_DONE) + == PS_STATE_FLAGS_TODO_TO_DONE(state->flags & PS_STATE_FLAG_EARLY_TODO)) + event_request_timer(ps_early_event, context, EVENT_NULL_DELAY); +} + /* ps_early_tests - start the early (before protocol) tests */ void ps_early_tests(PS_STATE *state) @@ -231,7 +266,8 @@ void ps_early_tests(PS_STATE *state) * Run a DNS blocklist query. */ if ((state->flags & PS_STATE_FLAG_DNSBL_TODO) != 0) - ps_dnsbl_request(state->smtp_client_addr); + ps_dnsbl_request(state->smtp_client_addr, ps_early_dnsbl_event, + (char *) state); /* * Wait for the client to respond or for DNS lookup to complete. diff --git a/postfix/src/postscreen/postscreen_state.c b/postfix/src/postscreen/postscreen_state.c index b449f957a..06a037fab 100644 --- a/postfix/src/postscreen/postscreen_state.c +++ b/postfix/src/postscreen/postscreen_state.c @@ -227,10 +227,12 @@ const char *ps_print_state_flags(int flags, const char *context) "PREGR_FAIL", PS_STATE_FLAG_PREGR_FAIL, "PREGR_PASS", PS_STATE_FLAG_PREGR_PASS, "PREGR_TODO", PS_STATE_FLAG_PREGR_TODO, + "PREGR_DONE", PS_STATE_FLAG_PREGR_DONE, "DNSBL_FAIL", PS_STATE_FLAG_DNSBL_FAIL, "DNSBL_PASS", PS_STATE_FLAG_DNSBL_PASS, "DNSBL_TODO", PS_STATE_FLAG_DNSBL_TODO, + "DNSBL_DONE", PS_STATE_FLAG_DNSBL_DONE, "PIPEL_FAIL", PS_STATE_FLAG_PIPEL_FAIL, "PIPEL_PASS", PS_STATE_FLAG_PIPEL_PASS, diff --git a/postfix/src/util/dict_thash.c b/postfix/src/util/dict_thash.c index 116ab5bca..c6fb98184 100644 --- a/postfix/src/util/dict_thash.c +++ b/postfix/src/util/dict_thash.c @@ -169,7 +169,7 @@ DICT *dict_thash_open(const char *path, int open_flags, int dict_flags) dict_thash->dict.lookup = dict_thash_lookup; dict_thash->dict.sequence = dict_thash_sequence; dict_thash->dict.close = dict_thash_close; - dict_thash->dict.flags = dict_flags | DICT_FLAG_FIXED; + dict_thash->dict.flags = dict_flags | DICT_FLAG_DUP_WARN | DICT_FLAG_FIXED; if (dict_flags & DICT_FLAG_FOLD_FIX) dict_thash->dict.fold_buf = vstring_alloc(10); dict_thash->info = 0; diff --git a/postfix/src/util/events.c b/postfix/src/util/events.c index 50751db96..c8cc6aeca 100644 --- a/postfix/src/util/events.c +++ b/postfix/src/util/events.c @@ -61,7 +61,8 @@ /* event_request_timer() causes the specified callback function to /* be called with the specified context argument after \fIdelay\fR /* seconds, or as soon as possible thereafter. The delay should -/* not be negative. +/* not be negative (the manifest EVENT_NULL_DELAY provides for +/* convenient zero-delay notification). /* The event argument is equal to EVENT_TIME. /* Only one timer request can be active per (callback, context) pair. /* Calling event_request_timer() with an existing (callback, context) diff --git a/postfix/src/util/events.h b/postfix/src/util/events.h index 200f7594b..6ea89119a 100644 --- a/postfix/src/util/events.h +++ b/postfix/src/util/events.h @@ -45,8 +45,9 @@ extern void event_fork(void); /* * Dummies. */ -#define EVENT_NULL_TYPE 0 +#define EVENT_NULL_TYPE (0) #define EVENT_NULL_CONTEXT ((char *) 0) +#define EVENT_NULL_DELAY (0) /* LICENSE /* .ad