Wietse Venema:
> Melvin Vermeeren:
> > In conclusion I believe "var_smtpd_sasl_enable" can be removed from
> > src/smtpd/ smtpd_check.c completely, together with the "if
> > (var_smtpd_sasl_enable)" conditionals. Could you post your thoughts on
> > this?
> 
> You can try that. But your argument has holes because there is code
> like this:
> 
>       if (state->sasl_enabled && state->sasl_enabled[0]) {
>           do_something_with_state(state, *cpp, def_acl);
> 
> where do_something_with_state() may access state->sasl_mechanism
> or state->sasl_sender which will not contain the expected values
> that they would have gotten from a real SASL backend.
> 
> Therefore every such function will need guards like:
> 
>     if (state->sasl_mechanism && state->sasl_mechanism[0])
>       do_something_with_sasl_mechanism
> 
> Some guards may already exist, but this will have to be verified
> exhaustively for every function.

In postfix-3.6-20200316 src/smtpd/smtpd_check.c currently four 
if(var_smtpd_sasl_enable) exist. After those checks additional checks are 
performed, one of:

a. if (state->sasl_username && state->sasl_username[0])
b. if (state->sender && *state->sender)

Once those pass at least one of the following three functions is invoked:

1. check_sasl_access(), after a.
2. reject_auth_sender_login_mismatch(), after b.
3. reject_unauth_sender_login_mismatch(), after b.

1. seems to map directly to a check_access() call, which appears to only 
lookup things in the configured tables.
2. appears to only lookup things in the sender login maps.
3. is similar to 2, but does an even simpler lookup in the sender login maps.

Of these three, check_sasl_access() is the only one that that does not check 
state->sasl_username formally, instead it is checked where it is called. I 
would be surprised if any of the check/table logic did modify SASL state, so I 
believe things are safe as is.

Note that only smtpd_check.c needs modification. I will try to write a simple 
patch within the next few days and do some testing, will keep you up-to-date. 
Thanks for the info so far.

Melvin.

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to