Re: ral(4): add RT3290 support

2018-09-25 Thread Kevin Lo
On Mon, Sep 17, 2018 at 09:34:06PM -0400, James Hastings wrote:
> 
> 
> Ported from original vendor driver.
> RT3290 is similar to RT5390 but integrates WLAN + Bluetooth on single chip.
> Bluetooth not supported.

First off, thank you for working on this.  I tested it on amd64,
ifconfig ral0 up will cause system hang totally.  The adapter is good since
it works on Linux *shrug*.

> New 4kb firmware at /etc/firmware/ral-rt3290 for this chip only.
> New routines to read efuse rom and control wlan core.
> Tested on RT3090 and RT5390.
^^
Typo?  Did you test on RT3290?  Thanks.



Re: unveil manpage tweak 2

2018-09-25 Thread Jason McIntyre
On Wed, Sep 26, 2018 at 01:49:00PM +0800, Michael Mikonos wrote:
> Re-reading the unveil manual I found a typo which isn't flagged
> by a spell checker. Does anyone prefer just doing s/paths// though?
> 

morning. ok for your diff - no opinion on the s/paths// suggestion.

jmc

> 
> Index: unveil.2
> ===
> RCS file: /cvs/src/lib/libc/sys/unveil.2,v
> retrieving revision 1.10
> diff -u -p -u -r1.10 unveil.2
> --- unveil.2  26 Sep 2018 02:54:34 -  1.10
> +++ unveil.2  26 Sep 2018 05:44:09 -
> @@ -108,7 +108,7 @@ This means that a directory that is remo
>  .Fn unveil
>  will appear to not exist.
>  .Pp
> -Non-directories paths are remembered by name within their containing
> +Non-directory paths are remembered by name within their containing
>  directory, and so may be created, removed, or re-created after a call to
>  .Fn unveil
>  and still appear to exist.
> 



unveil manpage tweak 2

2018-09-25 Thread Michael Mikonos
Re-reading the unveil manual I found a typo which isn't flagged
by a spell checker. Does anyone prefer just doing s/paths// though?


Index: unveil.2
===
RCS file: /cvs/src/lib/libc/sys/unveil.2,v
retrieving revision 1.10
diff -u -p -u -r1.10 unveil.2
--- unveil.226 Sep 2018 02:54:34 -  1.10
+++ unveil.226 Sep 2018 05:44:09 -
@@ -108,7 +108,7 @@ This means that a directory that is remo
 .Fn unveil
 will appear to not exist.
 .Pp
-Non-directories paths are remembered by name within their containing
+Non-directory paths are remembered by name within their containing
 directory, and so may be created, removed, or re-created after a call to
 .Fn unveil
 and still appear to exist.



getent: use more appropiate types/limits around strtonum()

2018-09-25 Thread Klemens Nanni
Replace `long long id' with appropiate types and names, use smaller
limits where applicable and move variable declarations up out of loops.

This makes the code clearer and a tad simpler while staying consistent
across databases.

Feedback? OK?

Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.18
diff -u -p -r1.18 getent.c
--- getent.c25 Sep 2018 19:51:39 -  1.18
+++ getent.c25 Sep 2018 22:21:22 -
@@ -190,8 +190,10 @@ ethers(int argc, char *argv[])
 static int
 group(int argc, char *argv[])
 {
-   int i, rv = RV_OK;
struct group*gr;
+   const char  *err;
+   gid_t   gid;
+   int i, rv = RV_OK;
 
setgroupent(1);
if (argc == 2) {
@@ -199,11 +201,9 @@ group(int argc, char *argv[])
GROUPPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, &err);
-
+   gid = strtonum(argv[i], 0, GID_MAX, &err);
if (!err)
-   gr = getgrgid((gid_t)id);
+   gr = getgrgid(gid);
else
gr = getgrnam(argv[i]);
if (gr != NULL)
@@ -291,8 +291,10 @@ hosts(int argc, char *argv[])
 static int
 passwd(int argc, char *argv[])
 {
-   int i, rv = RV_OK;
struct passwd   *pw;
+   const char  *err;
+   uid_t   uid;
+   int i, rv = RV_OK;
 
setpassent(1);
if (argc == 2) {
@@ -300,11 +302,9 @@ passwd(int argc, char *argv[])
PASSWDPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, &err);
-
+   uid = strtonum(argv[i], 0, UID_MAX, &err);
if (!err)
-   pw = getpwuid((uid_t)id);
+   pw = getpwuid(uid);
else
pw = getpwnam(argv[i]);
if (pw != NULL)
@@ -327,6 +327,8 @@ static int
 protocols(int argc, char *argv[])
 {
struct protoent *pe;
+   const char  *err;
+   int proto;
int i, rv = RV_OK;
 
setprotoent(1);
@@ -335,11 +337,9 @@ protocols(int argc, char *argv[])
PROTOCOLSPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, &err);
-
+   proto = strtonum(argv[i], 0, INT_MAX, &err);
if (!err)
-   pe = getprotobynumber((int)id);
+   pe = getprotobynumber(proto);
else
pe = getprotobyname(argv[i]);
if (pe != NULL)
@@ -362,6 +362,8 @@ static int
 rpc(int argc, char *argv[])
 {
struct rpcent   *re;
+   const char  *err;
+   int rpc;
int i, rv = RV_OK;
 
setrpcent(1);
@@ -370,11 +372,9 @@ rpc(int argc, char *argv[])
RPCPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long id = strtonum(argv[i], 0, UINT_MAX, &err);
-
+   rpc = strtonum(argv[i], 0, INT_MAX, &err);
if (!err)
-   re = getrpcbynumber((int)id);
+   re = getrpcbynumber(rpc);
else
re = getrpcbyname(argv[i]);
if (re != NULL)
@@ -397,6 +397,9 @@ static int
 services(int argc, char *argv[])
 {
struct servent  *se;
+   const char  *err;
+   char*proto;
+   int serv;
int i, rv = RV_OK;
 
setservent(1);
@@ -405,15 +408,11 @@ services(int argc, char *argv[])
SERVICESPRINT;
} else {
for (i = 2; i < argc; i++) {
-   const char  *err;
-   long long   id;
-   char *proto = strchr(argv[i], '/');
-
-   if (proto != NULL)
+   if ((proto = strchr(argv[i], '/')) != NULL)
*proto++ = '\0';
-   id = strtonum(argv[i], 0, UINT_MAX, &err);
+   serv = strtonum(argv[i], 0, IPPORT_HILASTAUTO, &err);
if (!err)
-

Re: OpenBGPd Feature Request / Question if the Feature Request

2018-09-25 Thread Tom Smyth
Hello Gregory
,please find my responses in line

On Tue 25 Sep 2018, 15:54 Gregory Edigarov,  wrote:

>
>
> the whole discussion here reminds me somewhat about my idea I wanted to
> realize some time ago:
> suppose we have an imaginary "fast" router, which does hardware
> assisted  forwarding, and an OpenBGP(on a different host) lets call it
> RDE, where it will provide routes for this fast router, to be put into
> forwarding engine.
> then we would have the following advantages:
> 1. nice way to configure whatever we need in BGP (because we all know
> that bgpd.conf is a very nice thing)
>

Openbgpd in nice for a control plane

2. real packet forwarding will take place on the hardware, specifically
> built for this purpose.
>
Standard l3 switches can do this as long as they have
1. Sufficient cam /asic memory to accomodate the routes you intend to load
into switch forwarding plane
2. The switch os needs to support bgp and have sufficent memory + cpu to
allow some route filtering between rib and fib on the switch
3. Probably ospf also would be neededon the switch

3. if somebody would manufacture such say "dumb" forwarding planes, they
> would definitely cost less then a fully featured router.
>

Yeah i think a decent l3 switch (broadcom trident ii ) or better would do
what you are asking for

>
> is this a feasible idea or am I thinking completely wrong


I dont think u need a special manufacturer for what im proposing .


Re: [patch] Perl: update Scalar-List-Utils to 1.50

2018-09-25 Thread Andrew Hewus Fresh
On Tue, Sep 25, 2018 at 09:43:57PM +0100, Stuart Henderson wrote:
> On 2018/09/25 19:02, Charlene Wendling wrote:
> > Hi,  
> > 
> > I'm proposing here a diff to update Scalar-List-Utils to 1.50.
> > 
> > It will be needed to import p5-List-AllUtils, that in turn would allow
> > to import p5-GeoIP2 (currently in -wip) and update others like
> > p5-Audio-MPD.
 
> > 
> > [1] https://metacpan.org/changes/distribution/Scalar-List-Utils
>
> A quick and dirty search for '(List|Scalar|Sub)::Util' in source to all
> p5-* ports suggests it's used by 400+ things there, plus it's used by other
> bits of perl core so, even if it seems a relatively safe update, my
> feeling is that this is a bit late for 6.4.

That's kinda where the discussion we had led, that it was likely too
late for 6.4, but some folks were hoping and willing to put up a patch
and I said out-of-band that if sthen@ OKs it, I'm OK with it too :-)

1.50 is the version that's in perl 5.28 and I didn't find any reports of
breakage in a fairly quick search of the perl5-porters mailing list.


> When I looked at changelog it seems to me that 1.45 does have a breaking
> change, the old "uniq" was moved to "uniqstr" and a slightly different
> "uniq" was added, I couldn't say if this is _actually_ likely to be a
> problem though.

You'll notice that uniq was added in 1.44, while in-tree we have 1.42
(with patches), so although they did change uniq, we don't have either
version.


> My preferred approach would be to update all of perl core sometime not
> too long after a release (and after some initial ports testing) really..
> I know it's a bunch more work but I'm pretty sure we have the same
> problem with other modules too..

I do plan to get to that update shortly after the tree unlocks, and had
hoped to get to it before it got too late for 6.4, but sadly . . . .

l8rZ,
-- 
andrew - http://afresh1.com

Software doesn't do what you want it to do, it does what you tell it do.
  -- Stefan G. Weichinger.



Re: slow vim and malloc

2018-09-25 Thread Stuart Henderson
On 2018/09/25 22:24, Matthias Schmidt wrote:
> hi Stuart,
> 
> * Stuart Henderson wrote:
> > 
> > These settings in .vimrc make it quite noticeable
> > 
> > syn
> > set foldmethod=syntax
> > set foldlevelstart=1
> 
> Totally forget about this thread.  I had the same issue and the
> following options made vim + syntax highlighting at least a bit more
> usable for me.
> 
> let sh_minlines=100
> let sh_maxlines=600
> set synmaxcol=300
> 
> Cheers and HTH
> 
>   Matthias
> 

I've had a play with various "set re" options to use the old and new
RE engines, new engine speeds range from "similar to the old one" to
"much much slower than the old one" in my tests (depending on what
it's doing and depending on malloc.conf flags).

I considered changing the port back to using re=1 by default, but
I ran into out-of-bounds reads in some cases when opening a file with
syntax-highlighting enabled and paging through to the very end
(especially with "hard" malloc.conf flags) so have put it to one side
for now ...



Re: getent: usage() is void

2018-09-25 Thread Todd C. Miller
On Tue, 25 Sep 2018 21:43:41 +0200, Klemens Nanni wrote:

> OK?

I think you should use __dead here too.  E.g.

static void __dead usage(void);

 - todd

> Index: getent.c
> ===
> RCS file: /cvs/src/usr.bin/getent/getent.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 getent.c
> --- getent.c  25 Sep 2018 06:48:48 -  1.16
> +++ getent.c  25 Sep 2018 19:41:04 -
> @@ -55,7 +55,7 @@
>  
>  #include 
>  
> -static int   usage(void);
> +static void  usage(void);
>  static int   ethers(int, char *[]);
>  static int   group(int, char *[]);
>  static int   hosts(int, char *[]);
> @@ -115,12 +115,11 @@ main(int argc, char *argv[])
>   return RV_USAGE;
>  }
>  
> -static int
> +static void
>  usage(void)
>  {
>   fprintf(stderr, "usage: %s database [key ...]\n", __progname);
>   exit(RV_USAGE);
> - /* NOTREACHED */
>  }
>  
>  /*



Re: [patch] Perl: update Scalar-List-Utils to 1.50

2018-09-25 Thread Stuart Henderson
On 2018/09/25 19:02, Charlene Wendling wrote:
> Hi,  
> 
> I'm proposing here a diff to update Scalar-List-Utils to 1.50.
> 
> It will be needed to import p5-List-AllUtils, that in turn would allow
> to import p5-GeoIP2 (currently in -wip) and update others like
> p5-Audio-MPD.
> 
> The changelog indicates no backward incompatible changes between this
> version and 1.42 we're currently shipping [1].
> 
> Testing: 
> 
> - It passes Perl global test suite. 
> - 'make test' for p5-List-AllUtils runs fine as well, p5-Audio-MPD
> builds and runs with no issues.
> - Actually i've been asked to make it public for further testing ;)
> 
> Any comment/feedback is welcome! 
> 
> Charlène. 
> 
> 
> [1] https://metacpan.org/changes/distribution/Scalar-List-Utils



A quick and dirty search for '(List|Scalar|Sub)::Util' in source to all
p5-* ports suggests it's used by 400+ things there, plus it's used by other
bits of perl core so, even if it seems a relatively safe update, my
feeling is that this is a bit late for 6.4.

When I looked at changelog it seems to me that 1.45 does have a breaking
change, the old "uniq" was moved to "uniqstr" and a slightly different
"uniq" was added, I couldn't say if this is _actually_ likely to be a
problem though.

My preferred approach would be to update all of perl core sometime not
too long after a release (and after some initial ports testing) really..
I know it's a bunch more work but I'm pretty sure we have the same
problem with other modules too..



Re: slow vim and malloc

2018-09-25 Thread Matthias Schmidt
hi Stuart,

* Stuart Henderson wrote:
> 
> These settings in .vimrc make it quite noticeable
> 
> syn
> set foldmethod=syntax
> set foldlevelstart=1

Totally forget about this thread.  I had the same issue and the
following options made vim + syntax highlighting at least a bit more
usable for me.

let sh_minlines=100
let sh_maxlines=600
set synmaxcol=300

Cheers and HTH

Matthias



Re: [patch] Add IPv6 description for `-T' option in netcat manual

2018-09-25 Thread Jason McIntyre
On Tue, Sep 25, 2018 at 09:11:42AM +0800, Nan Xiao wrote:
> Hi tech@,
> 
> According to netcat source code, the `-T' option not only takes effect
> in IPv4 but also IPv6:
> 
>   if (Tflag != -1) {
>   if (af == AF_INET && setsockopt(s, IPPROTO_IP,
>   IP_TOS, &Tflag, sizeof(Tflag)) == -1)
>   err(1, "set IP ToS");
> 
>   else if (af == AF_INET6 && setsockopt(s, IPPROTO_IPV6,
>   IPV6_TCLASS, &Tflag, sizeof(Tflag)) == -1)
>   err(1, "set IPv6 traffic class");
>   }
> 
> So I think the following patch should be more accurate:
> 

fixed, thanks.
jmc

> diff --git nc.1 nc.1
> index cb391288f15..5588daf87ae 100644
> --- nc.1
> +++ nc.1
> @@ -241,7 +241,7 @@ Cannot be used together with
>  or
>  .Fl x .
>  .It Fl T Ar keyword
> -Change the IPv4 TOS value or the TLS options.
> +Change the IPv4 TOS/IPv6 Traffic Class value or the TLS options.
>  .Pp
>  For TLS options,
>  .Ar keyword
> @@ -269,7 +269,7 @@ for further details).
>  Specifying TLS options requires
>  .Fl c .
>  .Pp
> -For the IPv4 TOS value,
> +For the IPv4 TOS/IPv6 Traffic Class value,
>  .Ar keyword
>  may be one of
>  .Cm critical ,
> 
> 
> Thanks!
> 
> -- 
> Best Regards
> Nan Xiao
> 



Re: Minor clarification in ttys(5) man page

2018-09-25 Thread Jason McIntyre
On Sun, Sep 23, 2018 at 06:00:23PM -0400, Katherine Rohl wrote:
> You can reload the ttys file by sending SIGHUP to the init process. 
> init(8) mentions this but ttys(5) does not, which can be confusing for 
> users who don't know that. I've added a note to this effect to ttys(5) 
> referencing init(8).
> 
> Index: ttys.5
> ===
> RCS file: /cvs/src/libexec/getty/ttys.5,v
> retrieving revision 1.11
> diff -u -p -r1.11 ttys.5
> --- ttys.522 Oct 2008 22:16:16 -  1.11
> +++ ttys.523 Sep 2018 20:21:23 -
> @@ -129,6 +129,15 @@ may be followed by a quoted command stri
>   will execute
>   .Em before
>   starting the command specified by the second field.
> +.El
> +.Pp
> +Changes to the ttys file will not take effect until it is reloaded by
> +.Xr init 8 ,
> +which can be triggered without a reboot by sending it a terminal line 
> hangup
> +(HUP) signal. See the
> +.Xr init 8
> +man page for more information.
> +
>   .Sh FILES
>   .Bl -tag -width /etc/ttys -compact
>   .It Pa /etc/ttys
> 

i committed a shorter version of this (included below).
thanks for the mail,

jmc

Index: ttys.5
===
RCS file: /cvs/src/libexec/getty/ttys.5,v
retrieving revision 1.11
diff -u -r1.11 ttys.5
--- ttys.5  22 Oct 2008 22:16:16 -  1.11
+++ ttys.5  25 Sep 2018 20:01:15 -
@@ -129,6 +129,12 @@
 will execute
 .Em before
 starting the command specified by the second field.
+.Pp
+Changes to the ttys file take effect after it has been reloaded by
+.Xr init 8 ,
+which can be triggered by sending it a
+.Dv HUP
+signal.
 .Sh FILES
 .Bl -tag -width /etc/ttys -compact
 .It Pa /etc/ttys



getent: usage() is void

2018-09-25 Thread Klemens Nanni
OK?

Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.16
diff -u -p -r1.16 getent.c
--- getent.c25 Sep 2018 06:48:48 -  1.16
+++ getent.c25 Sep 2018 19:41:04 -
@@ -55,7 +55,7 @@
 
 #include 
 
-static int usage(void);
+static voidusage(void);
 static int ethers(int, char *[]);
 static int group(int, char *[]);
 static int hosts(int, char *[]);
@@ -115,12 +115,11 @@ main(int argc, char *argv[])
return RV_USAGE;
 }
 
-static int
+static void
 usage(void)
 {
fprintf(stderr, "usage: %s database [key ...]\n", __progname);
exit(RV_USAGE);
-   /* NOTREACHED */
 }
 
 /*



[patch] Perl: update Scalar-List-Utils to 1.50

2018-09-25 Thread Charlene Wendling
Hi,  

I'm proposing here a diff to update Scalar-List-Utils to 1.50.

It will be needed to import p5-List-AllUtils, that in turn would allow
to import p5-GeoIP2 (currently in -wip) and update others like
p5-Audio-MPD.

The changelog indicates no backward incompatible changes between this
version and 1.42 we're currently shipping [1].

Testing: 

- It passes Perl global test suite. 
- 'make test' for p5-List-AllUtils runs fine as well, p5-Audio-MPD
builds and runs with no issues.
- Actually i've been asked to make it public for further testing ;)

Any comment/feedback is welcome! 

Charlène. 


[1] https://metacpan.org/changes/distribution/Scalar-List-Utils


Scalar-List-Utils.diff
Description: Binary data


Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 11:42:26PM +0800, Michael Mikonos wrote:
> On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote:
> > On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> > > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > > > This is an example of better to start at just hoisting the code that
> > > > opens the many fds and put them all inside open_files(). After that it's
> > > > just a matter of calling pledge("stdio") and we are done.
> > > > 
> > > > Of course that after this is done we can still make a list of all the 
> > > > files
> > > > we need to open and unveil them, but not the way it's done here.
> > > > 
> > > > Once I get back home from $DAYJOB I'll try to have a look at this.
> > > 
> > > After open_files() the wpath pledge can be dropped. rpath is still
> > > needed because /tmp files are reopened for read in output(). cpath
> > > is needed because /tmp files are unlinked at the end. This patch
> > > adds a pledge call, but is it better to just move the first pledge()
> > > down?
> > > 
> > 
> > you could try with the "tmppath" promise. I will allow opening/creating
> > files on /tmp and unlinking them (but not sure it will cover all yacc
> > need as it is designed for mkstemp(3) family). Unveil for such
> > operations are fine too, without explicit unveil(2) call.
> > 
> 
> Ah, I see what you mean. pledging "tmppath" is kind of like unveil
> because the allowed operations only work under /tmp.
> It's possible to do this after calling open_files() because the only
> files (re)opened later are in /tmp.

My description was wrong. tmppath allows unlink of /tmp files at the
end. rpath is still needed to reopen the /tmp files.

> 
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/yacc/main.c,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 main.c
> --- main.c25 May 2017 20:11:03 -  1.29
> +++ main.c25 Sep 2018 15:38:18 -
> @@ -346,12 +346,16 @@ open_files(void)
>  int
>  main(int argc, char *argv[])
>  {
> - if (pledge("stdio rpath wpath cpath", NULL) == -1)
> + if (pledge("stdio rpath wpath cpath tmppath", NULL) == -1)
>   fatal("pledge: invalid arguments");
>  
>   set_signals();
>   getargs(argc, argv);
>   open_files();
> +
> + if (pledge("stdio rpath tmppath", NULL) == -1)
> + fatal("pledge: invalid arguments");
> +
>   reader();
>   lr0();
>   lalr();



Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 05:25:54PM +0200, Sebastien Marie wrote:
> On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> > On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > > This is an example of better to start at just hoisting the code that
> > > opens the many fds and put them all inside open_files(). After that it's
> > > just a matter of calling pledge("stdio") and we are done.
> > > 
> > > Of course that after this is done we can still make a list of all the 
> > > files
> > > we need to open and unveil them, but not the way it's done here.
> > > 
> > > Once I get back home from $DAYJOB I'll try to have a look at this.
> > 
> > After open_files() the wpath pledge can be dropped. rpath is still
> > needed because /tmp files are reopened for read in output(). cpath
> > is needed because /tmp files are unlinked at the end. This patch
> > adds a pledge call, but is it better to just move the first pledge()
> > down?
> > 
> 
> you could try with the "tmppath" promise. I will allow opening/creating
> files on /tmp and unlinking them (but not sure it will cover all yacc
> need as it is designed for mkstemp(3) family). Unveil for such
> operations are fine too, without explicit unveil(2) call.
> 

Ah, I see what you mean. pledging "tmppath" is kind of like unveil
because the allowed operations only work under /tmp.
It's possible to do this after calling open_files() because the only
files (re)opened later are in /tmp.


Index: main.c
===
RCS file: /cvs/src/usr.bin/yacc/main.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 main.c
--- main.c  25 May 2017 20:11:03 -  1.29
+++ main.c  25 Sep 2018 15:38:18 -
@@ -346,12 +346,16 @@ open_files(void)
 int
 main(int argc, char *argv[])
 {
-   if (pledge("stdio rpath wpath cpath", NULL) == -1)
+   if (pledge("stdio rpath wpath cpath tmppath", NULL) == -1)
fatal("pledge: invalid arguments");
 
set_signals();
getargs(argc, argv);
open_files();
+
+   if (pledge("stdio rpath tmppath", NULL) == -1)
+   fatal("pledge: invalid arguments");
+
reader();
lr0();
lalr();



Re: yacc + unveil

2018-09-25 Thread Sebastien Marie
On Tue, Sep 25, 2018 at 11:15:43PM +0800, Michael Mikonos wrote:
> On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> > This is an example of better to start at just hoisting the code that
> > opens the many fds and put them all inside open_files(). After that it's
> > just a matter of calling pledge("stdio") and we are done.
> > 
> > Of course that after this is done we can still make a list of all the files
> > we need to open and unveil them, but not the way it's done here.
> > 
> > Once I get back home from $DAYJOB I'll try to have a look at this.
> 
> After open_files() the wpath pledge can be dropped. rpath is still
> needed because /tmp files are reopened for read in output(). cpath
> is needed because /tmp files are unlinked at the end. This patch
> adds a pledge call, but is it better to just move the first pledge()
> down?
> 

you could try with the "tmppath" promise. I will allow opening/creating
files on /tmp and unlinking them (but not sure it will cover all yacc
need as it is designed for mkstemp(3) family). Unveil for such
operations are fine too, without explicit unveil(2) call.

-- 
Sebastien Marie



Re: yacc + unveil

2018-09-25 Thread Michael Mikonos
On Tue, Sep 25, 2018 at 03:22:38PM +0100, Ricardo Mestre wrote:
> This is an example of better to start at just hoisting the code that
> opens the many fds and put them all inside open_files(). After that it's
> just a matter of calling pledge("stdio") and we are done.
> 
> Of course that after this is done we can still make a list of all the files
> we need to open and unveil them, but not the way it's done here.
> 
> Once I get back home from $DAYJOB I'll try to have a look at this.

After open_files() the wpath pledge can be dropped. rpath is still
needed because /tmp files are reopened for read in output(). cpath
is needed because /tmp files are unlinked at the end. This patch
adds a pledge call, but is it better to just move the first pledge()
down?


Index: main.c
===
RCS file: /cvs/src/usr.bin/yacc/main.c,v
retrieving revision 1.29
diff -u -p -u -r1.29 main.c
--- main.c  25 May 2017 20:11:03 -  1.29
+++ main.c  25 Sep 2018 15:07:35 -
@@ -352,6 +352,10 @@ main(int argc, char *argv[])
set_signals();
getargs(argc, argv);
open_files();
+
+   if (pledge("stdio rpath cpath", NULL) == -1)
+   fatal("pledge: invalid arguments");
+
reader();
lr0();
lalr();



Re: OpenBGPd Feature Request / Question if the Feature Request

2018-09-25 Thread Gregory Edigarov



On 22.09.18 17:11, Tom Smyth wrote:

Hello Stuart,
Thanks for the feedback , I have responded to your feedback in line,
On Sat, 22 Sep 2018 at 10:07, Stuart Henderson  wrote:

Interesting idea but I think the method you're suggesting puts you at
higher risk of things *not* reaching their destination - if you have
good and not-so-good transits, the diffs are likely to be things you
don't want anyway and could interfere with correct routing.

I see your point I had not considered that failure mode.


Some providers have a bit of a problem with "stuck" routes (if there's
a stuck more-specific route at one provider, that will be a difference,
and you'll send traffic there on a road to nowhere instead of to a valid
less-specific).

I get what you are saying however this would still be a problem if I had full
views  (if I understand your point correctly, any more specific network that is
invalid would still blackhole traffic in the case where we have full tables )

also with that risk in mind I (think) such a setup would be better than
defaulting to a transit provider who has sent a withdraw message on a prefix



Another issue is that a good provider may have filtered a dubious
announcement (hijack attempt), a less fastidious one might not.

If I wanted to identify *whether* a transit provider is sending such
routes, analysing a diff of the announcements between them and another
provider is quite a good way to find them.


I agree with you  ... there would have to be some steady state comparison
between the two (or more) Transit providers in normal conditions...
the usual threats (hijacks etc would have to be dealt with with the usual
IRR filtersetc


My suggestion if you're trying to use the hardware in this way: only use
transit providers who can be trusted to generally provide good transit
(which rules out a few ;) and just use defaults plus maybe allow some
extra through filters to encourage certain traffic to go a certain way,
or to balance load if your ports are hot.

Yeah ... that would be the (rough) plan .. .or build workaround filters
on the less well configured transit providers,

there are other things to consider which makes it a tough(er) problem to solve
what happens when your default provider looses connectivity with (most) of
its peers then it would be better to swap the default route to the other transit
provider and install exception routes (if any)


the whole discussion here reminds me somewhat about my idea I wanted to 
realize some time ago:
suppose we have an imaginary "fast" router, which does hardware 
assisted  forwarding, and an OpenBGP(on a different host) lets call it 
RDE, where it will provide routes for this fast router, to be put into 
forwarding engine.

then we would have the following advantages:
1. nice way to configure whatever we need in BGP (because we all know 
that bgpd.conf is a very nice thing)
2. real packet forwarding will take place on the hardware, specifically 
built for this purpose.
3. if somebody would manufacture such say "dumb" forwarding planes, they 
would definitely cost less then a fully featured router.


is this a feasible idea or am I thinking completely wrong ?



Re: yacc + unveil

2018-09-25 Thread Ricardo Mestre
This is an example of better to start at just hoisting the code that
opens the many fds and put them all inside open_files(). After that it's
just a matter of calling pledge("stdio") and we are done.

Of course that after this is done we can still make a list of all the files
we need to open and unveil them, but not the way it's done here.

Once I get back home from $DAYJOB I'll try to have a look at this.

On 08:04 Tue 25 Sep , Theo de Raadt wrote:
> Theo de Raadt  wrote:
> 
> > Michael Mikonos  wrote:
> > 
> > > On Mon, Sep 24, 2018 at 10:53:47PM -0600, Theo de Raadt wrote:
> > > > Ugh.  A diff which doens't check error returns.  Averting my gaze
> > > > is similar to "no way".  Hope you have another quarter, because you
> > > > need to try again
> > > 
> > > Oops... new coin inserted. I decided to create a fatal_perror()
> > > function which behaves like perror(). yacc's error.c didn't seem
> > > to have anything like that. Now if either pledge() or unveil()
> > > fails we see the appropriate error string instead of always seeing
> > > "invalid arguments" for pledge().
> > 
> > I don't understand parts of the diff.
> > 
> > > Index: defs.h
> > > ===
> > > RCS file: /cvs/src/usr.bin/yacc/defs.h,v
> > > retrieving revision 1.18
> > > diff -u -p -u -r1.18 defs.h
> > > --- defs.h2 Dec 2014 15:56:22 -   1.18
> > > +++ defs.h25 Sep 2018 05:34:27 -
> > > @@ -307,6 +307,7 @@ extern void closure(short *, int);
> > >  extern void finalize_closure(void);
> > >  
> > >  extern __dead void fatal(char *);
> > > +extern __dead void fatal_perror(char *);
> > >  
> > >  extern void reflexive_transitive_closure(unsigned *, int);
> > >  extern __dead void done(int);
> > > Index: error.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/yacc/error.c,v
> > > retrieving revision 1.14
> > > diff -u -p -u -r1.14 error.c
> > > --- error.c   8 Mar 2014 01:05:39 -   1.14
> > > +++ error.c   25 Sep 2018 05:34:27 -
> > > @@ -35,6 +35,8 @@
> > >  
> > >  /* routines for printing error messages  */
> > >  
> > > +#include 
> > > +
> > >  #include "defs.h"
> > >  
> > >  
> > > @@ -45,6 +47,12 @@ fatal(char *msg)
> > >   done(2);
> > >  }
> > >  
> > > +void
> > > +fatal_perror(char *msg)
> > > +{
> > > + fprintf(stderr, "%s: %s\n", msg, strerror(errno));
> > > + done(2);
> > > +}
> > >  
> > >  void
> > >  no_space(void)
> > > Index: main.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/yacc/main.c,v
> > > retrieving revision 1.29
> > > diff -u -p -u -r1.29 main.c
> > > --- main.c25 May 2017 20:11:03 -  1.29
> > > +++ main.c25 Sep 2018 05:34:27 -
> > > @@ -305,10 +305,14 @@ open_files(void)
> > >   create_file_names();
> > >  
> > >   if (input_file == 0) {
> > > + if (unveil(input_file_name, "r") == -1)
> > > + fatal_perror("unveil");
> > >   input_file = fopen(input_file_name, "r");
> > 
> > At this point, all files are allowed.
> > So you restrict it to one.
> > Then you open it.
> > You won't open it again.
> > Why does it remain on the permitted open list?
> > Why not just open it, and not have it on the permitted open list?
> > 
> > Meanwhile, unveil hasn't been blocked.  So if someone finds a bug
> > at this point which gives them control, they can continue calling
> > unveil, and get full filesystem access back.
> 
> 
> As another way of explaining, the logic you created goes like this
> 
>allow only this file
>open it
>do some more calculation
>oh wait, allow this file also!
>open it
>do some more calculation
>OH WAIT, here is another file to open...
> 



[patch]Modify the example code in write(2) manual

2018-09-25 Thread Nan Xiao
Hi tech@,

I am reading write(2) manual, and come across the following example:

for (off = 0; off < bsz; off += nw)
if ((nw = write(d, buf + off, bsz - off)) == 0 || nw == -1)
err(1, "write");

I am just wondering when the write(2) will return 0? If in some cases,
it will indeed return 0, according to the manual:

> Upon successful completion the number of bytes which were written is
returned. Otherwise, a -1 is returned and the global variable errno is
set to indicate the error.

Because the errno is only set when return value is -1, if write(2)
returns 0, the errno should be an undefined value, and "err(1,
"write");" also won't print correct information.

If write(2) won't return 0, my following patch fixes the example code:

diff --git write.2 write.2
index c1686b1a910..db134959002 100644
--- write.2
+++ write.2
@@ -164,7 +164,7 @@ ssize_t nw;
 int d;

 for (off = 0; off < bsz; off += nw)
-   if ((nw = write(d, buf + off, bsz - off)) == 0 || nw == -1)
+   if ((nw = write(d, buf + off, bsz - off)) == -1)
err(1, "write");
 .Ed
 .Sh ERRORS

Thanks!

-- 
Best Regards
Nan Xiao



Re: yacc + unveil

2018-09-25 Thread Theo de Raadt
Theo de Raadt  wrote:

> Michael Mikonos  wrote:
> 
> > On Mon, Sep 24, 2018 at 10:53:47PM -0600, Theo de Raadt wrote:
> > > Ugh.  A diff which doens't check error returns.  Averting my gaze
> > > is similar to "no way".  Hope you have another quarter, because you
> > > need to try again
> > 
> > Oops... new coin inserted. I decided to create a fatal_perror()
> > function which behaves like perror(). yacc's error.c didn't seem
> > to have anything like that. Now if either pledge() or unveil()
> > fails we see the appropriate error string instead of always seeing
> > "invalid arguments" for pledge().
> 
> I don't understand parts of the diff.
> 
> > Index: defs.h
> > ===
> > RCS file: /cvs/src/usr.bin/yacc/defs.h,v
> > retrieving revision 1.18
> > diff -u -p -u -r1.18 defs.h
> > --- defs.h  2 Dec 2014 15:56:22 -   1.18
> > +++ defs.h  25 Sep 2018 05:34:27 -
> > @@ -307,6 +307,7 @@ extern void closure(short *, int);
> >  extern void finalize_closure(void);
> >  
> >  extern __dead void fatal(char *);
> > +extern __dead void fatal_perror(char *);
> >  
> >  extern void reflexive_transitive_closure(unsigned *, int);
> >  extern __dead void done(int);
> > Index: error.c
> > ===
> > RCS file: /cvs/src/usr.bin/yacc/error.c,v
> > retrieving revision 1.14
> > diff -u -p -u -r1.14 error.c
> > --- error.c 8 Mar 2014 01:05:39 -   1.14
> > +++ error.c 25 Sep 2018 05:34:27 -
> > @@ -35,6 +35,8 @@
> >  
> >  /* routines for printing error messages  */
> >  
> > +#include 
> > +
> >  #include "defs.h"
> >  
> >  
> > @@ -45,6 +47,12 @@ fatal(char *msg)
> > done(2);
> >  }
> >  
> > +void
> > +fatal_perror(char *msg)
> > +{
> > +   fprintf(stderr, "%s: %s\n", msg, strerror(errno));
> > +   done(2);
> > +}
> >  
> >  void
> >  no_space(void)
> > Index: main.c
> > ===
> > RCS file: /cvs/src/usr.bin/yacc/main.c,v
> > retrieving revision 1.29
> > diff -u -p -u -r1.29 main.c
> > --- main.c  25 May 2017 20:11:03 -  1.29
> > +++ main.c  25 Sep 2018 05:34:27 -
> > @@ -305,10 +305,14 @@ open_files(void)
> > create_file_names();
> >  
> > if (input_file == 0) {
> > +   if (unveil(input_file_name, "r") == -1)
> > +   fatal_perror("unveil");
> > input_file = fopen(input_file_name, "r");
> 
> At this point, all files are allowed.
> So you restrict it to one.
> Then you open it.
> You won't open it again.
> Why does it remain on the permitted open list?
> Why not just open it, and not have it on the permitted open list?
> 
> Meanwhile, unveil hasn't been blocked.  So if someone finds a bug
> at this point which gives them control, they can continue calling
> unveil, and get full filesystem access back.


As another way of explaining, the logic you created goes like this

   allow only this file
   open it
   do some more calculation
   oh wait, allow this file also!
   open it
   do some more calculation
   OH WAIT, here is another file to open...



Re: yacc + unveil

2018-09-25 Thread Theo de Raadt
Michael Mikonos  wrote:

> On Mon, Sep 24, 2018 at 10:53:47PM -0600, Theo de Raadt wrote:
> > Ugh.  A diff which doens't check error returns.  Averting my gaze
> > is similar to "no way".  Hope you have another quarter, because you
> > need to try again
> 
> Oops... new coin inserted. I decided to create a fatal_perror()
> function which behaves like perror(). yacc's error.c didn't seem
> to have anything like that. Now if either pledge() or unveil()
> fails we see the appropriate error string instead of always seeing
> "invalid arguments" for pledge().

I don't understand parts of the diff.

> Index: defs.h
> ===
> RCS file: /cvs/src/usr.bin/yacc/defs.h,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 defs.h
> --- defs.h2 Dec 2014 15:56:22 -   1.18
> +++ defs.h25 Sep 2018 05:34:27 -
> @@ -307,6 +307,7 @@ extern void closure(short *, int);
>  extern void finalize_closure(void);
>  
>  extern __dead void fatal(char *);
> +extern __dead void fatal_perror(char *);
>  
>  extern void reflexive_transitive_closure(unsigned *, int);
>  extern __dead void done(int);
> Index: error.c
> ===
> RCS file: /cvs/src/usr.bin/yacc/error.c,v
> retrieving revision 1.14
> diff -u -p -u -r1.14 error.c
> --- error.c   8 Mar 2014 01:05:39 -   1.14
> +++ error.c   25 Sep 2018 05:34:27 -
> @@ -35,6 +35,8 @@
>  
>  /* routines for printing error messages  */
>  
> +#include 
> +
>  #include "defs.h"
>  
>  
> @@ -45,6 +47,12 @@ fatal(char *msg)
>   done(2);
>  }
>  
> +void
> +fatal_perror(char *msg)
> +{
> + fprintf(stderr, "%s: %s\n", msg, strerror(errno));
> + done(2);
> +}
>  
>  void
>  no_space(void)
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/yacc/main.c,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 main.c
> --- main.c25 May 2017 20:11:03 -  1.29
> +++ main.c25 Sep 2018 05:34:27 -
> @@ -305,10 +305,14 @@ open_files(void)
>   create_file_names();
>  
>   if (input_file == 0) {
> + if (unveil(input_file_name, "r") == -1)
> + fatal_perror("unveil");
>   input_file = fopen(input_file_name, "r");

At this point, all files are allowed.
So you restrict it to one.
Then you open it.
You won't open it again.
Why does it remain on the permitted open list?
Why not just open it, and not have it on the permitted open list?

Meanwhile, unveil hasn't been blocked.  So if someone finds a bug
at this point which gives them control, they can continue calling
unveil, and get full filesystem access back.

>   if (input_file == 0)
>   open_error(input_file_name);
>   }
> + if (unveil("/tmp", "crw") == -1)
> + fatal_perror("unveil");
>   fd = mkstemp(action_file_name);

Same concern.

>   if (fd == -1 || (action_file = fdopen(fd, "w")) == NULL)
>   open_error(action_file_name);
> @@ -318,11 +322,15 @@ open_files(void)
>   open_error(text_file_name);
>  
>   if (vflag) {
> + if (unveil(verbose_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   verbose_file = fopen(verbose_file_name, "w");

Same concern.

>   if (verbose_file == 0)
>   open_error(verbose_file_name);
>   }
>   if (dflag) {
> + if (unveil(defines_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   defines_file = fopen(defines_file_name, "w");

Same concern.

>   if (defines_file == NULL)
>   open_write_error(defines_file_name);
> @@ -330,24 +338,30 @@ open_files(void)
>   if (fd == -1 || (union_file = fdopen(fd, "w")) == NULL)
>   open_error(union_file_name);
>   }
> + if (unveil(output_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   output_file = fopen(output_file_name, "w");

Same concern.

>   if (output_file == 0)
>   open_error(output_file_name);
>  
>   if (rflag) {
> + if (unveil(code_file_name, "cw") == -1)
> + fatal_perror("unveil");
>   code_file = fopen(code_file_name, "w");

Same concern.

The best approach for using unveil should be more similar to pledge -
calculate the valid security ruleset early on.  Hoist initialization
earlier, and hoist security initialization code even earlier if possible.

That means, try to seperate the generation of the paths way up front,
unveil them, then pledge without unveil or unveil(NULL,NULL), and then
let the post-initialization part of the program continue.

>   if (code_file == 0)
>   open_error(code_file_name);
>   } else
>   code_file = output_file;
> + if (unveil(NULL, NULL) == -1)
> +

Re: bgpd ROA validation

2018-09-25 Thread Job Snijders
On Tue, Sep 25, 2018 at 12:23:48PM +0200, Claudio Jeker wrote:
> On Sat, Sep 22, 2018 at 09:48:24PM +, Job Snijders wrote:
> > Seems we are getting very close. Some suggestions to simplify the
> > experience for the end user.
> > 
> > Let's start with supporting just one (unnamed) roa-set, so far I've
> > really not come across a use case where multiple ROA tables are
> > useful.  I say this having implemented origin validation on both
> > ISPs and IXPs.
> > 
> > The semantics of the Origin Validation procedure that apply to
> > "Validated ROA Payloads" are not compatible with how the ARIN WHOIS
> > OriginAS semantics, so roa-set will never be used for ARIN WHOIS
> > data.
> 
> Please explain further, because honestly both the ruleset that
> arouteserver generates for ARIN WHOIS OriginAS and ROA are doing
> the same. They connect a source-as with a prefix. This is what a
> roa-set is giving you. It implements a way to quickly lookup a 2 key
> element (AS + prefix). 

The big difference between IRR, ARIN WHOIS and RPKI is that the former
two carry no exclusivity, and the latter (RPKI) semantically means
someting different than IRR or WHOIS data can convey.

When an object from IRR or WHOIS indicates that something like
"192.0.2.0/24 source-as 65001", it does *not* mean that 192.0.2.0/24
can't be originated by other ASNs. There may be other sources (other
IRRs) that also have route objects covering the prefix, or not. It just
means that 65001 is one of the valid source ASNs, it is a rather weak
statement.

However, if the same information "192.0.2.0/24 source-as 65001" comes
from a RPKI ROA, it means that *ALL OTHER* announcements from different
source ASNs or with different prefix lengths are invalid.

You can't apply the origin validation procedure if the information fed
into the procedure has no proof of termination.

Phrased differently: this is why you can do RPKI Origin Validation on
all your EBGP sessions, including your transit providers. But with IRR
based filters you can't really apply them on you transit providers.
IRR/WHOIS are only an allowlist, where as RPKI ROAs is a combined allow
+ blocklist.

> Depending on how this table is used it can be used for both cases.
> This is the technical view based on looking at rulesets and thinking
> on how to write them in a way that is fast and understandable.  I
> understand that from an administration point of view RPKI ROA is much
> stronger and can therefor be used more strictly (e.g. with the simple
> rule deny quick from ebgp roa-set RPKI invalid). The generic origin
> validatiation as a supporting measure to IRR based prefixset filtering
> is only allowing additional prefixes based on the roa-set (e.g. match
> from ebgp \ large-community $INTCOMM_ORIGIN_OK roa-set ARINDB valid \
> set large-community $INTCOMM_PREF_OK). They are two different things
> but can use the same filter building blocks.  Maybe naming it roa-set
> is wrong (since it is too much connected with RPKI) but the lookup
> table is usable for more than just RPKI. This is why I think
> supporting multiple tables is benefitial. I also agree that a
> simplified user experience for RPKI is a good thing, it should be
> simple to use.

At this point in time I do not thing we should support multiple tables.
Let's finish RPKI ROA origin validation and later, separately, look if
we can optimise for IRR/WHOIS data.

> > There currently is 1 RPKI, and therefor we should have just 1 roa-set.
> 
> I would fully agree here if ARIN would make their trust anchor freely
> distributable. 

That is entirely unrelated, and being worked on outside of OpenBSD
scope.

> But yes in general only one RPKI table is needed.  My point is that
> roa-set is more generic than RPKI. It is not an RPKI only thing. It
> can be used for more than that and we should support this as well
> since it makes for much better and efficient configs.

I disagree, roa-set is an RPKI only thing. There currently is nothing
else to support. No other type of published routing information has the
same semantics as a ROA and are thus suitable for the origin validation
procedure.

> > The advantage of having only one roa-set is that it does not need to be
> > referenced in the policies.
> > 
> > I propose the patch is amended to accomodate the following:
> > 
> > roa-set {
> > 165.254.255.0/24 source-as 15562
> > 193.0.0.0/21 source-as 
> > }
> > 
> > deny from any ovs invalid
> > match from any ovs valid set community local-as:42
> > match from any ovs unknown set community local-as:43
> > 
> > I suggest the filter keyword 'ovs' (origin validation state) is
> > introduced to allow easy matching. I think this looks better. If the
> > roa-set is not defined or empty, all route announcements are 'unknown'.
> 
> If we want to use ovs then the naming scheme should be the same as for the
> extended community. 

Ah, so instead of what I proposed 'unknown' we should use 'not-found'.
OK that mak

Re: bgpd ROA validation

2018-09-25 Thread Claudio Jeker
On Sat, Sep 22, 2018 at 09:48:24PM +, Job Snijders wrote:
> Hi claudio,
> 
> Seems we are getting very close. Some suggestions to simplify the
> experience for the end user.
> 
> Let's start with supporting just one (unnamed) roa-set, so far I've
> really not come across a use case where multiple ROA tables are useful.
> I say this having implemented origin validation on both ISPs and IXPs.
> 
> The semantics of the Origin Validation procedure that apply to
> "Validated ROA Payloads" are not compatible with how the ARIN WHOIS
> OriginAS semantics, so roa-set will never be used for ARIN WHOIS data.

Please explain further, because honestly both the ruleset that
arouteserver generates for ARIN WHOIS OriginAS and ROA are doing
the same. They connect a source-as with a prefix. This is what a
roa-set is giving you. It implements a way to quickly lookup a 2 key
element (AS + prefix). Depending on how this table is used it can be used
for both cases. This is the technical view based on looking at rulesets and
thinking on how to write them in a way that is fast and understandable.
I understand that from an administration point of view RPKI ROA is much
stronger and can therefor be used more strictly (e.g. with the simple rule
deny quick from ebgp roa-set RPKI invalid). The generic origin validatiation
as a supporting measure to IRR based prefixset filtering is only allowing
additional prefixes based on the roa-set (e.g. match from ebgp \
large-community $INTCOMM_ORIGIN_OK roa-set ARINDB valid \
set large-community $INTCOMM_PREF_OK). They are two different things but
can use the same filter building blocks.
Maybe naming it roa-set is wrong (since it is too much connected with
RPKI) but the lookup table is usable for more than just RPKI. This is why
I think supporting multiple tables is benefitial. I also agree that a
simplified user experience for RPKI is a good thing, it should be simple
to use.

> There currently is 1 RPKI, and therefor we should have just 1 roa-set.

I would fully agree here if ARIN would make their trust anchor freely
distributable. But yes in general only one RPKI table is needed.
My point is that roa-set is more generic than RPKI. It is not an RPKI only
thing. It can be used for more than that and we should support this as
well since it makes for much better and efficient configs.
 
> The advantage of having only one roa-set is that it does not need to be
> referenced in the policies.
> 
> I propose the patch is amended to accomodate the following:
> 
>   roa-set {
>   165.254.255.0/24 source-as 15562
>   193.0.0.0/21 source-as 
>   }
> 
>   deny from any ovs invalid
>   match from any ovs valid set community local-as:42
>   match from any ovs unknown set community local-as:43
> 
> I suggest the filter keyword 'ovs' (origin validation state) is
> introduced to allow easy matching. I think this looks better. If the
> roa-set is not defined or empty, all route announcements are 'unknown'.

If we want to use ovs then the naming scheme should be the same as for the
extended community. At least if we reuse this keyword it should work the
same. Also the side-effect is that the two configs look fairly similar
which can be good or bad:
match from any ovs valid set community local-as:42 
match from any ext-community ovs valid set community local-as:42 

I'm not against this, just wanted to bring it up.

Also I think unknown should be renamed not-found. I will do that since
not-found is what the RFC is using.

I will rework the diff considering the input.
-- 
:wq Claudio



Re: [patch] Add IPv6 description for `-T' option in netcat manual

2018-09-25 Thread Jason McIntyre
On Tue, Sep 25, 2018 at 03:25:51PM +0800, Nan Xiao wrote:
> Hi Jason,
> 
> According to IPv6(https://en.wikipedia.org/wiki/IPv6_packet) and IPv4
> TOS(https://en.wikipedia.org/wiki/Type_of_service), Traffic class and
> TOS seem have the same structure: the first six bits are DS field and
> the last two bits for Explicit Congestion Notification.
> 
> Thanks!
> 

hi. fair enough. so i'll commit this shortly unless i hear any
objections.

jmc

> Best Regards
> Nan Xiao
> On Tue, Sep 25, 2018 at 2:37 PM Jason McIntyre  wrote:
> >
> > On Tue, Sep 25, 2018 at 09:11:42AM +0800, Nan Xiao wrote:
> > > Hi tech@,
> > >
> >
> > morning.
> >
> > > According to netcat source code, the `-T' option not only takes effect
> > > in IPv4 but also IPv6:
> > >
> > >   if (Tflag != -1) {
> > >   if (af == AF_INET && setsockopt(s, IPPROTO_IP,
> > >   IP_TOS, &Tflag, sizeof(Tflag)) == -1)
> > >   err(1, "set IP ToS");
> > >
> > >   else if (af == AF_INET6 && setsockopt(s, IPPROTO_IPV6,
> > >   IPV6_TCLASS, &Tflag, sizeof(Tflag)) == -1)
> > >   err(1, "set IPv6 traffic class");
> > >   }
> > >
> > > So I think the following patch should be more accurate:
> > >
> > > diff --git nc.1 nc.1
> > > index cb391288f15..5588daf87ae 100644
> > > --- nc.1
> > > +++ nc.1
> > > @@ -241,7 +241,7 @@ Cannot be used together with
> > >  or
> > >  .Fl x .
> > >  .It Fl T Ar keyword
> > > -Change the IPv4 TOS value or the TLS options.
> > > +Change the IPv4 TOS/IPv6 Traffic Class value or the TLS options.
> > >  .Pp
> > >  For TLS options,
> > >  .Ar keyword
> > > @@ -269,7 +269,7 @@ for further details).
> > >  Specifying TLS options requires
> > >  .Fl c .
> > >  .Pp
> > > -For the IPv4 TOS value,
> > > +For the IPv4 TOS/IPv6 Traffic Class value,
> > >  .Ar keyword
> > >  may be one of
> >
> > one question: are the keywords identical for both v4 and 6?
> >
> > if so, anyone want to ok this?
> > if not, the diff needs work.
> >
> > also, i think traffic class should be lower case.
> >
> > jmc
> >
> > >  .Cm critical ,
> > >
> > >
> > > Thanks!
> > >
> > > --
> > > Best Regards
> > > Nan Xiao
> > >
> >
> 



Re: [patch] Add IPv6 description for `-T' option in netcat manual

2018-09-25 Thread Nan Xiao
Hi Jason,

According to IPv6(https://en.wikipedia.org/wiki/IPv6_packet) and IPv4
TOS(https://en.wikipedia.org/wiki/Type_of_service), Traffic class and
TOS seem have the same structure: the first six bits are DS field and
the last two bits for Explicit Congestion Notification.

Thanks!

Best Regards
Nan Xiao
On Tue, Sep 25, 2018 at 2:37 PM Jason McIntyre  wrote:
>
> On Tue, Sep 25, 2018 at 09:11:42AM +0800, Nan Xiao wrote:
> > Hi tech@,
> >
>
> morning.
>
> > According to netcat source code, the `-T' option not only takes effect
> > in IPv4 but also IPv6:
> >
> >   if (Tflag != -1) {
> >   if (af == AF_INET && setsockopt(s, IPPROTO_IP,
> >   IP_TOS, &Tflag, sizeof(Tflag)) == -1)
> >   err(1, "set IP ToS");
> >
> >   else if (af == AF_INET6 && setsockopt(s, IPPROTO_IPV6,
> >   IPV6_TCLASS, &Tflag, sizeof(Tflag)) == -1)
> >   err(1, "set IPv6 traffic class");
> >   }
> >
> > So I think the following patch should be more accurate:
> >
> > diff --git nc.1 nc.1
> > index cb391288f15..5588daf87ae 100644
> > --- nc.1
> > +++ nc.1
> > @@ -241,7 +241,7 @@ Cannot be used together with
> >  or
> >  .Fl x .
> >  .It Fl T Ar keyword
> > -Change the IPv4 TOS value or the TLS options.
> > +Change the IPv4 TOS/IPv6 Traffic Class value or the TLS options.
> >  .Pp
> >  For TLS options,
> >  .Ar keyword
> > @@ -269,7 +269,7 @@ for further details).
> >  Specifying TLS options requires
> >  .Fl c .
> >  .Pp
> > -For the IPv4 TOS value,
> > +For the IPv4 TOS/IPv6 Traffic Class value,
> >  .Ar keyword
> >  may be one of
>
> one question: are the keywords identical for both v4 and 6?
>
> if so, anyone want to ok this?
> if not, the diff needs work.
>
> also, i think traffic class should be lower case.
>
> jmc
>
> >  .Cm critical ,
> >
> >
> > Thanks!
> >
> > --
> > Best Regards
> > Nan Xiao
> >
>