Ping.
On Sun, Oct 09, 2022 at 08:01:01AM +0200, Martijn van Duren wrote:
> Bit focused on other things atm and not familiar with this part of the
> code, but some comments.
>
> On Sat, 2022-10-08 at 12:18 -0600, Chris Waddey wrote:
>> A message with a single successful recipient but with a failed
>> rcpt to: command afterward generates an incorrect Received: header.
...
>> The following patch fixes the problem:
>> Index: smtp_session.c
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
>> retrieving revision 1.432
>> diff -u -p -r1.432 smtp_session.c
>> --- smtp_session.c 1 Jul 2021 07:42:16 -0000 1.432
>> +++ smtp_session.c 8 Oct 2022 18:04:51 -0000
>> @@ -2732,6 +2732,7 @@ static void
>> smtp_message_begin(struct smtp_tx *tx)
>> {
>> struct smtp_session *s;
>> + struct smtp_rcpt *srfp;
>> int (*m_printf)(struct smtp_tx *, const char *, ...);
>> m_printf = smtp_message_printf;
>> @@ -2780,10 +2781,13 @@ smtp_message_begin(struct smtp_tx *tx)
>> }
>> }
>> + /* If we get failed "RCPT TO" commands that modify tx->evp, then
>> + * make sure we use the real recipient for the "for" clause */
> See style(9) how comments should be formatted:
> /*
> * Multi-line comments look like this. Make them real sentences.
> * Fill them so they look like real paragraphs.
> */
Got it.
>> if (tx->rcptcount == 1) {
>> + srfp = TAILQ_FIRST(&tx->rcpts);
>> m_printf(tx, "\n\tfor <%s@%s>",
>> - tx->evp.rcpt.user,
>> - tx->evp.rcpt.domain);
>> + srfp->maddr.user,
>> + srfp->maddr.domain);
> I don't see anything wrong with this, but does the code still do the correct
> thing with multiple recipients?
> RCPT TO: <exists1>
> RCPT TO: <exists2>
> RCPT TO: <nonexistent>
> or
> RCPT TO: <exists1>
> RCPT TO: <nonexistent>
> RCPT TO: <exists2>
For reference, testing the two above scenarios with the original gives
the following Received: header (except date and message id
differences):
Received: by thief.private (OpenSMTPD) with ESMTP id 4f5144f3; Sun, 9 Oct 2022
12:09:22 -0600 (MDT)
With the patch it still gives the following for both scenarios:
Received: by thief.private (OpenSMTPD) with ESMTP id 50bf7075; Sun, 9 Oct 2022
12:14:32 -0600 (MDT)
Modified patch with comment fix (feel free to remove the comment
entirely if you think it's not needed):
Index: smtp_session.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c 1 Jul 2021 07:42:16 -0000 1.432
+++ smtp_session.c 9 Oct 2022 18:15:53 -0000
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
struct smtp_session *s;
+ struct smtp_rcpt *srfp;
int (*m_printf)(struct smtp_tx *, const char *, ...);
m_printf = smtp_message_printf;
@@ -2780,10 +2781,15 @@ smtp_message_begin(struct smtp_tx *tx)
}
}
+ /*
+ * If we get failed "RCPT TO" commands that modify tx->evp, then
+ * make sure we use the real recipient for the "for" clause
+ */
if (tx->rcptcount == 1) {
+ srfp = TAILQ_FIRST(&tx->rcpts);
m_printf(tx, "\n\tfor <%s@%s>",
- tx->evp.rcpt.user,
- tx->evp.rcpt.domain);
+ srfp->maddr.user,
+ srfp->maddr.domain);
}
m_printf(tx, ";\n\t%s\n", time_to_text(time(&tx->time)));
Patch with no comment:
Index: smtp_session.c
===================================================================
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.432
diff -u -p -r1.432 smtp_session.c
--- smtp_session.c 1 Jul 2021 07:42:16 -0000 1.432
+++ smtp_session.c 9 Oct 2022 18:18:44 -0000
@@ -2732,6 +2732,7 @@ static void
smtp_message_begin(struct smtp_tx *tx)
{
struct smtp_session *s;
+ struct smtp_rcpt *srfp;
int (*m_printf)(struct smtp_tx *, const char *, ...);
m_printf = smtp_message_printf;
@@ -2781,9 +2782,10 @@ smtp_message_begin(struct smtp_tx *tx)
}
if (tx->rcptcount == 1) {
+ srfp = TAILQ_FIRST(&tx->rcpts);
m_printf(tx, "\n\tfor <%s@%s>",
- tx->evp.rcpt.user,
- tx->evp.rcpt.domain);
+ srfp->maddr.user,
+ srfp->maddr.domain);
}
m_printf(tx, ";\n\t%s\n", time_to_text(time(&tx->time)));
