From: Victor Duchovni (no email)
Date: Fri Nov 30 2007 - 12:46:23 EST
On Fri, Nov 30, 2007 at 01:48:36PM +0000, Keean Schupke wrote:
> There are three configuration variables, "smtp_sasl_auth_cache_enable"
Fine.
> (obvious), "smtp_sasl_auth_cache_name" (the db file for caching, if
> blank or not defined, a ram only cache is used), and
> "smtp_sasl_auth_cache_time" (how long failures remain in the cache,
> defaults to 90 days).
What happened to the suggestion of using the "verify" service? The patch
as implemented potentially has multiple processes writing the table,
we don't support this mode of operation. You seem to be working around
that by turning off both the idle timeout and the max use counter, and
presumably using a transport with a process limit of 1, so you have just
one delivery agent running "forever", but this is not a good idea.
Using "verify" would make these issues moot. Why not?
> +void smtp_sasl_post_init(void) {
> + if (*var_smtp_sasl_auth_cache_name == 0) {
> + var_use_limit = 0;
> + var_idle_limit = 0;
> + }
> +}
Why do we need a post-init call to just set two variables?
> int smtp_sasl_authenticate(SMTP_SESSION *session, DSN_BUF *why)
> {
> const char *myname = "smtp_sasl_authenticate";
> SMTP_RESP *resp;
> const char *mechanism;
> int result;
> char *line;
> + VSTRING *buf = 0;
> + VSTRING *get_buf = 0;
> + VSTRING *put_buf = 0;
Best to avoid automatic pointers to malloced memory, use one of the buffers
in "state", or use a static buffer allocated once.
> /*
> + * Check auth failure cache, and fail if this (relayhost,user,pass)
> + * triple matches an entry that is new enough.
> + */
> + if (var_smtp_sasl_auth_cache_enable && (buf = vstring_alloc(10))) {
> + vstring_sprintf(buf, "%s:%s:%s", session->sasl_username,
> + session->host, session->sasl_passwd);
Don't store the unhashed password in this table. You are leaking
sensitive data. Use a pre-image attack resistant hash function. SHA-1
should do. You can get SHA-1 from OpenSSL's libcrypto.
> if (var_smtp_sasl_auth_soft_bounce && resp->code / 100 == 5) {
> STR(resp->dsn_buf)[0] = '4';
> }
> // end: KPS20071119 - fix: Incorrect authentication data
The original Postfix code has no C++ style comments, so this context
diff is not based on unmodified Postfix code. Please don't add C++
comments to Postfix source code.
-- Viktor. Disclaimer: off-list followups get on-list replies or get ignored. Please do not ignore the "Reply-To" header. To unsubscribe from the postfix-users list, visit http://www.postfix.org/lists.html or click the link below: <mailto:?body=unsubscribe%20postfix-users> If my response solves your problem, the best way to thank me is to not send an "it worked, thanks" follow-up. If you must respond, please put "It worked, thanks" in the "Subject" so I can delete these quickly.
|
|
|