Patch: Support NOTIFY ESMTP parameter in SMFIR_ADDRCPT_PAR

[Andrew Ayer]

Hi,

The following patch adds support for setting the NOTIFY ESMTP parameter
via the SMFIR_ADDRCPT_PAR milter command, as per the milter spec
(previously, Postfix ignored all ESMTP parameters passed to this milter
command).

The patch is simple and only touches two functions because most of the
required pieces were already there. All I needed to do was split the
argument list, parse the NOTIFY parameter (using the existing
dsn_notify_mask() function), and pass the result as the last argument to
cleanup_addr_bcc_dsn(), instead of always passing DEF_DSN_NOTIFY. I've
tried to mimic the existing code style as much as possible.

I'm writing a milter for which this would be quite useful. Could this
patch be applied to Postfix?

Thanks,
Andrew


diff -ruN postfix-2.11-20131122/src/cleanup/cleanup_milter.c postfix-2.11-20131122-mine/src/cleanup/cleanup_milter.c
--- postfix-2.11-20131122/src/cleanup/cleanup_milter.c 2013-11-14 11:56:41.000000000 -0800
+++ postfix-2.11-20131122-mine/src/cleanup/cleanup_milter.c 2013-11-22 20:52:38.527301470 -0800
@@ -1388,9 +1388,10 @@
return (CLEANUP_OUT_OK(state) ? 0 : cleanup_milter_error(state, 0));
}

-/* cleanup_add_rcpt - append recipient address */
+/* cleanup_add_rcpt_par - append recipient address, with ESMTP arguments */

-static const char *cleanup_add_rcpt(void *context, const char *ext_rcpt)
+static const char *cleanup_add_rcpt_par(void *context, const char *ext_rcpt,
+ const char *esmtp_args)
{
const char *myname = "cleanup_add_rcpt";
CLEANUP_STATE *state = (CLEANUP_STATE *) context;
@@ -1400,6 +1401,26 @@
TOK822 *tree;
TOK822 *tp;
VSTRING *int_rcpt_buf;
+ ARGV *esmtp_argv;
+ int dsn_notify = 0;
+ int i;
+ const char *arg;
+
+ if (esmtp_args[0]) {
+ esmtp_argv = argv_split(esmtp_args, " ");
+ for (i = 0; i < esmtp_argv->argc; ++i) {
+ arg = esmtp_argv->argv[i];
+ if (strncasecmp(arg, "NOTIFY=", 7) == 0) {
+ if (dsn_notify || (dsn_notify = dsn_notify_mask(arg + 7)) == 0)
+ msg_warn("%s: %s: Bad NOTIFY parameter syntax",
+ state->queue_id, myname);
+ } else {
+ msg_warn("%s: %s: ignoring ESMTP argument \"%.100s\"",
+ state->queue_id, myname, arg);
+ }
+ }
+ argv_free(esmtp_argv);
+ }

if (msg_verbose)
msg_info("%s: \"%s\"", myname, ext_rcpt);
@@ -1454,7 +1475,8 @@
}
}
tok822_free_tree(tree);
- cleanup_addr_bcc_dsn(state, STR(int_rcpt_buf), NO_DSN_ORCPT, DEF_DSN_NOTIFY);
+ cleanup_addr_bcc_dsn(state, STR(int_rcpt_buf), NO_DSN_ORCPT,
+ dsn_notify ? dsn_notify : DEF_DSN_NOTIFY);
vstring_free(int_rcpt_buf);
if (addr_count == 0) {
msg_warn("%s: ignoring attempt from Milter to add null recipient",
@@ -1494,18 +1516,11 @@
return (CLEANUP_OUT_OK(state) ? 0 : cleanup_milter_error(state, 0));
}

-/* cleanup_add_rcpt_par - append recipient address, ignore ESMTP arguments */
+/* cleanup_add_rcpt - append recipient address */

-static const char *cleanup_add_rcpt_par(void *context, const char *ext_rcpt,
- const char *esmtp_args)
+static const char *cleanup_add_rcpt(void *context, const char *ext_rcpt)
{
- const char *myname = "cleanup_add_rcpt";
- CLEANUP_STATE *state = (CLEANUP_STATE *) context;
-
- if (esmtp_args[0])
- msg_warn("%s: %s: ignoring ESMTP arguments \"%.100s\"",
- state->queue_id, myname, esmtp_args);
- return (cleanup_add_rcpt(context, ext_rcpt));
+ return (cleanup_add_rcpt_par(context, ext_rcpt, ""));
}

/* cleanup_del_rcpt - remove recipient and all its expansions */

[Viktor Dukhovni]

Post by Andrew Ayer
The patch is simple and only touches two functions because most of the
required pieces were already there. All I needed to do was split the
argument list, parse the NOTIFY parameter (using the existing
dsn_notify_mask() function), and pass the result as the last argument to
cleanup_addr_bcc_dsn(), instead of always passing DEF_DSN_NOTIFY. I've
tried to mimic the existing code style as much as possible.

Simple context-free splitting is in principle not sufficient:

RCPT TO:<"perverse NOTIFY=bad address"@example.com> NOTIFY=good

Though the smtpd(8) parser for RCPT TO may not cover 100% of the
torture-test that is the RFC 5321 "RCPT TO" or "MAIL FROM" grammar,
it comes much closer... Look at extract_addr() in src/smtpd/smtpd.c.

[Andrew Ayer]

On Sat, 23 Nov 2013 18:47:13 +0000

Post by Viktor Dukhovni

Post by Andrew Ayer
The patch is simple and only touches two functions because most of the
required pieces were already there. All I needed to do was split the
argument list, parse the NOTIFY parameter (using the existing
dsn_notify_mask() function), and pass the result as the last argument to
cleanup_addr_bcc_dsn(), instead of always passing DEF_DSN_NOTIFY. I've
tried to mimic the existing code style as much as possible.

Though the smtpd(8) parser for RCPT TO may not cover 100% of the
torture-test that is the RFC 5321 "RCPT TO" or "MAIL FROM" grammar,
it comes much closer... Look at extract_addr() in src/smtpd/smtpd.c.

Thanks for taking a look at this.

I'm actually only parsing RCPT TO ESMTP parameters here, not an entire
RCPT TO command, and ESMTP parameter values are not allowed to contain
space characters[1]. If a parameter value contains an address (e.g.
ORCPT), then it's encoded using "xtext."[2] So I believe it should be
quite sufficient to split on space character here.

Regards,

Andrew

[1] https://tools.ietf.org/html/rfc1869#section-6 and
http://tools.ietf.org/html/rfc5321#section-4.1.2

[2] http://tools.ietf.org/html/rfc3461#section-4.2

[Viktor Dukhovni]

Post by Andrew Ayer

Post by Viktor Dukhovni
Though the smtpd(8) parser for RCPT TO may not cover 100% of the
torture-test that is the RFC 5321 "RCPT TO" or "MAIL FROM" grammar,
it comes much closer... Look at extract_addr() in src/smtpd/smtpd.c.

Thanks for taking a look at this.
I'm actually only parsing RCPT TO ESMTP parameters here, not an entire
RCPT TO command, and ESMTP parameter values are not allowed to contain
space characters[1]. If a parameter value contains an address (e.g.
ORCPT), then it's encoded using "xtext."[2] So I believe it should be
quite sufficient to split on space character here.

You're right, I did not look at the patch sufficiently closely. The
splitting into "rcpt", "dsn_args" happens before your code is reached.

[Wietse Venema]

Post by Andrew Ayer
On Sat, 23 Nov 2013 18:47:13 +0000

Post by Viktor Dukhovni

Post by Andrew Ayer
The patch is simple and only touches two functions because most of the
required pieces were already there. All I needed to do was split the
argument list, parse the NOTIFY parameter (using the existing
dsn_notify_mask() function), and pass the result as the last argument to
cleanup_addr_bcc_dsn(), instead of always passing DEF_DSN_NOTIFY. I've
tried to mimic the existing code style as much as possible.

Though the smtpd(8) parser for RCPT TO may not cover 100% of the
torture-test that is the RFC 5321 "RCPT TO" or "MAIL FROM" grammar,
it comes much closer... Look at extract_addr() in src/smtpd/smtpd.c.

Thanks for taking a look at this.
I'm actually only parsing RCPT TO ESMTP parameters here, not an entire
RCPT TO command, and ESMTP parameter values are not allowed to contain
space characters[1]. If a parameter value contains an address (e.g.
ORCPT), then it's encoded using "xtext."[2] So I believe it should be
quite sufficient to split on space character here.

That covers the syntax of all supported ESMTP parameters, so there
should be no problems syntax-wise. The code looks good. Maybe someone
else will find time to add ORCPT support later.

Wietse

Post by Andrew Ayer
Regards,
Andrew
[1] https://tools.ietf.org/html/rfc1869#section-6 and
http://tools.ietf.org/html/rfc5321#section-4.1.2
[2] http://tools.ietf.org/html/rfc3461#section-4.2

[Wietse Venema]

Post by Wietse Venema

Post by Andrew Ayer
I'm actually only parsing RCPT TO ESMTP parameters here, not an entire
RCPT TO command, and ESMTP parameter values are not allowed to contain
space characters[1]. If a parameter value contains an address (e.g.
ORCPT), then it's encoded using "xtext."[2] So I believe it should be
quite sufficient to split on space character here.

That covers the syntax of all supported ESMTP parameters, so there
should be no problems syntax-wise. The code looks good. Maybe someone
else will find time to add ORCPT support later.

Done, including ORCPT support. The other features, ENVID and RET,
would also require code changes elsewhere in the cleanup server.

Wietse