Re: smtpd filters: banner hostname
On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> 28 août 2019 09:04 "Martijn van Duren" a
> écrit:
>
>> Currently looking into writing an spf filter based on libopensmtpd.
>> While working through the spec I found in RFC7208 section 7.3 that:
>> The "r" macro expands to the name of the receiving MTA.
>> In other words the hostname presented in the banner. Unfortunately we
>> also support the hostnames directive, which supports ip-hostname
>> mappings via dynamic tables, which makes it impossible to transfer via
>> "config|".
>>
>> This is a major change that can break (and in the case of libopensmtpd
>> will break) parsers. We're currently at 0.1, so I don't know if we want
>> push it to 1 just yet, or if we want to call 1 release-stable and just
>> bump it to 0.2 for now since we don't have a release yet with filters.
>>
>> thoughts?
>>
>
> I'm sorry but I'm unsure I understand what you're trying to do with the
> banner,
> can you explain ?
spf has an "exp" modifier, which allows the reject message to be
specified by the spf administrator. To do so it can utilize several
macros. One option being "%{r}", which expands to:
domain name of host performing the check.
I can use gethostname(3), but that could contradict the banner, hence I
want to use the hostname used in the banner.
>
> If there's need for the hostname presented in the banner to be passed to
> filters,
> which makes sense, it needs its own reporting event which is basically the
> server
> side of the link-identify event.
>
> One thing for sure, you can't put that info in the link-connect event because
> the
> banner is displayed _after_ link-connect and while in smtp-in we already know
> the
> hostname we'll use in the banner, this isn't the case for smtp-out which will
> not
> be able to infer that information before actually seeing the banner.
You're right. I'll cook up something else.
>
>
>> Index: lka.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
>> retrieving revision 1.239
>> diff -u -p -r1.239 lka.c
>> --- lka.c 26 Jul 2019 06:30:13 - 1.239
>> +++ lka.c 28 Aug 2019 06:28:33 -
>> @@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
>> const char *heloname;
>> const char *filter_name;
>> const char *result;
>> + const char *banner;
>> struct sockaddr_storage ss_src, ss_dest;
>> int filter_response;
>> int filter_phase;
>> @@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
>> m_get_int(, );
>> m_get_sockaddr(, (struct sockaddr *)_src);
>> m_get_sockaddr(, (struct sockaddr *)_dest);
>> + m_get_string(, );
>> m_end();
>>
>> - lka_report_smtp_link_connect(direction, , reqid, rdns, fcrdns, _src,
>> _dest);
>> + lka_report_smtp_link_connect(direction, , reqid, rdns,
>> + fcrdns, _src, _dest, banner);
>> return;
>>
>> case IMSG_REPORT_SMTP_LINK_DISCONNECT:
>> Index: lka_report.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
>> retrieving revision 1.24
>> diff -u -p -r1.24 lka_report.c
>> --- lka_report.c 18 Aug 2019 16:52:02 - 1.24
>> +++ lka_report.c 28 Aug 2019 06:28:33 -
>> @@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
>> }
>>
>> void
>> -lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
>> uint64_t reqid, const char
>> *rdns,
>> - int fcrdns,
>> +lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
>> + uint64_t reqid, const char *rdns, int fcrdns,
>> const struct sockaddr_storage *ss_src,
>> - const struct sockaddr_storage *ss_dest)
>> + const struct sockaddr_storage *ss_dest, const char *banner)
>> {
>> char src[NI_MAXHOST + 5];
>> char dest[NI_MAXHOST + 5];
>> @@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char
>> }
>>
>> report_smtp_broadcast(reqid, direction, tv, "link-connect",
>> - "%016"PRIx64"|%s|%s|%s|%s\n",
>> - reqid, rdns, fcrdns_str, src, dest);
>> + "%016"PRIx64"|%s|%s|%s|%s|%s\n",
>> + reqid, rdns, fcrdns_str, src, dest, banner);
>> }
>>
>> void
>> Index: report_smtp.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/report_smtp.c,v
>> retrieving revision 1.8
>> diff -u -p -r1.8 report_smtp.c
>> --- report_smtp.c 26 Jul 2019 06:30:13 - 1.8
>> +++ report_smtp.c 28 Aug 2019 06:28:33 -
>> @@ -46,7 +46,7 @@
>> void
>> report_smtp_link_connect(const char *direction, uint64_t qid, const char
>> *rdns, int fcrdns,
>> const struct sockaddr_storage *ss_src,
>> - const struct sockaddr_storage *ss_dest)
>> + const struct sockaddr_storage *ss_dest, const char *banner)
>> {
>> struct timeval tv;
>>
>> @@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
>> m_add_int(p_lka, fcrdns);
>> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
>> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
>> + m_add_string(p_lka, banner);
>> m_close(p_lka);
>> }
>>
>> Index: smtp_session.c
>>
Re: smtpd filters: banner hostname
28 août 2019 09:04 "Martijn van Duren" a
écrit:
> Currently looking into writing an spf filter based on libopensmtpd.
> While working through the spec I found in RFC7208 section 7.3 that:
> The "r" macro expands to the name of the receiving MTA.
> In other words the hostname presented in the banner. Unfortunately we
> also support the hostnames directive, which supports ip-hostname
> mappings via dynamic tables, which makes it impossible to transfer via
> "config|".
>
> This is a major change that can break (and in the case of libopensmtpd
> will break) parsers. We're currently at 0.1, so I don't know if we want
> push it to 1 just yet, or if we want to call 1 release-stable and just
> bump it to 0.2 for now since we don't have a release yet with filters.
>
> thoughts?
>
I'm sorry but I'm unsure I understand what you're trying to do with the banner,
can you explain ?
If there's need for the hostname presented in the banner to be passed to
filters,
which makes sense, it needs its own reporting event which is basically the
server
side of the link-identify event.
One thing for sure, you can't put that info in the link-connect event because
the
banner is displayed _after_ link-connect and while in smtp-in we already know
the
hostname we'll use in the banner, this isn't the case for smtp-out which will
not
be able to infer that information before actually seeing the banner.
> Index: lka.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 lka.c
> --- lka.c 26 Jul 2019 06:30:13 - 1.239
> +++ lka.c 28 Aug 2019 06:28:33 -
> @@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
> const char *heloname;
> const char *filter_name;
> const char *result;
> + const char *banner;
> struct sockaddr_storage ss_src, ss_dest;
> int filter_response;
> int filter_phase;
> @@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
> m_get_int(, );
> m_get_sockaddr(, (struct sockaddr *)_src);
> m_get_sockaddr(, (struct sockaddr *)_dest);
> + m_get_string(, );
> m_end();
>
> - lka_report_smtp_link_connect(direction, , reqid, rdns, fcrdns, _src,
> _dest);
> + lka_report_smtp_link_connect(direction, , reqid, rdns,
> + fcrdns, _src, _dest, banner);
> return;
>
> case IMSG_REPORT_SMTP_LINK_DISCONNECT:
> Index: lka_report.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 lka_report.c
> --- lka_report.c 18 Aug 2019 16:52:02 - 1.24
> +++ lka_report.c 28 Aug 2019 06:28:33 -
> @@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
> }
>
> void
> -lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
> uint64_t reqid, const char
> *rdns,
> - int fcrdns,
> +lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
> + uint64_t reqid, const char *rdns, int fcrdns,
> const struct sockaddr_storage *ss_src,
> - const struct sockaddr_storage *ss_dest)
> + const struct sockaddr_storage *ss_dest, const char *banner)
> {
> char src[NI_MAXHOST + 5];
> char dest[NI_MAXHOST + 5];
> @@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char
> }
>
> report_smtp_broadcast(reqid, direction, tv, "link-connect",
> - "%016"PRIx64"|%s|%s|%s|%s\n",
> - reqid, rdns, fcrdns_str, src, dest);
> + "%016"PRIx64"|%s|%s|%s|%s|%s\n",
> + reqid, rdns, fcrdns_str, src, dest, banner);
> }
>
> void
> Index: report_smtp.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/report_smtp.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 report_smtp.c
> --- report_smtp.c 26 Jul 2019 06:30:13 - 1.8
> +++ report_smtp.c 28 Aug 2019 06:28:33 -
> @@ -46,7 +46,7 @@
> void
> report_smtp_link_connect(const char *direction, uint64_t qid, const char
> *rdns, int fcrdns,
> const struct sockaddr_storage *ss_src,
> - const struct sockaddr_storage *ss_dest)
> + const struct sockaddr_storage *ss_dest, const char *banner)
> {
> struct timeval tv;
>
> @@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
> m_add_int(p_lka, fcrdns);
> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
> m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
> + m_add_string(p_lka, banner);
> m_close(p_lka);
> }
>
> Index: smtp_session.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
> retrieving revision 1.407
> diff -u -p -r1.407 smtp_session.c
> --- smtp_session.c 14 Aug 2019 21:11:25 - 1.407
> +++ smtp_session.c 28 Aug 2019 06:28:33 -
> @@ -2029,7 +2029,7 @@ smtp_connected(struct smtp_session *s)
> smtp_filter_begin(s);
>
> report_smtp_link_connect("smtp-in", s->id, s->rdns, s->fcrdns, >ss,
> - >listener->ss);
> + >listener->ss, s->smtpname);
>
> smtp_filter_phase(FILTER_CONNECT, s, ss_to_text(>ss));
> }
> Index: smtpd.h
>
Re: smtpd filters: banner hostname
On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> 28 août 2019 09:04 "Martijn van Duren" a
> écrit:
>
>> Currently looking into writing an spf filter based on libopensmtpd.
>> While working through the spec I found in RFC7208 section 7.3 that:
>> The "r" macro expands to the name of the receiving MTA.
>> In other words the hostname presented in the banner. Unfortunately we
>> also support the hostnames directive, which supports ip-hostname
>> mappings via dynamic tables, which makes it impossible to transfer via
>> "config|".
>>
>> This is a major change that can break (and in the case of libopensmtpd
>> will break) parsers. We're currently at 0.1, so I don't know if we want
>> push it to 1 just yet, or if we want to call 1 release-stable and just
>> bump it to 0.2 for now since we don't have a release yet with filters.
>>
>> thoughts?
>>
>
> I'm sorry but I'm unsure I understand what you're trying to do with the
> banner,
> can you explain ?
>
> If there's need for the hostname presented in the banner to be passed to
> filters,
> which makes sense, it needs its own reporting event which is basically the
> server
> side of the link-identify event.
>
> One thing for sure, you can't put that info in the link-connect event because
> the
> banner is displayed _after_ link-connect and while in smtp-in we already know
> the
> hostname we'll use in the banner, this isn't the case for smtp-out which will
> not
> be able to infer that information before actually seeing the banner.
>
So the diff below implements report|link-greeting
I haven't implemented the smtp-out case, since none of the smtp-out
cases appear to be currently implemented.
Does this read better?
martijn@
Index: lka.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
retrieving revision 1.239
diff -u -p -r1.239 lka.c
--- lka.c 26 Jul 2019 06:30:13 - 1.239
+++ lka.c 28 Aug 2019 08:32:33 -
@@ -84,6 +84,8 @@ lka_imsg(struct mproc *p, struct imsg *i
const char *response;
const char *ciphers;
const char *address;
+ const char *domain;
+ const char *textstring;
const char *helomethod;
const char *heloname;
const char *filter_name;
@@ -391,6 +393,19 @@ lka_imsg(struct mproc *p, struct imsg *i
m_end();
lka_report_smtp_link_connect(direction, , reqid, rdns,
fcrdns, _src, _dest);
+ return;
+
+ case IMSG_REPORT_SMTP_LINK_GREETING:Index: lka.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
retrieving revision 1.239
diff -u -p -r1.239 lka.c
--- lka.c 26 Jul 2019 06:30:13 - 1.239
+++ lka.c 28 Aug 2019 08:32:33 -
@@ -84,6 +84,8 @@ lka_imsg(struct mproc *p, struct imsg *i
const char *response;
const char *ciphers;
const char *address;
+ const char *domain;
+ const char *textstring;
const char *helomethod;
const char *heloname;
const char *filter_name;
@@ -391,6 +393,19 @@ lka_imsg(struct mproc *p, struct imsg *i
m_end();
lka_report_smtp_link_connect(direction, , reqid, rdns,
fcrdns, _src, _dest);
+ return;
+
+ case IMSG_REPORT_SMTP_LINK_GREETING:
+ m_msg(, imsg);
+ m_get_string(, );
+ m_get_timeval(, );
+ m_get_id(, );
+ m_get_string(, );
+ m_get_string(, );
+ m_end();
+
+ lka_report_smtp_link_greeting(direction, reqid, , domain,
+ textstring);
return;
case IMSG_REPORT_SMTP_LINK_DISCONNECT:
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.41
diff -u -p -r1.41 lka_filter.c
--- lka_filter.c18 Aug 2019 16:52:02 - 1.41
+++ lka_filter.c28 Aug 2019 08:32:33 -
@@ -35,7 +35,7 @@
#include "smtpd.h"
#include "log.h"
-#definePROTOCOL_VERSION"0.1"
+#definePROTOCOL_VERSION"0.2"
struct filter;
struct filter_session;
Index: lka_report.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
retrieving revision 1.24
diff -u -p -r1.24 lka_report.c
--- lka_report.c18 Aug 2019 16:52:02 - 1.24
+++ lka_report.c28 Aug 2019 08:32:33 -
@@ -35,7 +35,7 @@
#include "smtpd.h"
#include "log.h"
-#definePROTOCOL_VERSION"0.1"
+#definePROTOCOL_VERSION"0.2"
struct reporter_proc {
TAILQ_ENTRY(reporter_proc) entries;
smtpd filters: banner hostname
Currently looking into writing an spf filter based on libopensmtpd.
While working through the spec I found in RFC7208 section 7.3 that:
The "r" macro expands to the name of the receiving MTA.
In other words the hostname presented in the banner. Unfortunately we
also support the hostnames directive, which supports ip-hostname
mappings via dynamic tables, which makes it impossible to transfer via
"config|".
This is a major change that can break (and in the case of libopensmtpd
will break) parsers. We're currently at 0.1, so I don't know if we want
push it to 1 just yet, or if we want to call 1 release-stable and just
bump it to 0.2 for now since we don't have a release yet with filters.
thoughts?
martijn@
Index: lka.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
retrieving revision 1.239
diff -u -p -r1.239 lka.c
--- lka.c 26 Jul 2019 06:30:13 - 1.239
+++ lka.c 28 Aug 2019 06:28:33 -
@@ -88,6 +88,7 @@ lka_imsg(struct mproc *p, struct imsg *i
const char *heloname;
const char *filter_name;
const char *result;
+ const char *banner;
struct sockaddr_storage ss_src, ss_dest;
int filter_response;
int filter_phase;
@@ -388,9 +389,11 @@ lka_imsg(struct mproc *p, struct imsg *i
m_get_int(, );
m_get_sockaddr(, (struct sockaddr *)_src);
m_get_sockaddr(, (struct sockaddr *)_dest);
+ m_get_string(, );
m_end();
- lka_report_smtp_link_connect(direction, , reqid, rdns,
fcrdns, _src, _dest);
+ lka_report_smtp_link_connect(direction, , reqid, rdns,
+ fcrdns, _src, _dest, banner);
return;
case IMSG_REPORT_SMTP_LINK_DISCONNECT:
Index: lka_report.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
retrieving revision 1.24
diff -u -p -r1.24 lka_report.c
--- lka_report.c18 Aug 2019 16:52:02 - 1.24
+++ lka_report.c28 Aug 2019 06:28:33 -
@@ -165,10 +165,10 @@ report_smtp_broadcast(uint64_t reqid, co
}
void
-lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
uint64_t reqid, const char *rdns,
-int fcrdns,
+lka_report_smtp_link_connect(const char *direction, struct timeval *tv,
+uint64_t reqid, const char *rdns, int fcrdns,
const struct sockaddr_storage *ss_src,
-const struct sockaddr_storage *ss_dest)
+const struct sockaddr_storage *ss_dest, const char *banner)
{
charsrc[NI_MAXHOST + 5];
chardest[NI_MAXHOST + 5];
@@ -207,8 +207,8 @@ lka_report_smtp_link_connect(const char
}
report_smtp_broadcast(reqid, direction, tv, "link-connect",
- "%016"PRIx64"|%s|%s|%s|%s\n",
- reqid, rdns, fcrdns_str, src, dest);
+ "%016"PRIx64"|%s|%s|%s|%s|%s\n",
+ reqid, rdns, fcrdns_str, src, dest, banner);
}
void
Index: report_smtp.c
===
RCS file: /cvs/src/usr.sbin/smtpd/report_smtp.c,v
retrieving revision 1.8
diff -u -p -r1.8 report_smtp.c
--- report_smtp.c 26 Jul 2019 06:30:13 - 1.8
+++ report_smtp.c 28 Aug 2019 06:28:33 -
@@ -46,7 +46,7 @@
void
report_smtp_link_connect(const char *direction, uint64_t qid, const char
*rdns, int fcrdns,
const struct sockaddr_storage *ss_src,
-const struct sockaddr_storage *ss_dest)
+const struct sockaddr_storage *ss_dest, const char *banner)
{
struct timeval tv;
@@ -60,6 +60,7 @@ report_smtp_link_connect(const char *dir
m_add_int(p_lka, fcrdns);
m_add_sockaddr(p_lka, (const struct sockaddr *)ss_src);
m_add_sockaddr(p_lka, (const struct sockaddr *)ss_dest);
+ m_add_string(p_lka, banner);
m_close(p_lka);
}
Index: smtp_session.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtp_session.c,v
retrieving revision 1.407
diff -u -p -r1.407 smtp_session.c
--- smtp_session.c 14 Aug 2019 21:11:25 - 1.407
+++ smtp_session.c 28 Aug 2019 06:28:33 -
@@ -2029,7 +2029,7 @@ smtp_connected(struct smtp_session *s)
smtp_filter_begin(s);
report_smtp_link_connect("smtp-in", s->id, s->rdns, s->fcrdns, >ss,
- >listener->ss);
+ >listener->ss, s->smtpname);
smtp_filter_phase(FILTER_CONNECT, s, ss_to_text(>ss));
}
Index: smtpd.h
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.h,v
retrieving revision 1.632
diff -u -p -r1.632 smtpd.h
--- smtpd.h 23 Aug 2019 07:09:52 - 1.632
+++ smtpd.h 28 Aug 2019 06:28:33 -
@@ -1330,7 +1330,7 @@ struct io *lka_proc_get_io(const char *)
Re: smtpd filters: banner hostname
On Wed, Aug 28, 2019 at 10:33:29AM +0200, Martijn van Duren wrote:
> On 8/28/19 9:23 AM, gil...@poolp.org wrote:
> > 28 ao??t 2019 09:04 "Martijn van Duren" a
> > ??crit:
> >
> >> Currently looking into writing an spf filter based on libopensmtpd.
> >> While working through the spec I found in RFC7208 section 7.3 that:
> >> The "r" macro expands to the name of the receiving MTA.
> >> In other words the hostname presented in the banner. Unfortunately we
> >> also support the hostnames directive, which supports ip-hostname
> >> mappings via dynamic tables, which makes it impossible to transfer via
> >> "config|".
> >>
> >> This is a major change that can break (and in the case of libopensmtpd
> >> will break) parsers. We're currently at 0.1, so I don't know if we want
> >> push it to 1 just yet, or if we want to call 1 release-stable and just
> >> bump it to 0.2 for now since we don't have a release yet with filters.
> >>
> >> thoughts?
> >>
> >
> > I'm sorry but I'm unsure I understand what you're trying to do with the
> > banner,
> > can you explain ?
> >
> > If there's need for the hostname presented in the banner to be passed to
> > filters,
> > which makes sense, it needs its own reporting event which is basically the
> > server
> > side of the link-identify event.
> >
> > One thing for sure, you can't put that info in the link-connect event
> > because the
> > banner is displayed _after_ link-connect and while in smtp-in we already
> > know the
> > hostname we'll use in the banner, this isn't the case for smtp-out which
> > will not
> > be able to infer that information before actually seeing the banner.
> >
> So the diff below implements report|link-greeting
>
> I haven't implemented the smtp-out case, since none of the smtp-out
> cases appear to be currently implemented.
>
> Does this read better?
>
No sorry, I'm very confused :-)
Let me explain my understanding just to make sure we're talking about
the same thing.
You said:
> spf has an "exp" modifier, which allows the reject message to be
> specified by the spf administrator. To do so it can utilize several
> macros. One option being "%{r}", which expands to:
> domain name of host performing the check.
So my understanding is that you'd like a filter to be able to use the
result from an SPF lookup to reject a message in your filter and have
the hostname that was used in the banner part of the message.
If that's so, I think the diff below is both not enough and a bit too
specific, preventing filters with different use-cases than yours from
using the information as easily:
1- first, i think your report event is too specific, keep in mind the
report events are used to build the internal state of the session,
so that you can basically duplicate struct smtp_session from smtpd
in a filter. the textstring part here is not really related to the
state of the session more than it is related to your use-case.
2- then, your use case is to allow rejecting with a custom message so
this can't be done with report events, which are informative, this
requires a filter event which allow taking decisions (like reject,
in this case). So I think that what's missing here is a phase that
allows filters to take a decision before printing the banner and I
think THIS is where your textstring goes, the filter receives that
report event with the hostname, it receives the filter request for
the banner and it can decide to write a custom banner.
I may want to write filters that keep track of the hostname for which
the MX is operating and that do not necessarily do anything at banner
so to me tying banner stuff outside of a banner phase is not correct.
Does it make sense ?
> Index: lka.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 lka.c
> --- lka.c 26 Jul 2019 06:30:13 - 1.239
> +++ lka.c 28 Aug 2019 08:32:33 -
> @@ -84,6 +84,8 @@ lka_imsg(struct mproc *p, struct imsg *i
> const char *response;
> const char *ciphers;
> const char *address;
> + const char *domain;
> + const char *textstring;
> const char *helomethod;
> const char *heloname;
> const char *filter_name;
> @@ -391,6 +393,19 @@ lka_imsg(struct mproc *p, struct imsg *i
> m_end();
>
> lka_report_smtp_link_connect(direction, , reqid, rdns,
> fcrdns, _src, _dest);
> + return;
> +
> + case IMSG_REPORT_SMTP_LINK_GREETING:Index: lka.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka.c,v
> retrieving revision 1.239
> diff -u -p -r1.239 lka.c
> --- lka.c 26 Jul 2019 06:30:13 - 1.239
> +++ lka.c 28 Aug 2019 08:32:33 -
> @@ -84,6 +84,8 @@ lka_imsg(struct
Re: smtpd filters: banner hostname
28 août 2019 14:07 "Martijn van Duren" a
écrit:
>>
>> So my understanding is that you'd like a filter to be able to use the
>> result from an SPF lookup to reject a message in your filter and have
>> the hostname that was used in the banner part of the message.
>
> Yes, because that's the only point in the transaction where I can be
> 100% certain on which hostname the transaction took place. This is
> because of listen on * hostnames , where table can change at
> runtime based on its backend.
>
Ok, so far so good :-)
>> If that's so, I think the diff below is both not enough and a bit too
>> specific, preventing filters with different use-cases than yours from
>> using the information as easily:
>>
>> 1- first, i think your report event is too specific, keep in mind the
>> report events are used to build the internal state of the session,
>> so that you can basically duplicate struct smtp_session from smtpd
>> in a filter. the textstring part here is not really related to the
>> state of the session more than it is related to your use-case.
>
> It's not related to my use-case. I included it for completeness.
> E.g. if someone wants to monitor what mailservers are being connected to
> in case of when we implement the smtp-out case.
> Since there's no uniform way to distil this information (it's freetext
> after all) I thought I'd return the textstring as specified by RFC5321.
> If you see harm in having the textstring part in the filters I don't
> mind dropping them, but I expect at some point people will ask for it.
>
There's no harm just as there's no harm adding more information to pretty
much any reporting event: "no harm" shouldn't be the argument.
The hostname makes a lot of sense because it is part of the session, this
means that currently you can't fully rebuild a session state in a filter,
we need a report event to allow filters to learn that hostname for sure.
The textstring part doesn't because:
a- it isn't a session state, the SMTP engine doesn't track it, it serves no
purpose in a session
b- ALL report events will have the same "ESMTP OpenSMTPD" value, it carries no
information, it is hardcoded
c- if there is a use-case that really required it, it can still be obtained
with protocol-server events
For b-, smtp-out will not be hardcoded but c- is still valid and if it so
happens that displaying the banner part is a very common use-case, we are
still allowed to create a specific report event.
For the time being, I think your diff without textstring is ok
>> 2- then, your use case is to allow rejecting with a custom message so
>> this can't be done with report events, which are informative, this
>> requires a filter event which allow taking decisions (like reject,
>> in this case). So I think that what's missing here is a phase that
>> allows filters to take a decision before printing the banner and I
>> think THIS is where your textstring goes, the filter receives that
>> report event with the hostname, it receives the filter request for
>> the banner and it can decide to write a custom banner.
>
> No, SPF filters at HELO/EHLO and MAIL FROM. If a host is rejected
> (because of a -all match) and the spf-record contains an "exp" (explain)
> entry it resolves fetches the txt record of where "exp" points to.
> Said record is then being evaluated by domain-spec and allows for the
> expansion of "%{r}", which evaluates to the domainname of the host
> doing the spf-checking.
>
Ok, so you can use actual filter hooks
> Say I would mail from openbsd+t...@imperialat.at to gil...@poolp.org and
> changed the txt imperialat.at to "v=spf1 mx -all exp:exp.imperialat.at"
> and txt exp.imperialat.at would be: "Message rejected by %{r}"
> If mx2.pool.org would reject the message because the sending ip is not
> listed in the spf imperialat.at, the resulting rejecting status would
> be:
> "550 5.7.1 Message rejected by mx2.poolp.org".
>
> See RFC7208 section 6.2, 7.2, 7,3 and 8.4
>
> So for the above I would need to know what the hostname of the
> parsing agent is. According to RFC5321 this information is available in
> the greeting directly after the status code. So if I were to register
> this on report|smtp-in|link-greeting I would have the domain name and I
> can cache this information on my session variable until I (potentially)
> need it during filter|helo, filter|ehlo and filter|mailfrom.
>
I understand and do you agree that link-greeting _without_ textstring is
not preventing you from doing that ?
Because nowhere is the textstring used in this pattern.
>> I may want to write filters that keep track of the hostname for which
>> the MX is operating and that do not necessarily do anything at banner
>> so to me tying banner stuff outside of a banner phase is not correct.
>>
>> Does it make sense ?
>
> I don't want to do anything at banner, I just want to know the banner,
> or more specifically, the hostname in the banner.
>
Yes sorry my bad, but this still covers the
Re: smtpd filters: banner hostname
28 août 2019 15:10 "Martijn van Duren" a écrit: > On 8/28/19 2:52 PM, gil...@poolp.org wrote: > >> Yes sorry my bad, but this still covers the textstring argument: >> >> We need to be able to reject at banner and this is going to be implemented >> as a filter phase, so basically if we expect people to need it for filters >> the proper place to pass your textstring to filters is in that filter phase. > > I'll commit it without the textstring. > ok ! > I just don't understand what you mean by reject at banner. > We only allow filtering on smtp-in and since we are the once that > produce the banner, how will you filter at that point? I still haven't fully gone through this but the bottom line is: - today we have report link-connect and we have filter phase connect - link-connect makes sense in getting info from the connection to build session, this is good - filter phase connect is kinda of weird as it gets the same info as the previous report event which is not right and it answers results in the banner displayed, it's kind of a strange filter phase in my opinion I think the filter phase connect should really be a filter phase "greeting" and this would get the session id to retrieve session state accumulated from report link-connect, as well as the banner as input, resulting in a proceed, rewrite, reject or disconnect action. It would make more sense but i haven't gone completely through this to make sure its correct, I'll make a proposal soon.
Re: smtpd filters: banner hostname
28 août 2019 12:26 "Gilles Chehade" a écrit:
> On Wed, Aug 28, 2019 at 10:33:29AM +0200, Martijn van Duren wrote:
>
>> On 8/28/19 9:23 AM, gil...@poolp.org wrote:
>> 28 ao??t 2019 09:04 "Martijn van Duren" a
>> ??crit:
>>
>> Currently looking into writing an spf filter based on libopensmtpd.
>> While working through the spec I found in RFC7208 section 7.3 that:
>> The "r" macro expands to the name of the receiving MTA.
>> In other words the hostname presented in the banner. Unfortunately we
>> also support the hostnames directive, which supports ip-hostname
>> mappings via dynamic tables, which makes it impossible to transfer via
>> "config|".
>>
>> This is a major change that can break (and in the case of libopensmtpd
>> will break) parsers. We're currently at 0.1, so I don't know if we want
>> push it to 1 just yet, or if we want to call 1 release-stable and just
>> bump it to 0.2 for now since we don't have a release yet with filters.
>>
>> thoughts?
>>
>> I'm sorry but I'm unsure I understand what you're trying to do with the
>> banner,
>> can you explain ?
>>
>> If there's need for the hostname presented in the banner to be passed to
>> filters,
>> which makes sense, it needs its own reporting event which is basically the
>> server
>> side of the link-identify event.
>>
>> One thing for sure, you can't put that info in the link-connect event
>> because the
>> banner is displayed _after_ link-connect and while in smtp-in we already
>> know the
>> hostname we'll use in the banner, this isn't the case for smtp-out which
>> will not
>> be able to infer that information before actually seeing the banner.
>>
>> So the diff below implements report|link-greeting
>>
>> I haven't implemented the smtp-out case, since none of the smtp-out
>> cases appear to be currently implemented.
>>
>> Does this read better?
>
> No sorry, I'm very confused :-)
>
> Let me explain my understanding just to make sure we're talking about
> the same thing.
>
> You said:
>
>> spf has an "exp" modifier, which allows the reject message to be
>> specified by the spf administrator. To do so it can utilize several
>> macros. One option being "%{r}", which expands to:
>> domain name of host performing the check.
>
> So my understanding is that you'd like a filter to be able to use the
> result from an SPF lookup to reject a message in your filter and have
> the hostname that was used in the banner part of the message.
>
> If that's so, I think the diff below is both not enough and a bit too
> specific, preventing filters with different use-cases than yours from
> using the information as easily:
>
> 1- first, i think your report event is too specific, keep in mind the
> report events are used to build the internal state of the session,
> so that you can basically duplicate struct smtp_session from smtpd
> in a filter. the textstring part here is not really related to the
> state of the session more than it is related to your use-case.
>
> 2- then, your use case is to allow rejecting with a custom message so
> this can't be done with report events, which are informative, this
> requires a filter event which allow taking decisions (like reject,
> in this case). So I think that what's missing here is a phase that
> allows filters to take a decision before printing the banner and I
> think THIS is where your textstring goes, the filter receives that
> report event with the hostname, it receives the filter request for
> the banner and it can decide to write a custom banner.
>
> I may want to write filters that keep track of the hostname for which
> the MX is operating and that do not necessarily do anything at banner
> so to me tying banner stuff outside of a banner phase is not correct.
>
> Does it make sense ?
>
By the way, as a side note, I already had a use-case which required a
banner filtering phase to allow rejecting at banner and which doesn't
need the server hostname so this tends to go in favour of what I said
above:
my filter would not register to receive the server hostname because I
don't care about that, but I'd still need to get the banner so I will
be able to take a decision to disconnect with a different message.
Re: smtpd filters: banner hostname
On 8/28/19 2:52 PM, gil...@poolp.org wrote: > > Yes sorry my bad, but this still covers the textstring argument: > > We need to be able to reject at banner and this is going to be implemented > as a filter phase, so basically if we expect people to need it for filters > the proper place to pass your textstring to filters is in that filter phase. > I'll commit it without the textstring. I just don't understand what you mean by reject at banner. We only allow filtering on smtp-in and since we are the once that produce the banner, how will you filter at that point?
Re: Make filter line handling more developer friendly
On Mon, Aug 26, 2019 at 12:04:06PM +0200, Martijn van Duren wrote:
> On 8/26/19 11:42 AM, Sebastien Marie wrote:
> > On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
> >> Right now all we get is "Misbehaving filter", which doesn't tell the
> >> developer a lot.
> >>
> >> Diff below does the following:
> >> - Make the register_hooks actually fail on misbehaviour.
> >> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> >> - Hoist some checks from lka_filter_process_response to processor_io
> >> - Make lka_filter_process_response void (it fatals now)
> >> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> >> - change str{,n}casecmp to str{,n}cmp for consistency.
> >> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> >> - restructure lka_filter_process_response to be shorter.
> >> and most importantly
> >> - Add a descriptive fatal() minefield.
> >>
> >> -12 LoC.
> >>
> >> Tested with filter-dnsbl with both a successful and rejected connection.
> >>
> >> OK?
> >
> > just one remark.
> >
> >> Index: lka_filter.c
> >> ===
> >> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> >> retrieving revision 1.41
> >> diff -u -p -r1.41 lka_filter.c
> >> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> >> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> >> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> >>struct filter_session *fs;
> >>
> >>(void)strlcpy(buffer, line, sizeof buffer);
> >> - if ((ep = strchr(buffer, '|')) == NULL)
> >> - return 0;
> >> - *ep = 0;
> >> + ep = strchr(buffer, '|');
> >> + ep[0] = '\0';
> >
> > is it possible to buffer to not have '|' ? if yes, you could deference NULL.
> >
> >
> You're absolutely right.
> It's not a risk, since both would crash smtpd, but doing the check would
> result in a clearer error message, which is the purpose of this
> endeavour.
>
ok gilles@
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 26 Aug 2019 10:03:43 -
> @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> static void filter_result_disconnect(uint64_t, const char *);
>
> static void filter_session_io(struct io *, int, void *);
> -int lka_filter_process_response(const char *, const char *);
> +void lka_filter_process_response(const char *, const char *);
>
>
> struct filter_session {
> @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> hook += 8;
> }
> else
> - return;
> + fatalx("Invalid message direction: %s", hook);
>
> for (i = 0; i < nitems(filter_execs); i++)
> if (strcmp(hook, filter_execs[i].phase_name) == 0)
> break;
> if (i == nitems(filter_execs))
> - return;
> + fatalx("Unrecognized report name: %s", hook);
>
> iter = NULL;
> while (dict_iter(, , _name, (void **)))
> @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> }
> }
>
> -int
> +void
> lka_filter_process_response(const char *name, const char *line)
> {
> uint64_t reqid;
> @@ -430,87 +430,68 @@ lka_filter_process_response(const char *
>
> (void)strlcpy(buffer, line, sizeof buffer);
> if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing token: %s", line);
> + ep[0] = '\0';
>
> kind = buffer;
> - if (strcmp(kind, "register") == 0)
> - return 1;
> -
> - if (strcmp(kind, "filter-result") != 0 &&
> - strcmp(kind, "filter-dataline") != 0)
> - return 0;
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing reqid: %s", line);
> + ep[0] = '\0';
>
> token = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && token == ULONG_MAX)
> - return 0;
> + fatalx("Invalid token: %s", line);
> + if (errno == ERANGE && token == ULLONG_MAX)
> + fatal("Invalid token: %s", line);
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatal("Missing directive: %s", line);
> + ep[0] = '\0';
>
> reqid = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && reqid == ULONG_MAX)
> - return 0;
> + fatalx("Invalid reqid: %s", line);
> + if (errno == ERANGE && reqid == ULLONG_MAX)
> + fatal("Invalid reqid: %s", line);
>
display reasons for wifi association failures in ifconfig
I find myself regularly asking people to run 'ifconfig iwm0 debug' to
figure out reasons behind association failures from dmesg output.
I would like to make this information more accessible. Making it part
of regular ifconfig output seems like a reasonable thing to do. This
becomes possible now that node information is cached across scans.
The current net80211 code is already computing failure reason codes which
are visible in 'ifconfig debug' output. So far such codes were magic numbers.
We can simply create macros for them and expose them in ieee80211_ioctl.h,
and add another code to represent the "wrong-WPA-key" situation.
To avoid changing the existing ifconfig output too much, the following
association failure reasons are not displayed:
- wrong SSID
- open vs. encrypted network
(They remain available at the ioctl layer which could be useful for
third-party wifi tools in the ports tree.)
I have never heard from people having problems figuring out the above
two failure cases so I don't think it's worth adding them to virtually
every line of 'ifconfig scan' output.
However, the following cases are causing regular cries for help:
- wrong WPA protocol version (number 1 reason for cries for help)
- wrong WPA key
- user forgets about previously hard-coded channel
- user forgets about previously hard-coded BSSID
And, of course, any of these problems can occur in combination.
With the diff below, ifconfig output in such situations looks as follows:
# ifconfig iwm0 nwid stsp.name wpakey wrong-key
Now ifconfig iwm0 scan indicates wrong WPA key after a failed WPA handshake:
nwid stsp.name chan 52 bssid 28:af:44:01:4f:38 71% HT-MCS15
privacy,short_slottime,wpa2 !(wpakey)
(Configure AP to run WPA1 only.)
# ifconfig iwm0 nwid stsp.name wpakey correct-key
ifconfig iwm0 scan indicates wrong WPA protocol version:
nwid stsp.name chan 52 bssid 28:af:44:01:4f:38 52% HT-MCS15
privacy,short_slottime,wpa1 !(wpaproto)
(user hard-codes the channel)
# ifconfig iwm0 chan 36
Now ifconfig iwm0 scan indicates WPA1-only AP on wrong channel:
nwid stsp.name chan 52 bssid 28:af:44:01:4f:38 79% HT-MCS15
privacy,short_slottime,wpa1 !(chan,wpaproto)
(user hard-codes BSSID)
WPA1-only AP on wrong channel with wrong BSSID:
# ifconfig iwm0 bssid 00:1c:c2:08:c6:65
now ifconfig iwm0 scan indicates WPA1-only AP, wrong channel, and wrong BSSID:
nwid stsp.name chan 52 bssid 28:af:44:01:4f:38 56% HT-MCS15
privacy,short_slottime,wpa1 !(chan,bssid,wpaproto)
Is this going in the right direction?
There are no man page changes yet but the various reason failure codes
would have to be properly documented, of course.
To test the diff below, recompile the patched kernel, install the patched
ieee80211_ioctl.h header (e.g. via 'make includes'), and recompile ifconfig.
If ifconfig displays garbage with this patch applied, your kernel and
ifconfig binary are out of sync.
diff refs/heads/master refs/heads/assocfail
blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
blob + 03e4818ef5fa11d7e5dbf8307affb9f0ea5522a0
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -2348,10 +2348,27 @@ print_cipherset(u_int32_t cipherset)
}
void
+print_assoc_failures(uint32_t assoc_fail, const char *intro, const char *outro)
+{
+ /* Filter out the most obvious failure cases. */
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
+
+ if (assoc_fail == 0)
+ return;
+
+ printf("%s", intro);
+ printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS);
+ printf("%s", outro);
+}
+
+void
ieee80211_status(void)
{
int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
- int ibssid, iwpa;
+ int ibssid, iwpa, assocfail = 0;
struct ieee80211_nwid nwid;
struct ieee80211_join join;
struct ieee80211_nwkey nwkey;
@@ -2431,11 +2448,15 @@ ieee80211_status(void)
bzero(, sizeof(nr));
bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
- if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
- if (nr.nr_max_rssi)
- printf(" %u%%", IEEE80211_NODEREQ_RSSI());
- else
- printf(" %ddBm", nr.nr_rssi);
+ if (ioctl(s, SIOCG80211NODE, ) == 0) {
+ if (nr.nr_rssi) {
+ if (nr.nr_max_rssi)
+ printf(" %u%%",
+ IEEE80211_NODEREQ_RSSI());
+ else
+ printf(" %ddBm", nr.nr_rssi);
+ }
+ assocfail = nr.nr_assoc_fail;
Re: flex {c,m}alloc() checks
On Wed, Aug 28, 2019 at 10:07:32AM +0800, Michael Mikonos wrote: > On Sun, Aug 25, 2019 at 02:58:47PM +0200, Otto Moerbeek wrote: > > On Sun, Aug 25, 2019 at 08:32:04PM +0800, Michael Mikonos wrote: > > > > > Hello, > > > > > > I noticed that flex is too trusting and assumes > > > calloc/malloc will always succeed. Hopefully I > > > caught all of them. > > > I tried to follow the existing idiom of > > > calling flexerror() and passing strings via > > > the _() macro. OK? > > > > Does upstream have anything like this? You could consider using the > > xmalloc idiom (i.e. have separate functions that do the checks). > > Upstream has the _() macro and also calls flexerror() on allocation > failure. To me it is also nicer adding an xmalloc/xcalloc. > That would be a bigger patch though as the calls currently > checking malloc/calloc return value get modified too. > > - Michael > I'd say go for the x* solution, -Otto
Re: smtpd filters: banner hostname
On 8/28/19 12:26 PM, Gilles Chehade wrote:
> On Wed, Aug 28, 2019 at 10:33:29AM +0200, Martijn van Duren wrote:
>> On 8/28/19 9:23 AM, gil...@poolp.org wrote:
>>> 28 ao??t 2019 09:04 "Martijn van Duren" a
>>> ??crit:
>>>
Currently looking into writing an spf filter based on libopensmtpd.
While working through the spec I found in RFC7208 section 7.3 that:
The "r" macro expands to the name of the receiving MTA.
In other words the hostname presented in the banner. Unfortunately we
also support the hostnames directive, which supports ip-hostname
mappings via dynamic tables, which makes it impossible to transfer via
"config|".
This is a major change that can break (and in the case of libopensmtpd
will break) parsers. We're currently at 0.1, so I don't know if we want
push it to 1 just yet, or if we want to call 1 release-stable and just
bump it to 0.2 for now since we don't have a release yet with filters.
thoughts?
>>>
>>> I'm sorry but I'm unsure I understand what you're trying to do with the
>>> banner,
>>> can you explain ?
>>>
>>> If there's need for the hostname presented in the banner to be passed to
>>> filters,
>>> which makes sense, it needs its own reporting event which is basically the
>>> server
>>> side of the link-identify event.
>>>
>>> One thing for sure, you can't put that info in the link-connect event
>>> because the
>>> banner is displayed _after_ link-connect and while in smtp-in we already
>>> know the
>>> hostname we'll use in the banner, this isn't the case for smtp-out which
>>> will not
>>> be able to infer that information before actually seeing the banner.
>>>
>> So the diff below implements report|link-greeting
>>
>> I haven't implemented the smtp-out case, since none of the smtp-out
>> cases appear to be currently implemented.
>>
>> Does this read better?
>>
>
> No sorry, I'm very confused :-)
>
> Let me explain my understanding just to make sure we're talking about
> the same thing.
>
> You said:
>
>> spf has an "exp" modifier, which allows the reject message to be
>> specified by the spf administrator. To do so it can utilize several
>> macros. One option being "%{r}", which expands to:
>> domain name of host performing the check.
>
>
> So my understanding is that you'd like a filter to be able to use the
> result from an SPF lookup to reject a message in your filter and have
> the hostname that was used in the banner part of the message.
Yes, because that's the only point in the transaction where I can be
100% certain on which hostname the transaction took place. This is
because of listen on * hostnames , where table can change at
runtime based on its backend.
>
> If that's so, I think the diff below is both not enough and a bit too
> specific, preventing filters with different use-cases than yours from
> using the information as easily:
>
> 1- first, i think your report event is too specific, keep in mind the
>report events are used to build the internal state of the session,
>so that you can basically duplicate struct smtp_session from smtpd
>in a filter. the textstring part here is not really related to the
>state of the session more than it is related to your use-case.
It's not related to my use-case. I included it for completeness.
E.g. if someone wants to monitor what mailservers are being connected to
in case of when we implement the smtp-out case.
Since there's no uniform way to distil this information (it's freetext
after all) I thought I'd return the textstring as specified by RFC5321.
If you see harm in having the textstring part in the filters I don't
mind dropping them, but I expect at some point people will ask for it.
>
> 2- then, your use case is to allow rejecting with a custom message so
>this can't be done with report events, which are informative, this
>requires a filter event which allow taking decisions (like reject,
>in this case). So I think that what's missing here is a phase that
>allows filters to take a decision before printing the banner and I
>think THIS is where your textstring goes, the filter receives that
>report event with the hostname, it receives the filter request for
>the banner and it can decide to write a custom banner.
No, SPF filters at HELO/EHLO and MAIL FROM. If a host is rejected
(because of a -all match) and the spf-record contains an "exp" (explain)
entry it resolves fetches the txt record of where "exp" points to.
Said record is then being evaluated by domain-spec and allows for the
expansion of "%{r}", which evaluates to the domainname of the host
doing the spf-checking.
Say I would mail from openbsd+t...@imperialat.at to gil...@poolp.org and
changed the txt imperialat.at to "v=spf1 mx -all exp:exp.imperialat.at"
and txt exp.imperialat.at would be: "Message rejected by %{r}"
If mx2.pool.org would reject the message because the sending ip is not
listed in the spf imperialat.at, the resulting
Re: display reasons for wifi association failures in ifconfig
Martin Pieuchot wrote: > Is there any value to keep the IFF_DEBUG output if ifconfig(8) already > report the failures? Yes. "debug" gives the play-by-play in dmesg, which is very different from the limited information this new mechanism provides.
Re: flex {c,m}alloc() checks
> I'd say go for the x* solution,
>
> -Otto
Sure. When I looked at this again there are also realloc() return
value checks missing, so I added xcalloc(), xmalloc() and xrealloc().
An old (unused) function yy_flex_xmalloc() gets removed. When building
this I checked the resulting .o files using nm(1) to make sure I didn't
miss any direct calls to calloc/malloc/realloc. However, scan.o and
parse.o were left alone. Does this look better?
Index: flexdef.h
===
RCS file: /cvs/src/usr.bin/lex/flexdef.h,v
retrieving revision 1.15
diff -u -r1.15 flexdef.h
--- flexdef.h 19 Nov 2015 23:48:06 - 1.15
+++ flexdef.h 28 Aug 2019 13:23:15 -
@@ -920,8 +920,9 @@
/* Output a yy_trans_info structure. */
extern void transition_struct_out PROTO ((int, int));
-/* Only needed when using certain broken versions of bison to build parse.c. */
-extern void *yy_flex_xmalloc PROTO ((int));
+extern void *xmalloc PROTO ((size_t));
+extern void *xcalloc PROTO ((size_t, size_t));
+extern void *xrealloc PROTO ((void *, size_t));
/* from file nfa.c */
Index: buf.c
===
RCS file: /cvs/src/usr.bin/lex/buf.c,v
retrieving revision 1.7
diff -u -r1.7 buf.c
--- buf.c 20 Nov 2015 18:54:49 - 1.7
+++ buf.c 28 Aug 2019 13:23:15 -
@@ -79,9 +79,7 @@
size_t tsz;
tsz = strlen(fmt) + strlen(s) + 1;
- t = malloc(tsz);
- if (!t)
- flexfatal(_("Allocation of buffer to print string failed"));
+ t = xmalloc(tsz);
snprintf(t, tsz, fmt, s);
buf = buf_strappend(buf, t);
free(t);
@@ -105,9 +103,7 @@
2 * strlen(filename) + /* filename with possibly all
backslashes escaped */
(int) (1 + log10(abs(lineno))) +/* line number */
1; /* NUL */
- t = malloc(tsz);
- if (!t)
- flexfatal(_("Allocation of buffer for line directive failed"));
+ t = xmalloc(tsz);
dst = t + snprintf(t, tsz, "#line %d \"", lineno);
for (src = filename; *src; *dst++ = *src++)
if (*src == '\\') /* escape backslashes */
@@ -181,10 +177,7 @@
val = val ? val : "";
strsz = strlen(fmt) + strlen(def) + strlen(val) + 2;
- str = malloc(strsz);
- if (!str)
- flexfatal(_("Allocation of buffer for m4 def failed"));
-
+ str = xmalloc(strsz);
snprintf(str, strsz, fmt, def, val);
buf_append(buf, , 1);
return buf;
@@ -203,10 +196,7 @@
size_t strsz;
strsz = strlen(fmt) + strlen(def) + 2;
- str = malloc(strsz);
- if (!str)
- flexfatal(_("Allocation of buffer for m4 undef failed"));
-
+ str = xmalloc(strsz);
snprintf(str, strsz, fmt, def);
buf_append(buf, , 1);
return buf;
Index: dfa.c
===
RCS file: /cvs/src/usr.bin/lex/dfa.c,v
retrieving revision 1.8
diff -u -r1.8 dfa.c
--- dfa.c 19 Nov 2015 23:20:34 - 1.8
+++ dfa.c 28 Aug 2019 13:23:15 -
@@ -523,15 +523,12 @@
* So we'll have to realloc() on the way...
* we'll wait until we can calculate yynxt_tbl->td_hilen.
*/
- yynxt_tbl =
- (struct yytbl_data *) calloc (1,
- sizeof (struct
- yytbl_data));
+ yynxt_tbl = xcalloc(1, sizeof(struct yytbl_data));
yytbl_data_init (yynxt_tbl, YYTD_ID_NXT);
yynxt_tbl->td_hilen = 1;
yynxt_tbl->td_lolen = num_full_table_rows;
yynxt_tbl->td_data = yynxt_data =
- (flex_int32_t *) calloc (yynxt_tbl->td_lolen *
+ xcalloc(yynxt_tbl->td_lolen *
yynxt_tbl->td_hilen,
sizeof (flex_int32_t));
yynxt_curr = 0;
@@ -715,7 +712,7 @@
/* Each time we hit here, it's another td_hilen, so we
realloc. */
yynxt_tbl->td_hilen++;
yynxt_tbl->td_data = yynxt_data =
- (flex_int32_t *) realloc (yynxt_data,
+ xrealloc(yynxt_data,
yynxt_tbl->td_hilen *
yynxt_tbl->td_lolen *
sizeof (flex_int32_t));
Index: filter.c
===
RCS file: /cvs/src/usr.bin/lex/filter.c,v
retrieving revision 1.9
diff -u -r1.9 filter.c
--- filter.c30 Aug 2017 02:54:07 - 1.9
+++ filter.c28 Aug 2019
Re: display reasons for wifi association failures in ifconfig
On Wed, Aug 28, 2019 at 10:39:24AM -0300, Martin Pieuchot wrote:
> On 28/08/19(Wed) 12:54, Stefan Sperling wrote:
> > +print_assoc_failures(uint32_t assoc_fail, const char *intro, const char
> > *outro)
> > +{
> > + /* Filter out the most obvious failure cases. */
> > + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
> > + if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
> > + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
> > + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
>
> It's not clear to me why you suggest we should filter out these failure
> cases.
Because these failures happen all the time. Annotating almost every
line with !essid and !privacy whenever you run 'scan' is not useful.
Try the diff without that chunk and you'll see what I mean.
> Is there any value to keep the IFF_DEBUG output if ifconfig(8) already
> report the failures?
I think so. The debug output provides a snapshot of what the stack
was seeing at that particular point in time, while ifconfig provides
an async view on what was cached when it ran an ioctl.
> > + /* Keep recorded association failures for this BSS/ESS intact. */
> > + if (IEEE80211_ADDR_EQ(ic->ic_bss->ni_macaddr, selbs->ni_macaddr) ||
> > + (ic->ic_des_esslen > 0 && ic->ic_des_esslen == selbs->ni_esslen &&
> > + memcmp(ic->ic_des_essid, selbs->ni_essid, selbs->ni_esslen) == 0))
> > + assoc_fail = ic->ic_bss->ni_assoc_fail;
> > +
> > (*ic->ic_node_copy)(ic, ic->ic_bss, selbs);
> > ni = ic->ic_bss;
> > + ni->ni_assoc_fail |= assoc_fail;
>
> I couldn't convince myself why two divers are overwriting `ic_node_copy',
It looks like this override is required for ieee80211_ibss_merge().
The generic copy function does not copy driver-private state information,
such as the current Tx rate in case of ath(4) which is maintained while
the driver is in RUN state.
An IBSS merge may be necessary whenever a beacon from another device in the
same IBSS is received, which could happen in RUN state. See ath_recv_mgmt().
The non-IBSS modes don't call node_copy() while the driver is in RUN state
so it's a non-issue in those modes. Most drivers don't support IBSS in the
first place, and most people never use IBSS mode. But it's worth keeping
IBSS mode since makes it possible to use OpenBSD in wireless mesh networks,
e.g. in combination with the net/olsrd port.
> but shouldn't this logic be part of ieee80211_node_copy() instead? It is
> just to work around the copy, right?
With the ic_node_copy() abstraction in place, it makes more sense to
keep this code where I have added it, I think.
> > + ic->ic_curmode = ieee80211_chan2mode(ic, ni->ni_chan);
> >
> > /* Make sure we send valid rates in an association request. */
> > if (ic->ic_opmode == IEEE80211_M_STA)
> > blob - b2b80656a4b4aa907b4d729ecaecb81827739c52
> > blob + ddd746729a48b15fa3c21d85390682263d2934d9
> > --- sys/net80211/ieee80211_node.h
> > +++ sys/net80211/ieee80211_node.h
> > @@ -353,6 +353,16 @@ struct ieee80211_node {
> > u_int16_t ni_qos_txseqs[IEEE80211_NUM_TID];
> > u_int16_t ni_qos_rxseqs[IEEE80211_NUM_TID];
> > int ni_fails; /* failure count to associate */
> > + uint32_tni_assoc_fail; /* assoc failure reasons */
> > +#define IEEE80211_NODE_ASSOCFAIL_CHAN 0x01
> > +#define IEEE80211_NODE_ASSOCFAIL_IBSS 0x02
> > +#define IEEE80211_NODE_ASSOCFAIL_PRIVACY 0x04
> > +#define IEEE80211_NODE_ASSOCFAIL_BASIC_RATE0x08
> > +#define IEEE80211_NODE_ASSOCFAIL_ESSID 0x10
> > +#define IEEE80211_NODE_ASSOCFAIL_BSSID 0x20
> > +#define IEEE80211_NODE_ASSOCFAIL_WPA_PROTO 0x40
> > +#define IEEE80211_NODE_ASSOCFAIL_WPA_KEY 0x80
> > +
> > int ni_inact; /* inactivity mark count */
> > int ni_txrate; /* index to ni_rates[] */
> > int ni_state;
> > blob - ba8c1872a2df596a0667ecb7c306f7e7f9dd0ffc
> > blob + daf43cd6457134728ce5dd485229036ea4822445
> > --- sys/net80211/ieee80211_pae_input.c
> > +++ sys/net80211/ieee80211_pae_input.c
> > @@ -650,6 +650,7 @@ ieee80211_recv_4way_msg3(struct ieee80211com *ic,
> > ether_sprintf(ni->ni_macaddr)));
> > ni->ni_port_valid = 1;
> > ieee80211_set_link_state(ic, LINK_STATE_UP);
> > + ni->ni_assoc_fail = 0;
>
> There are other places where `ni_port_valid' is set to 1 and another
> one where the link is set to UP where the flags aren't cleared.
Thanks, there were two missing cases in ieee80211_pae_input.c which matter.
Updated diff below.
The others are only relevant for hostap mode, which I am deliberately
ignoring for the purposes of this diff.
> Another question, could these flags be used in AP mode? To report
> client errors?
In theory yes, but AP mode uses entirely different code paths and
should
Re: display reasons for wifi association failures in ifconfig
On Wed, Aug 28, 2019 at 04:18:41PM +0200, Stefan Sperling wrote:
> Updated diff below.
And this is another update which changes display of failure codes
from "!(foo.bar)" to "!foo,!bar", as suggested off-list by Theo.
This should make the output easier to parse.
diff refs/heads/master refs/heads/assocfail
blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
blob + f41377b2823392dbd0b26894517b13a26ab31206
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -2348,10 +2348,58 @@ print_cipherset(u_int32_t cipherset)
}
void
+print_assoc_failures(uint32_t assoc_fail)
+{
+ const char *comma = "";
+
+ /* Filter out the most obvious failure cases. */
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
+
+ if (assoc_fail == 0)
+ return;
+
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_CHAN) {
+ printf("%s!chan", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_IBSS) {
+ printf("%s!ibss", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY) {
+ printf("%s!privacy", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_BASIC_RATE) {
+ printf("%s!basicrate", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_ESSID) {
+ printf("%s!essid", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_BSSID) {
+ printf("%s!bssid", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO) {
+ printf("%s!wpaproto", comma);
+ comma = ",";
+ }
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_WPA_KEY) {
+ printf("%s!wpakey", comma);
+ comma = ",";
+ }
+}
+
+void
ieee80211_status(void)
{
int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
- int ibssid, iwpa;
+ int ibssid, iwpa, assocfail = 0;
struct ieee80211_nwid nwid;
struct ieee80211_join join;
struct ieee80211_nwkey nwkey;
@@ -2431,11 +2479,15 @@ ieee80211_status(void)
bzero(, sizeof(nr));
bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
- if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
- if (nr.nr_max_rssi)
- printf(" %u%%", IEEE80211_NODEREQ_RSSI());
- else
- printf(" %ddBm", nr.nr_rssi);
+ if (ioctl(s, SIOCG80211NODE, ) == 0) {
+ if (nr.nr_rssi) {
+ if (nr.nr_max_rssi)
+ printf(" %u%%",
+ IEEE80211_NODEREQ_RSSI());
+ else
+ printf(" %ddBm", nr.nr_rssi);
+ }
+ assocfail = nr.nr_assoc_fail;
}
}
@@ -2478,6 +2530,11 @@ ieee80211_status(void)
putchar(' ');
printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
}
+
+ if (assocfail) {
+ putchar(' ');
+ print_assoc_failures(assocfail);
+ }
putchar('\n');
if (show_join)
join_status();
@@ -2751,6 +2808,8 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
IEEE80211_NODEREQ_STATE_BITS);
+ else if (nr->nr_assoc_fail)
+ print_assoc_failures(nr->nr_assoc_fail);
}
void
blob - 425ee5f5a726e3fe0445c82debb0469d4878407f
blob + 6b3b8a5834db39d5b2f2dc7693f42d0b5adbcfbb
--- sys/net80211/ieee80211_ioctl.c
+++ sys/net80211/ieee80211_ioctl.c
@@ -104,6 +104,7 @@ ieee80211_node2req(struct ieee80211com *ic, const stru
nr->nr_txseq = ni->ni_txseq;
nr->nr_rxseq = ni->ni_rxseq;
nr->nr_fails = ni->ni_fails;
+ nr->nr_assoc_fail = ni->ni_assoc_fail; /* flag values are the same */
nr->nr_inact = ni->ni_inact;
nr->nr_txrate = ni->ni_txrate;
nr->nr_state = ni->ni_state;
@@ -821,7 +822,11 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
break;
case SIOCG80211NODE:
nr = (struct ieee80211_nodereq *)data;
- ni = ieee80211_find_node(ic, nr->nr_macaddr);
+ if (ic->ic_bss &&
+ IEEE80211_ADDR_EQ(nr->nr_macaddr,
Re: display reasons for wifi association failures in ifconfig
On Wed, Aug 28, 2019 at 04:52:10PM +0200, Stefan Sperling wrote:
> On Wed, Aug 28, 2019 at 04:18:41PM +0200, Stefan Sperling wrote:
> > Updated diff below.
>
> And this is another update which changes display of failure codes
> from "!(foo.bar)" to "!foo,!bar", as suggested off-list by Theo.
>
> This should make the output easier to parse.
Now with less new lines added by making printb_status() more flexible.
diff refs/heads/master refs/heads/assocfail
blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
blob + 944d782715e3cc70330ab5fa4441eed8e1a051f9
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -641,7 +641,7 @@ int getinfo(struct ifreq *, int);
void getsock(int);
void printgroupattribs(char *);
void printif(char *, int);
-void printb_status(unsigned short, unsigned char *);
+void printb_status(unsigned short, unsigned char *, const char *);
const char *get_linkstate(int, int);
void status(int, struct sockaddr_dl *, int);
__dead voidusage(void);
@@ -2348,10 +2348,25 @@ print_cipherset(u_int32_t cipherset)
}
void
+print_assoc_failures(uint32_t assoc_fail)
+{
+ /* Filter out the most obvious failure cases. */
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
+
+ if (assoc_fail == 0)
+ return;
+
+ printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS, "!");
+}
+
+void
ieee80211_status(void)
{
int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
- int ibssid, iwpa;
+ int ibssid, iwpa, assocfail = 0;
struct ieee80211_nwid nwid;
struct ieee80211_join join;
struct ieee80211_nwkey nwkey;
@@ -2431,11 +2446,15 @@ ieee80211_status(void)
bzero(, sizeof(nr));
bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
- if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
- if (nr.nr_max_rssi)
- printf(" %u%%", IEEE80211_NODEREQ_RSSI());
- else
- printf(" %ddBm", nr.nr_rssi);
+ if (ioctl(s, SIOCG80211NODE, ) == 0) {
+ if (nr.nr_rssi) {
+ if (nr.nr_max_rssi)
+ printf(" %u%%",
+ IEEE80211_NODEREQ_RSSI());
+ else
+ printf(" %ddBm", nr.nr_rssi);
+ }
+ assocfail = nr.nr_assoc_fail;
}
}
@@ -2476,8 +2495,13 @@ ieee80211_status(void)
if (ioctl(s, SIOCG80211FLAGS, (caddr_t)) == 0 &&
ifr.ifr_flags) {
putchar(' ');
- printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
+ printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS, NULL);
}
+
+ if (assocfail) {
+ putchar(' ');
+ print_assoc_failures(assocfail);
+ }
putchar('\n');
if (show_join)
join_status();
@@ -2731,7 +2755,7 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
/* ESS is the default, skip it */
nr->nr_capinfo &= ~IEEE80211_CAPINFO_ESS;
if (nr->nr_capinfo) {
- printb_status(nr->nr_capinfo, IEEE80211_CAPINFO_BITS);
+ printb_status(nr->nr_capinfo, IEEE80211_CAPINFO_BITS, NULL);
if (nr->nr_capinfo & IEEE80211_CAPINFO_PRIVACY) {
if (nr->nr_rsnprotos) {
if (nr->nr_rsnprotos & IEEE80211_WPA_PROTO_WPA2)
@@ -2750,7 +2774,9 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
- IEEE80211_NODEREQ_STATE_BITS);
+ IEEE80211_NODEREQ_STATE_BITS, NULL);
+ else if (nr->nr_assoc_fail)
+ print_assoc_failures(nr->nr_assoc_fail);
}
void
@@ -4525,7 +4551,7 @@ trunk_status(void)
printf("\t\t%s lacp actor state ",
rpbuf[i].rp_portname);
printb_status(lp->actor_state,
- LACP_STATE_BITS);
+ LACP_STATE_BITS, NULL);
putchar('\n');
printf("\t\t%s lacp partner "
@@ -4540,12 +4566,12 @@ trunk_status(void)
printf("\t\t%s lacp partner state ",
rpbuf[i].rp_portname);
printb_status(lp->partner_state,
-
Re: display reasons for wifi association failures in ifconfig
On 28/08/19(Wed) 12:54, Stefan Sperling wrote:
> I find myself regularly asking people to run 'ifconfig iwm0 debug' to
> figure out reasons behind association failures from dmesg output.
>
> I would like to make this information more accessible. Making it part
> of regular ifconfig output seems like a reasonable thing to do. This
> becomes possible now that node information is cached across scans.
>
> The current net80211 code is already computing failure reason codes which
> are visible in 'ifconfig debug' output. So far such codes were magic numbers.
> We can simply create macros for them and expose them in ieee80211_ioctl.h,
> and add another code to represent the "wrong-WPA-key" situation.
>
> To avoid changing the existing ifconfig output too much, the following
> association failure reasons are not displayed:
> [...]
> Is this going in the right direction?
I love it. Comments below
> diff refs/heads/master refs/heads/assocfail
> blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
> blob + 03e4818ef5fa11d7e5dbf8307affb9f0ea5522a0
> --- sbin/ifconfig/ifconfig.c
> +++ sbin/ifconfig/ifconfig.c
> @@ -2348,10 +2348,27 @@ print_cipherset(u_int32_t cipherset)
> }
>
> void
> +print_assoc_failures(uint32_t assoc_fail, const char *intro, const char
> *outro)
> +{
> + /* Filter out the most obvious failure cases. */
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
> + if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
> + assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
It's not clear to me why you suggest we should filter out these failure
cases.
> +
> + if (assoc_fail == 0)
> + return;
> +
> + printf("%s", intro);
> + printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS);
> + printf("%s", outro);
> +}
> +
> +void
> ieee80211_status(void)
> {
> int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
> - int ibssid, iwpa;
> + int ibssid, iwpa, assocfail = 0;
> struct ieee80211_nwid nwid;
> struct ieee80211_join join;
> struct ieee80211_nwkey nwkey;
> @@ -2431,11 +2448,15 @@ ieee80211_status(void)
> bzero(, sizeof(nr));
> bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
> strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
> - if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
> - if (nr.nr_max_rssi)
> - printf(" %u%%", IEEE80211_NODEREQ_RSSI());
> - else
> - printf(" %ddBm", nr.nr_rssi);
> + if (ioctl(s, SIOCG80211NODE, ) == 0) {
> + if (nr.nr_rssi) {
> + if (nr.nr_max_rssi)
> + printf(" %u%%",
> + IEEE80211_NODEREQ_RSSI());
> + else
> + printf(" %ddBm", nr.nr_rssi);
> + }
> + assocfail = nr.nr_assoc_fail;
> }
> }
>
> @@ -2478,6 +2499,10 @@ ieee80211_status(void)
> putchar(' ');
> printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
> }
> +
> + if (assocfail)
> + print_assoc_failures(assocfail, " !(", ")");
> +
> putchar('\n');
> if (show_join)
> join_status();
> @@ -2751,6 +2776,8 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
> if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
> printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
> IEEE80211_NODEREQ_STATE_BITS);
> + else if (nr->nr_assoc_fail)
> + print_assoc_failures(nr->nr_assoc_fail, "!(", ") ");
> }
>
> void
> blob - 425ee5f5a726e3fe0445c82debb0469d4878407f
> blob + 6b3b8a5834db39d5b2f2dc7693f42d0b5adbcfbb
> --- sys/net80211/ieee80211_ioctl.c
> +++ sys/net80211/ieee80211_ioctl.c
> @@ -104,6 +104,7 @@ ieee80211_node2req(struct ieee80211com *ic, const stru
> nr->nr_txseq = ni->ni_txseq;
> nr->nr_rxseq = ni->ni_rxseq;
> nr->nr_fails = ni->ni_fails;
> + nr->nr_assoc_fail = ni->ni_assoc_fail; /* flag values are the same */
> nr->nr_inact = ni->ni_inact;
> nr->nr_txrate = ni->ni_txrate;
> nr->nr_state = ni->ni_state;
> @@ -821,7 +822,11 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
> break;
> case SIOCG80211NODE:
> nr = (struct ieee80211_nodereq *)data;
> - ni = ieee80211_find_node(ic, nr->nr_macaddr);
> + if (ic->ic_bss &&
> + IEEE80211_ADDR_EQ(nr->nr_macaddr, ic->ic_bss->ni_macaddr))
> + ni = ic->ic_bss;
> + else
> + ni = ieee80211_find_node(ic, nr->nr_macaddr);
> if (ni == NULL) {
> error = ENOENT;
> break;
> blob
Re: [PATCH] Avoid leftover temporary mount points when using -P (mfs)
On Sat, Aug 17, 2019 at 12:13:50PM -0300, Rafael Neves wrote: > Hi, > > Submitting to tech@ to broader audience. > > When using -P option in mfs with a directory or a block device that > doen't exist, for example when the device roams, newfs(2) leaves > leftovers of temporary mount points. > > With my /etc/fstab: > ca7552589896b01e.b none swap sw > ca7552589896b01e.a / ffs rw 1 1 > ca7552589896b01e.k /home ffs rw,nodev,nosuid 1 2 > #ca7552589896b01e.d /tmp ffs rw,nodev,nosuid 1 2 > swap /tmp mfs rw,nodev,nosuid,-s=300M 0 0 > ca7552589896b01e.f /usr ffs rw,nodev 1 2 > ca7552589896b01e.g /usr/X11R6 ffs rw,nodev 1 2 > ca7552589896b01e.h /usr/local ffs rw,nodev,wxallowed 1 2 > ca7552589896b01e.j /usr/build ffs rw,noperm,noauto 1 2 > swap /usr/obj mfs rw,nodev,nosuid,noauto,-s=4G,-P=/foo/bar 0 0 > ca7552589896b01e.i /usr/src ffs rw,nodev,nosuid 1 2 > ca7552589896b01e.e /var ffs rw,nodev,nosuid 1 2 > > Result when trying to mount /usr/obj: > root@orus [rneves]# mount /usr/obj > mount_mfs: cannot stat /foo/bar: No such file or directory > root@orus [rneves]# mount > /dev/sd2a on / type ffs (local) > /dev/sd2k on /home type ffs (local, nodev, nosuid) > mfs:28249 on /tmp type mfs (asynchronous, local, nodev, nosuid, > size=614400 512-blocks) > /dev/sd2f on /usr type ffs (local, nodev) > /dev/sd2g on /usr/X11R6 type ffs (local, nodev) > /dev/sd2h on /usr/local type ffs (local, nodev, wxallowed) > /dev/sd2i on /usr/src type ffs (local, nodev, nosuid) > /dev/sd2e on /var type ffs (local, nodev, nosuid) > mfs:44634 on /tmp/mntoMG6WmZTT7 type mfs (asynchronous, local, nodev, > nosuid, size=8388608 512-blocks) > > > Tracking down the issue I found that: > + When -P is specified, pop != NULL. > + After fork, waitformount() is called. It creates the temporary > places to store the data. > + copy() is called, and it it fails the following umount() and > rmdir() is not called, leaving the temporary mounts. > > As copy() can fail in a couple of ways, I thought about the following change: > + Make all errors a warning, but after then return to the first > caller indicating an error. Getting the erro the clean up is > done, and exit(1). > > + Make copy() return an int: -1 in fail, and 0 otherwise. > + isdir(), gettmpmnt(): Convert errors to warnings + return(-1). > > There is a change of messages printing if you don't have a /tmp > is read-only, first the error of cannot fstat, and after > "Cannot create tmp mountpoint for -P". > > There still a chance to get a inconsistent state: if there is no > /bin/pax, or errors in do_exec(). But erros in do_exec seem very > critical, the same with a missing /bin/pax. So I didn't change them. > > Otto@ said that another alternative is using atexit(3), but we > pointed out what it is very difficult to get it right, and almost > always has race conditions. Given that and what manpage says I have no > hope that I can get it right. > > What do you think? > > Note that the check in line 519 (newfs.c) was changed to add the new > possible return value. Actually, currently it is not allowed -P with a > read-only /tmp, because in this case gettmpmnt() returns 0, and by this > early check newfs(2) throws an error. Actually, gettmpmnt() must changed > to it work properly. The `created` if uses a <= by symmetry. > > But this is a differente issue, that I think could be changed in a > separated diff. > > Regards, > Rafael Neves > > > Patch: > > > Index: newfs.c > === > RCS file: /cvs/src/sbin/newfs/newfs.c,v > retrieving revision 1.112 > diff -u -p -r1.112 newfs.c > --- newfs.c 28 Jun 2019 13:32:45 - 1.112 > +++ newfs.c 17 Aug 2019 14:27:46 - > @@ -147,7 +147,7 @@ struct disklabel *getdisklabel(char *, i > static void waitformount(char *, pid_t); > static int do_exec(const char *, const char *, char *const[]); > static int isdir(const char *); > -static void copy(char *, char *); > +static int copy(char *, char *); > static int gettmpmnt(char *, size_t); > #endif > > @@ -179,6 +179,7 @@ main(int argc, char *argv[]) > #ifdef MFS > char mountfromname[BUFSIZ]; > char *pop = NULL, node[PATH_MAX]; > + int ret; Please move this one inside the scope where it is used, see below > pid_t pid; > struct stat mountpoint; > #endif > @@ -516,7 +517,7 @@ havelabel: > struct mfs_args args; > char tmpnode[PATH_MAX]; here > > - if (pop != NULL && gettmpmnt(tmpnode, sizeof(tmpnode)) == 0) > + if (pop != NULL && gettmpmnt(tmpnode, sizeof(tmpnode)) <= 0) > errx(1, "Cannot create tmp mountpoint for -P"); > memset(, 0, sizeof(args)); > args.base = membase; > @@
unveils in ping and traceroute
ping and traceroute are setuid programs, so increased access-reduction
features are worthwhile.
they can both lock their filesystem visibility to "readonly" very early on.
the attack model being prevented against is very obscure. it imagines a
bug in something between start-of-program and call-to-pledge (which
entirely removes filesystem access). implying a getaddrinfo related
bug. meanwhile, there is privdrop as another protection.
these still feel like improvements.
Index: usr.sbin/traceroute/traceroute.c
===
RCS file: /cvs/src/usr.sbin/traceroute/traceroute.c,v
retrieving revision 1.161
diff -u -p -u -r1.161 traceroute.c
--- usr.sbin/traceroute/traceroute.c28 Jun 2019 13:32:51 - 1.161
+++ usr.sbin/traceroute/traceroute.c27 Aug 2019 17:56:56 -
@@ -327,6 +327,12 @@ main(int argc, char *argv[])
uid_touid, uid;
gid_tgid;
+ /* Cannot pledge due to special setsockopt()s below */
+ if (unveil("/", "r") == -1)
+ err(1, "unveil");
+ if (unveil(NULL, NULL) == -1)
+ err(1, "unveil");
+
if ((conf = calloc(1, sizeof(*conf))) == NULL)
err(1,NULL);
Index: sbin/ping/ping.c
===
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.237
diff -u -p -u -r1.237 ping.c
--- sbin/ping/ping.c20 Jul 2019 00:49:54 - 1.237
+++ sbin/ping/ping.c27 Aug 2019 17:56:17 -
@@ -264,6 +264,12 @@ main(int argc, char *argv[])
u_int rtableid = 0;
extern char *__progname;
+ /* Cannot pledge due to special setsockopt()s below */
+ if (unveil("/", "r") == -1)
+ err(1, "unveil");
+ if (unveil(NULL, NULL) == -1)
+ err(1, "unveil");
+
if (strcmp("ping6", __progname) == 0) {
v6flag = 1;
maxpayload = MAXPAYLOAD6;
Re: unveils in ping and traceroute
On Wed, Aug 28, 2019 at 12:03:07PM -0600, Theo de Raadt wrote:
> ping and traceroute are setuid programs, so increased access-reduction
> features are worthwhile.
>
> they can both lock their filesystem visibility to "readonly" very early on.
>
> the attack model being prevented against is very obscure. it imagines a
> bug in something between start-of-program and call-to-pledge (which
> entirely removes filesystem access). implying a getaddrinfo related
> bug. meanwhile, there is privdrop as another protection.
>
> these still feel like improvements.
I think so too. Restricting filesystem access early here only helps.
OK brynet@
> Index: usr.sbin/traceroute/traceroute.c
> ===
> RCS file: /cvs/src/usr.sbin/traceroute/traceroute.c,v
> retrieving revision 1.161
> diff -u -p -u -r1.161 traceroute.c
> --- usr.sbin/traceroute/traceroute.c 28 Jun 2019 13:32:51 - 1.161
> +++ usr.sbin/traceroute/traceroute.c 27 Aug 2019 17:56:56 -
> @@ -327,6 +327,12 @@ main(int argc, char *argv[])
> uid_touid, uid;
> gid_tgid;
>
> + /* Cannot pledge due to special setsockopt()s below */
> + if (unveil("/", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
> if ((conf = calloc(1, sizeof(*conf))) == NULL)
> err(1,NULL);
>
> Index: sbin/ping/ping.c
> ===
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.237
> diff -u -p -u -r1.237 ping.c
> --- sbin/ping/ping.c 20 Jul 2019 00:49:54 - 1.237
> +++ sbin/ping/ping.c 27 Aug 2019 17:56:17 -
> @@ -264,6 +264,12 @@ main(int argc, char *argv[])
> u_int rtableid = 0;
> extern char *__progname;
>
> + /* Cannot pledge due to special setsockopt()s below */
> + if (unveil("/", "r") == -1)
> + err(1, "unveil");
> + if (unveil(NULL, NULL) == -1)
> + err(1, "unveil");
> +
> if (strcmp("ping6", __progname) == 0) {
> v6flag = 1;
> maxpayload = MAXPAYLOAD6;
>
>
Re: display reasons for wifi association failures in ifconfig
On Wed, Aug 28, 2019 at 05:07:34PM +0200, Stefan Sperling wrote:
> On Wed, Aug 28, 2019 at 04:52:10PM +0200, Stefan Sperling wrote:
> > On Wed, Aug 28, 2019 at 04:18:41PM +0200, Stefan Sperling wrote:
> > > Updated diff below.
> >
> > And this is another update which changes display of failure codes
> > from "!(foo.bar)" to "!foo,!bar", as suggested off-list by Theo.
> >
> > This should make the output easier to parse.
>
> Now with less new lines added by making printb_status() more flexible.
Another (hopefully final) update:
Theo suggested putting ! directly into the bitstring macro which keeps
things simpler for ifconfig.
Also, a background scan implies that we are already associated. While we are
associated, we only care about failures for APs belonging to the network
we are associated to, so don't update failure flags for unrelated APs when
processing the result of a background scan.
And clear old failure flags once we have recomputed failures based on
latest scan results.
diff refs/heads/master refs/heads/assocfail
blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
blob + 42ad3819f5688690ebee46d01274f54907066e67
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -2348,10 +2348,25 @@ print_cipherset(u_int32_t cipherset)
}
void
+print_assoc_failures(uint32_t assoc_fail)
+{
+ /* Filter out the most obvious failure cases. */
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
+
+ if (assoc_fail == 0)
+ return;
+
+ printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS);
+}
+
+void
ieee80211_status(void)
{
int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
- int ibssid, iwpa;
+ int ibssid, iwpa, assocfail = 0;
struct ieee80211_nwid nwid;
struct ieee80211_join join;
struct ieee80211_nwkey nwkey;
@@ -2431,11 +2446,15 @@ ieee80211_status(void)
bzero(, sizeof(nr));
bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
- if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
- if (nr.nr_max_rssi)
- printf(" %u%%", IEEE80211_NODEREQ_RSSI());
- else
- printf(" %ddBm", nr.nr_rssi);
+ if (ioctl(s, SIOCG80211NODE, ) == 0) {
+ if (nr.nr_rssi) {
+ if (nr.nr_max_rssi)
+ printf(" %u%%",
+ IEEE80211_NODEREQ_RSSI());
+ else
+ printf(" %ddBm", nr.nr_rssi);
+ }
+ assocfail = nr.nr_assoc_fail;
}
}
@@ -2478,6 +2497,11 @@ ieee80211_status(void)
putchar(' ');
printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
}
+
+ if (assocfail) {
+ putchar(' ');
+ print_assoc_failures(assocfail);
+ }
putchar('\n');
if (show_join)
join_status();
@@ -2751,6 +2775,8 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
IEEE80211_NODEREQ_STATE_BITS);
+ else if (nr->nr_assoc_fail)
+ print_assoc_failures(nr->nr_assoc_fail);
}
void
blob - 425ee5f5a726e3fe0445c82debb0469d4878407f
blob + 6b3b8a5834db39d5b2f2dc7693f42d0b5adbcfbb
--- sys/net80211/ieee80211_ioctl.c
+++ sys/net80211/ieee80211_ioctl.c
@@ -104,6 +104,7 @@ ieee80211_node2req(struct ieee80211com *ic, const stru
nr->nr_txseq = ni->ni_txseq;
nr->nr_rxseq = ni->ni_rxseq;
nr->nr_fails = ni->ni_fails;
+ nr->nr_assoc_fail = ni->ni_assoc_fail; /* flag values are the same */
nr->nr_inact = ni->ni_inact;
nr->nr_txrate = ni->ni_txrate;
nr->nr_state = ni->ni_state;
@@ -821,7 +822,11 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
break;
case SIOCG80211NODE:
nr = (struct ieee80211_nodereq *)data;
- ni = ieee80211_find_node(ic, nr->nr_macaddr);
+ if (ic->ic_bss &&
+ IEEE80211_ADDR_EQ(nr->nr_macaddr, ic->ic_bss->ni_macaddr))
+ ni = ic->ic_bss;
+ else
+ ni = ieee80211_find_node(ic, nr->nr_macaddr);
if (ni == NULL) {
error = ENOENT;
break;
blob - 575a573d2e21e9863bdab5276471f32fd189cc57
blob + 6a53566afc4594afeecf9834055f31af3d021b9f
--- sys/net80211/ieee80211_ioctl.h
+++
Re: useless rtm_type in rtm_output
On Wed, Aug 28, 2019 at 06:58:28PM +0200, Alexander Bluhm wrote: > in rev 1.273 RTM_LOCK has been removed from net/rtsock.c. Since > then the big switch in rtm_output() has RTM_CHANGE as a single case. > It does not make sense to check rtm_type again. OK kn I also double checked that no occurence of "RTM_LOCK" is left in our tree.
useless rtm_type in rtm_output
Hi,
in rev 1.273 RTM_LOCK has been removed from net/rtsock.c. Since
then the big switch in rtm_output() has RTM_CHANGE as a single case.
It does not make sense to check rtm_type again.
For easier review I provide the diff -w output. Otherwise you would
mostly see the indent change.
ok?
bluhm
Index: net/rtsock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.289
diff -u -p -w -r1.289 rtsock.c
--- net/rtsock.c17 Jul 2019 19:57:32 - 1.289
+++ net/rtsock.c28 Aug 2019 16:41:37 -
@@ -977,8 +977,7 @@ rtm_output(struct rt_msghdr *rtm, struct
* If RTAX_GATEWAY is the argument we're trying to
* change, try to find a compatible route.
*/
- if ((rt == NULL) && (info->rti_info[RTAX_GATEWAY] != NULL) &&
- (rtm->rtm_type == RTM_CHANGE)) {
+ if ((rt == NULL) && (info->rti_info[RTAX_GATEWAY] != NULL)) {
rt = rtable_lookup(tableid, info->rti_info[RTAX_DST],
info->rti_info[RTAX_NETMASK], NULL, prio);
/* Ensure we don't pick a multipath one. */
@@ -1003,7 +1002,7 @@ rtm_output(struct rt_msghdr *rtm, struct
}
/*
-* RTM_CHANGE/LOCK need a perfect match.
+* RTM_CHANGE needs a perfect match.
*/
plen = rtable_satoplen(info->rti_info[RTAX_DST]->sa_family,
info->rti_info[RTAX_NETMASK]);
@@ -1012,8 +1011,6 @@ rtm_output(struct rt_msghdr *rtm, struct
break;
}
- switch (rtm->rtm_type) {
- case RTM_CHANGE:
if (info->rti_info[RTAX_GATEWAY] != NULL)
if (rt->rt_gateway == NULL ||
bcmp(rt->rt_gateway,
@@ -1128,8 +1125,6 @@ change:
rt->rt_locks |=
(rtm->rtm_inits & rtm->rtm_rmx.rmx_locks);
NET_UNLOCK();
- break;
- }
break;
case RTM_GET:
rt = rtable_lookup(tableid, info->rti_info[RTAX_DST],
Re: add caveats section in newfs about endianess
> Or, you could explain this more as an attribute of the filesystem: > > FFS filesystems are byte-order dependent, and thus not portable to > systems with a different endianness. > > (two n in endianness) > > what about this text in mount_ffs(8) where it makes more sense? Index: mount_ffs.8 === RCS file: /data/cvs/src/sbin/mount_ffs/mount_ffs.8,v retrieving revision 1.18 diff -u -p -r1.18 mount_ffs.8 --- mount_ffs.8 6 Oct 2016 13:16:21 - 1.18 +++ mount_ffs.8 28 Aug 2019 17:19:48 - @@ -77,6 +77,10 @@ For example, /dev/sd0a and 3eb7f9da875cb .Sq a partition. .Pp +As FFS filesystems are byte-order dependent, +.Nm +will error on a system using a different endianness. +.Pp This command is normally executed per file system by .Xr rc 8 at boot time using the
net80211: fix RSN IE parsing
While testing my assocfail diff I have found a bug. The beacon parsing
code will skip RSN information in beacons unless WPA is active in the
current configuration. The symptom of this was WPA2 APs being flagged
!wpaprotos sometimes. I've even seen scan results in debug output where
every line contained !rsn, just because WPA was disabled during the
scan (i.e. ic->ic_rsnprotos was set to zero).
In my opinion the implicit assumption that WPA is always enabled is wrong.
So instead of checking the current WPA configuration, check whether the
driver announces WPA support. This way, all nodes have their RSN IEs parsed.
This code attempts to infer the highest mutually supported WPA version.
It is safe to always assume that the highest version we support is WPA2.
I don't think we have any driver which supports WPA1 but not WPA2.
Should we have a driver which supports WPA1 but not WPA2, the driver
would need to be urgently fixed anyway.
ok?
diff fd4c5934aa7aaaf9db9d202db33c8f150f540cec /usr/src
blob - 31842d7d3fff5c31b84e2c72eb9ca56136ab25fa
file + sys/net80211/ieee80211_input.c
--- sys/net80211/ieee80211_input.c
+++ sys/net80211/ieee80211_input.c
@@ -1676,7 +1676,7 @@ ieee80211_recv_probe_resp(struct ieee80211com *ic, str
*/
if (rsnie != NULL &&
(ni->ni_supported_rsnprotos & IEEE80211_PROTO_RSN) &&
- (ic->ic_rsnprotos & IEEE80211_PROTO_RSN)) {
+ (ic->ic_caps & IEEE80211_C_RSN)) {
if (ieee80211_save_ie(rsnie, >ni_rsnie) == 0
#ifndef IEEE80211_STA_ONLY
&& ic->ic_opmode != IEEE80211_M_HOSTAP
@@ -1692,7 +1692,7 @@ ieee80211_recv_probe_resp(struct ieee80211com *ic, str
}
} else if (wpaie != NULL &&
(ni->ni_supported_rsnprotos & IEEE80211_PROTO_WPA) &&
- (ic->ic_rsnprotos & IEEE80211_PROTO_WPA)) {
+ (ic->ic_caps & IEEE80211_C_RSN)) {
if (ieee80211_save_ie(wpaie, >ni_rsnie) == 0
#ifndef IEEE80211_STA_ONLY
&& ic->ic_opmode != IEEE80211_M_HOSTAP
route link address length
Hi,
route(8) should provide the storage for struct sockaddr_dl to the
kernel when creating an IFP address. Instead it uses inet/inet6
autodetection also for the link address type.
Currently nothing bad happens as the kernel uses only the field
sdl_index which is within the size of a sockaddr_in.
ok?
bluhm
Index: sbin/route/route.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/route/route.c,v
retrieving revision 1.230
diff -u -p -r1.230 route.c
--- sbin/route/route.c 31 Mar 2019 11:30:35 - 1.230
+++ sbin/route/route.c 28 Aug 2019 22:01:00 -
@@ -579,7 +579,7 @@ newroute(int argc, char **argv)
case K_IFP:
if (!--argc)
usage(1+*argv);
- getaddr(RTA_IFP, af, *++argv, NULL);
+ getaddr(RTA_IFP, AF_LINK, *++argv, NULL);
break;
case K_GATEWAY:
if (!--argc)
@@ -798,7 +798,7 @@ getaddr(int which, int af, char *s, stru
{
sup su = NULL;
struct hostent *hp;
- int afamily, bits;
+ int aflength, afamily, bits;
if (af == AF_UNSPEC) {
if (strchr(s, ':') != NULL) {
@@ -809,7 +809,9 @@ getaddr(int which, int af, char *s, stru
aflen = sizeof(struct sockaddr_in);
}
}
- afamily = af; /* local copy of af so we can change it */
+ /* local copy of len and af so we can change it */
+ aflength = aflen;
+ afamily = af;
rtm_addrs |= which;
switch (which) {
@@ -824,6 +826,7 @@ getaddr(int which, int af, char *s, stru
break;
case RTA_IFP:
su = _ifp;
+ aflength = sizeof(struct sockaddr_dl);
afamily = AF_LINK;
break;
case RTA_IFA:
@@ -833,7 +836,7 @@ getaddr(int which, int af, char *s, stru
errx(1, "internal error");
/* NOTREACHED */
}
- su->sa.sa_len = aflen;
+ su->sa.sa_len = aflength;
su->sa.sa_family = afamily;
if (strcmp(s, "default") == 0) {
sxiccmu - A64 support
Hello Mark,
below is the patch I'm using for a while now on Pinebook and A64+.
I thought, if it's good enough maybe it could be accepted to the main
tree.
I used it with device tree entries below, apm was able to set lowest
and highest clock, running stable for more than a week.
It's actually same as H3 implementation but - since A64 already had
its own set of functions I replicated it there.
I had one problem with the implementation - "delay(1)" for PLL just
hung on Pinebook (was fine on A64+). It didn't look like it was
looping, i.e. breaking the loop after some threshold did not resume
the execution as far as I could tell.
Increasing the delay value by trial and error solved the issue for me.
/ {
compatible = "allwinner,sun50i-a64";
};
&{/} {
cpu0_opp_table: opp_table0 {
compatible = "operating-points-v2";
opp-shared;
opp-64800 {
opp-hz = /bits/ 64 <64800>;
opp-microvolt = <104>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-79200 {
opp-hz = /bits/ 64 <79200>;
opp-microvolt = <110>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-81600 {
opp-hz = /bits/ 64 <81600>;
opp-microvolt = <110>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-91200 {
opp-hz = /bits/ 64 <91200>;
opp-microvolt = <112>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-96000 {
opp-hz = /bits/ 64 <96000>;
opp-microvolt = <116>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-100800 {
opp-hz = /bits/ 64 <100800>;
opp-microvolt = <120>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-105600 {
opp-hz = /bits/ 64 <105600>;
opp-microvolt = <124>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-110400 {
opp-hz = /bits/ 64 <110400>;
opp-microvolt = <126>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
opp-115200 {
opp-hz = /bits/ 64 <115200>;
opp-microvolt = <130>;
clock-latency-ns = <244144>; /* 8 32k periods */
};
};
};
&{/cpus/cpu@0} {
operating-points-v2 = <_opp_table>;
clocks = < 21>;
clock-names = "cpu";
cpu-supply = <_dcdc2>;
};
&{/cpus/cpu@1} {
operating-points-v2 = <_opp_table>;
};
&{/cpus/cpu@2} {
operating-points-v2 = <_opp_table>;
};
&{/cpus/cpu@3} {
operating-points-v2 = <_opp_table>;
};
--
Krystian
Index: sys/dev/fdt/sxiccmu.c
===
RCS file: /cvs/src/sys/dev/fdt/sxiccmu.c,v
retrieving revision 1.22
diff -u -p -r1.22 sxiccmu.c
--- sys/dev/fdt/sxiccmu.c 12 Feb 2019 21:34:11 - 1.22
+++ sys/dev/fdt/sxiccmu.c 28 Aug 2019 22:16:15 -
@@ -971,13 +971,55 @@ sxiccmu_a23_get_frequency(struct sxiccmu
return 0;
}
+#define A64_PLL_CPUX_CTRL_REG 0x
+#define A64_PLL_CPUX_LOCK (1 << 28)
+#define A64_PLL_CPUX_OUT_EXT_DIVP(x) (((x) >> 16) & 0x3)
+#define A64_PLL_CPUX_OUT_EXT_DIVP_MASK (0x3 << 16)
+#define A64_PLL_CPUX_FACTOR_N(x) (((x) >> 8) & 0x1f)
+#define A64_PLL_CPUX_FACTOR_N_MASK (0x1f << 8)
+#define A64_PLL_CPUX_FACTOR_N_SHIFT8
+#define A64_PLL_CPUX_FACTOR_K(x) (((x) >> 4) & 0x3)
+#define A64_PLL_CPUX_FACTOR_K_MASK (0x3 << 4)
+#define A64_PLL_CPUX_FACTOR_K_SHIFT4
+#define A64_PLL_CPUX_FACTOR_M(x) (((x) >> 0) & 0x3)
+#define A64_PLL_CPUX_FACTOR_M_MASK (0x3 << 0)
+#define A64_CPUX_AXI_CFG_REG 0x0050
+#define A64_CPUX_CLK_SRC_SEL (0x3 << 16)
+#define A64_CPUX_CLK_SRC_SEL_LOSC (0x0 << 16)
+#define A64_CPUX_CLK_SRC_SEL_OSC24M(0x1 << 16)
+#define A64_CPUX_CLK_SRC_SEL_PLL_CPUX (0x2 << 16)
+
uint32_t
sxiccmu_a64_get_frequency(struct sxiccmu_softc *sc, uint32_t idx)
{
uint32_t parent;
uint32_t reg, div;
+ uint32_t k, m, n, p;
switch (idx) {
+ case A64_CLK_PLL_CPUX:
+ reg = SXIREAD4(sc, A64_PLL_CPUX_CTRL_REG);
+ k = A64_PLL_CPUX_FACTOR_K(reg) + 1;
+
Re: display reasons for wifi association failures in ifconfig
On Wed, Aug 28, 2019 at 07:12:58PM +0200, Stefan Sperling wrote:
> Another (hopefully final) update:
Turns out the previous diffs break the ramdisk kernel build.
Fixed version:
diff refs/heads/master refs/heads/assocfail
blob - 543e09f0d6e6b4e75456a21d4218468729b61a5e
blob + 42ad3819f5688690ebee46d01274f54907066e67
--- sbin/ifconfig/ifconfig.c
+++ sbin/ifconfig/ifconfig.c
@@ -2348,10 +2348,25 @@ print_cipherset(u_int32_t cipherset)
}
void
+print_assoc_failures(uint32_t assoc_fail)
+{
+ /* Filter out the most obvious failure cases. */
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_ESSID;
+ if (assoc_fail & IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY)
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_WPA_PROTO;
+ assoc_fail &= ~IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY;
+
+ if (assoc_fail == 0)
+ return;
+
+ printb_status(assoc_fail, IEEE80211_NODEREQ_ASSOCFAIL_BITS);
+}
+
+void
ieee80211_status(void)
{
int len, inwid, ijoin, inwkey, ipsk, ichan, ipwr;
- int ibssid, iwpa;
+ int ibssid, iwpa, assocfail = 0;
struct ieee80211_nwid nwid;
struct ieee80211_join join;
struct ieee80211_nwkey nwkey;
@@ -2431,11 +2446,15 @@ ieee80211_status(void)
bzero(, sizeof(nr));
bcopy(bssid.i_bssid, _macaddr, sizeof(nr.nr_macaddr));
strlcpy(nr.nr_ifname, name, sizeof(nr.nr_ifname));
- if (ioctl(s, SIOCG80211NODE, ) == 0 && nr.nr_rssi) {
- if (nr.nr_max_rssi)
- printf(" %u%%", IEEE80211_NODEREQ_RSSI());
- else
- printf(" %ddBm", nr.nr_rssi);
+ if (ioctl(s, SIOCG80211NODE, ) == 0) {
+ if (nr.nr_rssi) {
+ if (nr.nr_max_rssi)
+ printf(" %u%%",
+ IEEE80211_NODEREQ_RSSI());
+ else
+ printf(" %ddBm", nr.nr_rssi);
+ }
+ assocfail = nr.nr_assoc_fail;
}
}
@@ -2478,6 +2497,11 @@ ieee80211_status(void)
putchar(' ');
printb_status(ifr.ifr_flags, IEEE80211_F_USERBITS);
}
+
+ if (assocfail) {
+ putchar(' ');
+ print_assoc_failures(assocfail);
+ }
putchar('\n');
if (show_join)
join_status();
@@ -2751,6 +2775,8 @@ ieee80211_printnode(struct ieee80211_nodereq *nr)
if ((nr->nr_flags & IEEE80211_NODEREQ_AP) == 0)
printb_status(IEEE80211_NODEREQ_STATE(nr->nr_state),
IEEE80211_NODEREQ_STATE_BITS);
+ else if (nr->nr_assoc_fail)
+ print_assoc_failures(nr->nr_assoc_fail);
}
void
blob - 425ee5f5a726e3fe0445c82debb0469d4878407f
blob + 6b3b8a5834db39d5b2f2dc7693f42d0b5adbcfbb
--- sys/net80211/ieee80211_ioctl.c
+++ sys/net80211/ieee80211_ioctl.c
@@ -104,6 +104,7 @@ ieee80211_node2req(struct ieee80211com *ic, const stru
nr->nr_txseq = ni->ni_txseq;
nr->nr_rxseq = ni->ni_rxseq;
nr->nr_fails = ni->ni_fails;
+ nr->nr_assoc_fail = ni->ni_assoc_fail; /* flag values are the same */
nr->nr_inact = ni->ni_inact;
nr->nr_txrate = ni->ni_txrate;
nr->nr_state = ni->ni_state;
@@ -821,7 +822,11 @@ ieee80211_ioctl(struct ifnet *ifp, u_long cmd, caddr_t
break;
case SIOCG80211NODE:
nr = (struct ieee80211_nodereq *)data;
- ni = ieee80211_find_node(ic, nr->nr_macaddr);
+ if (ic->ic_bss &&
+ IEEE80211_ADDR_EQ(nr->nr_macaddr, ic->ic_bss->ni_macaddr))
+ ni = ic->ic_bss;
+ else
+ ni = ieee80211_find_node(ic, nr->nr_macaddr);
if (ni == NULL) {
error = ENOENT;
break;
blob - 575a573d2e21e9863bdab5276471f32fd189cc57
blob + 6a53566afc4594afeecf9834055f31af3d021b9f
--- sys/net80211/ieee80211_ioctl.h
+++ sys/net80211/ieee80211_ioctl.h
@@ -359,6 +359,8 @@ struct ieee80211_nodereq {
/* VHT */
uint8_t nr_vht_ss;
+
+ u_int32_t nr_assoc_fail; /* association failure reasons */
};
#define IEEE80211_NODEREQ_STATE(_s)(1 << _s)
@@ -378,6 +380,18 @@ struct ieee80211_nodereq {
#define SIOCG80211NODE _IOWR('i', 211, struct ieee80211_nodereq)
#define SIOCS80211NODE _IOW('i', 212, struct ieee80211_nodereq)
#define SIOCS80211DELNODE _IOW('i', 213, struct ieee80211_nodereq)
+
+#define IEEE80211_NODEREQ_ASSOCFAIL_CHAN 0x01
+#define IEEE80211_NODEREQ_ASSOCFAIL_IBSS 0x02
+#define IEEE80211_NODEREQ_ASSOCFAIL_PRIVACY0x04
+#define IEEE80211_NODEREQ_ASSOCFAIL_BASIC_RATE 0x08
+#define IEEE80211_NODEREQ_ASSOCFAIL_ESSID 0x10
+#define
validate addresses in routing message
Hi,
The kernel may crash as there is not enough input validation in
routing messages.
https://syzkaller.appspot.com/bug?id=e2076a6518b49730aefe64acf0a266f8e79685a5
Here the name of a routing label is not NUL terminated, but there
are more things that can go wrong. So I added some checks for
incoming routing addresses from userland that the kernel actually
uses.
It is not super strict as userland may provide incomplete addresses
that work anyway. I remember openvpn caused some problems in this
area. Could someone test it with this diff?
ok?
bluhm
Index: net/rtsock.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.290
diff -u -p -w -r1.290 rtsock.c
--- net/rtsock.c28 Aug 2019 20:54:24 - 1.290
+++ net/rtsock.c28 Aug 2019 21:23:34 -
@@ -1366,6 +1366,91 @@ rtm_xaddrs(caddr_t cp, caddr_t cplim, st
rtinfo->rti_info[i] = sa;
ADVANCE(cp, sa);
}
+ for (i = 0; i < RTAX_MAX; i++) {
+ size_t len, maxlen, size;
+
+ sa = rtinfo->rti_info[i];
+ if (sa == NULL)
+ continue;
+ maxlen = size = 0;
+ switch (i) {
+ case RTAX_DST:
+ case RTAX_GATEWAY:
+ case RTAX_SRC:
+ switch (sa->sa_family) {
+ case AF_INET:
+ size = sizeof(struct sockaddr_in);
+ break;
+#ifdef INET6
+ case AF_INET6:
+ size = sizeof(struct sockaddr_in6);
+ break;
+#endif
+#ifdef MPLS
+ case AF_MPLS:
+ size = sizeof(struct sockaddr_mpls);
+ break;
+#endif
+ }
+ break;
+ case RTAX_IFP:
+ if (sa->sa_family != AF_LINK)
+ return (EAFNOSUPPORT);
+ /*
+* XXX Should be sizeof(struct sockaddr_dl), but
+* route(8) has a bug and provides less memory.
+*/
+ size = 16;
+ break;
+ case RTAX_IFA:
+ switch (sa->sa_family) {
+ case AF_INET:
+ size = sizeof(struct sockaddr_in);
+ break;
+#ifdef INET6
+ case AF_INET6:
+ size = sizeof(struct sockaddr_in6);
+ break;
+#endif
+ default:
+ return (EAFNOSUPPORT);
+ }
+ break;
+ case RTAX_LABEL:
+ maxlen = RTLABEL_LEN;
+ size = sizeof(struct sockaddr_rtlabel);
+ break;
+#ifdef BFD
+ case RTAX_BFD:
+ size = sizeof(struct sockaddr_bfd);
+ break;
+#endif
+ case RTAX_DNS:
+ maxlen = RTDNS_LEN;
+ size = sizeof(struct sockaddr_rtdns);
+ break;
+ case RTAX_STATIC:
+ maxlen = RTSTATIC_LEN;
+ size = sizeof(struct sockaddr_rtstatic);
+ break;
+ case RTAX_SEARCH:
+ maxlen = RTSEARCH_LEN;
+ size = sizeof(struct sockaddr_rtsearch);
+ break;
+ }
+ if (size) {
+ if (sa->sa_len < size)
+ return (EINVAL);
+ }
+ if (maxlen) {
+ if (2 + maxlen >= size)
+ return (EINVAL);
+ len = strnlen(sa->sa_data, maxlen);
+ if (len >= maxlen || 2 + len >= sa->sa_len)
+ return (EINVAL);
+ break;
+ }
+ }
return (0);
}
Packet loss / ENOBUFs with kqueue(2) and tap(4)
Hi I am experiencing packet loss and ENOBUFs, I have a program with with two tap(4) interfaces and i am using kqueue(2) to determine when and which tap interface to process the packet. I email bugs@ over a month ago with no reply https://marc.info/?l=openbsd-bugs=156229879107900=2 so i thought i would try here, paraphrased below. Back Story: Over the last few years i have ported Solo5, used by MirageOS and others to run on OpenBSD's vmm(4). In Solo5 we have been working towards supporting multiple network interfaces and implemented this using kqueue(2) and tap(4). The Problem is demonstrated as follows. setting up two Tap interfaces, starting up the Unikernel(Solo5) on vmm. In another session flood pinging the first Tap interface, Solo5 handles this with no packets dropped. In another session ping the second Tap interface, then for every ping to the second interface a packet is dropped on the first. If you switch to a flood ping on the second tab interface, you will observe massive packet loss on both interfaces, and ping complaining about No buffer space available (ENOBUFS). see https://github.com/Solo5/solo5/issues/374 for more information. I have been able to reproduce this in a hacked up exampled program, available here https://github.com/adamsteen/test_net_2if. Please note this is hacked, generally butchered program, which demonstrates the problem. (if required i can try and clean up this test case) 01. git clone https://github.com/adamsteen/test_net_2if 02. cd test_net_2if 03. make 04. doas setup.sh (Setup up the Tap interfaces) 05. doas ./test_net_2if 06. in another seesion start a flood ping doas ping -f 10.0.0.2 07. Observe that the flood ping is functioning correctly, with no packets dropped. 08. In another session, start a normal ping ping 10.1.0.2 09. Observe that, for each ping sent to service1, a packet is dropped. 10. Kill the normal ping 11. start a flood ping doas ping -f 10.1.0.2 12. Observe massive packet loss on both interfaces, and ping complaining about No buffer space available (ENOBUFS). Cheers Adam ps Sorry for the poor english. dmesg: OpenBSD 6.5-current (GENERIC.MP) #123: Sat Jun 29 19:39:46 AWST 2019 ast...@x220.adamsteen.com.au:/sys/arch/amd64/compile/GENERIC.MP real mem = 17041059840 (16251MB) avail mem = 16514461696 (15749MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.6 @ 0xdae9c000 (64 entries) bios0: vendor LENOVO version "8DET69WW (1.39 )" date 07/18/2013 bios0: LENOVO 4291N58 acpi0 at bios0: ACPI 4.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP SLIC SSDT SSDT SSDT HPET APIC MCFG ECDT ASF! TCPA SSDT SSDT \ DMAR UEFI UEFI UEFI acpi0: wakeup devices LID_(S3) SLPB(S3) IGBE(S4) EXP4(S4) EXP7(S4) EHC1(S3) EHC2(S3) \ HDEF(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2492.31 MHz, 06-2a-07 cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,D \ S,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2 \ ,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,RDTS \ CP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 99MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.1.2, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Core(TM) i5-2520M CPU @ 2.50GHz, 2491.91 MHz, 06-2a-07 cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,D \ S,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2 \ ,SSSE3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,NXE,RDTS \ CP,LONG,LAHF,PERF,ITSC,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 24 pins acpimcfg0 at acpi0 acpimcfg0: addr 0xf800, bus 0-63 acpiec0 at acpi0 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (PEG_) acpiprt2 at acpi0: bus 2 (EXP1) acpiprt3 at acpi0: bus 3 (EXP2) acpiprt4 at acpi0: bus 5 (EXP4) acpiprt5 at acpi0: bus 13 (EXP5) acpiprt6 at acpi0: bus -1 (EXP7) acpicpu0 at acpi0: C3(350@104 io@0x415), C1(1000@1 halt), PSS acpicpu1 at acpi0: C3(350@104 io@0x415), C1(1000@1 halt), PSS acpipwrres0 at acpi0: PUBS, resource for EHC1, EHC2 acpitz0 at acpi0: critical temperature is 99 degC acpibtn0 at acpi0: LID_ acpibtn1 at acpi0: SLPB acpipci0 at acpi0 PCI0: 0x 0x0011 0x0001 acpicmos0 at acpi0 acpibat0 at acpi0: BAT0 model "42T4861" serial 16466 type LION oem "SANYO" acpiac0 at acpi0: AC unit online
smtpd reports - expectation management
Since we don't support any smtp-out events at time of writing, so we
might as well throw an error if people try to register one. This way
people won't be surprised if the registration succeeds, but no event
ever arrives.
OK?
martijn@
Index: lka_report.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_report.c,v
retrieving revision 1.26
diff -u -p -r1.26 lka_report.c
--- lka_report.c28 Aug 2019 15:50:36 - 1.26
+++ lka_report.c29 Aug 2019 05:01:21 -
@@ -107,10 +107,13 @@ lka_report_register_hook(const char *nam
subsystem = _in;
hook += 8;
}
+#if 0
+ /* No smtp-out event has been implemented yet */
else if (strncmp(hook, "smtp-out|", 9) == 0) {
subsystem = _out;
hook += 9;
}
+#endif
else
fatalx("Invalid message direction: %s", hook);
let tun(4) and tap(4) receive larger than if_mtu bytes packets
tun_dev_write currently checks if the packet being written into the
kernel is less than the current if_mtu of the interface. we don't really
have a conflation of the mtu with the mru in any other driver, so i'd
like to remove it from tun(4).
we can also avoid a check at runtime about what type of interface
it is by using the if_hdrlen value. tun needs the 4 byte af shim,
and tap should have a whole ethernet header before ether_input gets
to look at it.
ok?
Index: if_tun.c
===
RCS file: /cvs/src/sys/net/if_tun.c,v
retrieving revision 1.187
diff -u -p -r1.187 if_tun.c
--- if_tun.c10 Jun 2019 21:55:16 - 1.187
+++ if_tun.c29 Aug 2019 02:53:35 -
@@ -838,8 +838,7 @@ tun_dev_write(struct tun_softc *tp, stru
ifp = >tun_if;
TUNDEBUG(("%s: tunwrite\n", ifp->if_xname));
- if (uio->uio_resid == 0 || uio->uio_resid > ifp->if_mtu +
- (tp->tun_flags & TUN_LAYER2 ? ETHER_HDR_LEN : sizeof(*th))) {
+ if (uio->uio_resid < ifp->if_hdrlen) {
TUNDEBUG(("%s: len=%d!\n", ifp->if_xname, uio->uio_resid));
return (EMSGSIZE);
}
