June 23, 2023 11:58 AM, "Omar Polo" <o...@omarpolo.com> wrote:
> another diff from the -portable repo: > > https://github.com/OpenSMTPD/OpenSMTPD/pull/1150 > > per rfc-5321 ยง 4.1.1.9 the NOOP command allows optionally one argument > that we SHOULD ignore. > > The original diff set the check function in the commands table to NULL > because NOOP is not a phase that can have filters. However, I prefer > to stay on the safe side and add a smtp_check_noop() function. > Alternatively, we could allow a NULL check function in > smtp_session_imsg(). > > the rfc specifies only one optional string, while here for semplicity > it's relaxed to allow anything. > > diff /usr/src > commit - 8def1c1c2777f0b5175283f8116e1eaab1f1962a > path + /usr/src > blob - 1686f03e96deeb5e6ea8b065456e04c27c752c8c > file + usr.sbin/smtpd/smtp_session.c > --- usr.sbin/smtpd/smtp_session.c > +++ usr.sbin/smtpd/smtp_session.c > @@ -212,6 +212,7 @@ static int smtp_check_noparam(struct smtp_session *, > static int smtp_check_mail_from(struct smtp_session *, const char *); > static int smtp_check_rcpt_to(struct smtp_session *, const char *); > static int smtp_check_data(struct smtp_session *, const char *); > +static int smtp_check_noop(struct smtp_session *, const char *); > static int smtp_check_noparam(struct smtp_session *, const char *); > > static void smtp_filter_phase(enum filter_phase, struct smtp_session *, const > char *); > @@ -276,7 +277,7 @@ static struct { > { CMD_DATA, FILTER_DATA, "DATA", smtp_check_data, smtp_proceed_data }, > { CMD_RSET, FILTER_RSET, "RSET", smtp_check_rset, smtp_proceed_rset }, > { CMD_QUIT, FILTER_QUIT, "QUIT", smtp_check_noparam, smtp_proceed_quit }, > - { CMD_NOOP, FILTER_NOOP, "NOOP", smtp_check_noparam, smtp_proceed_noop }, > + { CMD_NOOP, FILTER_NOOP, "NOOP", smtp_check_noop, smtp_proceed_noop }, > { CMD_HELP, FILTER_HELP, "HELP", smtp_check_noparam, smtp_proceed_help }, > { CMD_WIZ, FILTER_WIZ, "WIZ", smtp_check_noparam, smtp_proceed_wiz }, > { CMD_COMMIT, FILTER_COMMIT, ".", smtp_check_noparam, smtp_proceed_commit }, > @@ -1343,8 +1344,8 @@ smtp_command(struct smtp_session *s, char *line) > break; > > case CMD_NOOP: > - if (!smtp_check_noparam(s, args)) > - break; > + if (!smtp_check_noop(s, args)) > + break; > smtp_filter_phase(FILTER_NOOP, s, NULL); > break; > > @@ -1631,6 +1632,12 @@ smtp_check_noparam(struct smtp_session *s, const char > } > > static int > +smtp_check_noop(struct smtp_session *s, const char *args) > +{ > + return 1; > +} > + > +static int > smtp_check_noparam(struct smtp_session *s, const char *args) > { > if (args != NULL) { This reads fine and you did well adding an smtp_check_noop() because it leaves room for hooking this command in filters which is something OpenSMTPD could benefit from as it would allow a filter to detect people doing NOOP loops and kick them. Just my 2cents
