Re: smtpd: add support for cidr in hostname resolution for spf walk
> Hello
Hi Eric,
> > Hello,
> >
> > Here's a patch for smtpctl spf resolution, adding support for target
> > specified as a hostname + cidr.
> >
> > Yes, SPF lets you specify targets like a:example.com/24.
> >
> > Due to the async and recursive nature of DNS resolution in spfwalk.c,
> > it's kind of hard to pass data around without too much memory
> > allocation, so I decided to just pass cidr(s) as integer value instead
> > of keeping the original string and passing pointers around.
> >
> > Comments welcome, thanks!
>
> I find it confusing that everything is stuffed into the sender struct,
> including the callback. I don't really see the point. If you don't
> need to pass the target value over to the callback, there is no need
> to have it in the struct either, especially since the pointer is only
> valid at the time you call the lookup function.
Indeed, passing the target around isn't strictly needed, this is rather
a leftover from a previous implementation were both CIDRs were string
pointers pointing to the target.
> Another point, I don't understand how the parse_sender() function is
> supposed to work. Can you give examples with cidr4 and cidr6?
Per the SPF RFC[0], the "a" and "mx" mechanism can have an IPv4 cidr,
or an IPv6 cidr, or both like “a:example.com/24”, “a:example.com//64”,
or “a:example.com/24/64” which specifies both cidr for the IPv4 and cidr
for the IPv6 resolutions of the name.
And yes, those are actually used out there, like by my bank company.
So the function parses the target for a "/cidr4" and strtonum() it to
cidr4, then parses for another "/cidr6" and strtonum() to cidr6.
[0] https://tools.ietf.org/rfc/rfc7208.txt
Here's an updated patch, plus two independant fixes for smtpd.h
(missing header limits.h for PATH_MAX and imsg.h for img structure).
struct sender target removed
smtpd.h header added for xmalloc()
fixed parse_sender for CIDRs like "//num".
Index: usr.sbin/smtpd/smtpd.h
===
RCS file: /var/cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.642
diff -u -r1.642 smtpd.h
--- usr.sbin/smtpd/smtpd.h 3 Nov 2019 23:58:51 - 1.642
+++ usr.sbin/smtpd/smtpd.h 23 Nov 2019 20:19:17 -
@@ -22,6 +22,8 @@
#define nitems(_a) (sizeof((_a)) / sizeof((_a)[0]))
#endif
+#include
+#include
#include
#include
#include
Index: usr.sbin/smtpd/spfwalk.c
===
RCS file: /var/cvs/src/usr.sbin/smtpd/spfwalk.c,v
retrieving revision 1.15
diff -u -r1.15 spfwalk.c
--- usr.sbin/smtpd/spfwalk.c11 Nov 2019 17:20:25 - 1.15
+++ usr.sbin/smtpd/spfwalk.c23 Nov 2019 21:13:11 -
@@ -35,20 +35,28 @@
#include
#define LINE_MAX 1024
+#include "smtpd.h"
#include "smtpd-defines.h"
#include "smtpd-api.h"
#include "unpack_dns.h"
#include "parser.h"
+struct sender {
+ void(*cb)(struct dns_rr *, struct sender *);
+ int cidr4;
+ int cidr6;
+};
+
int spfwalk(int, struct parameter *);
-static voiddispatch_txt(struct dns_rr *);
-static voiddispatch_mx(struct dns_rr *);
-static voiddispatch_a(struct dns_rr *);
-static voiddispatch_(struct dns_rr *);
-static voidlookup_record(int, const char *, void (*)(struct dns_rr *));
+static voiddispatch_txt(struct dns_rr *, struct sender *);
+static voiddispatch_mx(struct dns_rr *, struct sender *);
+static voiddispatch_a(struct dns_rr *, struct sender *);
+static voiddispatch_(struct dns_rr *, struct sender *);
+static voidlookup_record(int, const char *, struct sender *);
static voiddispatch_record(struct asr_result *, void *);
static ssize_t parse_txt(const char *, size_t, char *, size_t);
+static int parse_sender(char *, struct sender *);
int ip_v4 = 0;
int ip_v6 = 0;
@@ -59,6 +67,7 @@
int
spfwalk(int argc, struct parameter *argv)
{
+ struct sendersdr;
const char *ip_family = NULL;
char*line = NULL;
size_t linesize = 0;
@@ -81,12 +90,15 @@
dict_init();
event_init();
+ sdr.cidr4 = sdr.cidr6 = -1;
+ sdr.cb = dispatch_txt;
+
while ((linelen = getline(, , stdin)) != -1) {
while (linelen-- > 0 && isspace(line[linelen]))
line[linelen] = '\0';
if (linelen > 0)
- lookup_record(T_TXT, line, dispatch_txt);
+ lookup_record(T_TXT, line, );
}
free(line);
@@ -100,20 +112,23 @@
}
void
-lookup_record(int type, const char *record, void (*cb)(struct dns_rr *))
+lookup_record(int type, const char *record, struct sender *sdr)
{
struct asr_query *as;
+ struct sender *nsdr;
as = res_query_async(record, C_IN, type, NULL);
if (as == NULL)
err(1, "res_query_async");
- event_asr_run(as, dispatch_record, cb);
+ nsdr
Re: smtpd: add support for cidr in hostname resolution for spf walk
On Tue, Nov 12, 2019 at 02:16:25PM +0100, Quentin Rameau wrote:
Hello
> Hello,
>
> Here's a patch for smtpctl spf resolution, adding support for target
> specified as a hostname + cidr.
>
> Yes, SPF lets you specify targets like a:example.com/24.
>
> Due to the async and recursive nature of DNS resolution in spfwalk.c,
> it's kind of hard to pass data around without too much memory
> allocation, so I decided to just pass cidr(s) as integer value instead
> of keeping the original string and passing pointers around.
>
> Comments welcome, thanks!
I find it confusing that everything is stuffed into the sender struct,
including the callback. I don't really see the point. If you don't
need to pass the target value over to the callback, there is no need
to have it in the struct either, especially since the pointer is only
valid at the time you call the lookup function.
Another point, I don't understand how the parse_sender() function is
supposed to work. Can you give examples with cidr4 and cidr6?
Eric.
>
> Index: usr.sbin/smtpd/spfwalk.c
> ===
> RCS file: /var/cvs/src/usr.sbin/smtpd/spfwalk.c,v
> retrieving revision 1.15
> diff -u -r1.15 spfwalk.c
> --- usr.sbin/smtpd/spfwalk.c 11 Nov 2019 17:20:25 - 1.15
> +++ usr.sbin/smtpd/spfwalk.c 12 Nov 2019 12:43:25 -
> @@ -40,15 +40,24 @@
> #include "unpack_dns.h"
> #include "parser.h"
>
> +struct sender {
> + void(*cb)(struct dns_rr *, struct sender *);
> + char *target;
> + int cidr4;
> + int cidr6;
> +};
> +
> +void *xmalloc(size_t);
> int spfwalk(int, struct parameter *);
>
> -static void dispatch_txt(struct dns_rr *);
> -static void dispatch_mx(struct dns_rr *);
> -static void dispatch_a(struct dns_rr *);
> -static void dispatch_(struct dns_rr *);
> -static void lookup_record(int, const char *, void (*)(struct dns_rr *));
> +static void dispatch_txt(struct dns_rr *, struct sender *);
> +static void dispatch_mx(struct dns_rr *, struct sender *);
> +static void dispatch_a(struct dns_rr *, struct sender *);
> +static void dispatch_(struct dns_rr *, struct sender *);
> +static void lookup_record(int, struct sender *);
> static void dispatch_record(struct asr_result *, void *);
> static ssize_t parse_txt(const char *, size_t, char *, size_t);
> +static int parse_sender(struct sender *);
>
> int ip_v4 = 0;
> int ip_v6 = 0;
> @@ -59,6 +68,7 @@
> int
> spfwalk(int argc, struct parameter *argv)
> {
> + struct sendersdr;
> const char *ip_family = NULL;
> char*line = NULL;
> size_t linesize = 0;
> @@ -81,12 +91,17 @@
> dict_init();
> event_init();
>
> + sdr.cidr4 = sdr.cidr6 = -1;
> + sdr.cb = dispatch_txt;
> +
> while ((linelen = getline(, , stdin)) != -1) {
> while (linelen-- > 0 && isspace(line[linelen]))
> line[linelen] = '\0';
>
> - if (linelen > 0)
> - lookup_record(T_TXT, line, dispatch_txt);
> + if (linelen > 0) {
> + sdr.target = line;
> + lookup_record(T_TXT, );
> + }
> }
>
> free(line);
> @@ -100,20 +115,23 @@
> }
>
> void
> -lookup_record(int type, const char *record, void (*cb)(struct dns_rr *))
> +lookup_record(int type, struct sender *sdr)
> {
> struct asr_query *as;
> + struct sender *nsdr;
>
> - as = res_query_async(record, C_IN, type, NULL);
> + as = res_query_async(sdr->target, C_IN, type, NULL);
> if (as == NULL)
> err(1, "res_query_async");
> - event_asr_run(as, dispatch_record, cb);
> + nsdr = xmalloc(sizeof(*nsdr));
> + *nsdr = *sdr;
> + event_asr_run(as, dispatch_record, (void *)nsdr);
> }
>
> void
> dispatch_record(struct asr_result *ar, void *arg)
> {
> - void (*cb)(struct dns_rr *) = arg;
> + struct sender *sdr = arg;
> struct unpack pack;
> struct dns_header h;
> struct dns_query q;
> @@ -121,7 +139,7 @@
>
> /* best effort */
> if (ar->ar_h_errno && ar->ar_h_errno != NO_DATA)
> - return;
> + goto end;
>
> unpack_init(, ar->ar_data, ar->ar_datalen);
> unpack_header(, );
> @@ -130,19 +148,22 @@
> for (; h.ancount; h.ancount--) {
> unpack_rr(, );
> /**/
> - cb();
> + sdr->cb(, sdr);
> }
> +end:
> + free(sdr);
> }
>
> void
> -dispatch_txt(struct dns_rr *rr)
> +dispatch_txt(struct dns_rr *rr, struct sender *sdr)
> {
> + char buf[4096];
> + char *argv[512];
> + char buf2[512];
> + struct sender lsdr;
> struct in6_addr ina;
> -char buf[4096];
> -char buf2[512];
> -char *in = buf;
> -char *argv[512];
> -char **ap = argv;
> + char **ap = argv;
> + char *in = buf;
> char
smtpd: add support for cidr in hostname resolution for spf walk
Hello,
Here's a patch for smtpctl spf resolution, adding support for target
specified as a hostname + cidr.
Yes, SPF lets you specify targets like a:example.com/24.
Due to the async and recursive nature of DNS resolution in spfwalk.c,
it's kind of hard to pass data around without too much memory
allocation, so I decided to just pass cidr(s) as integer value instead
of keeping the original string and passing pointers around.
Comments welcome, thanks!
Index: usr.sbin/smtpd/spfwalk.c
===
RCS file: /var/cvs/src/usr.sbin/smtpd/spfwalk.c,v
retrieving revision 1.15
diff -u -r1.15 spfwalk.c
--- usr.sbin/smtpd/spfwalk.c11 Nov 2019 17:20:25 - 1.15
+++ usr.sbin/smtpd/spfwalk.c12 Nov 2019 12:43:25 -
@@ -40,15 +40,24 @@
#include "unpack_dns.h"
#include "parser.h"
+struct sender {
+ void(*cb)(struct dns_rr *, struct sender *);
+ char *target;
+ int cidr4;
+ int cidr6;
+};
+
+void *xmalloc(size_t);
int spfwalk(int, struct parameter *);
-static voiddispatch_txt(struct dns_rr *);
-static voiddispatch_mx(struct dns_rr *);
-static voiddispatch_a(struct dns_rr *);
-static voiddispatch_(struct dns_rr *);
-static voidlookup_record(int, const char *, void (*)(struct dns_rr *));
+static voiddispatch_txt(struct dns_rr *, struct sender *);
+static voiddispatch_mx(struct dns_rr *, struct sender *);
+static voiddispatch_a(struct dns_rr *, struct sender *);
+static voiddispatch_(struct dns_rr *, struct sender *);
+static voidlookup_record(int, struct sender *);
static voiddispatch_record(struct asr_result *, void *);
static ssize_t parse_txt(const char *, size_t, char *, size_t);
+static int parse_sender(struct sender *);
int ip_v4 = 0;
int ip_v6 = 0;
@@ -59,6 +68,7 @@
int
spfwalk(int argc, struct parameter *argv)
{
+ struct sendersdr;
const char *ip_family = NULL;
char*line = NULL;
size_t linesize = 0;
@@ -81,12 +91,17 @@
dict_init();
event_init();
+ sdr.cidr4 = sdr.cidr6 = -1;
+ sdr.cb = dispatch_txt;
+
while ((linelen = getline(, , stdin)) != -1) {
while (linelen-- > 0 && isspace(line[linelen]))
line[linelen] = '\0';
- if (linelen > 0)
- lookup_record(T_TXT, line, dispatch_txt);
+ if (linelen > 0) {
+ sdr.target = line;
+ lookup_record(T_TXT, );
+ }
}
free(line);
@@ -100,20 +115,23 @@
}
void
-lookup_record(int type, const char *record, void (*cb)(struct dns_rr *))
+lookup_record(int type, struct sender *sdr)
{
struct asr_query *as;
+ struct sender *nsdr;
- as = res_query_async(record, C_IN, type, NULL);
+ as = res_query_async(sdr->target, C_IN, type, NULL);
if (as == NULL)
err(1, "res_query_async");
- event_asr_run(as, dispatch_record, cb);
+ nsdr = xmalloc(sizeof(*nsdr));
+ *nsdr = *sdr;
+ event_asr_run(as, dispatch_record, (void *)nsdr);
}
void
dispatch_record(struct asr_result *ar, void *arg)
{
- void (*cb)(struct dns_rr *) = arg;
+ struct sender *sdr = arg;
struct unpack pack;
struct dns_header h;
struct dns_query q;
@@ -121,7 +139,7 @@
/* best effort */
if (ar->ar_h_errno && ar->ar_h_errno != NO_DATA)
- return;
+ goto end;
unpack_init(, ar->ar_data, ar->ar_datalen);
unpack_header(, );
@@ -130,19 +148,22 @@
for (; h.ancount; h.ancount--) {
unpack_rr(, );
/**/
- cb();
+ sdr->cb(, sdr);
}
+end:
+ free(sdr);
}
void
-dispatch_txt(struct dns_rr *rr)
+dispatch_txt(struct dns_rr *rr, struct sender *sdr)
{
+ char buf[4096];
+ char *argv[512];
+ char buf2[512];
+ struct sender lsdr;
struct in6_addr ina;
-char buf[4096];
-char buf2[512];
-char *in = buf;
-char *argv[512];
-char **ap = argv;
+ char **ap = argv;
+ char *in = buf;
char *end;
ssize_t n;
@@ -173,6 +194,8 @@
if (**ap == '+' || **ap == '?')
(*ap)++;
+ lsdr.cidr4 = lsdr.cidr6 = -1;
+
if (strncasecmp("ip4:", *ap, 4) == 0) {
if ((ip_v4 == 1 || ip_both == 1) &&
inet_net_pton(AF_INET, *(ap) + 4,
@@ -190,35 +213,55 @@
if (strcasecmp("a", *ap) == 0) {
print_dname(rr->rr_dname, buf2, sizeof(buf2));
buf2[strlen(buf2) - 1] = '\0';
- lookup_record(T_A, buf2, dispatch_a);
- lookup_record(T_, buf2, dispatch_);
+
