On Thu, Jan 30, 2025 at 07:08:36PM +0000, Jason McIntyre wrote:
> On Thu, Jan 30, 2025 at 10:43:41AM -0800, patrick keshishian wrote:
> > Bumping this diff/correction for comment.
> > 
> > Adding Omar Polo and Jason McIntyre to Cc - I hope that is OK.
> > 
> > 
> > On Thu, Jan 23, 2025 at 06:12:57PM -0800, patrick keshishian wrote:
> > > Hi guys,
> > > 
> > > Fiddling with the smtpd-filters(7) interface I came to realize
> > > the inconsistency between the documentation and what the actual
> > > code sends over for the "filter" request for the "connect" phase.
> > > 
> > > I am assuming the documentation is incorrect - basic copy and
> > > paste from the "report" event for "link-connect".  The latter
> > > stating that the event specific param for link-connect includes:
> > > 
> > > 	link-connect: rdns fcrdns src dest
> > > 
> > > which is copied for "connect" phase in the FILTER REQUESTS
> > > section:
> > > 
> > > 	connect: rdns fcrdns src dest
> > > 
> > > While in fact, the "connect" phase param consists only of "rdns"
> > > and "src".
> > > 
> > > Here are actual examples logged by my filter program demonstrating
> > > difference between "report ... link-connect" and "filter ... connect":
> > > 
> > > $ telnet localhost 25
> > > 
> > > report|0.7|1737667524.180287|smtp-in|link-connect|ed211f331e52ea06|localhost|pass|127.0.0.1:13455|127.0.0.1:25
> > > filter|0.7|1737667524.181485|smtp-in|connect|ed211f331e52ea06|df4279847dbec98a|localhost|127.0.0.1
> > > 
> > > 
> > > $ telnet ::1 25
> > > 
> > > report|0.7|1737618643.739838|smtp-in|link-connect|156e8e029558f448|localhost|pass|[::1]:28610|[::1]:25
> > > filter|0.7|1737618643.740355|smtp-in|connect|156e8e029558f448|1d8054e802f0ca74|localhost|[::1]
> > > 
> > > $ mail user
> > > Subject: testing
> > > hi
> > > .
> > > EOT
> > > 
> > > report|0.7|1737665197.527146|smtp-in|link-connect|1f61d2743cb71695|myhost.mydomain|pass|unix:/var/run/smtpd.sock|unix:/var/run/smtpd.sock
> > > filter|0.7|1737665197.527570|smtp-in|connect|1f61d2743cb71695|3d87240f23bb8951|myhost.mydomain|local
> > > 
> > > 
> > > The diff defines format of "src" in each of the three possible
> > > forms (IPv4, IPv6 and Unix socket).
> > > 
> > > The first (inline) diff is against OPENBSD_7_5, and the 2nd
> > > is against HEAD.
> > > 
> > > 
> > > Thoughts?
> > > 
> > > --patrick
> > > 
> > > 
> 
> hi. i can;t comment on the correctness of the diff but i would suggest
> changing the wording a little:
> 
> > > 
> > > Index: smtpd-filters.7
> > > ===================================================================
> > > RCS file: /cvs/obsd/src/usr.sbin/smtpd/smtpd-filters.7,v
> > > retrieving revision 1.10
> > > diff -u -p -u -p -r1.10 smtpd-filters.7
> > > --- smtpd-filters.7	7 Jul 2023 14:52:00 -0000	1.10
> > > +++ smtpd-filters.7	24 Jan 2025 01:19:08 -0000
> > > @@ -462,7 +462,7 @@ Fields are separated by the
> > >  .Dq |
> > >  character.
> > >  .Bd -literal -offset indent
> > > -filter|0.5|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d|mail.openbsd.org|pass|199.185.178.25:33174|45.77.67.80:25
> > > +filter|0.7|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d|mail.openbsd.org|199.185.178.25
> > >  .Ed
> > >  .Pp
> > >  The format consists of a protocol prefix containing the stream,
> > > @@ -473,7 +473,7 @@ the filtering phase,
> > >  the unique session identifier and an opaque token that the filter
> > >  should provide in its response:
> > >  .Bd -literal -offset indent
> > > -filter|0.5|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d
> > > +filter|0.7|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d
> > >  .Ed
> > >  .Pp
> > >  It is followed by a suffix containing the phase-specific parameters of the
> > > @@ -481,7 +481,7 @@ filter request,
> > >  also separated by
> > >  .Dq | :
> > >  .Bd -literal -offset indent
> > > -mail.openbsd.org|pass|199.185.178.25:33174|45.77.67.80:25
> > > +mail.openbsd.org|199.185.178.25
> > >  .Ed
> > >  .Pp
> > >  Unlike with report events,
> > > @@ -550,9 +550,19 @@ filter-dataline|7641df9771b4ed00|1ef1c20
> > >  .Pp
> > >  The list of events and event-specific parameters for smtp-in are as follows:
> > >  .Bl -tag -width Ds
> > > -.It Ic connect : Ar rdns fcrdns src dest
> > > +.It Ic connect : Ar rdns src
> > >  This request is emitted after connection,
> > >  before the banner is displayed.
> > > +.Pp
> > > +.Ar src
> > > +contains either the IP address of the source address,
> > > +in the format
> > > +.Dq a.b.c.d
> > > +in case of IPv4, or
> > > +.Dq [x:x:x:x:x:x:x:x]
> > > +in case of IPv6, or
> > > +.Dq local
> > > +in case of UNIX socket.
> > >  .It Ic helo : Ar identity
> > >  This request is emitted after the client has emitted
> > >  .Dq HELO .
> > > 
> 
> i think this is a bit wordy. i think you could just say
> 
> 	contains either the IP address of the source
> 	(a.b.c.d for IPv4 or [x:..] for IPv6)
> 	or
> 	.Dq local
> 	(for UNIX sockets).
> 
> maybe you don;t even have to mention the formats for v4/6 if it's clear
> enough.

Thanks for the feedback.
Updated diff (against HEAD) inline at the end.

--patrick



Index: smtpd-filters.7
===================================================================
RCS file: /cvs/obsd/src/usr.sbin/smtpd/smtpd-filters.7,v
retrieving revision 1.13
diff -u -p -u -p -r1.13 smtpd-filters.7
--- smtpd-filters.7	5 Nov 2024 19:36:53 -0000	1.13
+++ smtpd-filters.7	30 Jan 2025 19:31:56 -0000
@@ -465,7 +465,7 @@ Fields are separated by the
 .Dq |
 character.
 .Bd -literal -offset indent
-filter|0.7|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d|mail.openbsd.org|pass|199.185.178.25:33174|45.77.67.80:25
+filter|0.7|1576146008.006099|smtp-in|connect|7641df9771b4ed00|1ef1c203cc576e5d|mail.openbsd.org|199.185.178.25
 .Ed
 .Pp
 The format consists of a protocol prefix containing the stream,
@@ -484,7 +484,7 @@ filter request,
 also separated by
 .Dq | :
 .Bd -literal -offset indent
-mail.openbsd.org|pass|199.185.178.25:33174|45.77.67.80:25
+mail.openbsd.org|199.185.178.25
 .Ed
 .Pp
 Unlike with report events,
@@ -553,9 +553,16 @@ filter-dataline|7641df9771b4ed00|1ef1c20
 .Pp
 The list of events and event-specific parameters for smtp-in are as follows:
 .Bl -tag -width Ds
-.It Ic connect : Ar rdns fcrdns src dest
+.It Ic connect : Ar rdns src
 This request is emitted after connection,
 before the banner is displayed.
+.Pp
+.Ar src
+contains either the IP address of the source
+(a.b.c.d for IPv4 or [x:..] IPv6)
+or
+.Dq local
+(for UNIX sockets).
 .It Ic helo : Ar identity
 This request is emitted after the client has emitted
 .Dq HELO .