Download raw body.
smtpd: implement RFC9422 LIMITS extension utilization
Kirill A. Korinsky <kirill@korins.ky> wrote:
> On Wed, 11 Mar 2026 08:30:25 +0100,
> Martijn van Duren <openbsd+tech@list.imperialat.at> wrote:
> >
> > On 3/11/26 00:18, Martijn van Duren wrote:
> > > EHLO,
> > >
> > > While I haven't seen this in the wild, it might be nice to announce our
> > > limits to the world. RFC9422 specifies 3 limits that can be announced:
> > > MAILMAX, RCPTMAX, and RCPTDOMAINMAX. RCPTDOMAINMAX isn't a feature we
> > > support as far as I'm aware, but MAILMAX, and RCPTMAX are implemented,
> > > so why not announce them and save everybody a bit of time where
> > > supported.
> > >
> > > OK?
> > >
> > > martijn@
> > > And here's a diff to parse and use MAILMAX and RCPTMAX.
> >
> > OK?
okay op@, left some nits and suggestions inline. Since the release is
not too far in the future, I'd like this (fixed) diff to hit the tree
fairly soon, so we still have some time to test it in the real world.
(not that I expect breakages, but...)
> > martijn@
> >
> > diff d8042db37a6bbfc3219d85f0b9c4cafc1a1924c2 b90a19db558de4a73e6a07a023332ad78d9d5e86
> > commit - d8042db37a6bbfc3219d85f0b9c4cafc1a1924c2
> > commit + b90a19db558de4a73e6a07a023332ad78d9d5e86
> > blob - 3524b378129873422b2d238b563adcef08f705be
> > blob + a207b2ef8f6cf664368b54952de97d344cc2f094
> > --- usr.sbin/smtpd/limit.c
> > +++ usr.sbin/smtpd/limit.c
> > @@ -40,6 +40,7 @@ limit_mta_set_defaults(struct mta_limits *limits)
> > limits->discdelay_route = 3;
> >
> > limits->max_mail_per_session = 100;
> > + limits->max_rcpt_per_transaction = 0;
> > limits->sessdelay_transaction = 0;
> > limits->sessdelay_keepalive = 10;
> >
> > @@ -86,6 +87,8 @@ limit_mta_set(struct mta_limits *limits, const char *k
> >
> > else if (!strcmp(key, "session-mail-max"))
> > limits->max_mail_per_session = value;
> > + else if (!strcmp(key, "transaction-rcpt-max"))
> > + limits->max_rcpt_per_transaction = value;
> > else if (!strcmp(key, "session-transaction-delay"))
> > limits->sessdelay_transaction = value;
> > else if (!strcmp(key, "session-keepalive"))
> > blob - 8022d23ecb02e4654968912bb112c2042701e89e
> > blob + 91785c5acc5aa4266c25d68ba7a1e23ac683a16e
> > --- usr.sbin/smtpd/mta.c
> > +++ usr.sbin/smtpd/mta.c
> > @@ -44,10 +44,12 @@
> > #define RELAY_HOLDQ 0x02
> >
> > static void mta_setup_dispatcher(struct dispatcher *);
> > +static void mta_task_split(struct mta_task *, size_t);
> > static void mta_handle_envelope(struct envelope *, const char *);
> > static void mta_query_smarthost(struct envelope *);
> > static void mta_on_smarthost(struct envelope *, const char *);
> > static void mta_query_mx(struct mta_relay *);
> > +static void mta_query_limits(struct mta_relay *);
> > static void mta_query_secret(struct mta_relay *);
> > static void mta_query_preference(struct mta_relay *);
> > static void mta_query_source(struct mta_relay *);
> > @@ -649,11 +651,21 @@ mta_route_collect(struct mta_relay *relay, struct mta_
> > }
> >
> > struct mta_task *
> > -mta_route_next_task(struct mta_relay *relay, struct mta_route *route)
> > +mta_route_next_task(struct mta_relay *relay, size_t rcptmax)
> > {
> > struct mta_task *task;
> > + size_t min;
> >
> > if ((task = TAILQ_FIRST(&relay->tasks))) {
> > + min = 0;
> > + if (relay->limits->max_rcpt_per_transaction != 0)
> > + min = relay->limits->max_rcpt_per_transaction;
> > + if (rcptmax != 0 && rcptmax < min)
> > + min = rcptmax;
> > + if (min == 0)
> > + min = 100;
>
> It reads like min is default limits->max_mail_per_session, if it is, why not
> reuse that value here?
agreed. as discussed privately with Martijn this could actually be just
as simple as
if (min != 0)
mta_task_split(task, min);
> > + mta_task_split(task, min);
> > +
> > TAILQ_REMOVE(&relay->tasks, task, entry);
> > relay->ntask -= 1;
> > task->relay = NULL;
> > @@ -684,6 +696,49 @@ mta_route_next_task(struct mta_relay *relay, struct mt
> > }
> >
> > static void
> > +mta_task_split(struct mta_task *task0, size_t limit)
> > +{
> > + struct mta_relay *relay;
> > + struct mta_task *task;
> > + struct mta_envelope *e0, *e;
> > + size_t n;
> > +
> > + if (limit == 0 || task0->nenvelopes <= limit)
> > + return;
> > +
> > + log_debug("debug: mta: msgid:%08" PRIx32 " %s "
> > + "too many envelopes (limit %zu), splitting %zu "
> > + "envelopes over %zu tasks",
> > + task0->msgid, mta_relay_to_text(task0->relay), limit, task0->nenvelopes,
> > + (task0->nenvelopes / limit) + (task0->nenvelopes % limit == 0 ? 0 : 1));
> > +
> > + n = 0;
> > + TAILQ_FOREACH(e0, &task0->envelopes, entry) {
> > + if (++n == limit)
> > + break;
> > + }
> > + relay = task0->relay;
> > + task = NULL;
> > + n = 0;
> > + while ((e = TAILQ_NEXT(e0, entry)) != NULL) {
> > + if (task == NULL) {
> > + task = xmemdup(task0, sizeof *task0);
instead of trying to be clever here, I'd prefer if we just allocated a
new empty task struct and reassing the various fields. We're almost
doing it anyway, and I think it would be safer in case of future changes
to the mta_task structure.
Also, could you please reorder the lines so we fill task before touching
relay or other vars?
> > + TAILQ_INIT(&task->envelopes);
> > + task->nenvelopes = 0;
> > + relay->ntask += 1;
> > + TAILQ_INSERT_TAIL(&relay->tasks, task, entry);
> > + task->sender = xstrdup(task0->sender);
> > + stat_increment("mta.task", 1);
> > + }
> > + TAILQ_REMOVE(&task0->envelopes, e, entry);
> > + TAILQ_INSERT_TAIL(&task->envelopes, e, entry);
> > + e->task = task;
> > + if (++task->nenvelopes == limit)
> > + task = NULL;
> > + }
> > +}
> > +
> > +static void
> > mta_handle_envelope(struct envelope *evp, const char *smarthost)
> > {
> > struct mta_relay *relay;
> > @@ -770,9 +825,18 @@ mta_handle_envelope(struct envelope *evp, const char *
> > if (task->msgid == evpid_to_msgid(evp->id))
> > break;
> >
> > + if (task != NULL) {
> > + if (task->relay->limits == NULL)
> > + mta_query_limits(task->relay);
> > + if (task->relay->limits->max_rcpt_per_transaction != 0 &&
> > + task->relay->limits->max_rcpt_per_transaction <
> > + task->nenvelopes)
> > + task = NULL;
> > + }
> > if (task == NULL) {
> > task = xmalloc(sizeof *task);
> > TAILQ_INIT(&task->envelopes);
> > + task->nenvelopes = 0;
> > task->relay = relay;
> > relay->ntask += 1;
> > TAILQ_INSERT_TAIL(&relay->tasks, task, entry);
> > @@ -817,6 +881,7 @@ mta_handle_envelope(struct envelope *evp, const char *
> > e->dsn_ret = evp->dsn_ret;
> >
> > TAILQ_INSERT_TAIL(&task->envelopes, e, entry);
> > + task->nenvelopes++;
> > log_debug("debug: mta: received evp:%016" PRIx64
> > " for <%s>", e->id, e->dest);
> >
> > blob - 8aa23779f8f46fb4ec69517a724563eae2a0515f
> > blob + eb63136ea840a44d9480ec844036ec0def638ff8
> > --- usr.sbin/smtpd/mta_session.c
> > +++ usr.sbin/smtpd/mta_session.c
> > @@ -76,12 +76,14 @@ enum mta_state {
> > #define MTA_HANGON 0x2000
> > #define MTA_RECONN 0x4000
> >
> > -#define MTA_EXT_STARTTLS 0x01
> > -#define MTA_EXT_PIPELINING 0x02
> > -#define MTA_EXT_AUTH 0x04
> > -#define MTA_EXT_AUTH_PLAIN 0x08
> > -#define MTA_EXT_AUTH_LOGIN 0x10
> > -#define MTA_EXT_SIZE 0x20
> > +#define MTA_EXT_STARTTLS 0x001
> > +#define MTA_EXT_PIPELINING 0x002
> > +#define MTA_EXT_AUTH 0x004
> > +#define MTA_EXT_AUTH_PLAIN 0x008
> > +#define MTA_EXT_AUTH_LOGIN 0x010
> > +#define MTA_EXT_SIZE 0x020
> > +#define MTA_EXT_LIMITS_MAILMAX 0x100
> > +#define MTA_EXT_LIMITS_RCPTMAX 0x200
> >
> > struct mta_session {
> > uint64_t id;
> > @@ -105,6 +107,8 @@ struct mta_session {
> > int ext;
> >
> > size_t ext_size;
> > + size_t ext_mailmax;
> > + size_t ext_rcptmax;
> >
> > size_t msgtried;
> > size_t msgcount;
> > @@ -712,7 +716,9 @@ again:
> > break;
> > }
> >
> > - if (s->msgcount >= s->relay->limits->max_mail_per_session) {
> > + if (s->msgcount >= s->relay->limits->max_mail_per_session ||
> > + (s->ext & MTA_EXT_LIMITS_MAILMAX &&
> > + s->msgcount >= s->ext_mailmax)) {
> > log_debug("debug: mta: "
> > "%p: cannot send more message to relay %s", s,
> > mta_relay_to_text(s->relay));
> > @@ -730,7 +736,8 @@ again:
> > fatalx("task should be NULL at this point");
> >
> > if (s->task == NULL)
> > - s->task = mta_route_next_task(s->relay, s->route);
> > + s->task = mta_route_next_task(s->relay,
> > + s->ext & MTA_EXT_LIMITS_RCPTMAX ? s->ext_rcptmax : 0);
> > if (s->task == NULL) {
> > log_debug("debug: mta: %p: no task for relay %s",
> > s, mta_relay_to_text(s->relay));
> > @@ -1198,10 +1205,12 @@ static void
> > mta_io(struct io *io, int evt, void *arg)
> > {
> > struct mta_session *s = arg;
> > - char *line, *msg, *p;
> > + char *line, *msg, *p0, *p;
> > size_t len;
> > const char *error;
> > - int cont;
> > + int cont, ext, fail;
> > + long l;
> > + size_t *limit;
> >
> > log_trace(TRACE_IO, "mta: %p: %s %s", s, io_strevent(evt),
> > io_strio(io));
> > @@ -1280,6 +1289,41 @@ mta_io(struct io *io, int evt, void *arg)
> > s->ext_size = strtonum(msg+5, 0, UINT32_MAX, &error);
> > if (error == NULL)
> > s->ext |= MTA_EXT_SIZE;
> > + } else if (strncmp(msg, "LIMITS ", 7) == 0) {
> > + p0 = msg + 7;
> > + while (p0[0] != '\0') {
> > + limit = NULL;
> > + ext = 0;
> > + if (strncmp(p0, "MAILMAX=", 8) == 0) {
> > + ext = MTA_EXT_LIMITS_MAILMAX;
> > + p0 += 8;
> > + limit = &s->ext_mailmax;
> > + } else if (strncmp(p0, "RCPTMAX=", 8) == 0) {
> > + ext = MTA_EXT_LIMITS_RCPTMAX;
> > + p0 += 8;
> > + limit = &s->ext_rcptmax;
> > + }
> > +
> > + if (limit != NULL) {
> > + errno = 0;
> > + l = strtol(p0, &p, 10);
> > +
> > + fail = errno != 0 || p0 == p;
> > + fail |= l <= 0;
> > + fail |=
> > + p[0] != ' ' && p[0] != '\0';
> > +
> > + if (!fail) {
> > + *limit = (size_t)l;
> > + s->ext |= ext;
> > + }
> > + }
> > + p = strchr(p0, ' ');
> > + if (p != NULL)
> > + p0 = p + 1;
> > + else
> > + p0 = strchr(p0, '\0');
suggestion: since we're mutating the line anyway, can't we loop over the
"words" with strsep() and parse the numbers with strtonum()?
less arithmetics and less use of APIs which makes me uneasy =)
thanks!
smtpd: implement RFC9422 LIMITS extension utilization