On Mon, 2020-09-14 at 10:28 +0200, Sebastien Marie wrote:
> On Mon, Sep 14, 2020 at 09:32:46AM +0200, 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?
> >
> > 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;
> > + }
>
> maybe escape the output before it reachs the terminal ? the string
> comes from untrusted source...
I think the code below is the easiest solution, since '?' it's not part
of a domainname or the macro syntax.
Also marked the final "spf walk" in the manpage based on feedback from
jmc.
> > 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 .
> >
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 08:52:23 -0000
@@ -117,7 +117,17 @@ lookup_record(int type, const char *reco
{
struct asr_query *as;
struct target *ntgt;
+ size_t i;
+ if (strchr(record, '%') != NULL) {
+ for (i = 0; record[i] != '\0'; i++) {
+ if (!isprint(record[i]))
+ record[i] = '?';
+ }
+ 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 08:52:23 -0000
@@ -247,8 +247,13 @@ 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.
+.Cm spf walk
+cannot provide full results in these cases.
.It Cm trace Ar subsystem
Enables real-time tracing of
.Ar subsystem .
