Re: [External] : Re: pf route-to issues
On Sun, Jan 24, 2021 at 11:51 PM David Gwynne wrote:
> On Mon, Jan 25, 2021 at 02:50:12AM +0100, Alexandr Nedvedicky wrote:
> > Hello,
> >
> > >
> > > ok. i don't know how to split up the rest of the change though.
> > >
> > > here's an updated diff that includes the rest of the kernel changes and
> > > the pfctl and pf.conf tweaks.
> > >
> > > it's probably useful for me to try and explain at a high level what
> > > i think the semantics should be, otherwise we might end up arguing
> about
> > > which bits of the current config i broke.
> > >
> > > so, from an extremely high level point of view, and apologies if
> > > this is condescending, pf sits between the network stack and an
> > > interface that a packet travels on. for connections handled by the
> > > local box, this means packets come from the stack and get an output
> > > interface selected by a route lookup, then pf checks it, and then
> > > it goes out the selected interface. replies come into an interface,
> > > get checked by pf, and then enter the stack. when forwarding, a
> > > packet comes into an interface, pf checks it, the stack does a route
> > > lookup to pick an interface, pf checks it again, and then it goes
> > > out the interface.
> > >
> > > so what does it mean when route-to (or reply-to) gets involved? i'm
> > > saying that when route-to is applied to a packet, pf takes the packet
> > > away from the stack and immediately forwards it toward to specified
> > > destination address. for a packet entering the system, ie, when the
> > > packet is going from the interface into the stack, route-to should
> > > pretend that it is forwarding the packet and basically push it
> > > straight out an interface. however, like normal forwarding via the
> > > stack, there might be some policy on packets leaving that interface
> that
> > > you want to apply, so pf should run pf_test in that situation so the
> > > policy can be applied. this is especially useful if you need to apply
> > > nat-to when packets leave a particular interface.
> > >
> > > however, if you route-to when a packet is on the way out of the
> > > stack, i'm arguing that pf should not run again against that packet.
> > > currently route-to rules run pf_test again if the interface the packet
> > > is routed out of changes, which means pf runs multiple times against a
> > > packet if rules keep changing which interface it goes out. this means
> > > there's loop prevention in pf to mitigate against this, and weird
> > > potentials for multiple states to be created when nat gets involved.
> > >
> > > for simplicity, both in terms of reasoning and code i think pf should
> > > only be run once when a packet enters the system, and only once when it
> > > leaves the system. the only reason i can come up with for running
> > > pf_test multiple times when route-to changes the outgoing interface is
> > > so you can check the packet with "pass out on $new_if" type rules. we
> > > don't rerun pf again when nat/rdr changes addresses, so this feels
> > > inconsistent to me.
> >
> > I understand that simple is better here, so I won't object
> > if we will lean towards simplified model above. However I still
> > would like to share my view on current PF.
> >
> > the way I understand how things (should) work currently is fairly
> simple:
> >
> > we always run pf_test() as packet crosses interface.
> > packet can cross interface either in outbound or
> > inbound direction.
>
> That's how I understand the current code. I'm proposing that we change
> the semantics so they are:
>
> - we always run pf_test as a packet enters or leaves the network stack.
> - pf is able to filter or apply policy based on various attributes
> of the packet such as addresses and ports, but also metadata about
> the packet such as the current prio, or the interface it came
> from or is going to.
> - changing a packet or it's metadata does not cause a rerun of pf_test.
> - route-to on an incoming packet basically bypasses the default
> stack processing with a "fast route" out of the stack.
>
> > this way we can always create a complex route-to loops,
> > however it can also solve some route-to vs. NAT issues.
> > consider those fairly innocent rules:
> >
> > 8<---8<---8<--8<
> > table { 10.10.10.10, 172.16.1.1 }
> >
> > pass out on em0 from 192.168.1.0/24 to any route-to
> > pass out on em1 from 192.168.1.0 to any nat-to (em1)
> > pass out on em2 all
> > 8<---8<---8<--8<
> >
> > Rules above should currently work, but will stop if we will
> > go with simplified model.
>
> The entries in make the packet go out em1 and em2?
>
> I'm ok with breaking configs like that. We don't run pf_test again for
> other changes to the packet, so if we do want to support something like
> that I think we should make the following work:
>
> # pf_pdesc kif is em0
Re: [External] : Re: pf route-to issues
On Mon, Jan 25, 2021 at 02:50:12AM +0100, Alexandr Nedvedicky wrote:
> Hello,
>
> >
> > ok. i don't know how to split up the rest of the change though.
> >
> > here's an updated diff that includes the rest of the kernel changes and
> > the pfctl and pf.conf tweaks.
> >
> > it's probably useful for me to try and explain at a high level what
> > i think the semantics should be, otherwise we might end up arguing about
> > which bits of the current config i broke.
> >
> > so, from an extremely high level point of view, and apologies if
> > this is condescending, pf sits between the network stack and an
> > interface that a packet travels on. for connections handled by the
> > local box, this means packets come from the stack and get an output
> > interface selected by a route lookup, then pf checks it, and then
> > it goes out the selected interface. replies come into an interface,
> > get checked by pf, and then enter the stack. when forwarding, a
> > packet comes into an interface, pf checks it, the stack does a route
> > lookup to pick an interface, pf checks it again, and then it goes
> > out the interface.
> >
> > so what does it mean when route-to (or reply-to) gets involved? i'm
> > saying that when route-to is applied to a packet, pf takes the packet
> > away from the stack and immediately forwards it toward to specified
> > destination address. for a packet entering the system, ie, when the
> > packet is going from the interface into the stack, route-to should
> > pretend that it is forwarding the packet and basically push it
> > straight out an interface. however, like normal forwarding via the
> > stack, there might be some policy on packets leaving that interface that
> > you want to apply, so pf should run pf_test in that situation so the
> > policy can be applied. this is especially useful if you need to apply
> > nat-to when packets leave a particular interface.
> >
> > however, if you route-to when a packet is on the way out of the
> > stack, i'm arguing that pf should not run again against that packet.
> > currently route-to rules run pf_test again if the interface the packet
> > is routed out of changes, which means pf runs multiple times against a
> > packet if rules keep changing which interface it goes out. this means
> > there's loop prevention in pf to mitigate against this, and weird
> > potentials for multiple states to be created when nat gets involved.
> >
> > for simplicity, both in terms of reasoning and code i think pf should
> > only be run once when a packet enters the system, and only once when it
> > leaves the system. the only reason i can come up with for running
> > pf_test multiple times when route-to changes the outgoing interface is
> > so you can check the packet with "pass out on $new_if" type rules. we
> > don't rerun pf again when nat/rdr changes addresses, so this feels
> > inconsistent to me.
>
> I understand that simple is better here, so I won't object
> if we will lean towards simplified model above. However I still
> would like to share my view on current PF.
>
> the way I understand how things (should) work currently is fairly simple:
>
> we always run pf_test() as packet crosses interface.
> packet can cross interface either in outbound or
> inbound direction.
That's how I understand the current code. I'm proposing that we change
the semantics so they are:
- we always run pf_test as a packet enters or leaves the network stack.
- pf is able to filter or apply policy based on various attributes
of the packet such as addresses and ports, but also metadata about
the packet such as the current prio, or the interface it came
from or is going to.
- changing a packet or it's metadata does not cause a rerun of pf_test.
- route-to on an incoming packet basically bypasses the default
stack processing with a "fast route" out of the stack.
> this way we can always create a complex route-to loops,
> however it can also solve some route-to vs. NAT issues.
> consider those fairly innocent rules:
>
> 8<---8<---8<--8<
> table { 10.10.10.10, 172.16.1.1 }
>
> pass out on em0 from 192.168.1.0/24 to any route-to
> pass out on em1 from 192.168.1.0 to any nat-to (em1)
> pass out on em2 all
> 8<---8<---8<--8<
>
> Rules above should currently work, but will stop if we will
> go with simplified model.
The entries in make the packet go out em1 and em2?
I'm ok with breaking configs like that. We don't run pf_test again for
other changes to the packet, so if we do want to support something like
that I think we should make the following work:
# pf_pdesc kif is em0
match out on em0 from 192.168.1.0/24 to any route-to
# pf_pdesc kif is now em1
pass out on em1 from 192.168.1.0 to any nat-to (em1)
pass out on em2 all
This is more in line with how NAT rules operate.
> I'll be OK
Re: grep: add --null flag
On 2021-01-25, Sebastian Benoit wrote:
> Sebastian Benoit(be...@openbsd.org) on 2021.01.25 00:27:05 +0100:
> > Theo de Raadt(dera...@openbsd.org) on 2021.01.24 16:01:32 -0700:
> > > Stuart Henderson wrote:
> > >
> > > > On 2021/01/24 12:10, Theo de Raadt wrote:
> > > > > I completely despise that the option is called "--null".
> > > > >
> > > > > Someone was a complete idiot.
> > > >
> > > > gnu grep has both --null and -z for this (why do they do that?!).
> > > > If it's added as --null it should be added as -z too.
> > > >
> > > > Looking at Debian codesearch most things using it as --null use other
> > > > long options that we don't have. Maybe just adding as -z would be
> > > > enough. It does seem a useful and fairly widely supported feature.
> > >
> > > Yes, maybe just add -z.
> >
> > Actually it's "-Z, --null". The lowercase -z in gnu grep is
> >
> >-z, --null-data
> > Treat input and output data as sequences of lines, each
> > terminated by a zero byte (the ASCII NUL character) instead of
> > a newline. Like the -Z or --null option, this option can be
> > used with commands like sort-z to process arbitrary file
> > names.
>
> And we already have -z for "force grep to behave as zgrep".
>
> Diff below with tedu@ suggestion and changed manpage text.
What happened to -Z?
>
> >
> > Note that we have the -z in sort(1), which at least is consistent.
> > That also is a non-posix extension.
> >
> > > > Should have been -0 to match xargs and be similar to find's -print0
> > > > but it's too late for that now.
> > >
> > > Yes it should have been -0.
> > >
> > > But unfortunately some an uneducated idiot got involved. None of this
> > > is standardized. Unix script portability is being ruined by idiots, not
> > > just the people proposing it or writing it originally, but also the people
> > > who don't say "wrong" quickly enough. And much of this is because of
> > > intentional development silos.
>
> diff --git usr.bin/grep/grep.1 usr.bin/grep/grep.1
> index 5cc228df222..e1edae7e432 100644
> --- usr.bin/grep/grep.1
> +++ usr.bin/grep/grep.1
> @@ -49,6 +49,7 @@
> .Op Fl -context Ns Op = Ns Ar num
> .Op Fl -label Ns = Ns Ar name
> .Op Fl -line-buffered
> +.Op Fl -null
> .Op Ar pattern
> .Op Ar
> .Ek
> @@ -297,6 +298,16 @@ instead of the filename before lines.
> Force output to be line buffered.
> By default, output is line buffered when standard output is a terminal
> and block buffered otherwise.
> +.It Fl -null
> +Output a zero byte instead of the character that normally follows a
> +file name.
> +This option makes the output unambiguous, even in the presence of file
> +names containing unusual characters like newlines, making the output
> +suitable for use with the
> +.Fl 0
> +option to
> +.Xr xargs 1 .
> +This option is a non-POSIX extension and may not be portable.
> .El
> .Sh EXIT STATUS
> The
> diff --git usr.bin/grep/grep.c usr.bin/grep/grep.c
> index f41b5e20ca6..279d949fae7 100644
> --- usr.bin/grep/grep.c
> +++ usr.bin/grep/grep.c
> @@ -80,6 +80,7 @@ int vflag; /* -v: only show non-matching lines */
> int wflag; /* -w: pattern must start and end on word boundaries */
> int xflag; /* -x: pattern must match entire line */
> int lbflag;/* --line-buffered */
> +int nullflag; /* --null */
> const char *labelname; /* --label=name */
>
> int binbehave = BIN_FILE_BIN;
> @@ -89,6 +90,7 @@ enum {
> HELP_OPT,
> MMAP_OPT,
> LINEBUF_OPT,
> + NULL_OPT,
> LABEL_OPT,
> };
>
> @@ -134,6 +136,7 @@ static const struct option long_options[] =
> {"mmap",no_argument,NULL, MMAP_OPT},
> {"label", required_argument, NULL, LABEL_OPT},
> {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> + {"null",no_argument,NULL, NULL_OPT},
> {"after-context", required_argument, NULL, 'A'},
> {"before-context", required_argument, NULL, 'B'},
> {"context", optional_argument, NULL, 'C'},
> @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> case LINEBUF_OPT:
> lbflag = 1;
> break;
> + case NULL_OPT:
> + nullflag = 1;
> + break;
> case HELP_OPT:
> default:
> usage();
> diff --git usr.bin/grep/grep.h usr.bin/grep/grep.h
> index b3d24ae662b..37e295d4d40 100644
> --- usr.bin/grep/grep.h
> +++ usr.bin/grep/grep.h
> @@ -68,7 +68,7 @@ extern int cflags, eflags;
> extern intAflag, Bflag, Eflag, Fflag, Hflag, Lflag,
>Rflag, Zflag,
>bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
> - sflag, vflag, wflag, xflag;
> + sflag, vflag, wflag, xflag, nullflag;
> extern
Re: [External] : Re: pf route-to issues
Hello,
>
> ok. i don't know how to split up the rest of the change though.
>
> here's an updated diff that includes the rest of the kernel changes and
> the pfctl and pf.conf tweaks.
>
> it's probably useful for me to try and explain at a high level what
> i think the semantics should be, otherwise we might end up arguing about
> which bits of the current config i broke.
>
> so, from an extremely high level point of view, and apologies if
> this is condescending, pf sits between the network stack and an
> interface that a packet travels on. for connections handled by the
> local box, this means packets come from the stack and get an output
> interface selected by a route lookup, then pf checks it, and then
> it goes out the selected interface. replies come into an interface,
> get checked by pf, and then enter the stack. when forwarding, a
> packet comes into an interface, pf checks it, the stack does a route
> lookup to pick an interface, pf checks it again, and then it goes
> out the interface.
>
> so what does it mean when route-to (or reply-to) gets involved? i'm
> saying that when route-to is applied to a packet, pf takes the packet
> away from the stack and immediately forwards it toward to specified
> destination address. for a packet entering the system, ie, when the
> packet is going from the interface into the stack, route-to should
> pretend that it is forwarding the packet and basically push it
> straight out an interface. however, like normal forwarding via the
> stack, there might be some policy on packets leaving that interface that
> you want to apply, so pf should run pf_test in that situation so the
> policy can be applied. this is especially useful if you need to apply
> nat-to when packets leave a particular interface.
>
> however, if you route-to when a packet is on the way out of the
> stack, i'm arguing that pf should not run again against that packet.
> currently route-to rules run pf_test again if the interface the packet
> is routed out of changes, which means pf runs multiple times against a
> packet if rules keep changing which interface it goes out. this means
> there's loop prevention in pf to mitigate against this, and weird
> potentials for multiple states to be created when nat gets involved.
>
> for simplicity, both in terms of reasoning and code i think pf should
> only be run once when a packet enters the system, and only once when it
> leaves the system. the only reason i can come up with for running
> pf_test multiple times when route-to changes the outgoing interface is
> so you can check the packet with "pass out on $new_if" type rules. we
> don't rerun pf again when nat/rdr changes addresses, so this feels
> inconsistent to me.
I understand that simple is better here, so I won't object
if we will lean towards simplified model above. However I still
would like to share my view on current PF.
the way I understand how things (should) work currently is fairly simple:
we always run pf_test() as packet crosses interface.
packet can cross interface either in outbound or
inbound direction.
this way we can always create a complex route-to loops,
however it can also solve some route-to vs. NAT issues.
consider those fairly innocent rules:
8<---8<---8<--8<
table { 10.10.10.10, 172.16.1.1 }
pass out on em0 from 192.168.1.0/24 to any route-to
pass out on em1 from 192.168.1.0 to any nat-to (em1)
pass out on em2 all
8<---8<---8<--8<
Rules above should currently work, but will stop if we will
go with simplified model.
I'll be OK with your simplified model if it will make things
more explicit:
route-to option should be applied on inbound rules
only
reply-to option should be applied on outbound rule
only
dup-to option can go either way (in/out)
does it make sense? IMO yes, because doing route-to
on outbound path feels unnatural to me.
>
> this also breaks the ability to do route-to without states. is there a
> reason to do that apart from the DSR type things? did we agree that
> those use cases could be handled by sloppy states instead?
If I remember correct we need to make 'keep state' mandatory
for route-to so it can work well with pfsync(4), right?
>
> lastly, the "argument" or address specified with route-to (and
> reply-to and dup-to) is a destination address, not a next-hop. this
> has been discussed on the lists a couple of times before, so i won't
> go over it again, except to reiterate that it allows pf to force
> "sticky" path selection while opening up the possibility for ecmp
> and failover for where that path traverses.
I keep forgetting about it as I still stick to current interpretation.
I've seen changes to pfctl. Diff below still allows rule:
pass in on net0 from 192.168.1.0/24 to any
Re: [External] : Re: tell pfctl(8) route-to and reply-to accept next-hop only
> On 25 Jan 2021, at 10:43, Alexandr Nedvedicky > wrote: > > hello, > > On Fri, Jan 22, 2021 at 05:32:47PM +1000, David Gwynne wrote: >> I tried this diff, and it broke the ability to use dynamic addresses. >> ie, the following rules should work: >> >> pass in on gre52 inet proto icmp route-to (gre49:peer) >> pass in on vmx0 inet proto icmp route-to (gre:peer) > >I see, I did not know those should work. We are suffering a bit from not having a high level design :/ >> >> however, other forms of dynamic interface addresses should fail. or do >> we want to support route-to if0:broadcast too? > >I can't think of any valid reason why 'ifp0:broadcast' should work. this >seems to be poor hack to work around some awkward glitch. I would prefer >to disable this option now. We can always add it later, when we will >understand the true purpose. Agreed. Cheers, dlg > > > thanks and > regards > sashan
Re: [External] : Re: tell pfctl(8) route-to and reply-to accept next-hop only
hello, On Fri, Jan 22, 2021 at 05:32:47PM +1000, David Gwynne wrote: > I tried this diff, and it broke the ability to use dynamic addresses. > ie, the following rules should work: > > pass in on gre52 inet proto icmp route-to (gre49:peer) > pass in on vmx0 inet proto icmp route-to (gre:peer) I see, I did not know those should work. > > however, other forms of dynamic interface addresses should fail. or do > we want to support route-to if0:broadcast too? I can't think of any valid reason why 'ifp0:broadcast' should work. this seems to be poor hack to work around some awkward glitch. I would prefer to disable this option now. We can always add it later, when we will understand the true purpose. thanks and regards sashan
Re: grep: add --null flag
Sebastian Benoit(be...@openbsd.org) on 2021.01.25 00:27:05 +0100:
> Theo de Raadt(dera...@openbsd.org) on 2021.01.24 16:01:32 -0700:
> > Stuart Henderson wrote:
> >
> > > On 2021/01/24 12:10, Theo de Raadt wrote:
> > > > I completely despise that the option is called "--null".
> > > >
> > > > Someone was a complete idiot.
> > >
> > > gnu grep has both --null and -z for this (why do they do that?!).
> > > If it's added as --null it should be added as -z too.
> > >
> > > Looking at Debian codesearch most things using it as --null use other
> > > long options that we don't have. Maybe just adding as -z would be
> > > enough. It does seem a useful and fairly widely supported feature.
> >
> > Yes, maybe just add -z.
>
> Actually it's "-Z, --null". The lowercase -z in gnu grep is
>
>-z, --null-data
> Treat input and output data as sequences of lines, each
> terminated by a zero byte (the ASCII NUL character) instead of
> a newline. Like the -Z or --null option, this option can be
> used with commands like sort-z to process arbitrary file
> names.
And we already have -z for "force grep to behave as zgrep".
Diff below with tedu@ suggestion and changed manpage text.
>
> Note that we have the -z in sort(1), which at least is consistent.
> That also is a non-posix extension.
>
> > > Should have been -0 to match xargs and be similar to find's -print0
> > > but it's too late for that now.
> >
> > Yes it should have been -0.
> >
> > But unfortunately some an uneducated idiot got involved. None of this
> > is standardized. Unix script portability is being ruined by idiots, not
> > just the people proposing it or writing it originally, but also the people
> > who don't say "wrong" quickly enough. And much of this is because of
> > intentional development silos.
diff --git usr.bin/grep/grep.1 usr.bin/grep/grep.1
index 5cc228df222..e1edae7e432 100644
--- usr.bin/grep/grep.1
+++ usr.bin/grep/grep.1
@@ -49,6 +49,7 @@
.Op Fl -context Ns Op = Ns Ar num
.Op Fl -label Ns = Ns Ar name
.Op Fl -line-buffered
+.Op Fl -null
.Op Ar pattern
.Op Ar
.Ek
@@ -297,6 +298,16 @@ instead of the filename before lines.
Force output to be line buffered.
By default, output is line buffered when standard output is a terminal
and block buffered otherwise.
+.It Fl -null
+Output a zero byte instead of the character that normally follows a
+file name.
+This option makes the output unambiguous, even in the presence of file
+names containing unusual characters like newlines, making the output
+suitable for use with the
+.Fl 0
+option to
+.Xr xargs 1 .
+This option is a non-POSIX extension and may not be portable.
.El
.Sh EXIT STATUS
The
diff --git usr.bin/grep/grep.c usr.bin/grep/grep.c
index f41b5e20ca6..279d949fae7 100644
--- usr.bin/grep/grep.c
+++ usr.bin/grep/grep.c
@@ -80,6 +80,7 @@ intvflag; /* -v: only show non-matching lines */
int wflag; /* -w: pattern must start and end on word boundaries */
int xflag; /* -x: pattern must match entire line */
int lbflag;/* --line-buffered */
+int nullflag; /* --null */
const char *labelname; /* --label=name */
int binbehave = BIN_FILE_BIN;
@@ -89,6 +90,7 @@ enum {
HELP_OPT,
MMAP_OPT,
LINEBUF_OPT,
+ NULL_OPT,
LABEL_OPT,
};
@@ -134,6 +136,7 @@ static const struct option long_options[] =
{"mmap",no_argument,NULL, MMAP_OPT},
{"label", required_argument, NULL, LABEL_OPT},
{"line-buffered", no_argument,NULL, LINEBUF_OPT},
+ {"null",no_argument,NULL, NULL_OPT},
{"after-context", required_argument, NULL, 'A'},
{"before-context", required_argument, NULL, 'B'},
{"context", optional_argument, NULL, 'C'},
@@ -436,6 +439,9 @@ main(int argc, char *argv[])
case LINEBUF_OPT:
lbflag = 1;
break;
+ case NULL_OPT:
+ nullflag = 1;
+ break;
case HELP_OPT:
default:
usage();
diff --git usr.bin/grep/grep.h usr.bin/grep/grep.h
index b3d24ae662b..37e295d4d40 100644
--- usr.bin/grep/grep.h
+++ usr.bin/grep/grep.h
@@ -68,7 +68,7 @@ extern int cflags, eflags;
extern int Aflag, Bflag, Eflag, Fflag, Hflag, Lflag,
Rflag, Zflag,
bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
-sflag, vflag, wflag, xflag;
+sflag, vflag, wflag, xflag, nullflag;
extern int binbehave;
extern const char *labelname;
diff --git usr.bin/grep/util.c usr.bin/grep/util.c
index e16d08e7d85..546bd09dc3b 100644
--- usr.bin/grep/util.c
+++ usr.bin/grep/util.c
@@ -172,13 +172,13 @@ procfile(char *fn)
if
Re: grep: add --null flag
Theo de Raadt(dera...@openbsd.org) on 2021.01.24 16:01:32 -0700: > Stuart Henderson wrote: > > > On 2021/01/24 12:10, Theo de Raadt wrote: > > > I completely despise that the option is called "--null". > > > > > > Someone was a complete idiot. > > > > gnu grep has both --null and -z for this (why do they do that?!). > > If it's added as --null it should be added as -z too. > > > > Looking at Debian codesearch most things using it as --null use other > > long options that we don't have. Maybe just adding as -z would be > > enough. It does seem a useful and fairly widely supported feature. > > Yes, maybe just add -z. Actually it's "-Z, --null". The lowercase -z in gnu grep is -z, --null-data Treat input and output data as sequences of lines, each terminated by a zero byte (the ASCII NUL character) instead of a newline. Like the -Z or --null option, this option can be used with commands like sort-z to process arbitrary file names. Note that we have the -z in sort(1), which at least is consistent. That also is a non-posix extension. > > Should have been -0 to match xargs and be similar to find's -print0 > > but it's too late for that now. > > Yes it should have been -0. > > But unfortunately some an uneducated idiot got involved. None of this > is standardized. Unix script portability is being ruined by idiots, not > just the people proposing it or writing it originally, but also the people > who don't say "wrong" quickly enough. And much of this is because of > intentional development silos. >
Re: grep: add --null flag
Stuart Henderson wrote: > On 2021/01/24 12:10, Theo de Raadt wrote: > > I completely despise that the option is called "--null". > > > > Someone was a complete idiot. > > gnu grep has both --null and -z for this (why do they do that?!). > If it's added as --null it should be added as -z too. > > Looking at Debian codesearch most things using it as --null use other > long options that we don't have. Maybe just adding as -z would be > enough. It does seem a useful and fairly widely supported feature. Yes, maybe just add -z. > Should have been -0 to match xargs and be similar to find's -print0 > but it's too late for that now. Yes it should have been -0. But unfortunately some an uneducated idiot got involved. None of this is standardized. Unix script portability is being ruined by idiots, not just the people proposing it or writing it originally, but also the people who don't say "wrong" quickly enough. And much of this is because of intentional development silos.
Re: grep: add --null flag
On 2021/01/24 12:10, Theo de Raadt wrote: > I completely despise that the option is called "--null". > > Someone was a complete idiot. gnu grep has both --null and -z for this (why do they do that?!). If it's added as --null it should be added as -z too. Looking at Debian codesearch most things using it as --null use other long options that we don't have. Maybe just adding as -z would be enough. It does seem a useful and fairly widely supported feature. Should have been -0 to match xargs and be similar to find's -print0 but it's too late for that now.
Re: [PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
On Sun, Jan 24, 2021 at 01:01:45PM -0700, Alex Henrie wrote: > On Sun, Jan 24, 2021 at 10:51 AM Otto Moerbeek wrote: > > > > Please stop pushing your diff to this list. So far nobody showed any > > interest. > > I am definitely interested. Bernhard Voelker seemed to express > interest as well, conditional on -q being added to POSIX first.[1] > Also, a --quiet flag was proposed back in 2001 by Roman Czyborra [2] > and Jim Meyering expressed support for the idea.[3] > > -Alex > > [1] https://lists.gnu.org/archive/html/coreutils/2021-01/msg00043.html > [2] https://lists.gnu.org/archive/html/bug-sh-utils/2001-05/msg00024.html > [3] https://lists.gnu.org/archive/html/bug-sh-utils/2001-05/msg00039.html "This list" is the OpenBSD tech list, sorry I did leave out this context info. -Otto
Re: [PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
On Sun, Jan 24, 2021 at 10:51 AM Otto Moerbeek wrote: > > Please stop pushing your diff to this list. So far nobody showed any > interest. I am definitely interested. Bernhard Voelker seemed to express interest as well, conditional on -q being added to POSIX first.[1] Also, a --quiet flag was proposed back in 2001 by Roman Czyborra [2] and Jim Meyering expressed support for the idea.[3] -Alex [1] https://lists.gnu.org/archive/html/coreutils/2021-01/msg00043.html [2] https://lists.gnu.org/archive/html/bug-sh-utils/2001-05/msg00024.html [3] https://lists.gnu.org/archive/html/bug-sh-utils/2001-05/msg00039.html
Re: grep: add --null flag
I completely despise that the option is called "--null".
Someone was a complete idiot.
Ted Unangst wrote:
> On 2021-01-22, Omar Polo wrote:
> >
> > quasi three-weekly ping.
> >
> > Is this such a bad idea?
>
> This seems reasonable. I think there's no need to change print line calls
> though, just patch the implementation once.
>
> >
> > (TBH: I have still to look at how to write a regression test for this)
> >
> > Omar Polo writes:
> >
> > > Hello,
> > >
> > > There are various program, especially the one written with GNU grep in
> > > mind, that expects various flags that grep in base doesn't have. While
> > > some of the flags (like --color) can be easily worked out (i.e. by
> > > patching/customising these programs) one thing that it isn't easily
> > > workable is --null, because it alters the way grep outputs its data.
> > >
> > > --null makes grep output an ASCII NUL byte after the file name, so a
> > > program can parse the output of grep unambiguously even when the file
> > > names contains "funny" characters, such as a newline.
> > >
> > > GNU grep isn't the only one with a --null flag, also FreeBSD and NetBSD
> > > grep do (at least by looking at their manpages), so it's somewhat
> > > widespread.
> > >
> > > I searched the archives on marc.info but I haven't seen a previous
> > > discussion about this.
> > >
> > > The following patch was tried against GNU grep (installed from packages)
> > > and seem to behave consistently.
> > >
> > > I used the same text of the FreeBSD/NetBSD manpage for the description
> > > of the --null option, but I really dislike it: it feels way to verbose
> > > for what it's trying to say, but I wasn't able to come up with something
> > > better.
> > >
> > > I'm not familiar at all with the grep codebase, so I hope I'm not
> > > missing something. If this has some chances of being accepted, I guess
> > > I should also add some regress test; I'll try to work on them soon, but
> > > in the meantime I'm sending this.
> > >
> > > Thanks,
> > >
> > > Omar Polo
> >
> >
> > diff e992327fc31d0277a6f8518613a7db1b9face78b /home/op/w/openbsd-src
> > blob - 5cc228df222c54a0553f289b5da8bbbe6afd171e
> > file + usr.bin/grep/grep.1
> > --- usr.bin/grep/grep.1
> > +++ usr.bin/grep/grep.1
> > @@ -49,6 +49,7 @@
> > .Op Fl -context Ns Op = Ns Ar num
> > .Op Fl -label Ns = Ns Ar name
> > .Op Fl -line-buffered
> > +.Op Fl -null
> > .Op Ar pattern
> > .Op Ar
> > .Ek
> > @@ -297,6 +298,25 @@ instead of the filename before lines.
> > Force output to be line buffered.
> > By default, output is line buffered when standard output is a terminal
> > and block buffered otherwise.
> > +.It Fl -null
> > +Output a zero byte (the ASCII NUL character) instead of the character
> > +that normally follows a file name.
> > +For example,
> > +.Nm Fl l Fl -null
> > +outputs a zero byte after each file name instead of the usual newline.
> > +This option makes the output unambiguous, even in the presence of file
> > +names containing unusual characters like newlines.
> > +This option can be used with commands like
> > +.Xr find 1
> > +.Fl print0 Ns ,
> > +.Xr perl 1
> > +.Fl 0 Ns ,
> > +.Xr sort 1
> > +.Fl z Ns , and
> > +.Xr args 1
> > +.Fl 0
> > +to process arbitrary file names, even those that contain newline
> > +characters.
> > .El
> > .Sh EXIT STATUS
> > The
> > blob - f41b5e20ca68c9e9a36d2f7dd3c44329c621f29b
> > file + usr.bin/grep/grep.c
> > --- usr.bin/grep/grep.c
> > +++ usr.bin/grep/grep.c
> > @@ -80,6 +80,7 @@ intvflag; /* -v: only show non-matching
> > lines */
> > int wflag; /* -w: pattern must start and end on word
> > boundaries */
> > int xflag; /* -x: pattern must match entire line */
> > int lbflag;/* --line-buffered */
> > +int nullflag; /* --null */
> > const char *labelname; /* --label=name */
> >
> > int binbehave = BIN_FILE_BIN;
> > @@ -89,6 +90,7 @@ enum {
> > HELP_OPT,
> > MMAP_OPT,
> > LINEBUF_OPT,
> > + NULL_OPT,
> > LABEL_OPT,
> > };
> >
> > @@ -134,6 +136,7 @@ static const struct option long_options[] =
> > {"mmap",no_argument,NULL, MMAP_OPT},
> > {"label", required_argument, NULL, LABEL_OPT},
> > {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> > + {"null",no_argument,NULL, NULL_OPT},
> > {"after-context", required_argument, NULL, 'A'},
> > {"before-context", required_argument, NULL, 'B'},
> > {"context", optional_argument, NULL, 'C'},
> > @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> > case LINEBUF_OPT:
> > lbflag = 1;
> > break;
> > + case NULL_OPT:
> > + nullflag = 1;
> > + break;
> > case HELP_OPT:
> > default:
> > usage();
> > blob -
Re: grep: add --null flag
On 2021-01-22, Omar Polo wrote:
>
> quasi three-weekly ping.
>
> Is this such a bad idea?
This seems reasonable. I think there's no need to change print line calls
though, just patch the implementation once.
>
> (TBH: I have still to look at how to write a regression test for this)
>
> Omar Polo writes:
>
> > Hello,
> >
> > There are various program, especially the one written with GNU grep in
> > mind, that expects various flags that grep in base doesn't have. While
> > some of the flags (like --color) can be easily worked out (i.e. by
> > patching/customising these programs) one thing that it isn't easily
> > workable is --null, because it alters the way grep outputs its data.
> >
> > --null makes grep output an ASCII NUL byte after the file name, so a
> > program can parse the output of grep unambiguously even when the file
> > names contains "funny" characters, such as a newline.
> >
> > GNU grep isn't the only one with a --null flag, also FreeBSD and NetBSD
> > grep do (at least by looking at their manpages), so it's somewhat
> > widespread.
> >
> > I searched the archives on marc.info but I haven't seen a previous
> > discussion about this.
> >
> > The following patch was tried against GNU grep (installed from packages)
> > and seem to behave consistently.
> >
> > I used the same text of the FreeBSD/NetBSD manpage for the description
> > of the --null option, but I really dislike it: it feels way to verbose
> > for what it's trying to say, but I wasn't able to come up with something
> > better.
> >
> > I'm not familiar at all with the grep codebase, so I hope I'm not
> > missing something. If this has some chances of being accepted, I guess
> > I should also add some regress test; I'll try to work on them soon, but
> > in the meantime I'm sending this.
> >
> > Thanks,
> >
> > Omar Polo
>
>
> diff e992327fc31d0277a6f8518613a7db1b9face78b /home/op/w/openbsd-src
> blob - 5cc228df222c54a0553f289b5da8bbbe6afd171e
> file + usr.bin/grep/grep.1
> --- usr.bin/grep/grep.1
> +++ usr.bin/grep/grep.1
> @@ -49,6 +49,7 @@
> .Op Fl -context Ns Op = Ns Ar num
> .Op Fl -label Ns = Ns Ar name
> .Op Fl -line-buffered
> +.Op Fl -null
> .Op Ar pattern
> .Op Ar
> .Ek
> @@ -297,6 +298,25 @@ instead of the filename before lines.
> Force output to be line buffered.
> By default, output is line buffered when standard output is a terminal
> and block buffered otherwise.
> +.It Fl -null
> +Output a zero byte (the ASCII NUL character) instead of the character
> +that normally follows a file name.
> +For example,
> +.Nm Fl l Fl -null
> +outputs a zero byte after each file name instead of the usual newline.
> +This option makes the output unambiguous, even in the presence of file
> +names containing unusual characters like newlines.
> +This option can be used with commands like
> +.Xr find 1
> +.Fl print0 Ns ,
> +.Xr perl 1
> +.Fl 0 Ns ,
> +.Xr sort 1
> +.Fl z Ns , and
> +.Xr args 1
> +.Fl 0
> +to process arbitrary file names, even those that contain newline
> +characters.
> .El
> .Sh EXIT STATUS
> The
> blob - f41b5e20ca68c9e9a36d2f7dd3c44329c621f29b
> file + usr.bin/grep/grep.c
> --- usr.bin/grep/grep.c
> +++ usr.bin/grep/grep.c
> @@ -80,6 +80,7 @@ int vflag; /* -v: only show non-matching lines */
> int wflag; /* -w: pattern must start and end on word boundaries */
> int xflag; /* -x: pattern must match entire line */
> int lbflag;/* --line-buffered */
> +int nullflag; /* --null */
> const char *labelname; /* --label=name */
>
> int binbehave = BIN_FILE_BIN;
> @@ -89,6 +90,7 @@ enum {
> HELP_OPT,
> MMAP_OPT,
> LINEBUF_OPT,
> + NULL_OPT,
> LABEL_OPT,
> };
>
> @@ -134,6 +136,7 @@ static const struct option long_options[] =
> {"mmap",no_argument,NULL, MMAP_OPT},
> {"label", required_argument, NULL, LABEL_OPT},
> {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> + {"null",no_argument,NULL, NULL_OPT},
> {"after-context", required_argument, NULL, 'A'},
> {"before-context", required_argument, NULL, 'B'},
> {"context", optional_argument, NULL, 'C'},
> @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> case LINEBUF_OPT:
> lbflag = 1;
> break;
> + case NULL_OPT:
> + nullflag = 1;
> + break;
> case HELP_OPT:
> default:
> usage();
> blob - b3d24ae662beb72c5632190c5c819bcc92f0389a
> file + usr.bin/grep/grep.h
> --- usr.bin/grep/grep.h
> +++ usr.bin/grep/grep.h
> @@ -68,7 +68,7 @@ extern int cflags, eflags;
> extern intAflag, Bflag, Eflag, Fflag, Hflag, Lflag,
>Rflag, Zflag,
>bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag,
> - sflag,
Re: unwind: silence "udp connect failed" errors
On Sun, Jan 24, 2021 at 07:24:07PM +0100, Florian Obser wrote:
> On Sun, Jan 24, 2021 at 01:06:31PM +0100, Klemens Nanni wrote:
> > On Sun, Jan 24, 2021 at 12:52:50PM +0100, Theo Buehler wrote:
> > > Probably better to sync first with the corresponding unbound commit
> > > https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
> > > then adjust udp_connect_needs_log() as needed.
> > Good call, thanks.
> >
> > Here's the combined diff that syncs with unbound and adds EADDRNOTAVAIL
> > in the same fashion.
> >
> > In case that is OK, I'd commit sync and addition separately.
> >
> > Feedback? OK?
>
> I consider the libunbound directory off-limits, it is not a fork and
> it should not be a fork.
>
> We currently carry two diffs in there and I want to get rid of them
> (see at the end).
>
> Especialy the local syslog one is stupid. I think we are doing
> something wrong or libunbound does something wrong, not sure.
>
> Anyway, I think what we should actually do is disable logging in
> libunbound unless we crank logging to debug in unwind. There is
> nothing interesting comming out of libunbound for us.
>
> This leaves the 2nd problem, why is it going off the rails so badly if
> there is no v4? Is it also doing this for v6 when that's not present?
> Try resolving dnssec-failed.org, it's delegated to comcast which uses
> 5 nameservers all with v4 and v6 addresses. Since dnssec validation is
> intentionally broken unwind tries to talk to *all* nameservers, the
> log does not stop scrolling. It's so bad it actually times out on IPv6
> only.
(lib)unbound is very aggressive in finding a answer that validates. If
a reply fails validation, it will continue to recurse to see if it can
find a valid one. Other recursors use different apporoaches, e.g.
PowerDNS recursor cuts of the recursion if it finds an answer, even
if fails validation. I'd say if some of your nameservers provide
non-validating answers, fix them.
-Otto
>
> Is this a bug in (lib)unbound or is it doing what it's supposed to do?
>
> It is possible that from the point of view of (lib)unbound it is
> behaving perfectly fine, if you don't have an address family you are
> supposed to hit the available button in unbound.conf.
>
> If that is the case we should do the same, automatically. I mean
> that's the whole point of unwind, figure out what works and use that.
> Meaning if we detect that we don't have working IPv4 set use-ipv4: no.
>
> commit d273f78b8643bdb01f621260eb323123b774e431
> Author: florian
> Date: Fri Dec 6 13:08:48 2019 +
>
> Stop fiddling with openlog / closelog in libunbound. unwind handles
> this. We need to find a way to properly upstream this.
> OK otto
>
> diff --git libunbound/util/log.c libunbound/util/log.c
> index dfbb2334994..e8e987963c5 100644
> --- libunbound/util/log.c
> +++ libunbound/util/log.c
> @@ -109,16 +109,20 @@ log_init(const char* filename, int use_syslog, const
> char* chrootdir)
> fclose(cl);
> }
> #ifdef HAVE_SYSLOG_H
> +#if 0/* unwind handles syslog for us */
> if(logging_to_syslog) {
> closelog();
> logging_to_syslog = 0;
> }
> +#endif
> if(use_syslog) {
> /* do not delay opening until first write, because we may
>* chroot and no longer be able to access dev/log and so on */
> /* the facility is LOG_DAEMON by default, but
>* --with-syslog-facility=LOCAL[0-7] can override it */
> +#if 0/* unwind handles syslog for us */
> openlog(ident, LOG_NDELAY, UB_SYSLOG_FACILITY);
> +#endif
> logging_to_syslog = 1;
> lock_basic_unlock(_lock);
> return;
>
> commit 81b0d744ff77e26ea69cee28aed10081d3973fe8
> Author: otto
> Date: Sat Dec 14 19:56:26 2019 +
>
> Be less aggressive pre-allocating memory; ok florian@
>
> diff --git libunbound/util/alloc.c libunbound/util/alloc.c
> index 7e9618931ca..e9613b10dcd 100644
> --- libunbound/util/alloc.c
> +++ libunbound/util/alloc.c
> @@ -113,7 +113,7 @@ alloc_init(struct alloc_cache* alloc, struct alloc_cache*
> super,
> alloc->last_id -= 1;/* for compiler portability. */
> alloc->last_id |= alloc->next_id;
> alloc->next_id += 1;/* because id=0 is special. */
> - alloc->max_reg_blocks = 100;
> + alloc->max_reg_blocks = 10;
> alloc->num_reg_blocks = 0;
> alloc->reg_list = NULL;
> alloc->cleanup = NULL;
>
>
> >
> >
> > Index: libunbound/services/outside_network.c
> > ===
> > RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
> > retrieving revision 1.9
> > diff -u -p -r1.9 outside_network.c
> > --- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -
> > 1.9
> > +++ libunbound/services/outside_network.c 24 Jan 2021
Re: route sourceaddr: simplify code & get out of ART
On 23/01/21(Sat) 21:59, Vitaliy Makkoveev wrote:
> Hello.
>
> According the code `ifaddr’ struct has `ifa_refcnt’ field. Also it seems `ifa’
> could exist while corresponding `ifp’ was destroyed. Is this true for `rt’
> case? Should `ifa_refcnt' be bumped while you return `ifa’?
What is stored is a "struct rtentry". This data structure is properly
refcounted in this diff. When the last reference of `rt' is freed
ifafree() is called. So there's no need to mess with `ifa' directly.
> > On 9 Jan 2021, at 20:50, Denis Fondras wrote:
> >
> > This diff place the user-set source address outside of struct art_root and
> > make
> > the code more readable (to me).
> >
> > Based on a concept by mpi@
> >
> > Index: net/art.h
> > ===
> > RCS file: /cvs/src/sys/net/art.h,v
> > retrieving revision 1.20
> > diff -u -p -r1.20 art.h
> > --- net/art.h 12 Nov 2020 15:25:28 - 1.20
> > +++ net/art.h 9 Jan 2021 16:04:02 -
> > @@ -42,7 +42,6 @@ struct art_root {
> > uint8_t ar_nlvl; /* [I] Number of levels */
> > uint8_t ar_alen; /* [I] Address length in bits */
> > uint8_t ar_off;/* [I] Offset of key in bytes */
> > - struct sockaddr *source;/* [K] optional src addr to use
> > */
> > };
> >
> > #define ISLEAF(e) (((unsigned long)(e) & 1) == 0)
> > Index: net/route.c
> > ===
> > RCS file: /cvs/src/sys/net/route.c,v
> > retrieving revision 1.397
> > diff -u -p -r1.397 route.c
> > --- net/route.c 29 Oct 2020 21:15:27 - 1.397
> > +++ net/route.c 9 Jan 2021 16:04:02 -
> > @@ -1192,9 +1192,9 @@ rt_ifa_del(struct ifaddr *ifa, int flags
> > if (flags & RTF_CONNECTED)
> > prio = ifp->if_priority + RTP_CONNECTED;
> >
> > - rtable_clearsource(rdomain, ifa->ifa_addr);
> > error = rtrequest_delete(, prio, ifp, , rdomain);
> > if (error == 0) {
> > + rt_sourceclear(rt, rdomain);
> > rtm_send(rt, RTM_DELETE, 0, rdomain);
> > if (flags & RTF_LOCAL)
> > rtm_addr(RTM_DELADDR, ifa);
> > Index: net/route.h
> > ===
> > RCS file: /cvs/src/sys/net/route.h,v
> > retrieving revision 1.183
> > diff -u -p -r1.183 route.h
> > --- net/route.h 29 Oct 2020 21:15:27 - 1.183
> > +++ net/route.h 9 Jan 2021 16:04:02 -
> > @@ -478,6 +478,9 @@ int rtrequest_delete(struct rt_addrinfo
> > int rt_if_track(struct ifnet *);
> > int rt_if_linkstate_change(struct rtentry *, void *, u_int);
> > int rtdeletemsg(struct rtentry *, struct ifnet *, u_int);
> > +
> > +struct ifaddr *rt_get_ifa(struct rtentry *, unsigned int);
> > +voidrt_sourceclear(struct rtentry *, unsigned int);
> > #endif /* _KERNEL */
> >
> > #endif /* _NET_ROUTE_H_ */
> > Index: net/rtable.c
> > ===
> > RCS file: /cvs/src/sys/net/rtable.c,v
> > retrieving revision 1.72
> > diff -u -p -r1.72 rtable.c
> > --- net/rtable.c7 Nov 2020 09:51:40 - 1.72
> > +++ net/rtable.c9 Jan 2021 16:04:02 -
> > @@ -365,44 +365,6 @@ rtable_alloc(unsigned int rtableid, unsi
> > return (art_alloc(rtableid, alen, off));
> > }
> >
> > -int
> > -rtable_setsource(unsigned int rtableid, int af, struct sockaddr *src)
> > -{
> > - struct art_root *ar;
> > -
> > - if ((ar = rtable_get(rtableid, af)) == NULL)
> > - return (EAFNOSUPPORT);
> > -
> > - ar->source = src;
> > -
> > - return (0);
> > -}
> > -
> > -struct sockaddr *
> > -rtable_getsource(unsigned int rtableid, int af)
> > -{
> > - struct art_root *ar;
> > -
> > - ar = rtable_get(rtableid, af);
> > - if (ar == NULL)
> > - return (NULL);
> > -
> > - return (ar->source);
> > -}
> > -
> > -void
> > -rtable_clearsource(unsigned int rtableid, struct sockaddr *src)
> > -{
> > - struct sockaddr *addr;
> > -
> > - addr = rtable_getsource(rtableid, src->sa_family);
> > - if (addr && (addr->sa_len == src->sa_len)) {
> > - if (memcmp(src, addr, addr->sa_len) == 0) {
> > - rtable_setsource(rtableid, src->sa_family, NULL);
> > - }
> > - }
> > -}
> > -
> > struct rtentry *
> > rtable_lookup(unsigned int rtableid, struct sockaddr *dst,
> > struct sockaddr *mask, struct sockaddr *gateway, uint8_t prio)
> > Index: net/rtable.h
> > ===
> > RCS file: /cvs/src/sys/net/rtable.h,v
> > retrieving revision 1.26
> > diff -u -p -r1.26 rtable.h
> > --- net/rtable.h7 Nov 2020 09:51:40 - 1.26
> > +++ net/rtable.h9 Jan 2021 16:04:02 -
> > @@ -39,9 +39,6 @@ unsigned int rtable_l2(unsigned int);
> > unsigned int rtable_loindex(unsigned int);
> >
Re: unwind: silence "udp connect failed" errors
On Sun, Jan 24, 2021 at 01:06:31PM +0100, Klemens Nanni wrote:
> On Sun, Jan 24, 2021 at 12:52:50PM +0100, Theo Buehler wrote:
> > Probably better to sync first with the corresponding unbound commit
> > https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
> > then adjust udp_connect_needs_log() as needed.
> Good call, thanks.
>
> Here's the combined diff that syncs with unbound and adds EADDRNOTAVAIL
> in the same fashion.
>
> In case that is OK, I'd commit sync and addition separately.
>
> Feedback? OK?
I consider the libunbound directory off-limits, it is not a fork and
it should not be a fork.
We currently carry two diffs in there and I want to get rid of them
(see at the end).
Especialy the local syslog one is stupid. I think we are doing
something wrong or libunbound does something wrong, not sure.
Anyway, I think what we should actually do is disable logging in
libunbound unless we crank logging to debug in unwind. There is
nothing interesting comming out of libunbound for us.
This leaves the 2nd problem, why is it going off the rails so badly if
there is no v4? Is it also doing this for v6 when that's not present?
Try resolving dnssec-failed.org, it's delegated to comcast which uses
5 nameservers all with v4 and v6 addresses. Since dnssec validation is
intentionally broken unwind tries to talk to *all* nameservers, the
log does not stop scrolling. It's so bad it actually times out on IPv6
only.
Is this a bug in (lib)unbound or is it doing what it's supposed to do?
It is possible that from the point of view of (lib)unbound it is
behaving perfectly fine, if you don't have an address family you are
supposed to hit the available button in unbound.conf.
If that is the case we should do the same, automatically. I mean
that's the whole point of unwind, figure out what works and use that.
Meaning if we detect that we don't have working IPv4 set use-ipv4: no.
commit d273f78b8643bdb01f621260eb323123b774e431
Author: florian
Date: Fri Dec 6 13:08:48 2019 +
Stop fiddling with openlog / closelog in libunbound. unwind handles
this. We need to find a way to properly upstream this.
OK otto
diff --git libunbound/util/log.c libunbound/util/log.c
index dfbb2334994..e8e987963c5 100644
--- libunbound/util/log.c
+++ libunbound/util/log.c
@@ -109,16 +109,20 @@ log_init(const char* filename, int use_syslog, const
char* chrootdir)
fclose(cl);
}
#ifdef HAVE_SYSLOG_H
+#if 0 /* unwind handles syslog for us */
if(logging_to_syslog) {
closelog();
logging_to_syslog = 0;
}
+#endif
if(use_syslog) {
/* do not delay opening until first write, because we may
* chroot and no longer be able to access dev/log and so on */
/* the facility is LOG_DAEMON by default, but
* --with-syslog-facility=LOCAL[0-7] can override it */
+#if 0 /* unwind handles syslog for us */
openlog(ident, LOG_NDELAY, UB_SYSLOG_FACILITY);
+#endif
logging_to_syslog = 1;
lock_basic_unlock(_lock);
return;
commit 81b0d744ff77e26ea69cee28aed10081d3973fe8
Author: otto
Date: Sat Dec 14 19:56:26 2019 +
Be less aggressive pre-allocating memory; ok florian@
diff --git libunbound/util/alloc.c libunbound/util/alloc.c
index 7e9618931ca..e9613b10dcd 100644
--- libunbound/util/alloc.c
+++ libunbound/util/alloc.c
@@ -113,7 +113,7 @@ alloc_init(struct alloc_cache* alloc, struct alloc_cache*
super,
alloc->last_id -= 1;/* for compiler portability. */
alloc->last_id |= alloc->next_id;
alloc->next_id += 1;/* because id=0 is special. */
- alloc->max_reg_blocks = 100;
+ alloc->max_reg_blocks = 10;
alloc->num_reg_blocks = 0;
alloc->reg_list = NULL;
alloc->cleanup = NULL;
>
>
> Index: libunbound/services/outside_network.c
> ===
> RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 outside_network.c
> --- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -
> 1.9
> +++ libunbound/services/outside_network.c 24 Jan 2021 12:03:38 -
> @@ -1745,6 +1745,36 @@ select_id(struct outside_network* outnet
> return 1;
> }
>
> +/** return true is UDP connect error needs to be logged */
> +static int udp_connect_needs_log(int err)
> +{
> + switch(err) {
> + case ECONNREFUSED:
> +# ifdef ENETUNREACH
> + case ENETUNREACH:
> +# endif
> +# ifdef EHOSTDOWN
> + case EHOSTDOWN:
> +# endif
> +# ifdef EHOSTUNREACH
> + case EHOSTUNREACH:
> +# endif
> +# ifdef ENETDOWN
> + case ENETDOWN:
> +# endif
> +# ifdef EADDRNOTAVAIL
> + case EADDRNOTAVAIL:
> +# endif
> + if(verbosity >= VERB_ALGO)
> +
Re: [PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
Otto Moerbeek wrote: > On Sun, Jan 24, 2021 at 01:18:46PM +0100, Alejandro Colomar wrote: > > > This is useful for using tee to just write to a file, > > at the end of a pipeline, > > without having to redirect to /dev/null > > > > Example: > > > > echo 'foo' | sudo tee -q /etc/foo; > > > > is equivalent to the old (and ugly) > > You keep repeating "ugly" as the reason you are wanting this. > > I consider adding special options to command to solve an imagined > issue that can be solved with a general concept like redirection ugly. > Please stop pushing your diff to this list. So far nobody showed any > interest. I also see ZERO reason for this change. This change will encourage the creation of non-portable scripts, which harms backwards compatibility and portability, while increasing the cognitive cost of building software in a simple and useable command ecosystem.
Re: [PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
On Sun, Jan 24, 2021 at 01:18:46PM +0100, Alejandro Colomar wrote:
> This is useful for using tee to just write to a file,
> at the end of a pipeline,
> without having to redirect to /dev/null
>
> Example:
>
> echo 'foo' | sudo tee -q /etc/foo;
>
> is equivalent to the old (and ugly)
You keep repeating "ugly" as the reason you are wanting this.
I consider adding special options to command to solve an imagined
issue that can be solved with a general concept like redirection ugly.
Please stop pushing your diff to this list. So far nobody showed any
interest.
-Otto
>
> echo 'foo' | sudo tee /etc/foo >/dev/null;
>
> Signed-off-by: Alejandro Colomar
> ---
>
> Resend as v3. I forgot to change the subject line.
> Everything else is the same as in
> <20210123145356.53962-1-alx.manpa...@gmail.com>.
>
> src/tee.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/src/tee.c b/src/tee.c
> index c81faea91..1dfa92cf2 100644
> --- a/src/tee.c
> +++ b/src/tee.c
> @@ -45,6 +45,9 @@ static bool append;
> /* If true, ignore interrupts. */
> static bool ignore_interrupts;
>
> +/* Don't write to stdout */
> +static bool quiet;
> +
> enum output_error
>{
> output_error_sigpipe, /* traditional behavior, sigpipe enabled. */
> @@ -61,6 +64,8 @@ static struct option const long_options[] =
>{"append", no_argument, NULL, 'a'},
>{"ignore-interrupts", no_argument, NULL, 'i'},
>{"output-error", optional_argument, NULL, 'p'},
> + {"quiet", no_argument, NULL, 'q'},
> + {"silent", no_argument, NULL, 'q'},
>{GETOPT_HELP_OPTION_DECL},
>{GETOPT_VERSION_OPTION_DECL},
>{NULL, 0, NULL, 0}
> @@ -93,6 +98,7 @@ Copy standard input to each FILE, and also to standard
> output.\n\
> "), stdout);
>fputs (_("\
>-pdiagnose errors writing to non pipes\n\
> + -q, --quiet, --silent don't write to standard output\n\
>--output-error[=MODE] set behavior on write error. See MODE below\n\
> "), stdout);
>fputs (HELP_OPTION_DESCRIPTION, stdout);
> @@ -130,8 +136,9 @@ main (int argc, char **argv)
>
>append = false;
>ignore_interrupts = false;
> + quiet = false;
>
> - while ((optc = getopt_long (argc, argv, "aip", long_options, NULL)) != -1)
> + while ((optc = getopt_long (argc, argv, "aipq", long_options, NULL)) != -1)
> {
>switch (optc)
> {
> @@ -151,6 +158,10 @@ main (int argc, char **argv)
> output_error = output_error_warn_nopipe;
>break;
>
> +case 'q':
> + quiet = true;
> + break;
> +
> case_GETOPT_HELP_CHAR;
>
> case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
> @@ -235,8 +246,9 @@ tee_files (int nfiles, char **files)
> break;
>
>/* Write to all NFILES + 1 descriptors.
> - Standard output is the first one. */
> - for (i = 0; i <= nfiles; i++)
> + Standard output is the first one.
> + If 'quiet' is true, write to descriptors 1 and above (omit stdout)
> */
> + for (i = quiet; i <= nfiles; i++)
> if (descriptors[i]
> && fwrite (buffer, bytes_read, 1, descriptors[i]) != 1)
>{
> --
> 2.30.0
>
Re: unwind(8): Implement DNS64 synthesis.
On Sun, Jan 24, 2021 at 04:55:25PM +0100, Florian Obser wrote:
> Are you sure you are running with the config you think you are running
> with? I can not reproduce and from the logging, especially the
> check_resolver_done bits it very much looks like you are running
> without any config at all.
I'm sorry, you are right wrt. to getting a synthesized answer:
that only happens with `dhcp' or `stub'.
> I don't know. So you find yourself in a IPv6 only network which does
> provide a NAT64 service, you'd rather not be able to reach IPv4-only
> services?
For user experience I'd say no, better do that and make things work.
On the other hand, when LAN resolvers are specifically disabled (not
enabled in `preferences {}') one might argue that NAT64 synthesis based
on the LAN's gateway should not be done either.
> Note that from the point of view of DNS you are talking to the right
> destination, we will only do synthesis if we are not receiving a bogus
> answer.
>
> From my point of view detecting presence of NAT64 and doing synthesis
> is independent of the resolving strategy preference. We are not asking
> dhcp or the stub to do resolving, we fire up one query to detect the
> NAT64 prefix.
Right.
> Are you saying you need to switch to turn off DNS64, or is it fine
> like this and you were just surprised?
It's fine and I've probably misinterpreted things in that I thought of
both detection and synthesis as part of the resolving strategy.
The diff is working and the code reads fine, thanks for your work and
feedback on this.
OK kn
Re: [PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
On 1/24/21 5:11 PM, Teran McKinney wrote: > On 2021-01-24 13-18-46, Alejandro Colomar wrote: >> This is useful for using tee to just write to a file, >> at the end of a pipeline, >> without having to redirect to /dev/null >> >> Example: >> >> echo 'foo' | sudo tee -q /etc/foo; >> >> is equivalent to the old (and ugly) >> >> echo 'foo' | sudo tee /etc/foo >/dev/null; >> >> Signed-off-by: Alejandro Colomar >> --- [...] > > Hi, > > Why is this a thing? > > The point of tee is to write a file *and* to stdout. If you don't want use > that, use: > > `> file` > > To overwrite. > > Or > > `>> file` > > To append. > > I guess the only reason this would be used is if you wanted to write > multiple files at the same time, which tee supports. > > -Teran > Hi Teran, The rationale is that protected files can't be modified with '>', unless you give superuser rights to the whole shell. If you want fine-grained control over the rights of the tools in the pipeline, you'll come to times where you want the write step to be the only one to have those, and you can't [sudo >] nor [sudo >>], which by the way I would consider an even better solution, but which would require much more work, and designing a completely new idiom for it, which would face much more objection too. Regards, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
Re: unwind(8): Implement DNS64 synthesis.
On Sun, Jan 24, 2021 at 11:12:49AM +0100, Klemens Nanni wrote:
> What I'm seeing here is that unwind forwards the very first query to my
> gateway (learned via SLAAC), that one succeeds, but all successive
> queries of A only domains do not work... that's what makes the query in
> my previous mail work. I can always reproduce it like that:
>
> $ doas rcctl restart unwind
> unwind(ok)
> unwind(ok)
> $ dig +noall +question +answer github.com
> ;github.com.IN
> github.com. 44 IN 64:ff9b::8c52:7904
> $ dig +noall +question +answer github.com
> ;github.com.IN
>
> Here's the output of `unwind -dv' (with 'udp connect failed' log spam
> for IPv4 addresses removed):
>
> check_resolver_done: dhcp: unknown
> check_resolver_done: stub: unknown
> check_resolver_done: oDoT-dhcp rcode: SERVFAIL
> check_resolver_done: recursor: unknown
> [127.0.0.1]:22827: github.com. IN ?
> try_next_resolver[+0ms]: recursor[validating] github.com. IN
> try_next_resolver[+6ms]: dhcp[validating] github.com. IN
> try_next_resolver[+6ms]: stub[resolving] github.com. IN
> try_next_resolver: could not find (any more) working resolvers
> resolve_done[stub]: github.com. IN rcode: NOERROR[0], elapsed:
> 1ms, running: 1
> check_resolver_done: oDoT-dhcp rcode: SERVFAIL
> [127.0.0.1]:5226: github.com. IN ?
> try_next_resolver[+0ms]: dhcp[validating] github.com. IN
> resolve_done[dhcp]: github.com. IN rcode: NOERROR[0], elapsed:
> 0ms, running: 1
>
Are you sure you are running with the config you think you are running
with? I can not reproduce and from the logging, especially the
check_resolver_done bits it very much looks like you are running
without any config at all.
$ obj/unwind -nvf ./unwind.conf
preference { recursor }
$ doas obj/unwind -dvf ./unwind.conf
check_resolver_done: recursor: unknown
[::1]:39989: ripe.net. IN ?
try_next_resolver[+0ms]: recursor[validating] ripe.net. IN
try_next_resolver: could not find (any more) working resolvers
resolve_done[recursor]: ripe.net. IN rcode: NOERROR[0], elapsed: 403ms,
running: 1
> > >
> > > So it seems unwind will effectively always synthesize, no matter what
> > > `preference {}' is used.
> > >
> > > I also verified that unwind always asks my autoconf resolver for
> > > ipv4only.arpa by using tcpdump and running with this diff
> >
> > Yes, that's intentional. That's how it learns the NAT64 prefix.
> Asking because my assumption was that, if I don't configured `stub' or
> `dhcp' in unwind.conf, the NAT64 would/should not be discovered.
> As in: Why does unwind do LAN specific DNS64 synthesis if I told it not
> to use any LAN specific resolvers.
>
> I'll assume my logic is flawed here.
[...]
> I assumed unwind would not do DNS64 at all in cases like
> `preference { resolver }'.
I don't know. So you find yourself in a IPv6 only network which does
provide a NAT64 service, you'd rather not be able to reach IPv4-only
services?
Note that from the point of view of DNS you are talking to the right
destination, we will only do synthesis if we are not receiving a bogus
answer.
>From my point of view detecting presence of NAT64 and doing synthesis
is independent of the resolving strategy preference. We are not asking
dhcp or the stub to do resolving, we fire up one query to detect the
NAT64 prefix.
Are you saying you need to switch to turn off DNS64, or is it fine
like this and you were just surprised?
--
I'm not entirely sure you are real.
Re: grep: add --null flag
Hi,
the diff looks good, i would change the wording in the manpage a bit, see
below.
Anyone else who wants to ok it?
/Benno
Omar Polo(o...@omarpolo.com) on 2021.01.22 12:19:08 +0100:
>
> quasi three-weekly ping.
>
> Is this such a bad idea?
>
> (TBH: I have still to look at how to write a regression test for this)
>
> Omar Polo writes:
>
> > Hello,
> >
> > There are various program, especially the one written with GNU grep in
> > mind, that expects various flags that grep in base doesn't have. While
> > some of the flags (like --color) can be easily worked out (i.e. by
> > patching/customising these programs) one thing that it isn't easily
> > workable is --null, because it alters the way grep outputs its data.
> >
> > --null makes grep output an ASCII NUL byte after the file name, so a
> > program can parse the output of grep unambiguously even when the file
> > names contains "funny" characters, such as a newline.
> >
> > GNU grep isn't the only one with a --null flag, also FreeBSD and NetBSD
> > grep do (at least by looking at their manpages), so it's somewhat
> > widespread.
> >
> > I searched the archives on marc.info but I haven't seen a previous
> > discussion about this.
> >
> > The following patch was tried against GNU grep (installed from packages)
> > and seem to behave consistently.
> >
> > I used the same text of the FreeBSD/NetBSD manpage for the description
> > of the --null option, but I really dislike it: it feels way to verbose
> > for what it's trying to say, but I wasn't able to come up with something
> > better.
> >
> > I'm not familiar at all with the grep codebase, so I hope I'm not
> > missing something. If this has some chances of being accepted, I guess
> > I should also add some regress test; I'll try to work on them soon, but
> > in the meantime I'm sending this.
> >
> > Thanks,
> >
> > Omar Polo
>
>
> diff e992327fc31d0277a6f8518613a7db1b9face78b /home/op/w/openbsd-src
> blob - 5cc228df222c54a0553f289b5da8bbbe6afd171e
> file + usr.bin/grep/grep.1
> --- usr.bin/grep/grep.1
> +++ usr.bin/grep/grep.1
> @@ -49,6 +49,7 @@
> .Op Fl -context Ns Op = Ns Ar num
> .Op Fl -label Ns = Ns Ar name
> .Op Fl -line-buffered
> +.Op Fl -null
> .Op Ar pattern
> .Op Ar
> .Ek
> @@ -297,6 +298,25 @@ instead of the filename before lines.
> Force output to be line buffered.
> By default, output is line buffered when standard output is a terminal
> and block buffered otherwise.
> +.It Fl -null
> +Output a zero byte (the ASCII NUL character) instead of the character
> +that normally follows a file name.
> +For example,
> +.Nm Fl l Fl -null
> +outputs a zero byte after each file name instead of the usual newline.
> +This option makes the output unambiguous, even in the presence of file
> +names containing unusual characters like newlines.
> +This option can be used with commands like
> +.Xr find 1
> +.Fl print0 Ns ,
> +.Xr perl 1
> +.Fl 0 Ns ,
> +.Xr sort 1
> +.Fl z Ns , and
> +.Xr args 1
xargs ?
> +.Fl 0
> +to process arbitrary file names, even those that contain newline
> +characters.
This list of commands is a bit verbose.
I would rephrase it to match find(1):
-null
Print the pathname of the current file to standard output, followed by
a null
character, suitable for use with the -0 option to xargs(1).
If we dont list all these commands in find(1), why here? The list might be
incomplete anyway.
> .El
> .Sh EXIT STATUS
> The
> blob - f41b5e20ca68c9e9a36d2f7dd3c44329c621f29b
> file + usr.bin/grep/grep.c
> --- usr.bin/grep/grep.c
> +++ usr.bin/grep/grep.c
> @@ -80,6 +80,7 @@ int vflag; /* -v: only show non-matching lines */
> int wflag; /* -w: pattern must start and end on word boundaries */
> int xflag; /* -x: pattern must match entire line */
> int lbflag;/* --line-buffered */
> +int nullflag; /* --null */
> const char *labelname; /* --label=name */
>
> int binbehave = BIN_FILE_BIN;
> @@ -89,6 +90,7 @@ enum {
> HELP_OPT,
> MMAP_OPT,
> LINEBUF_OPT,
> + NULL_OPT,
> LABEL_OPT,
> };
>
> @@ -134,6 +136,7 @@ static const struct option long_options[] =
> {"mmap",no_argument,NULL, MMAP_OPT},
> {"label", required_argument, NULL, LABEL_OPT},
> {"line-buffered", no_argument,NULL, LINEBUF_OPT},
> + {"null",no_argument,NULL, NULL_OPT},
> {"after-context", required_argument, NULL, 'A'},
> {"before-context", required_argument, NULL, 'B'},
> {"context", optional_argument, NULL, 'C'},
> @@ -436,6 +439,9 @@ main(int argc, char *argv[])
> case LINEBUF_OPT:
> lbflag = 1;
> break;
> + case NULL_OPT:
> + nullflag = 1;
> + break;
> case HELP_OPT:
> default:
>
snmpd: remove print_{verbose,debug}
Nothing seems to use them and I see no reason in the forseeable future
to start using them.
OK?
martijn@
Index: snmpd.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.91
diff -u -p -r1.91 snmpd.h
--- snmpd.h 22 Jan 2021 06:33:26 - 1.91
+++ snmpd.h 24 Jan 2021 13:48:15 -
@@ -780,8 +780,6 @@ ssize_t sendtofrom(int, void *, size_t,
socklen_t, struct sockaddr *, socklen_t);
ssize_t recvfromto(int, void *, size_t, int, struct sockaddr *,
socklen_t *, struct sockaddr *, socklen_t *);
-voidprint_debug(const char *, ...);
-voidprint_verbose(const char *, ...);
const char *log_in6addr(const struct in6_addr *);
const char *print_host(struct sockaddr_storage *, char *, size_t);
Index: util.c
===
RCS file: /cvs/src/usr.sbin/snmpd/util.c,v
retrieving revision 1.10
diff -u -p -r1.10 util.c
--- util.c 30 Jun 2020 17:11:49 - 1.10
+++ util.c 24 Jan 2021 13:48:15 -
@@ -152,30 +152,6 @@ recvfromto(int s, void *buf, size_t len,
return (ret);
}
-void
-print_debug(const char *emsg, ...)
-{
- va_list ap;
-
- if (log_getverbose() > 2) {
- va_start(ap, emsg);
- vfprintf(stderr, emsg, ap);
- va_end(ap);
- }
-}
-
-void
-print_verbose(const char *emsg, ...)
-{
- va_list ap;
-
- if (log_getverbose()) {
- va_start(ap, emsg);
- vfprintf(stderr, emsg, ap);
- va_end(ap);
- }
-}
-
const char *
log_in6addr(const struct in6_addr *addr)
{
[PATCH v3 (resend)] tee: Add -q, --quiet, --silent option to not write to stdout
This is useful for using tee to just write to a file,
at the end of a pipeline,
without having to redirect to /dev/null
Example:
echo 'foo' | sudo tee -q /etc/foo;
is equivalent to the old (and ugly)
echo 'foo' | sudo tee /etc/foo >/dev/null;
Signed-off-by: Alejandro Colomar
---
Resend as v3. I forgot to change the subject line.
Everything else is the same as in
<20210123145356.53962-1-alx.manpa...@gmail.com>.
src/tee.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/src/tee.c b/src/tee.c
index c81faea91..1dfa92cf2 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -45,6 +45,9 @@ static bool append;
/* If true, ignore interrupts. */
static bool ignore_interrupts;
+/* Don't write to stdout */
+static bool quiet;
+
enum output_error
{
output_error_sigpipe, /* traditional behavior, sigpipe enabled. */
@@ -61,6 +64,8 @@ static struct option const long_options[] =
{"append", no_argument, NULL, 'a'},
{"ignore-interrupts", no_argument, NULL, 'i'},
{"output-error", optional_argument, NULL, 'p'},
+ {"quiet", no_argument, NULL, 'q'},
+ {"silent", no_argument, NULL, 'q'},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{NULL, 0, NULL, 0}
@@ -93,6 +98,7 @@ Copy standard input to each FILE, and also to standard
output.\n\
"), stdout);
fputs (_("\
-pdiagnose errors writing to non pipes\n\
+ -q, --quiet, --silent don't write to standard output\n\
--output-error[=MODE] set behavior on write error. See MODE below\n\
"), stdout);
fputs (HELP_OPTION_DESCRIPTION, stdout);
@@ -130,8 +136,9 @@ main (int argc, char **argv)
append = false;
ignore_interrupts = false;
+ quiet = false;
- while ((optc = getopt_long (argc, argv, "aip", long_options, NULL)) != -1)
+ while ((optc = getopt_long (argc, argv, "aipq", long_options, NULL)) != -1)
{
switch (optc)
{
@@ -151,6 +158,10 @@ main (int argc, char **argv)
output_error = output_error_warn_nopipe;
break;
+case 'q':
+ quiet = true;
+ break;
+
case_GETOPT_HELP_CHAR;
case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
@@ -235,8 +246,9 @@ tee_files (int nfiles, char **files)
break;
/* Write to all NFILES + 1 descriptors.
- Standard output is the first one. */
- for (i = 0; i <= nfiles; i++)
+ Standard output is the first one.
+ If 'quiet' is true, write to descriptors 1 and above (omit stdout) */
+ for (i = quiet; i <= nfiles; i++)
if (descriptors[i]
&& fwrite (buffer, bytes_read, 1, descriptors[i]) != 1)
{
--
2.30.0
Re: unwind: silence "udp connect failed" errors
On Sun, Jan 24, 2021 at 12:52:50PM +0100, Theo Buehler wrote:
> Probably better to sync first with the corresponding unbound commit
> https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
> then adjust udp_connect_needs_log() as needed.
Good call, thanks.
Here's the combined diff that syncs with unbound and adds EADDRNOTAVAIL
in the same fashion.
In case that is OK, I'd commit sync and addition separately.
Feedback? OK?
Index: libunbound/services/outside_network.c
===
RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
retrieving revision 1.9
diff -u -p -r1.9 outside_network.c
--- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -
1.9
+++ libunbound/services/outside_network.c 24 Jan 2021 12:03:38 -
@@ -1745,6 +1745,36 @@ select_id(struct outside_network* outnet
return 1;
}
+/** return true is UDP connect error needs to be logged */
+static int udp_connect_needs_log(int err)
+{
+ switch(err) {
+ case ECONNREFUSED:
+# ifdef ENETUNREACH
+ case ENETUNREACH:
+# endif
+# ifdef EHOSTDOWN
+ case EHOSTDOWN:
+# endif
+# ifdef EHOSTUNREACH
+ case EHOSTUNREACH:
+# endif
+# ifdef ENETDOWN
+ case ENETDOWN:
+# endif
+# ifdef EADDRNOTAVAIL
+ case EADDRNOTAVAIL:
+# endif
+ if(verbosity >= VERB_ALGO)
+ return 1;
+ return 0;
+ default:
+ break;
+ }
+ return 1;
+}
+
+
/** Select random interface and port */
static int
select_ifport(struct outside_network* outnet, struct pending* pend,
@@ -1804,9 +1834,11 @@ select_ifport(struct outside_network* ou
/* connect() to the destination */
if(connect(fd, (struct sockaddr*)>addr,
pend->addrlen) < 0) {
- log_err_addr("udp connect failed",
- strerror(errno), >addr,
- pend->addrlen);
+ if(udp_connect_needs_log(errno)) {
+ log_err_addr("udp connect
failed",
+ strerror(errno),
>addr,
+ pend->addrlen);
+ }
sock_close(fd);
return 0;
}
Re: unwind: silence "udp connect failed" errors
On Sun, Jan 24, 2021 at 12:44:39PM +0100, Klemens Nanni wrote:
> unwind/libunbound always tries to connect to nameservers using both
> address families, even if only one is configured on the local machine.
>
> So on IPv6 only boxes for example syslog gets spammed with these
>
> Jan 24 12:23:06 eru unwind[38261]: [38261:0] error: udp connect failed: Can't
> assign requested address for 195.54.164.36 port 53
>
> grep -c 'unwind.*udp connect failed' /var/log/daemon
> 1278
>
> Diff below makes unwind not log this iff connect(2) yielded
> EADDRNOTAVAIL.
>
> This diff wouldn't make sense for libunbound upstream or say a router
> running unbound where addresses are configured statically and admins do
> want to see such connect failures.
>
> But with unwind on roaming clients I don't see much value in logging it,
> especially not if unwind eventually answers the query successfully.
>
> Feedback? Objections? OK?
Probably better to sync first with the corresponding unbound commit
https://cvsweb.openbsd.org/src/usr.sbin/unbound/services/outside_network.c#rev1.21
then adjust udp_connect_needs_log() as needed.
>
>
> Index: libunbound/services/outside_network.c
> ===
> RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 outside_network.c
> --- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -
> 1.9
> +++ libunbound/services/outside_network.c 24 Jan 2021 11:27:04 -
> @@ -1803,7 +1803,8 @@ select_ifport(struct outside_network* ou
> if(outnet->udp_connect) {
> /* connect() to the destination */
> if(connect(fd, (struct sockaddr*)>addr,
> - pend->addrlen) < 0) {
> + pend->addrlen) < 0 &&
> + errno != EADDRNOTAVAIL) {
> log_err_addr("udp connect failed",
> strerror(errno), >addr,
> pend->addrlen);
>
unwind: silence "udp connect failed" errors
unwind/libunbound always tries to connect to nameservers using both
address families, even if only one is configured on the local machine.
So on IPv6 only boxes for example syslog gets spammed with these
Jan 24 12:23:06 eru unwind[38261]: [38261:0] error: udp connect failed: Can't
assign requested address for 195.54.164.36 port 53
grep -c 'unwind.*udp connect failed' /var/log/daemon
1278
Diff below makes unwind not log this iff connect(2) yielded
EADDRNOTAVAIL.
This diff wouldn't make sense for libunbound upstream or say a router
running unbound where addresses are configured statically and admins do
want to see such connect failures.
But with unwind on roaming clients I don't see much value in logging it,
especially not if unwind eventually answers the query successfully.
Feedback? Objections? OK?
Index: libunbound/services/outside_network.c
===
RCS file: /cvs/src/sbin/unwind/libunbound/services/outside_network.c,v
retrieving revision 1.9
diff -u -p -r1.9 outside_network.c
--- libunbound/services/outside_network.c 11 Dec 2020 12:21:40 -
1.9
+++ libunbound/services/outside_network.c 24 Jan 2021 11:27:04 -
@@ -1803,7 +1803,8 @@ select_ifport(struct outside_network* ou
if(outnet->udp_connect) {
/* connect() to the destination */
if(connect(fd, (struct sockaddr*)>addr,
- pend->addrlen) < 0) {
+ pend->addrlen) < 0 &&
+ errno != EADDRNOTAVAIL) {
log_err_addr("udp connect failed",
strerror(errno), >addr,
pend->addrlen);
some fvwmrc defautlts changes
Hi, The patch below aims a making the default system.fvwmrc a bit less obsolete. - remove xman (was removed from xenocara base) - replace rlogin with ssh in modules - make the fg color for ssh / telnet prompts black so that's it's readable. Comments? oks? Index: sample.fvwmrc/system.fvwmrc === RCS file: /cvs/OpenBSD/xenocara/app/fvwm/sample.fvwmrc/system.fvwmrc,v retrieving revision 1.9 diff -u -p -u -r1.9 system.fvwmrc --- sample.fvwmrc/system.fvwmrc 30 Sep 2020 20:42:12 - 1.9 +++ sample.fvwmrc/system.fvwmrc 24 Jan 2021 11:03:08 - @@ -55,7 +55,6 @@ Style "xbiff" NoTitle, Sticky, Win Style "xcalc" Icon xcalc.xpm, NoButton 2,RandomPlacement,ClickToFocus Style "xmh" Icon mail1.xpm, NoIconTitle,StickyIcon Style "xmh"RandomPlacement, NoButton 2 -Style "xman" Icon xman.xpm, RandomPlacement, ClickToFocus Style "xmag" Icon mag_glass.xpm, RandomPlacement, ClickToFocus Style "xgraph" Icon graphs.xpm, RandomPlacement, ClickToFocus Style "xmosaic" Color Green/Yellow, ClickToFocus @@ -115,7 +114,6 @@ AddToMenu RootMenu "Root Menu" Title AddToMenu Utilities "Utilities" Title + "Top" Exec exec xterm -T Top -n Top -e top + "Calculator"Exec exec xcalc -+ "Xman" Exec exec xman + "Xmag" Exec exec xmag + "Editres" Exec exec editres + "" Nop @@ -200,7 +198,7 @@ AddToMenu Module-Popup "FvwmModules" Tit + "IconBox" Module FvwmIconBox + "IconMan" Module FvwmIconMan + "" Nop -+ "Form - Rlogin" Module FvwmForm Rlogin ++ "Form - Ssh"Module FvwmForm Ssh + "Form - MyFvwmTalk" Module FvwmForm MyFvwmTalk + "Form - QuitVerify" Module FvwmForm QuitVerify @@ -259,8 +257,8 @@ AddToFunc PrintReverseFunction "I" Rais AddToFunc Iconify-and-Raise "I" Iconify + "I" Raise -# RLOGIN machine fg bg -AddToFunc RLOGIN "I" Exec xterm -fg $1 -bg $2 -e rlogin $0 -8 +# SSH machine fg bg +AddToFunc SSH "I" Exec xterm -fg $1 -bg $2 -e ssh $0 # TELNET machine fg bg AddToFunc TELNET "I" Exec xterm -fg $1 -bg $2 -e telnet $0 @@ -421,43 +419,43 @@ Key F8A M CirculateDown # *FvwmIconBox"Fvwm*" - -# FvwmForm alias - rlogin or telnet to host via xterm -*RloginWarpPointer -*RloginFont *helvetica*m*r*n*12* -*RloginButtonFont *helvetica*m*o*n*12* -*RloginInputFont*cour*m*r*n*12* -*RloginFore Black -*RloginBack White -*RloginItemFore Yellow -*RloginItemBack Wheat +# FvwmForm alias - ssh or telnet to host via xterm +*SshWarpPointer +*SshFont *helvetica*m*r*n*12* +*SshButtonFont *helvetica*m*o*n*12* +*SshInputFont*cour*m*r*n*12* +*SshFore Black +*SshBack White +*SshItemFore Black +*SshItemBack Wheat # begin items -*RloginLine center -*RloginText "Login to Remote Host" -*RloginLine center -*RloginText "Host:" -*RloginInputHostName 30 "" -*RloginLine center -*RloginSelectionmeth single -*RloginChoice TN TN off "telnet" -*RloginChoice RL RL on "rlogin" -*RloginSelectionUserSel single -#*RloginChoice Default Default on "same user" -#*RloginChoice CustomCustomoff "user:" -*RloginText "(Userid:" -*RloginInputUserName 10 "" -*RloginText ")" -*RloginLine center -*RloginText "FG:" -*RloginInputFgColor 15 "" -*RloginText "BG:" -*RloginInputBgColor 15 "" -*RloginLine expand -*RloginButton quit "Login" ^M -*RloginCommand Exec xterm $(FgColor?-fg $(FgColor)) $(BgColor?-bg $(BgColor)) -T xterm@$(HostName) -e $(RL?rlogin) $(TN?telnet) $(HostName) $(RL?-8 $(UserName?-l $(UserName))) -*RloginButton restart "Clear" -*RloginCommand Beep -*RloginButton quit "Cancel" -*RloginCommand Nop +*SshLine center +*SshText "Login to Remote Host" +*SshLine center +*SshText "Host:" +*SshInputHostName 30 "" +*SshLine center +*SshSelectionmeth single +*SshChoice TN TN off "telnet" +*SshChoice RL RL on "ssh" +*SshSelectionUserSel single +#*SshChoice Default Default on "same user" +#*SshChoice CustomCustomoff "user:" +*SshText "(Userid:" +*SshInputUserName 10 "" +*SshText ")" +*SshLine center +*SshText "FG:" +*SshInputFgColor 15 "" +*SshText "BG:" +*SshInputBgColor 15 "" +*SshLine expand +*SshButton quit "Login" ^M +*SshCommand Exec xterm
Re: unwind(8): Implement DNS64 synthesis.
On Sun, Jan 24, 2021 at 10:14:22AM +0100, Florian Obser wrote:
> On Sun, Jan 24, 2021 at 09:35:26AM +0100, Klemens Nanni wrote:
> > On Thu, Jan 21, 2021 at 05:16:24PM +0100, Florian Obser wrote:
> > > When unwind(8) learns new autoconf resolvers (from dhcp or router
> > > advertisements) it checks if a DNS64 is present in this network
> > > location and tries to recover the IPv6 prefix used according to
> > > RFC7050.
> > I noticed that unwind learns autoconf resolvers regardless of "stub" or
> > "dhcp" being present in unwind.conf:
> >
> > $ doas rcctl restart unwind
> > unwind(ok)
> > unwind(ok)
> > $ unwind -nv
> > preference { recursor }
> > $ unwindctl status autoconf
> > autoconfiguration forwarders:
> > SLAAC[trunk0]: fd00::1
>
> Yes, the config is present, it's just not being used.
> You can also define forwarders or DoT forwarders and not use them.
>
> > $ dig +short @::1 github.com.
> > 64:ff9b::8c52:7903
> >
> > This happens without your DNS64 diff already.
>
> This does not make any sense, is your gateway intercepting DNS
> queries?
To be clear, "happens already" was specific to the config being
present/shown but not used.
Regarding my network: No, not at all. My gateway does NAT64 and DNS64
without any interception, `divert-to' or whatever.
What I'm seeing here is that unwind forwards the very first query to my
gateway (learned via SLAAC), that one succeeds, but all successive
queries of A only domains do not work... that's what makes the query in
my previous mail work. I can always reproduce it like that:
$ doas rcctl restart unwind
unwind(ok)
unwind(ok)
$ dig +noall +question +answer github.com
;github.com.IN
github.com. 44 IN 64:ff9b::8c52:7904
$ dig +noall +question +answer github.com
;github.com.IN
Here's the output of `unwind -dv' (with 'udp connect failed' log spam
for IPv4 addresses removed):
check_resolver_done: dhcp: unknown
check_resolver_done: stub: unknown
check_resolver_done: oDoT-dhcp rcode: SERVFAIL
check_resolver_done: recursor: unknown
[127.0.0.1]:22827: github.com. IN ?
try_next_resolver[+0ms]: recursor[validating] github.com. IN
try_next_resolver[+6ms]: dhcp[validating] github.com. IN
try_next_resolver[+6ms]: stub[resolving] github.com. IN
try_next_resolver: could not find (any more) working resolvers
resolve_done[stub]: github.com. IN rcode: NOERROR[0], elapsed:
1ms, running: 1
check_resolver_done: oDoT-dhcp rcode: SERVFAIL
[127.0.0.1]:5226: github.com. IN ?
try_next_resolver[+0ms]: dhcp[validating] github.com. IN
resolve_done[dhcp]: github.com. IN rcode: NOERROR[0], elapsed:
0ms, running: 1
> >
> > So it seems unwind will effectively always synthesize, no matter what
> > `preference {}' is used.
> >
> > I also verified that unwind always asks my autoconf resolver for
> > ipv4only.arpa by using tcpdump and running with this diff
>
> Yes, that's intentional. That's how it learns the NAT64 prefix.
Asking because my assumption was that, if I don't configured `stub' or
`dhcp' in unwind.conf, the NAT64 would/should not be discovered.
As in: Why does unwind do LAN specific DNS64 synthesis if I told it not
to use any LAN specific resolvers.
I'll assume my logic is flawed here.
> They way NAT64/DNS64 is normally setup is that the gateway performes
> NAT64 and Router Advertisements contain nameservers that perform
> DNS64. So we ask those for a name that is known to not have a
> record to force them to perform synthesis.
>
> As in, we have to ask those, we can't just do our own recurion and
> hope that someone intercepts our query. See check_dns64, that
> intentionally uses ASR to learn the prefix.
Sure, that makes sense; see above.
I assumed unwind would not do DNS64 at all in cases like
`preference { resolver }'.
Re: Wireguard: can't remove multiple peers at once.
Hi,
> On 14/01/2021, at 8:33 PM, YASUOKA Masahiko wrote:
>
> Hi,
>
> On Thu, 14 Jan 2021 08:54:36 +0900
> Yuichiro NAITO wrote:
>> Does anybody please review my code?
>>
>> Yasuoka-san is my coleague of my work.
>> So, he is interested in this topic. That’s why I CCed this mail.
>> I don’t mean he is an reviewer.
>>
>>> 2021/01/12 11:27、Yuichiro NAITO のメール:
>>> I have set up multiple peers in a wg0 interface,
>>> and tried to remove more than one peers at once.
>>> Ifconfig(1) only removes the first peer.
>>>
>>> Command line was like following.
>>>
>>> ```
>>> # ifconfig wg0 -wgpeer -wgpeer -wgpeer
>>> ```
>>>
>>> Only was removed.
>>>
>>> I think next peer pointer isn't calculated in case of removing peer
>>> in sys/net/if_wg.c: wg_ioctl_set() function.
>>>
>>> I have tried following patch that can fix this problem.
>
> Yes, the diff seems good.
>
> I made the following whitespace change.
>
>> @@ -2333,6 +2333,11 @@ wg_ioctl_set(struct wg_softc *sc, struct wg_data_io
>> *data)
>> }
>>
>> peer_p = (struct wg_peer_io *)aip_p;
>> +continue;
>> +next_peer:
>> +aip_p = _p->p_aips[0];
>> +aip_p += peer_o.p_aips_count;
>> +peer_p = (struct wg_peer_io *)aip_p;
>> }
>>
>> error:
>
> It seems we prefer putting goto labels at the beginning of the line.
>
>
> ok?
ok procter@
>
> Fix wg(4) ioctl to be able to handle multiple wgpeers.
> Diff from Yuichiro NAITO.
>
> Index: sys/net/if_wg.c
> ===
> RCS file: /cvs/src/sys/net/if_wg.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 if_wg.c
> --- sys/net/if_wg.c 1 Sep 2020 19:06:59 - 1.14
> +++ sys/net/if_wg.c 14 Jan 2021 07:26:48 -
> @@ -2270,7 +2270,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
>
> /* Peer must have public key */
> if (!(peer_o.p_flags & WG_PEER_HAS_PUBLIC))
> - continue;
> + goto next_peer;
>
> /* 0 = latest protocol, 1 = this protocol */
> if (peer_o.p_protocol_version != 0) {
> @@ -2283,7 +2283,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
> /* Get local public and check that peer key doesn't match */
> if (noise_local_keys(>sc_local, public, NULL) == 0 &&
> bcmp(public, peer_o.p_public, WG_KEY_SIZE) == 0)
> - continue;
> + goto next_peer;
>
> /* Lookup peer, or create if it doesn't exist */
> if ((peer = wg_peer_lookup(sc, peer_o.p_public)) == NULL) {
> @@ -2291,7 +2291,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
>* Also, don't create a new one if we only want to
>* update. */
> if (peer_o.p_flags & (WG_PEER_REMOVE|WG_PEER_UPDATE))
> - continue;
> + goto next_peer;
>
> if ((peer = wg_peer_create(sc,
> peer_o.p_public)) == NULL) {
> @@ -2303,7 +2303,7 @@ wg_ioctl_set(struct wg_softc *sc, struct
> /* Remove peer and continue if specified */
> if (peer_o.p_flags & WG_PEER_REMOVE) {
> wg_peer_destroy(peer);
> - continue;
> + goto next_peer;
> }
>
> if (peer_o.p_flags & WG_PEER_HAS_ENDPOINT)
> @@ -2332,6 +2332,11 @@ wg_ioctl_set(struct wg_softc *sc, struct
> aip_p++;
> }
>
> + peer_p = (struct wg_peer_io *)aip_p;
> + continue;
> +next_peer:
> + aip_p = _p->p_aips[0];
> + aip_p += peer_o.p_aips_count;
> peer_p = (struct wg_peer_io *)aip_p;
> }
>
>
Re: unwind(8): Implement DNS64 synthesis.
On Sun, Jan 24, 2021 at 09:35:26AM +0100, Klemens Nanni wrote:
> On Thu, Jan 21, 2021 at 05:16:24PM +0100, Florian Obser wrote:
> > When unwind(8) learns new autoconf resolvers (from dhcp or router
> > advertisements) it checks if a DNS64 is present in this network
> > location and tries to recover the IPv6 prefix used according to
> > RFC7050.
> I noticed that unwind learns autoconf resolvers regardless of "stub" or
> "dhcp" being present in unwind.conf:
>
> $ doas rcctl restart unwind
> unwind(ok)
> unwind(ok)
> $ unwind -nv
> preference { recursor }
> $ unwindctl status autoconf
> autoconfiguration forwarders:
> SLAAC[trunk0]: fd00::1
Yes, the config is present, it's just not being used.
You can also define forwarders or DoT forwarders and not use them.
> $ dig +short @::1 github.com.
> 64:ff9b::8c52:7903
>
> This happens without your DNS64 diff already.
This does not make any sense, is your gateway intercepting DNS
queries?
>
> So it seems unwind will effectively always synthesize, no matter what
> `preference {}' is used.
>
> I also verified that unwind always asks my autoconf resolver for
> ipv4only.arpa by using tcpdump and running with this diff
Yes, that's intentional. That's how it learns the NAT64 prefix.
They way NAT64/DNS64 is normally setup is that the gateway performes
NAT64 and Router Advertisements contain nameservers that perform
DNS64. So we ask those for a name that is known to not have a
record to force them to perform synthesis.
As in, we have to ask those, we can't just do our own recurion and
hope that someone intercepts our query. See check_dns64, that
intentionally uses ASR to learn the prefix.
>
> Index: libunbound/dns64/dns64.c
> ===
> RCS file: /cvs/src/sbin/unwind/libunbound/dns64/dns64.c,v
> retrieving revision 1.4
> diff -u -p -U0 -r1.4 dns64.c
> --- libunbound/dns64/dns64.c 29 Aug 2020 08:22:42 - 1.4
> +++ libunbound/dns64/dns64.c 24 Jan 2021 07:58:10 -
> @@ -65 +65 @@
> -static const char DEFAULT_DNS64_PREFIX[] = "64:ff9b::/96";
> +static const char DEFAULT_DNS64_PREFIX[] = "2001:db9::/96";
>
> Is that intentional?
that code is not being used.
>
> > The learned autoconf resolvers are then prevented from upgrading to
> > the validating state since DNS64 breaks DNSSEC.
> >
> > unwind(8) can now perform its own synthesis. If a query for a
> > record results in no answer we re-send the query for A and if that
> > leads to an answer we synthesize an answer using the learned
> > prefixes.
> Using different configurations/preferences has been working just fine
> for me in IPv6 only networks, i.e. unwind properly picks up the DNS64
> prefix (if for example changed with unbound.conf(5)'s `dns64-prefix').
>
So are you saying this works or does not? It kinda sounds like you
claim it already worked without the diff?
--
I'm not entirely sure you are real.
Re: unwind(8): Implement DNS64 synthesis.
On Thu, Jan 21, 2021 at 05:16:24PM +0100, Florian Obser wrote:
> When unwind(8) learns new autoconf resolvers (from dhcp or router
> advertisements) it checks if a DNS64 is present in this network
> location and tries to recover the IPv6 prefix used according to
> RFC7050.
I noticed that unwind learns autoconf resolvers regardless of "stub" or
"dhcp" being present in unwind.conf:
$ doas rcctl restart unwind
unwind(ok)
unwind(ok)
$ unwind -nv
preference { recursor }
$ unwindctl status autoconf
autoconfiguration forwarders:
SLAAC[trunk0]: fd00::1
$ dig +short @::1 github.com.
64:ff9b::8c52:7903
This happens without your DNS64 diff already.
So it seems unwind will effectively always synthesize, no matter what
`preference {}' is used.
I also verified that unwind always asks my autoconf resolver for
ipv4only.arpa by using tcpdump and running with this diff
Index: libunbound/dns64/dns64.c
===
RCS file: /cvs/src/sbin/unwind/libunbound/dns64/dns64.c,v
retrieving revision 1.4
diff -u -p -U0 -r1.4 dns64.c
--- libunbound/dns64/dns64.c29 Aug 2020 08:22:42 - 1.4
+++ libunbound/dns64/dns64.c24 Jan 2021 07:58:10 -
@@ -65 +65 @@
-static const char DEFAULT_DNS64_PREFIX[] = "64:ff9b::/96";
+static const char DEFAULT_DNS64_PREFIX[] = "2001:db9::/96";
Is that intentional?
> The learned autoconf resolvers are then prevented from upgrading to
> the validating state since DNS64 breaks DNSSEC.
>
> unwind(8) can now perform its own synthesis. If a query for a
> record results in no answer we re-send the query for A and if that
> leads to an answer we synthesize an answer using the learned
> prefixes.
Using different configurations/preferences has been working just fine
for me in IPv6 only networks, i.e. unwind properly picks up the DNS64
prefix (if for example changed with unbound.conf(5)'s `dns64-prefix').
