From: Victor Duchovni (Victor.DuchovniMorganStanley.com)
Date: Sat Dec 01 2007 - 13:03:27 CST

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Sat, Dec 01, 2007 at 05:30:00PM +0000, Keean Schupke wrote:

> Here is V2 of the patch. There is now a generic caching service
> (cache/cache.c) that effectively implements a dictionary service with
> "var_cache_name" as the cache backing store. It implements "query",
> "update" and "delete", but has no timeout policy, and does not hack
> the strings around like verify.c does (so all characters are valid in
> the key and the value).
>
> This leaves the client side to deal with any string mangling and
> timeout policy, without having to worry about what the backend might
> be doing.

Given that there is also a "var_cache_service", to allow different
clients to choose split caches, this seems reasonable. Given the generic
nature of the cache, the parameter that controls the cache database name
is mis-named.

    #define VAR_CACHE_NAME "smtp_sasl_auth_cache_name"

It should be something like "cache_map" (along the lines of
address_verify_map). And then in master.cf something along the lines of:

    master.cf:
        auth_cache unix ... cache
            -o cache_map=$smtp_auth_cache_map

    main.cf:
        # Optional:
            # smtp_auth_cache_map = btree:/var/lib/postfix/smtp_auth_cache

Minor drawback is that the "smtp_auth_cache_map" parameter won't be
known to "postconf".

> +static void smtp_sasl_make_cache_key(VSTRING *buf,const char *host,
> + const char *user,const char *pass) {
> + VSTRING *tmp = vstring_alloc(10);
> + vstring_sprintf(tmp,"%s;%s;%s",host,user,pass);
> + base64_encode(buf,(const char*)SHA1((unsigned char*)STR(tmp),
> + (unsigned long)VSTRING_LEN(tmp),0),SHA_DIGEST_LENGTH);
> + vstring_free(tmp);
> +}

Violation of Postfix indentation style... :-) The opening brace of a
function should be its own line... Variable declarations are followed
by a blank line and then the code.

> +static int smtp_sasl_parse_cache_value(char *buf, long *timestamp,
> + char **respdsn, char **respstr) {
> + if ((*respdsn = split_at(buf,':')) != 0
> + && (*respstr = split_at(*respdsn,':')) != 0 && alldig(buf)) {
> + *timestamp = atol(buf);
> + return (0);
> + }
> + msg_warn("bad smtp_sasl_auth_cache_map entry: %.100s", buf);
> + return (-1);
> +}

May want to validate the DSN string here, and perhaps use strtol()
or sscanf() instead of atol() to check for trailing junk, ...

> + char *cached_respstr = 0;
> + long now = (long) time((time_t *) 0);
> + long timestamp = 0;

Good, you are also "truncating" the time down to "long", so comparison
should yield decent results even if time_t is larger.

> + smtp_sasl_make_cache_key(buf, session->host,
> session->sasl_username, session->sasl_passwd);
> + if (cache_client_query(STR(buf), get_buf) == 0) {
> + if (vstring_strcpy(put_buf,STR(get_buf))
> + && smtp_sasl_parse_cache_value(STR(get_buf),&timestamp,
> + &cached_respdsn,&cached_respstr) == 0
> + && timestamp + var_smtp_sasl_auth_cache_time > now) {
> + dsb_update(why, cached_respdsn, DSB_DEF_ACTION, DSB_MTYPE_DNS,
> + session->host, var_procname, cached_respstr,
> + "SASL [CACHED] authentication failed; server %s : %s",
> + session->namaddr, STR(put_buf));
> + status = -1;
> + } else {
> + /* timeout or corrupt cache entry */
> + cache_client_delete(STR(buf));
> + }
> + }
> + vstring_free(put_buf);
> + vstring_free(get_buf);
> + vstring_free(buf);
> + return (status);
> +}

With SASL soft failures, the DSN should perhaps be downgraded from 5XX
to 4XX here? And validated somewhere to be either a 5XX or 4XX?

> if (msg_verbose)
> msg_info("%s: %s: SASL mechanisms %s",
> myname, session->namaddr, session->sasl_mechanism_list);
>
> + if (var_smtp_sasl_auth_cache_enable) {
> + if (smtp_sasl_cache_query(session,why)) { return (0); }
> + }
> +
> /*
> * Start the client side authentication protocol.
> */
> result = xsasl_client_first(session->sasl_client,
> session->sasl_mechanism_list,
> -348,10 +432,15
> dsb_update(why, resp->dsn, DSB_DEF_ACTION,
> DSB_MTYPE_DNS, session->host,
> var_procname, resp->str,
> "SASL authentication failed; server %s said: %s",
> session->namaddr, resp->str);
> +
> + if (var_smtp_sasl_auth_cache_enable) {
> + smtp_sasl_cache_update(session, resp);
> + }

Here, you should be more selective (code == 535) about which SASL auth
failures are cached as "bad password" errors. While soft-fail on AUTH
failure is reason agnostic, blocking all deliveries for the same password
requires more care.

--
        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:majordomopostfix.org?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.

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]