On Mon, Sep 14, 2020 at 09:49:30AM +0200, Giovanni Bechis wrote: > On 9/14/20 9:32 AM, Martijn van Duren wrote: > > On Mon, 2020-09-14 at 08:12 +0100, Stuart Henderson wrote: > >> On 2020/09/13 22:48, Giovanni Bechis wrote: > >>> "smtpctl spf walk" doesn't work as it should because it breaks when it > >>> finds > >>> macros as defined in RFC 7208. > >>> > >>> $ echo ryanair.com | smtpctl spf walk > >>> gives no output while dig reply is: > >>> $ dig txt ryanair.com | grep spf > >>> ryanair.com. 17 IN TXT "v=spf1 > >>> include:ryanair.com._nspf.vali.email > >>> include:%{i}._ip.%{h}._ehlo.%{d}._spf.vali.email ~all" > >> > >> "spf walk" should return a warning or an error in these cases. > > > > Was already working on that. How about the diff below? > > > > $ echo ryanair.com | ./smtpctl/obj/smtpctl spf walk > > smtpctl: lookup_record: %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email contains > > macros and can't be resolved > >> > >>> Is it worth mentioning in smtpctl in CAVEATS section or somewhere else ? > >> > >> Maybe in caveats, but if it's there it should be referenced in the > >> description of "spf walk" too, to make it easier to find. > >> > >> Text something like this? > >> > >> "SPF records may contain macros which cannot be included in a static list > >> and must be resolved dynamically at connection time. > >> spf walk cannot provide full results in these cases." > > > > Text reads fine to me. Added to diff below. > > While here I also changed the # to $ so not to give people the > > impression it should be run as root. > > > > OK? > > > ok giovanni@, thanks. > Giovanni > > > martijn@ > > > > Index: spfwalk.c > > =================================================================== > > RCS file: /cvs/src/usr.sbin/smtpd/spfwalk.c,v > > retrieving revision 1.17 > > diff -u -p -r1.17 spfwalk.c > > --- spfwalk.c 15 Mar 2020 16:34:57 -0000 1.17 > > +++ spfwalk.c 14 Sep 2020 07:31:03 -0000 > > @@ -118,6 +118,11 @@ lookup_record(int type, const char *reco > > struct asr_query *as; > > struct target *ntgt; > > > > + if (strchr(record, '%') != NULL) { > > + warnx("%s: %s contains macros and can't be resolved", __func__, > > + record); > > + return; > > + } > > as = res_query_async(record, C_IN, type, NULL); > > if (as == NULL) > > err(1, "res_query_async"); > > Index: smtpctl.8 > > =================================================================== > > RCS file: /cvs/src/usr.sbin/smtpd/smtpctl.8,v > > retrieving revision 1.64 > > diff -u -p -r1.64 smtpctl.8 > > --- smtpctl.8 18 Sep 2018 06:21:45 -0000 1.64 > > +++ smtpctl.8 14 Sep 2020 07:31:03 -0000 > > @@ -247,8 +247,12 @@ Shows if MTA, MDA and SMTP systems are c > > Recursively look up SPF records for the domains read from stdin. > > For example: > > .Bd -literal -offset indent > > -# smtpctl spf walk < domains.txt > > +$ smtpctl spf walk < domains.txt > > .Ed > > +.Pp > > +SPF records may contain macros which cannot be included in a static list > > and > > +must be resolved dynamically at connection time. > > +spf walk cannot provide full results in these cases. > > .It Cm trace Ar subsystem > > Enables real-time tracing of > > .Ar subsystem . > > >
ok from me too. but i think you should probably mark up "spf walk" in the last line of added text. jmc