Re: New mod_smtpd release
Rian Hunter wrote: Well not exactly. A module that parses headers can register itself as an input_filter for mod_smtpd. It can parse the headers and the headers can be accessed by a get_smtpd_headers(request_rec*) function or similar exported by the parsing module, and modules that rely on this module will know about that API. This module can even be included with the default mod_smptd package. Do you agree that this is possible? It should certainly be included with the mod_smtpd package. Is there any reason why the existing header parsing code for http shouldn't be used for this? AFAICS, the new things we have to deal with for SMTP are: - smtp handshake and envelope (which you've done) - spooling - dealing with multiple recipients - dealing with MIME message MIME decoding of multipart messages is something I'd like to see (for spam filtering), but it's not an immediate priority. If we have the initial RFC822 headers in r-headers_in, that'll be fine for now. Spooling and dealing with multiple recipients are things I'd like to think about now. ISTR posting about this before: I think each recipient needs a separate request processing cycle, because each recipient may have completely different processing rules. Do you think we can deal with that by creating a new suprequest for each recipient? If so, we'll need to allow filter hooks both before and after subrequesting, and pass each one the spooled input data. Either way, lacking header parsing in mod_smtpd is being impractically pedant since probably 99% of SMTP transfers involve messages in the RFC 2822/MIME formats. Although I think that maybe there will be a plugin that wants data from the DATA command verbatim. I still feel this needs some thought. Agreed. Maybe a hook for DATA could deal with pathological cases, with normal header/body handling the default? But I haven't thought it through: I wasn't even aware of the notion of smtp-without-RFC(2)822. -- Nick Kew
Re: New mod_smtpd release
Rian Hunter wrote: [chop] I think I should have looked harder before replying ... looks like you've done more than I realised. I guess what I was asking about slots easily in to data_post ? -- Nick Kew
Re: New mod_smtpd release
On Aug 16, 2005, at 6:37 AM, Nick Kew wrote: Spooling and dealing with multiple recipients are things I'd like to think about now. ISTR posting about this before: I think each recipient needs a separate request processing cycle, because each recipient may have completely different processing rules. Do you think we can deal with that by creating a new suprequest for each recipient? If so, we'll need to allow filter hooks both before and after subrequesting, and pass each one the spooled input data. I've thought about this and it has sort of been an avoided topic but I think the current plan is that in smtpd_run_queue (the hook where modules should queue or deliver the message) each registered hook will be called with the recipient list and access to the message until one of them returns something other than SMTPD_DECLINED or SMTPD_OK (unlike smtpd_run_rcpt, which will stop when a module returns SMTPD_OK). For instance let's say you have a mail message with two recipients: [EMAIL PROTECTED] and [EMAIL PROTECTED] and you want each to be dealt with by different modules because one is a local address and one should be relayed. The modules are mod_smtpd_queue_local and mod_smtpd_queue_relay. Both of the modules have some means of configuration which lets them know what email addresses they accept. When smtpd_run_queue is called on them, they only accept the email addresses they are configured for and ignore the rest. Of course if not one module returns SMTPD_OK then we respond with some sort of 4** error saying that we don't have a queue-er. When a server admin is configuring the accepted email addresses for each module he should be sure the their respective lists are mutually exclusive else he'll get the same addresses being handled by two modules which is probably bad news. This design seems to satisfy the modularity that I was expecting from mod_smtpd. Disabling local delivery or relay can be as simple as not loading a module in a server config. Maybe mod_smtpd can even specify a convention for email address acceptance lists like: SmtpAcceptList my_list1 mysql://foo SmtpAcceptList my_list2 regex:[EMAIL PROTECTED] LocalQueueAccept my_list1 RelayQueueAccept my_list2 and mod_smtpd_queue_{local,relay} both know that they accept mail from my_list1 and my_list2 repectively. The problem with this design is that you have repetitious string comparisons for each hook on smtp_run_queue. This is not a big problem except for when the recipient list is long (~40 recipients) and you have maybe three modules handling mail queuing. My short term solution is that each module remove mail addresses from the recipient list that they handled, so the list grows shorter as each queue hook is called (this also solves the problem of email addresses handled twice). Another short term fix for this is having mod_smtpd hash the recipient list (md5?) as another list passed so comparisons and lookups are quicker. I think this problem is a trade-off though for our modularity. Either way, lacking header parsing in mod_smtpd is being impractically pedant since probably 99% of SMTP transfers involve messages in the RFC 2822/MIME formats. Although I think that maybe there will be a plugin that wants data from the DATA command verbatim. I still feel this needs some thought. Agreed. Maybe a hook for DATA could deal with pathological cases, with normal header/body handling the default? But I haven't thought it through: I wasn't even aware of the notion of smtp-without-RFC(2)822. I figure we'll have an input filter registered and if the data doesn't look like RFC-(2)822, then it passes it on verbatim. -rian
Re: New mod_smtpd release
On Aug 16, 2005, at 6:47 AM, Nick Kew wrote: Rian Hunter wrote: [chop] I think I should have looked harder before replying ... looks like you've done more than I realised. I guess what I was asking about slots easily in to data_post ? Which question exactly? -rian
Re: New mod_smtpd release
Well there's also another problem. RFC 2821 (SMTP) doesn't define a particular message format for SMTP (in wide use there the RFC 822 and MIME message formats). I don't think that mod_smtpd should assume a RFC 822 or MIME message format since its strictly a SMTP module, that's why I agree with this I still think header parsing should be in another module. Of course this module is free to register itself as an mod_smtpd filter and do what it needs to do, but it shouldn't be part of the main mod_smtpd. That seems wise. Any weird thing can come through over SMTP, it could look very much unlike an email after all. You're handling the protocol in your module and that means the SMTP protocol as I understand, not MIME or anything.
Re: New mod_smtpd release
Jem Berkes [EMAIL PROTECTED] writes: Well there's also another problem. RFC 2821 (SMTP) doesn't define a particular message format for SMTP (in wide use there the RFC 822 and MIME message formats). I don't think that mod_smtpd should assume a RFC 822 or MIME message format since its strictly a SMTP module, that's why I agree with this Now I'm confused; 2821 S-2.3.1 defines SMTP content as headers + body. What am I overlooking? I still think header parsing should be in another module. Of course this module is free to register itself as an mod_smtpd filter and do what it needs to do, but it shouldn't be part of the main mod_smtpd. If you put in into a separate module, that means there will be no hooks which can expect the headers to be in r-headers_in. So every hook that needs them will need to parse it themselves, which seems absolutely redundant. Furthermore it is a requirement (MUST) for a 2821 compliant server to implement loop detection. That means at least one hook will *always* care about the Received: headers, for every smtp transaction. Why you wouldn't want to provide an API for hook authors to use for inspecting headers, seems like a very spartan choice, which IMO is counter to the spirit of httpd. -- Joe Schaefer
Re: New mod_smtpd release
On Aug 15, 2005, at 10:22 AM, Joe Schaefer wrote: Jem Berkes [EMAIL PROTECTED] writes: Well there's also another problem. RFC 2821 (SMTP) doesn't define a particular message format for SMTP (in wide use there the RFC 822 and MIME message formats). I don't think that mod_smtpd should assume a RFC 822 or MIME message format since its strictly a SMTP module, that's why I agree with this Now I'm confused; 2821 S-2.3.1 defines SMTP content as headers + body. What am I overlooking? 2821 s-2.3.1 says: If the content conforms to other contemporary standards, the headers form a collection of field/value pairs structured as in the message format specification [32]; the body, if structured, is defined according to MIME [12]. Personally I interpret this to mean that the content may not conform to other contemporary standards although I do doubt the existence of non-RFC 2822 header formats. I still think header parsing should be in another module. Of course this module is free to register itself as an mod_smtpd filter and do what it needs to do, but it shouldn't be part of the main mod_smtpd. If you put in into a separate module, that means there will be no hooks which can expect the headers to be in r-headers_in. So every hook that needs them will need to parse it themselves, which seems absolutely redundant. Furthermore it is a requirement (MUST) for a 2821 compliant server to implement loop detection. That means at least one hook will *always* care about the Received: headers, for every smtp transaction. Why you wouldn't want to provide an API for hook authors to use for inspecting headers, seems like a very spartan choice, which IMO is counter to the spirit of httpd. Well not exactly. A module that parses headers can register itself as an input_filter for mod_smtpd. It can parse the headers and the headers can be accessed by a get_smtpd_headers(request_rec*) function or similar exported by the parsing module, and modules that rely on this module will know about that API. This module can even be included with the default mod_smptd package. Do you agree that this is possible? The module that implements loop detection could rely on that module and also be included with the defautl mod_smtpd package. Either way, lacking header parsing in mod_smtpd is being impractically pedant since probably 99% of SMTP transfers involve messages in the RFC 2822/MIME formats. Although I think that maybe there will be a plugin that wants data from the DATA command verbatim. I still feel this needs some thought. -rian
Re: New mod_smtpd release
Rian Hunter [EMAIL PROTECTED] writes: Either way, lacking header parsing in mod_smtpd is being impractically pedant since probably 99% of SMTP transfers involve messages in the RFC 2822/MIME formats. Although I think that maybe there will be a plugin that wants data from the DATA command verbatim. I still feel this needs some thought. IMO a good bit of code to look at is Qpsmtpd::Transaction, which is what I think should correspond to a request_rec in mod_smtpd. Any per-transaction extensions you need can go into r-request_config. -- Joe Schaefer
Re: New mod_smtpd release
Joe Schaefer wrote: Rian Hunter [EMAIL PROTECTED] writes: Either way, lacking header parsing in mod_smtpd is being impractically pedant since probably 99% of SMTP transfers involve messages in the RFC 2822/MIME formats. Although I think that maybe there will be a plugin that wants data from the DATA command verbatim. I still feel this needs some thought. IMO a good bit of code to look at is Qpsmtpd::Transaction, which is what I think should correspond to a request_rec in mod_smtpd. Any per-transaction extensions you need can go into r-request_config. +1, Qpsmtpd has already solved many of these problems, there's little reason to spend lots of time doing it ourselves when we can just learn from what they've already done. -garrett
Re: New mod_smtpd release
Branko Čibej [EMAIL PROTECTED] writes: May I suggest you resend this patch, using svn diff instead of diff -pur to create it? You're diffing the SVN administrative directory... OK, here's a patch against mod_smtpd trunk that replaces the // comments with /**/: Property changes on: ___ Name: svn:ignore + smtp_core.slo smtp_protocol.slo aclocal.m4 autom4te.cache config.log config.status configure Makefile .libs Index: smtp_protocol.c === --- smtp_protocol.c (revision 232921) +++ smtp_protocol.c (working copy) @@ -90,7 +90,7 @@ handle_func = apr_hash_get(handlers, command, APR_HASH_KEY_STRING); in_data.msg = NULL; -// command not recognized +/* command not recognized */ if (!handle_func) { if (!smtpd_handle_unrecognized_command(r, in_data, command, buffer)) break; @@ -104,7 +104,7 @@ } end: - // flush any output we may have before disconnecting + /* flush any output we may have before disconnecting */ ap_rflush(r); return; } @@ -152,10 +152,10 @@ break; } - // default behavior: + /* default behavior: */ - // RFC 2821 states that when ehlo or helo is received, reset - // state + /* RFC 2821 states that when ehlo or helo is received, reset */ + /* state */ smtpd_reset_transaction(r); sr-helo = apr_pstrdup(sr-p, buffer); @@ -180,7 +180,7 @@ HANDLER_DECLARE(helo) { smtpd_request_rec *sr = smtpd_get_request_rec(r); - // bad syntax + /* bad syntax */ if (buffer[0] == '\0') { ap_rprintf(r, %d %s\r\n, 501, Syntax: HELO hostname); return 501; @@ -201,8 +201,8 @@ break; } - // RFC 2821 states that when ehlo or helo is received, reset - // state + /* RFC 2821 states that when ehlo or helo is received, reset */ + /* state */ smtpd_reset_transaction(r); sr-helo = apr_pstrdup(sr-p, buffer); @@ -216,13 +216,13 @@ smtpd_request_rec *sr = smtpd_get_request_rec(r); char *loc; - // already got mail + /* already got mail */ if (sr-state_vector == SMTPD_STATE_GOT_MAIL) { ap_rprintf(r, %d %s\r\n, 503, Error: Nested MAIL command); return 503; } - // bad syntax + /* bad syntax */ if ((loc = ap_strcasestr(buffer, from:)) == NULL) { ap_rprintf(r, %d %s\r\n, 501, Syntax: MAIL FROM:address); return 501; @@ -237,7 +237,7 @@ case SMTPD_DONE: return 1; case SMTPD_DONE_DISCONNECT: -// zero to disconnect +/* zero to disconnect */ return 0; case SMTPD_DENY: ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r-server, @@ -268,7 +268,7 @@ } else { ap_rprintf(r, %d %s, denied\r\n, 550, loc); } -// zero to disconnect +/* zero to disconnect */ return 0; case SMTPD_DENYSOFT_DISCONNECT: ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, r-server, @@ -279,13 +279,13 @@ } else { ap_rprintf(r, %d %s, temporarily denied\r\n, 450, loc); } -// zero to disconnect +/* zero to disconnect */ return 0; default: break; } - // default handling + /* default handling */ sr-mail_from = apr_pstrdup(sr-p, loc); sr-state_vector = SMTPD_STATE_GOT_MAIL; ap_rprintf(r, %d %s\r\n, 250, Ok); @@ -297,14 +297,14 @@ smtpd_request_rec *sr = smtpd_get_request_rec(r); char *loc; - // need mail first + /* need mail first */ if ((sr-state_vector != SMTPD_STATE_GOT_MAIL) (sr-state_vector != SMTPD_STATE_GOT_RCPT)) { ap_rprintf(r, %d %s\r\n, 503, Error: need MAIL command); return 503; } - // bad syntax + /* bad syntax */ if ((loc = ap_strcasestr(buffer, to:)) == NULL) { ap_rprintf(r, %d %s\r\n, 501, Syntax: RCPT TO:address); return 501; @@ -342,7 +342,7 @@ ap_rprintf(r, %d %s\r\n, 450, in_data-msg ? in_data-msg : relaying temporarily denied); return 0; - case SMTPD_OK: // recipient is okay + case SMTPD_OK: /* recipient is okay */ break; default: ap_rprintf(r, %d %s\r\n, 450, No plugin decided if relaying is @@ -350,7 +350,7 @@ return 450; } - // add a recipient + /* add a recipient */ (*((char **)apr_array_push(sr-rcpt_to))) = apr_pstrdup(sr-p, loc); sr-state_vector = SMTPD_STATE_GOT_RCPT; ap_rprintf(r, %d %s\r\n, 250, Ok); @@ -403,14 +403,14 @@ case SMTPD_DONE_DISCONNECT: return 0; case SMTPD_DENY: -// REVIEW: should we reset state here? -// smtpd_clear_request_rec(sr); +/* REVIEW: should we reset state here? */ +/* smtpd_clear_request_rec(sr); */ ap_rprintf(r, %d %s\r\n, 554, in_data-msg ? in_data-msg : Message denied); return 554; case SMTPD_DENYSOFT: -// REVIEW: should we reset state here? -// smtpd_clear_request_rec(sr); +/* REVIEW: should we reset state here? */ +/* smtpd_clear_request_rec(sr); */ ap_rprintf(r, %d %s\r\n, 451, in_data-msg ? in_data-msg :
Re: New mod_smtpd release
Joe Schaefer wrote: Rian Hunter [EMAIL PROTECTED] writes: You can checkout this code out from: http://svn.apache.org/repos/asf/httpd/mod_smtpd/trunk/ Very cool, thanks! I had some trouble compiling it, and I noticed you're using // comments alot. Here are two patches for that. May I suggest you resend this patch, using svn diff instead of diff -pur to create it? You're diffing the SVN administrative directory... (Or at the very least, tell diff to -x .svn.) -- Brane
Re: New mod_smtpd release
Branko Čibej [EMAIL PROTECTED] writes: May I suggest you resend this patch, using svn diff instead of diff -pur to create it? You're diffing the SVN administrative directory... Thanks. Here's another patch to add a skeleton STATUS file, using svn diff this time. Index: STATUS === --- STATUS (revision 0) +++ STATUS (revision 0) @@ -0,0 +1,49 @@ +mod_smtpd STATUS: -*-text-*- +Last modified at [$Date: $] + +The current version of this file can be found at: + + * http://svn.apache.org/repos/asf/httpd/mod_smtpd/trunk/STATUS + + +Release history: + + 0.9 under development. + + +Contributors looking for a mission: + + * Just do an egrep on TODO or XXX in the source. + + * Review the bug database at: http://issues.apache.org/bugzilla/ + + * Open bugs in the bug database. + + +CURRENT RELEASE NOTES: + + Virtual hosts a'la mod_ftpd don't work. + + +RELEASE SHOWSTOPPERS: + + + smtp_process_connection_internal should take a smtp_proto_rec argument + (which is what the current smtp_request_rec struct should be renamed to). + + +CURRENT VOTES: + + +TODO ISSUES: + + The request i/o is driven around ap_rgetline, when it really + should be using input filters. + + +WISH LIST: + + Link against libapreq2 so we can use its header and multipart parsers. + apreq's header parser would help in implementing rfc2821 loop-detection, + and in providing the header collection as r-headers_in for data + hooks to examine. -- Joe Schaefer
Re: New mod_smtpd release
On Aug 14, 2005, at 1:22 PM, Joe Schaefer wrote: snip... +CURRENT RELEASE NOTES: + + Virtual hosts a'la mod_ftpd don't work. It does work like this: Listen 80 Listen 25 NameVirtualHost *:80 NameVirtualHost *:25 VirtualHost *:80 ServerName localhost DocumentRoot htdocs /VirtualHost VirtualHost *:25 ServerName localhost SmtpProtocol On /VirtualHost no? +RELEASE SHOWSTOPPERS: + + + smtp_process_connection_internal should take a smtp_proto_rec argument + (which is what the current smtp_request_rec struct should be renamed to). I can easily rename smtpd_request_rec but I don't think I should pass it to smtpd_process_connection internal only because the hooks take a request_rec*. They need request_rec to use filters (even though i don't currently enable convenient use of filters yet). Unless I add a member to smtpd_proto_rec that is a pointer to the related request_rec (solely so filters can work) and use smtpd_proto_rec as the main structure. + +CURRENT VOTES: + + +TODO ISSUES: + + The request i/o is driven around ap_rgetline, when it really + should be using input filters. I have to look a little more into this. +WISH LIST: + + Link against libapreq2 so we can use its header and multipart parsers. + apreq's header parser would help in implementing rfc2821 loop- detection, + and in providing the header collection as r-headers_in for data + hooks to examine. Ideally header parsing should be done in an mod_smtpd plugin not in mod_smtpd. -rian
Re: New mod_smtpd release
Rian Hunter [EMAIL PROTECTED] writes: On Aug 14, 2005, at 1:22 PM, Joe Schaefer wrote: +RELEASE SHOWSTOPPERS: + + + smtp_process_connection_internal should take a smtp_proto_rec + argument (which is what the current smtp_request_rec struct + should be renamed to). I can easily rename smtpd_request_rec but I don't think I should pass it to smtpd_process_connection internal only because the hooks take a request_rec*. The hooks can still take a request_rec, but IMO the protocol's state management shouldn't be done from a request_rec. I'd still like to see one request correspond to one MAIL FROM/RCPT TO/DATA sequence, so that whenever the state gets reset, the whole request_rec pool gets cleaned up. They need request_rec to use filters (even though i don't currently enable convenient use of filters yet). The request_rec slot can be NULL for connection-level filters. But I'd create a request_rec sometime before I added an smtp protocol filter, which would just do the .-decoding, similar to how http_in deals with TE. [...] Ideally header parsing should be done in an mod_smtpd plugin not in mod_smtpd. I respectfully disagree, because I'd like different hooks to have increasing state information available to them through the request_rec. In particular I'd like to see the smtp filtering API match httpd, by first parsing the headers, but passing the rest of the data through r-input_filters, with smtp_in translating the final . line into an EOS bucket. -- Joe Schaefer
Re: New mod_smtpd release
On Aug 14, 2005, at 8:12 PM, Joe Schaefer wrote: Rian Hunter [EMAIL PROTECTED] writes: On Aug 14, 2005, at 1:22 PM, Joe Schaefer wrote: +RELEASE SHOWSTOPPERS: + + + smtp_process_connection_internal should take a smtp_proto_rec + argument (which is what the current smtp_request_rec struct + should be renamed to). I can easily rename smtpd_request_rec but I don't think I should pass it to smtpd_process_connection internal only because the hooks take a request_rec*. The hooks can still take a request_rec, but IMO the protocol's state management shouldn't be done from a request_rec. I'd still like to see one request correspond to one MAIL FROM/RCPT TO/DATA sequence, so that whenever the state gets reset, the whole request_rec pool gets cleaned up. This make sense. smtpd_request_rec does what you say. After looking at smtp_core.c it seems that request_rec isn't really something needed for mod_smtpd (and really never was). After figuring out the input filters situation, i'll probably do away with request_rec (since it isn't needed for connection-level filters) and just stick to smtpd_proto_rec. Any objections? They need request_rec to use filters (even though i don't currently enable convenient use of filters yet). The request_rec slot can be NULL for connection-level filters. But I'd create a request_rec sometime before I added an smtp protocol filter, which would just do the .-decoding, similar to how http_in deals with TE. Yeah I agree. Ideally header parsing should be done in an mod_smtpd plugin not in mod_smtpd. I respectfully disagree, because I'd like different hooks to have increasing state information available to them through the request_rec. In particular I'd like to see the smtp filtering API match httpd, by first parsing the headers, but passing the rest of the data through r-input_filters, with smtp_in translating the final . line into an EOS bucket. Well there's also another problem. RFC 2821 (SMTP) doesn't define a particular message format for SMTP (in wide use there the RFC 822 and MIME message formats). I don't think that mod_smtpd should assume a RFC 822 or MIME message format since its strictly a SMTP module, that's why I still think header parsing should be in another module. Of course this module is free to register itself as an mod_smtpd filter and do what it needs to do, but it shouldn't be part of the main mod_smtpd. The modules that will specifically rely on this header parsing module will know how to obtain the header information using the conventions specified by that parsing module. -rian
Re: New mod_smtpd release
Rian Hunter [EMAIL PROTECTED] writes: The request_rec slot can be NULL for connection-level filters. But I'd create a request_rec sometime before I added an smtp protocol filter, which would just do the .-decoding, similar to how http_in deals with TE. Yeah I agree. I'd be more than happy to work on the filters for mod_smtpd if you want to delegate that task. -- Joe Schaefer
Re: New mod_smtpd release
On Aug 12, 2005, at 5:57 PM, Rian Hunter wrote: This version of mod_smtpd is callback based, very similar to Qpsmtpd. Here is a list of all the hooks you can register: That's a beautiful cycle. When I added the plugin/extension/hook system to qpsmtpd way back when I borrowed many concepts from the httpd module system. :-) - ask -- http://www.askbjoernhansen.com/
New mod_smtpd release
Hi, I've checked in mod_smtpd 0.9 and its API should be completely frozen by now. This version of mod_smtpd is heavily based on Qpsmtpd, so the same extensibility you expect from Qpsmtpd can be achieved with this version of mod_smtpd. I haven't written any documentation yet but here is a quick run-down of how to use it: In your httpd.conf, make sure you have SmtpProtocol On, if you are setting up a virtualhost make sure the virtualHost container has the ServerName directive (duh). This version of mod_smtpd is callback based, very similar to Qpsmtpd. Here is a list of all the hooks you can register: smtpd_run_unrecognized_command smtpd_run_connect smtpd_run_reset_transaction smtpd_run_helo smtpd_run_ehlo smtpd_run_mail smtpd_run_rcpt smtpd_run_vrfy smtpd_run_quit smtpd_run_data smtpd_run_data_post smtpd_run_data_queue You can register a hook to one of these by calling: APR_OPTIONAL_HOOK(smtpd, /* hook name */ vrfy, /* function address */ default_vrfy, NULL, NULL, APR_HOOK_FIRST); In your register hooks function. Each hook you register should return smtpd_retcode, to see what retcodes make sense for each callback you should look at smtp_protocol.c until I write better documentation. To see what argument each different type of hook takes look at smtp_core.c. The code is very small and simple and shouldn't be too hard to figure out if you're familiar with apache modules. Currently it works with httpd 2.0 and up. You can checkout this code out from: http://svn.apache.org/repos/asf/httpd/mod_smtpd/trunk/ Have Fun! -rian