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.

jmc
.
> > 
> > 
> > 
> > 
> > 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	24 Jan 2025 01:58:22 -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,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 .
> > 
> >