httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian J. France" <l...@firehawksystems.com>
Subject Re: [mod_smtpd] patch need for the SIZE extension
Date Wed, 08 Feb 2006 15:56:25 GMT
On Feb 8, 2006, at 12:42 AM, Rian Hunter wrote:
> On Feb 7, 2006, at 8:15 PM, Brian J. France wrote:
>>   Before I started converting my other modules to the new code I  
>> figured I would start with writing a new module to handle the SIZE  
>> extension.  I needed to apply the following patch (link below) to  
>> the mod_smtpd code to get access at the max data size.
>
> +0, Well this is an interesting point. The max data size should be  
> settable and gettable from extending modules. In a way max_data is  
> a lot like the list of extensions: should it be set on every  
> connection or upon server initialization (ie should it's scope be  
> per connection or per mod_smtpd instance). I chose that the list of  
> extensions might want to be per-connection since modules may not  
> want to offer all clients all extensions, just the same way modules  
> may want to enforce different max_data sizes for different clients.  
> Where do you think these variables belong?
>
> One thing I do know is that max_data doesn't belong in mod_smtpd's  
> configuration structure and rather either in some other "per  
> mod_smtpd instance" structure accessible to extending modules or  
> just in scr (the per connection structure). Right? I think that  
> would be cleaner and more modular. We should talk about this some  
> more before I apply this patch.

+1, I like the idea of the storing the setting in the per connection  
instance.  Leaving the default in the core and copying it to an per  
connection struct would allow modules to tweak the setting per  
connection.

One other addition to the smtpd_session_rec I would like to see would  
be something like r->notes or r->subprocess_env.  Some way for  
modules to set flags that would change the behavior of another  
module.  As an example I might want to add a new modules that changes  
the max data size based on either connection ip or mail from  
address.  This module would want to set a flag telling the size  
module to not show the numerical size limit in the ehlo response as  
it might change based on the the mail from address.

>>   I hooked the mail from hook, check for a valid SIZE in  
>> mail_parameters and check to make sure it is not over the limit.   
>> If it is over the limit I can use smtpd_respond_oneline to send  
>> the 552 "message exceeds fixed maximum message size" line back to  
>> the client, but what should the function return to make it force a  
>> QUIT or REST command as anything but SMTPD_DONE sends more stuff  
>> to the client.
>>
>> Should I just return SMTPD_DONE and set scr->should_disconnect?   
>> Could we tweak it to support two different settings, one would  
>> only allow QUIT only and the other would allow QUIT and REST (to  
>> start over).
>
> +1, Since quit and rset are both handled by mod_smtpd there should  
> probably be another variable called scr->only_quit_or_rset, also  
> because I think there are other times when the client should only  
> issue a quit or rset. More discussion follows though:
>
> My basic design strategy has been that mod_smtpd should do as  
> little as possible or what will be practical to a large amount of  
> extending modules. If mod_smtpd sets a variable in a structure it  
> should use it. If an extending module sets a variable in a  
> structure it should use it. Having an "only_quit_or_rset" variable  
> in the scr structure with mod_smtpd consciously checking for it,  
> but never changing the value itself sort of violates that strategy.  
> That doesn't mean I'm against it because like I already mentioned I  
> think a lot of modules will need this sort of thing.
>
> Have you thought about hooking into all the commands, and then  
> sending a "503 Only QUIT and RSET" from your SIZE module until a  
> rset is received?
>
> I only added the scr->should_disconnect logic since it's what  
> should happen when a simple module wants to SMTPD_DENY some  
> connections. I didn't want every module that wanted to implement  
> connection policy to have to deal with being hooked into all the  
> commands, and just be able to return SMTPD_DENY.
>
> The scr->should_disconnect situation I just explained (where I  
> didn't want every modules to have to deal with hooking all the  
> commands for a redundant task) could be applied to any potential  
> module that wants to disallow any of the built in commands with  
> instead a bit-field that specifies which mod_smtpd core commands  
> are currently allowed. I think I like this idea the best, what do  
> you think?

+1, if I understand you currently.  Changing scr->should_disconnect  
to scr->allows_commands which is a bit-field of what commands that  
are allowed.  Then smtp_protocol.c handles denying command instead of  
having a module hook every command and doing it, right?

Brian




Mime
View raw message