On Aug 30, 2005, at 11:19 AM, Brian J. France wrote:

This past week I have finished up a few modules and ready for review.

http://www.brianfrance.com/software/apache/mod_smtpd_load.tar.gz

mod_smtpd_load:
This module allows rejecting connection (temporarily) based on server load It is not very cross platform (any os with getloadavg), but I am sure we
  can work on that.

I have finished mod_smtpd_access_dbi, but after talking with Paul on IRC I need to convert it to use mod_dbd instead of mod_dbi_pool. Working on that now and will post another message when done. I could build a tar ball of this module if anybody is interested as the the flow will not change, just how the
db connection is handled.

mod_smtpd_access_dbi:
    This module is similar to sendmails access file.  It allows
    checking of the ip/hostname/from/to items via a database to see if
    things should be rejected.  It uses mod_dbi_pool and libdbi.
Thanks to Paul for mod_dbi_pool and code examples from mod_vhost_dbi.

This is great!

A few things I have found while writing these modules. How about changing smtpd_return_data from this:

typedef struct smtpd_return_data {
    apr_pool_t *p;
    /* list of messages */
    apr_array_header_t *msgs;
} smtpd_return_data;

to something like this:

typedef struct smtpd_return_data {
    apr_pool_t *p;
    int code;
    /* list of messages */
    apr_array_header_t *user_msgs;
    apr_array_header_t *log_msgs;
} smtpd_return_data;

While doing the mod_smtpd_load module I found when I want to deny a connection I can set what message the user will get, but I also want to log a different message instead of the default "Connection Denied" (current I log my own and the default gets logged). Of course this might be another thread of how and what do we plan on logging.


IMO I think that logging twice is fine. mod_smtpd logs "Connection Denied" because a server admin may want to know what connections are getting denied for debugging and etc. and I feel its absolutely necessary, same with the MAIL command. If your module wants to log extra information, I think that's OK.

Maybe for clarity all mod_smtpd's logs can be prefixed with "mod_smtpd:" and other modules can follow suit.

I would also like to set the error code, because looking over rfc0821 I think it should return 452 or may be that needs to be a default for smtpd_run_connect soft errors (552 for hard errors). Should we allow the module to set the error code?

Error codes can be sent with SMTPD_DONE. Otherwise error codes won't change in the next fifty years for things like SMTPD_DENY and friends. I'll look into changing those specific codes. Most of the error codes I got from Postfix and Qpsmtpd but of course they aren't perfect.

In mod_smtpd_access_dbi I found it strange that I get a string that looks like this: " <[EMAIL PROTECTED]>" instead of "[EMAIL PROTECTED]". For the mail/rcpt hooks should we send a struct that has the full line sent, the data from the full line and the parsed email address? I have some code that duplicates the string and then remove spaces and < from the beginning and > from the end, but that seems like it should be done before my function is called. Another problem I can see is when we get into things like the size options:

MAIL FROM:<[EMAIL PROTECTED]> SIZE=500000

Do we want every module to have to parse the email address (removing < >) and the SIZE?

This is a very good point. Honestly I'm not really sure since I'm not experienced with writing a plugin based architecture, but I'm almost positive that most people will say that mod_smtpd should parse the email address and options along with it. When I first wrote mod_smtpd I figured I'd pass the un-parsed string after from: verbatim to let the modules decide what to do with it. Maybe to allow some crazy things. Either way, having mod_smtpd parse sounds like the right idea so how about this hook for mail:

smtp_run_mail(smtp_conn_rec *, smtp_return_data *, char *parsed_address, apr_array_header_t *options);

The options won't be stored as name->value pairs: more like an array of strings like "SIZE=50000". Does that sound good?

Rian Hunter

Reply via email to