Re: Removing PF

2019-03-31 Thread Ian McWilliam
"peeing on, or even integration into baby mulching
machines or atomic bombs to be dropped on Australia"

That's a lot of missing features to implement in one release cycle.

Ian McWilliam


From: owner-t...@openbsd.org  on behalf of Claudio 
Jeker 
Sent: Monday, 1 April 2019 4:01 PM
To: tech@openbsd.org
Subject: Removing PF

There have been internal discussions about OpenBSD also removing the pf
packet filter after the upcoming 6.5 release. Instead a switch to
using David Gwynne's new bpf filter will happen.
The benefits outweigh the drawbacks and the missing features will be
readily implemented in time for the 6.6 release.

--
:wq Claudio



Removing PF

2019-03-31 Thread Claudio Jeker
There have been internal discussions about OpenBSD also removing the pf
packet filter after the upcoming 6.5 release. Instead a switch to
using David Gwynne's new bpf filter will happen.
The benefits outweigh the drawbacks and the missing features will be
readily implemented in time for the 6.6 release.

-- 
:wq Claudio



sys/dev/pci/if_wb.c: repair "} if"

2019-03-31 Thread Christian Weisgerber
Some sort of merge error; FreeBSD, where this was imported from, always
had "else if".  Also, semantically more plausible.

ok?

Index: sys/dev/pci/if_wb.c
===
RCS file: /cvs/src/sys/dev/pci/if_wb.c,v
retrieving revision 1.69
diff -u -p -r1.69 if_wb.c
--- sys/dev/pci/if_wb.c 13 Jul 2017 17:45:00 -  1.69
+++ sys/dev/pci/if_wb.c 1 Apr 2019 04:11:22 -
@@ -660,7 +660,7 @@ wb_fixmedia(sc)
if (IFM_SUBTYPE(mii->mii_media_active) == IFM_10_T) {
media = mii->mii_media_active & ~IFM_10_T;
media |= IFM_100_TX;
-   } if (IFM_SUBTYPE(mii->mii_media_active) == IFM_100_TX) {
+   } else if (IFM_SUBTYPE(mii->mii_media_active) == IFM_100_TX) {
media = mii->mii_media_active & ~IFM_100_TX;
media |= IFM_10_T;
} else
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: sys/dev/x86emu/x86emu.c: repair "} if"

2019-03-31 Thread Theo de Raadt
Yes, very weird.

There are about 20 more in the base tree.

Some of them are missing "else", but some are not.

> I'm grepping the tree for "} if" lines...
> 
> I'm confident that these were intended as "else if", compare the
> corresponding ror_* functions.  Also, it doesn't actually change
> the result.
> 
> ok?
> 
> Index: sys/dev/x86emu/x86emu.c
> ===
> RCS file: /cvs/src/sys/dev/x86emu/x86emu.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 x86emu.c
> --- sys/dev/x86emu/x86emu.c   25 Nov 2018 19:52:08 -  1.10
> +++ sys/dev/x86emu/x86emu.c   1 Apr 2019 03:50:28 -
> @@ -7009,7 +7009,7 @@ rol_byte(struct x86emu *emu, uint8_t d, 
>   CONDITIONAL_SET_FLAG(s == 1 &&
>   XOR2((res & 0x1) + ((res >> 6) & 0x2)),
>   F_OF);
> - } if (s != 0) {
> + } else if (s != 0) {
>   /* set the new carry flag, Note that it is the low order bit
>* of the result!!!   */
>   CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
> @@ -7035,7 +7035,7 @@ rol_word(struct x86emu *emu, uint16_t d,
>   CONDITIONAL_SET_FLAG(s == 1 &&
>   XOR2((res & 0x1) + ((res >> 14) & 0x2)),
>   F_OF);
> - } if (s != 0) {
> + } else if (s != 0) {
>   /* set the new carry flag, Note that it is the low order bit
>* of the result!!!   */
>   CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
> @@ -7061,7 +7061,7 @@ rol_long(struct x86emu *emu, uint32_t d,
>   CONDITIONAL_SET_FLAG(s == 1 &&
>   XOR2((res & 0x1) + ((res >> 30) & 0x2)),
>   F_OF);
> - } if (s != 0) {
> + } else if (s != 0) {
>   /* set the new carry flag, Note that it is the low order bit
>* of the result!!!   */
>   CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 



sys/dev/x86emu/x86emu.c: repair "} if"

2019-03-31 Thread Christian Weisgerber
I'm grepping the tree for "} if" lines...

I'm confident that these were intended as "else if", compare the
corresponding ror_* functions.  Also, it doesn't actually change
the result.

ok?

Index: sys/dev/x86emu/x86emu.c
===
RCS file: /cvs/src/sys/dev/x86emu/x86emu.c,v
retrieving revision 1.10
diff -u -p -r1.10 x86emu.c
--- sys/dev/x86emu/x86emu.c 25 Nov 2018 19:52:08 -  1.10
+++ sys/dev/x86emu/x86emu.c 1 Apr 2019 03:50:28 -
@@ -7009,7 +7009,7 @@ rol_byte(struct x86emu *emu, uint8_t d, 
CONDITIONAL_SET_FLAG(s == 1 &&
XOR2((res & 0x1) + ((res >> 6) & 0x2)),
F_OF);
-   } if (s != 0) {
+   } else if (s != 0) {
/* set the new carry flag, Note that it is the low order bit
 * of the result!!!   */
CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
@@ -7035,7 +7035,7 @@ rol_word(struct x86emu *emu, uint16_t d,
CONDITIONAL_SET_FLAG(s == 1 &&
XOR2((res & 0x1) + ((res >> 14) & 0x2)),
F_OF);
-   } if (s != 0) {
+   } else if (s != 0) {
/* set the new carry flag, Note that it is the low order bit
 * of the result!!!   */
CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
@@ -7061,7 +7061,7 @@ rol_long(struct x86emu *emu, uint32_t d,
CONDITIONAL_SET_FLAG(s == 1 &&
XOR2((res & 0x1) + ((res >> 30) & 0x2)),
F_OF);
-   } if (s != 0) {
+   } else if (s != 0) {
/* set the new carry flag, Note that it is the low order bit
 * of the result!!!   */
CONDITIONAL_SET_FLAG(res & 0x1, F_CF);
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: iked(8): add support for IKEv2 Message Fragmentation

2019-03-31 Thread Tim Stewart

On 3/30/19 3:11 PM, Tobias Heider wrote:

Hi Stuart,

I'm glad to see people are using this.
There's some smaller fixes that I haven't sent to the list yet, so
probably I'll send an updated diff on monday.


I plan to start using this patch this week, likely as soon as you send 
the updated diff.  I started on the same feature in June of 2018, but 
other tasks took priority and it stalled.


I have a pretty good testbed for this, as I have several site-to-site 
links that drop UDP fragments and several road warriors that would 
experience fragment drops depending on the cell network they use.  I 
will report back on this thread with my findings.


Thanks for the patch!

-TimS




Regards,
Tobias

On 3/30/19 6:43 PM, Stuart Henderson wrote:

This diff hasn't gone anywhere recently - I've been using it since
Tobias posted it with no problems. Any comments on whether it should
go in, and if so, before/after 6.5? The feature is disabled by default.

Index: config.c
===
RCS file: /cvs/src/sbin/iked/config.c,v
retrieving revision 1.49
diff -u -p -r1.49 config.c
--- config.c27 Nov 2017 18:39:35 -  1.49
+++ config.c30 Mar 2019 17:41:33 -
@@ -94,12 +94,30 @@ config_free_kex(struct iked_kex *kex)
  }
  
  void

+config_free_fragments(struct iked_frag *frag)
+{
+   size_t i;
+   if (frag && frag->frag_arr) {
+   for (i = 0; i < frag->frag_count; i++) {
+   free(frag->frag_arr[i]->frag_data);
+   frag->frag_arr[i]->frag_data = NULL;
+   free(frag->frag_arr[i]);
+   frag->frag_arr[i] = NULL;
+   }
+   free(frag->frag_arr);
+   frag->frag_arr = NULL;
+   bzero(frag, sizeof(struct iked_frag));
+   }
+}
+
+void
  config_free_sa(struct iked *env, struct iked_sa *sa)
  {
timer_del(env, >sa_timer);
timer_del(env, >sa_keepalive);
timer_del(env, >sa_rekey);
  
+	config_free_fragments(>sa_fragments);

config_free_proposals(>sa_proposals, 0);
config_free_childsas(env, >sa_childsas, NULL, NULL);
sa_free_flows(env, >sa_flows);
@@ -925,6 +943,29 @@ config_setkeys(struct iked *env)
EVP_PKEY_free(key);
  
  	return (ret);

+}
+
+int
+config_setfragmentation(struct iked *env)
+{
+   unsigned int boolval;
+
+   boolval = env->sc_frag;
+   proc_compose(>sc_ps, PROC_IKEV2, IMSG_CTL_FRAGMENTATION,
+   , sizeof(boolval));
+   return (0);
+}
+
+int
+config_getfragmentation(struct iked *env, struct imsg *imsg)
+{
+   unsigned int boolval;
+
+   IMSG_SIZE_CHECK(imsg, );
+   memcpy(, imsg->data, sizeof(boolval));
+   env->sc_frag = boolval;
+   log_debug("%s: %sfragmentation", __func__, env->sc_frag ? "" : "no ");
+   return (0);
  }
  
  int

Index: iked.c
===
RCS file: /cvs/src/sbin/iked/iked.c,v
retrieving revision 1.36
diff -u -p -r1.36 iked.c
--- iked.c  27 Nov 2017 18:39:35 -  1.36
+++ iked.c  30 Mar 2019 17:41:33 -
@@ -251,6 +251,7 @@ parent_configure(struct iked *env)
fatal("pledge");
  
  	config_setmobike(env);

+   config_setfragmentation(env);
config_setcoupled(env, env->sc_decoupled ? 0 : 1);
config_setmode(env, env->sc_passive ? 1 : 0);
config_setocsp(env);
@@ -282,6 +283,7 @@ parent_reload(struct iked *env, int rese
config_setcompile(env, PROC_IKEV2);
  
  		config_setmobike(env);

+   config_setfragmentation(env);
config_setcoupled(env, env->sc_decoupled ? 0 : 1);
config_setmode(env, env->sc_passive ? 1 : 0);
config_setocsp(env);
Index: iked.conf.5
===
RCS file: /cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.53
diff -u -p -r1.53 iked.conf.5
--- iked.conf.5 31 Jan 2018 13:25:55 -  1.53
+++ iked.conf.5 30 Mar 2019 17:41:33 -
@@ -136,6 +136,12 @@ This is the default.
  .It Ic set decouple
  Don't load the negotiated SAs and flows from the kernel.
  This mode is only useful for testing and debugging.
+.It Ic set fragmentation
+Enable IKEv2 Message Fragmentation (RFC 7383) support.
+This allows IKEv2 to operate in environments that might block IP fragments.
+.It Ic set nofragmentation
+Disables IKEv2 Message Fragmentation support.
+This is the default.
  .It Ic set mobike
  Enable MOBIKE (RFC 4555) support.
  This is the default.
Index: iked.h
===
RCS file: /cvs/src/sbin/iked/iked.h,v
retrieving revision 1.119
diff -u -p -r1.119 iked.h
--- iked.h  6 Aug 2018 06:30:06 -   1.119
+++ iked.h  30 Mar 2019 17:41:33 -
@@ -362,6 +362,21 @@ struct iked_kex {
struct ibuf *kex_dhpeer;/* pointer to i or r */
  };

Re: httpd(8): Adapt to industry wide current best security practices

2019-03-31 Thread Bryan Steele
On Mon, Apr 01, 2019 at 02:30:22AM +0200, Florian Obser wrote:
> OK?
> 
> diff --git server_http.c server_http.c
> index 6c8549d2b41..f04a15bd056 100644
> --- server_http.c
> +++ server_http.c
> @@ -1176,7 +1176,7 @@ server_response(struct httpd *httpd, struct client *clt)
>   struct http_descriptor  *resp = clt->clt_descresp;
>   struct server   *srv = clt->clt_srv;
>   struct server_config*srv_conf = >srv_conf;
> - struct kv   *kv, key, *host;
> + struct kv   *kv, key, *host, *ua;
>   struct str_find  sm;
>   int  portval = -1, ret;
>   char*hostval, *query;
> @@ -1194,6 +1194,15 @@ server_response(struct httpd *httpd, struct client 
> *clt)
>   if ((desc->http_path = strdup(path)) == NULL)
>   goto fail;
>  
> + key.kv_key = "user-agent";

I think this should be "User-Agent" to match the other cases in
the file.

With that change, ok brynet@

> + if ((ua = kv_find(>http_headers, )) != NULL &&
> + ua->kv_value != NULL) {
> + if (strstr(ua->kv_value, "curl") != NULL) {
> + server_abort_http(clt, 403, "forbidden");
> + return (-1);
> + }
> + }
> +
>   key.kv_key = "Host";
>   if ((host = kv_find(>http_headers, )) != NULL &&
>   host->kv_value == NULL)
> 
> -- 
> I'm not entirely sure you are real.
> 
> 



Re: httpd(8): Adapt to industry wide current best security practices

2019-03-31 Thread Theo de Raadt
relayd needs this also.  It is a very often a bump in the wire,
protecting cisco devices.

Florian Obser  wrote:

> OK?
> 
> diff --git server_http.c server_http.c
> index 6c8549d2b41..f04a15bd056 100644
> --- server_http.c
> +++ server_http.c
> @@ -1176,7 +1176,7 @@ server_response(struct httpd *httpd, struct client *clt)
>   struct http_descriptor  *resp = clt->clt_descresp;
>   struct server   *srv = clt->clt_srv;
>   struct server_config*srv_conf = >srv_conf;
> - struct kv   *kv, key, *host;
> + struct kv   *kv, key, *host, *ua;
>   struct str_find  sm;
>   int  portval = -1, ret;
>   char*hostval, *query;
> @@ -1194,6 +1194,15 @@ server_response(struct httpd *httpd, struct client 
> *clt)
>   if ((desc->http_path = strdup(path)) == NULL)
>   goto fail;
>  
> + key.kv_key = "user-agent";
> + if ((ua = kv_find(>http_headers, )) != NULL &&
> + ua->kv_value != NULL) {
> + if (strstr(ua->kv_value, "curl") != NULL) {
> + server_abort_http(clt, 403, "forbidden");
> + return (-1);
> + }
> + }
> +
>   key.kv_key = "Host";
>   if ((host = kv_find(>http_headers, )) != NULL &&
>   host->kv_value == NULL)
> 
> -- 
> I'm not entirely sure you are real.
> 



httpd(8): Adapt to industry wide current best security practices

2019-03-31 Thread Florian Obser
OK?

diff --git server_http.c server_http.c
index 6c8549d2b41..f04a15bd056 100644
--- server_http.c
+++ server_http.c
@@ -1176,7 +1176,7 @@ server_response(struct httpd *httpd, struct client *clt)
struct http_descriptor  *resp = clt->clt_descresp;
struct server   *srv = clt->clt_srv;
struct server_config*srv_conf = >srv_conf;
-   struct kv   *kv, key, *host;
+   struct kv   *kv, key, *host, *ua;
struct str_find  sm;
int  portval = -1, ret;
char*hostval, *query;
@@ -1194,6 +1194,15 @@ server_response(struct httpd *httpd, struct client *clt)
if ((desc->http_path = strdup(path)) == NULL)
goto fail;
 
+   key.kv_key = "user-agent";
+   if ((ua = kv_find(>http_headers, )) != NULL &&
+   ua->kv_value != NULL) {
+   if (strstr(ua->kv_value, "curl") != NULL) {
+   server_abort_http(clt, 403, "forbidden");
+   return (-1);
+   }
+   }
+
key.kv_key = "Host";
if ((host = kv_find(>http_headers, )) != NULL &&
host->kv_value == NULL)

-- 
I'm not entirely sure you are real.



Re: smtpd.conf(5)/table(5) manuals: change `\-' to `-'

2019-03-31 Thread Jason McIntyre
On Sat, Mar 23, 2019 at 11:50:53PM -0600, Randy Hartman wrote:
> Change smtpd.conf(5) and table(5) man pages to represent hyphen
> as plain `-'.  According to mandoc_char(7), "[...] in manual pages
> just write plain `-' to represent hyphen, minus, and hyphen-minus."
> 
> Found while comparing ssl(8), starttls(8), and smtpd.conf(5).
> 

hi.

the current situation is that both formats are regarded as correct, and
the work neccessary to flip all pages one way or the other just isn;t
worth it.

to that end, ingo has just bumped mandoc_char(7) a little to hopefully
make this clearer.

thanks for the mail anyway,
jmc

> Index: smtpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
> retrieving revision 1.210
> diff -u -p -u -p -r1.210 smtpd.conf.5
> --- smtpd.conf.5  22 Dec 2018 08:54:02 -  1.210
> +++ smtpd.conf.5  24 Mar 2019 05:08:30 -
> @@ -110,12 +110,12 @@ The delivery
>  .Ar method
>  parameter may be one of the following:
>  .Bl -tag -width Ds
> -.It Cm expand\-only
> +.It Cm expand-only
>  Only accept the message if a delivery method was specified
>  in an aliases or
>  .Pa .forward
>  file.
> -.It Cm forward\-only
> +.It Cm forward-only
>  Only accept the message if the recipient results in a remote address
>  after the processing of aliases or forward file.
>  .It Cm lmtp Ar destination Op Ar rcpt-to
> @@ -223,7 +223,7 @@ with higher priority than mail exchanger
>  Advertise
>  .Ar heloname
>  as the hostname to other mail exchangers during the HELO phase.
> -.It Cm helo\-src Pf < Ar table Ns >
> +.It Cm helo-src Pf < Ar table Ns >
>  Use the mapping
>  .Ar table
>  to look up a hostname matching the source address,
> @@ -283,7 +283,7 @@ This option is usable only with
>  option.
>  The credential table format is described in
>  .Xr table 5 .
> -.It Cm mail\-from Ar mailaddr
> +.It Cm mail-from Ar mailaddr
>  Use
>  .Ar mailaddr
>  as the MAIL FROM address within the SMTP transaction.
> @@ -294,7 +294,7 @@ for the source IP address.
>  If the list contains more than one address, all of them are used
>  in such a way that traffic is routed as efficiently as possible.
>  .El
> -.It Ic bounce Cm warn\-interval Ar delay Op , Ar delay ...
> +.It Ic bounce Cm warn-interval Ar delay Op , Ar delay ...
>  Send warning messages to the envelope sender when temporary delivery
>  failures cause a message to remain on the queue for longer than
>  .Ar delay .
> @@ -308,7 +308,7 @@ At most four
>  .Ar delay
>  parameters can be specified.
>  The default is
> -.Qq Ic bounce Cm warn\-interval No 4h ,
> +.Qq Ic bounce Cm warn-interval No 4h ,
>  sending a single warning after four hours.
>  .It Ic ca Ar caname Cm cert Ar cafile
>  Associate the Certificate Authority (CA) certificate file
> @@ -355,7 +355,7 @@ or a credentials table
>  .Ar authtable ,
>  the format of which is described in
>  .Xr table 5 .
> -.It Cm auth\-optional Op Pf < Ar authtable Ns >
> +.It Cm auth-optional Op Pf < Ar authtable Ns >
>  Support SMTPAUTH optionally:
>  clients need not authenticate, but may do so.
>  This allows a
> @@ -386,13 +386,13 @@ The
>  table contains a mapping of IP addresses to hostnames.
>  If the address on which the connection arrives appears in the mapping,
>  the associated hostname is used.
> -.It Cm mask\-src
> +.It Cm mask-src
>  Omit the
>  .Sy from
>  part when prepending
>  .Dq Received
>  headers.
> -.It Cm no\-dsn
> +.It Cm no-dsn
>  Disable the DSN (Delivery Status Notification) extension.
>  .It Cm pki Ar pkiname
>  For secure connections,
> @@ -406,7 +406,7 @@ to prove a mail server's identity.
>  Listen on the given
>  .Ar port
>  instead of the default port 25.
> -.It Cm received\-auth
> +.It Cm received-auth
>  In
>  .Dq Received
>  headers, report whether the session was authenticated
> @@ -432,7 +432,7 @@ Clients connecting to the listener are t
>  Support STARTTLS, by default on port 25.
>  Mutually exclusive with
>  .Cm smtps .
> -.It Cm tls\-require Op Cm verify
> +.It Cm tls-require Op Cm verify
>  Like
>  .Cm tls ,
>  but force clients to establish a secure connection
> @@ -442,12 +442,12 @@ With the
>  option, clients must also provide a valid certificate
>  to establish an SMTP session.
>  .El
> -.It Ic listen on Cm socket Op Cm mask\-src
> +.It Ic listen on Cm socket Op Cm mask-src
>  Listen for incoming SMTP connections on the Unix domain socket
>  .Pa /var/run/smtpd.sock .
>  This is done by default, even if the directive is absent.
>  If the
> -.Cm mask\-src
> +.Cm mask-src
>  option is specified, printing of the HELO name, hostname, and IP
>  address of the originating host is suppressed in Received: header lines.
>  .\" XXX The option
> @@ -564,28 +564,28 @@ Specify that session's HELO / EHLO shoul
>  .Ar helo-name .
>  .It Xo
>  .Op Ic \&!
> -.Cm mail\-from
> +.Cm mail-from
>  .Ar sender | Pf < Ar sender Ns >
>  .Xc
>  Specify that transactions's MAIL FROM should match the string or list 

Re: bgpd: unbreak route origin validation

2019-03-31 Thread Denis Fondras
On Sun, Mar 31, 2019 at 06:03:01PM +0200, Claudio Jeker wrote:
> On Fri, Mar 22, 2019 at 09:25:32PM +0100, Denis Fondras wrote:
> > (better when the right diff is sent...)
> > 
> > ROV has been broken since the configuration reload changes.
> > 
> > Index: rde.c
> > ===
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > retrieving revision 1.466
> > diff -u -p -r1.466 rde.c
> > --- rde.c   13 Mar 2019 14:35:39 -  1.466
> > +++ rde.c   22 Mar 2019 15:36:41 -
> > @@ -2899,7 +2899,7 @@ rde_reload_done(void)
> > roa_old = conf->rde_roa;
> > as_sets_old = conf->as_sets;
> >  
> > -   copy_config(conf, nconf);
> > +   memcpy(conf, nconf, sizeof(struct bgpd_config));
> > SIMPLEQ_INIT(>rde_prefixsets);
> > SIMPLEQ_INIT(>rde_originsets);
> > SIMPLEQ_CONCAT(>rde_prefixsets, >rde_prefixsets);
> > 
> 
> Here is a diff that should work better. Your version introduces a use
> after free because of nconf->as_sets being freed but copied to conf
> beforehands.
> 
> This handles both, as_sets and rde_roa. OK?

OK @denis


> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.467
> diff -u -p -r1.467 rde.c
> --- rde.c 23 Mar 2019 13:09:56 -  1.467
> +++ rde.c 31 Mar 2019 13:52:01 -
> @@ -2899,11 +2899,15 @@ rde_reload_done(void)
>   roa_old = conf->rde_roa;
>   as_sets_old = conf->as_sets;
>  
> - memcpy(conf, nconf, sizeof(struct bgpd_config));
> - SIMPLEQ_INIT(>rde_prefixsets);
> - SIMPLEQ_INIT(>rde_originsets);
> + copy_config(conf, nconf);
> + /* need to copy the sets and roa table and clear them in nconf */
>   SIMPLEQ_CONCAT(>rde_prefixsets, >rde_prefixsets);
>   SIMPLEQ_CONCAT(>rde_originsets, >rde_originsets);
> + conf->rde_roa = nconf->rde_roa;
> + conf->as_sets = nconf->as_sets;
> + memset(>rde_roa, 0, sizeof(nconf->rde_roa));
> + nconf->as_sets = NULL;
> +
>   free_config(nconf);
>   nconf = NULL;
>  



improve rsync(1) manual page

2019-03-31 Thread Ingo Schwarze
Hi,

here are serveral bugfixes and improvements for the rsync(1) manual.

OK?
  Ingo


Bugfixes:
 * For -D and -l: s/Transfer/Also transfer/.
 * In the EXAMPLES, the renaming rsync -> openrsync caused a mess;
   the worst aspect is the --rsync-path in the last example.
   I don't have a good solution.  I fear we will have to edit the
   section again once we rename the program back to plain "rsync".

Missing information:
 * Change the misleading argument placeholder "portnumber" to "service";
   even the default ("rsync") isn't a number.  In the description,
   mention services(5), and also mention the default.
 * Document the missing option --numeric-ids.
 * Add the missing sections EXIT STATUS, HISTORY, AUTHORS.

Minor improvements:
 * Drop the redundant phrases "Archive mode" and "Dry-run mode"
   which are already clear form the long options right above: long
   options cause enough verbosity already, so at least give them
   the chance to convey some useful meaning.
 * Add missing full stop for -D.
 * Clarify which kind of program -e specifies; there are options
   for specifying two different kinds of programs.
 * Remove excessive verbosity from -g and -o, and improve precision:
- These options do *not* set the ID to match the source by default.
- The meaning of "verbatim" is unclear.
- Use the standard wording "user ID" and "group ID" like in
  chgrp(1) find(1) ls(1) chown(2) stat(2) group(5) passwd(5) chown(8);
  i can't see "group identifier" used anywhere else, and given
  that the distinction between "group name" and "group ID" matters
  here, let's avoid the unusual term.
 * Recommend -v with -n, or people may either wonder what the point is
   or incorrectly expect that -n might already imply -v.
 * Describe -o in a self-contained way.  It isn't so long that it
   would justify giving readers the runaround.
 * Do not call the source directory "the root directory";
   the latter term generally refers to /.
 * For --rsync-path, consistently use .Ar program, not .Ar prog, and
   correct the markup of the default remote program name "rsync".
 * Fix markup of argument syntax: host, module, and path have to be
   replaced separately and the punctuation in between has to remain
   exactly as shown in the manual, so do not mark up the punctuation
   with .Ar.  Also, "rsync" is a keyword in this context, to be given
   verbatim, not a placeholder for something else.
 * Move the compatibility statement into a STANDARDS section.
   Even though the rsync protocal may not be a formal standard,
   that's where such information fits best.
 * Remove commented out sections that will never make sense in this page.


Index: main.c
===
RCS file: /cvs/src/usr.bin/rsync/main.c,v
retrieving revision 1.42
diff -u -r1.42 main.c
--- main.c  31 Mar 2019 13:17:44 -  1.42
+++ main.c  31 Mar 2019 16:24:03 -
@@ -505,8 +505,9 @@
exit(rc);
 usage:
fprintf(stderr, "usage: %s"
-   " [-aDglnoprtv] [-e program] [--del] [--port=portnumber]\n"
-   "\t[--rsync-path=program] [--version] source ... directory\n",
+   " [-aDglnoprtv] [-e program] [--del] [--numeric-ids]\n"
+   "\t[--port=portnumber] [--rsync-path=program] [--version]\n"
+   "\tsource ... directory\n",
getprogname());
exit(1);
 }
Index: rsync.1
===
RCS file: /cvs/src/usr.bin/rsync/rsync.1,v
retrieving revision 1.15
diff -u -r1.15 rsync.1
--- rsync.1 31 Mar 2019 13:17:44 -  1.15
+++ rsync.1 31 Mar 2019 16:24:04 -
@@ -25,7 +25,8 @@
 .Op Fl aDglnoprtv
 .Op Fl e Ar program
 .Op Fl -del
-.Op Fl -port Ns = Ns Ar portnumber
+.Op Fl -numeric-ids
+.Op Fl -port Ns = Ns Ar service
 .Op Fl -rsync-path Ns = Ns Ar program
 .Op Fl -version
 .Ar source ...
@@ -47,13 +48,12 @@
 The arguments are as follows:
 .Bl -tag -width Ds
 .It Fl a , -archive
-Archive mode.
 Shorthand for
 .Fl Dgloprt .
 .It Fl D
-Transfer device and special files.
+Also transfer device and special files.
 Shorthand for
-.Fl -devices -specials
+.Fl -devices -specials .
 .It Fl -del , -delete
 Delete files in
 .Ar directory
@@ -63,36 +63,50 @@
 Only applicable with
 .Fl r .
 .It Fl -devices
-Transfer device files.
+Also transfer device files.
 .It Fl e Ar program , Fl -rsh Ns = Ns Ar program
-Specify alternative program, defaults to
+Specify alternative communication program, defaults to
 .Xr ssh 1 .
 .It Fl g , -group
-Set group identifier to match the source.
-Groups are matched by name: group
-.Qq kristaps
-with id 1000 on a remote server will be properly assigned group
-.Qq kristaps
-on the local machine with id 2000.
-If the sender's group is unknown on the local machine, it is used
-verbatim.
+Set the group name to match the source.
+If
+.Fl -numeric-ids
+is also given or if the remote group name is unknown on the local machine,
+set the 

Re: bgpd: unbreak route origin validation

2019-03-31 Thread Claudio Jeker
On Fri, Mar 22, 2019 at 09:25:32PM +0100, Denis Fondras wrote:
> (better when the right diff is sent...)
> 
> ROV has been broken since the configuration reload changes.
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.466
> diff -u -p -r1.466 rde.c
> --- rde.c 13 Mar 2019 14:35:39 -  1.466
> +++ rde.c 22 Mar 2019 15:36:41 -
> @@ -2899,7 +2899,7 @@ rde_reload_done(void)
>   roa_old = conf->rde_roa;
>   as_sets_old = conf->as_sets;
>  
> - copy_config(conf, nconf);
> + memcpy(conf, nconf, sizeof(struct bgpd_config));
>   SIMPLEQ_INIT(>rde_prefixsets);
>   SIMPLEQ_INIT(>rde_originsets);
>   SIMPLEQ_CONCAT(>rde_prefixsets, >rde_prefixsets);
> 

Here is a diff that should work better. Your version introduces a use
after free because of nconf->as_sets being freed but copied to conf
beforehands.

This handles both, as_sets and rde_roa. OK?
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.467
diff -u -p -r1.467 rde.c
--- rde.c   23 Mar 2019 13:09:56 -  1.467
+++ rde.c   31 Mar 2019 13:52:01 -
@@ -2899,11 +2899,15 @@ rde_reload_done(void)
roa_old = conf->rde_roa;
as_sets_old = conf->as_sets;
 
-   memcpy(conf, nconf, sizeof(struct bgpd_config));
-   SIMPLEQ_INIT(>rde_prefixsets);
-   SIMPLEQ_INIT(>rde_originsets);
+   copy_config(conf, nconf);
+   /* need to copy the sets and roa table and clear them in nconf */
SIMPLEQ_CONCAT(>rde_prefixsets, >rde_prefixsets);
SIMPLEQ_CONCAT(>rde_originsets, >rde_originsets);
+   conf->rde_roa = nconf->rde_roa;
+   conf->as_sets = nconf->as_sets;
+   memset(>rde_roa, 0, sizeof(nconf->rde_roa));
+   nconf->as_sets = NULL;
+
free_config(nconf);
nconf = NULL;
 



Re: fix the gpio pin for ar9287-based usb devices

2019-03-31 Thread Stefan Sperling
On Sun, Mar 31, 2019 at 06:09:37PM +0800, Kevin Lo wrote:
> Hi,
> 
> AR9287-based usb devices use GPIO pin 10 for LED, not 8.
> Tested with TP-LINK TL-WN821N V3.  ok?

OK

> 
> Index: sys/dev/ic/ar9287.c
> ===
> RCS file: /cvs/src/sys/dev/ic/ar9287.c,v
> retrieving revision 1.27
> diff -u -p -u -p -r1.27 ar9287.c
> --- sys/dev/ic/ar9287.c   29 Mar 2019 11:04:40 -  1.27
> +++ sys/dev/ic/ar9287.c   31 Mar 2019 10:12:22 -
> @@ -104,7 +104,7 @@ ar9287_attach(struct athn_softc *sc)
>   AR9287_HTC_EEP_START_LOC : AR9287_EEP_START_LOC;
>   sc->eep_size = sizeof(struct ar9287_eeprom);
>   sc->ngpiopins = (sc->flags & ATHN_FLAG_USB) ? 16 : 11;
> - sc->led_pin = 8;
> + sc->led_pin = (sc->flags & ATHN_FLAG_USB) ? 10 : 8;
>   sc->workaround = AR9285_WA_DEFAULT;
>   sc->ops.setup = ar9287_setup;
>   sc->ops.swap_rom = ar9287_swap_rom;
> 



fix the gpio pin for ar9287-based usb devices

2019-03-31 Thread Kevin Lo
Hi,

AR9287-based usb devices use GPIO pin 10 for LED, not 8.
Tested with TP-LINK TL-WN821N V3.  ok?

Index: sys/dev/ic/ar9287.c
===
RCS file: /cvs/src/sys/dev/ic/ar9287.c,v
retrieving revision 1.27
diff -u -p -u -p -r1.27 ar9287.c
--- sys/dev/ic/ar9287.c 29 Mar 2019 11:04:40 -  1.27
+++ sys/dev/ic/ar9287.c 31 Mar 2019 10:12:22 -
@@ -104,7 +104,7 @@ ar9287_attach(struct athn_softc *sc)
AR9287_HTC_EEP_START_LOC : AR9287_EEP_START_LOC;
sc->eep_size = sizeof(struct ar9287_eeprom);
sc->ngpiopins = (sc->flags & ATHN_FLAG_USB) ? 16 : 11;
-   sc->led_pin = 8;
+   sc->led_pin = (sc->flags & ATHN_FLAG_USB) ? 10 : 8;
sc->workaround = AR9285_WA_DEFAULT;
sc->ops.setup = ar9287_setup;
sc->ops.swap_rom = ar9287_swap_rom;



Re: bgpd: remove announce ... from the manpage too

2019-03-31 Thread Claudio Jeker
On Wed, Mar 27, 2019 at 09:43:31PM +0100, Sebastian Benoit wrote:
> 
> ok?

Sure.
 
> (benno_announce_doc.diff)
> 
> diff --git usr.sbin/bgpd/bgpd.conf.5 usr.sbin/bgpd/bgpd.conf.5
> index a6f975e935d..86adf872f64 100644
> --- usr.sbin/bgpd/bgpd.conf.5
> +++ usr.sbin/bgpd/bgpd.conf.5
> @@ -737,23 +737,6 @@ There are several neighbor properties:
>  .Bl -tag -width Ds -compact
>  .It Xo
>  .Ic announce
> -.Pq Ic all | default-route | none
> -.Xc
> -.Ic announce all
> -is a no-op.
> -.Ic announce none
> -and
> -.Ic announce default-route
> -are aliases for
> -.Ic export none
> -and
> -.Ic export default-route ,
> -respectively.
> -These three directives are provided for backward compatibility, but will
> -eventually be removed.
> -.Pp
> -.It Xo
> -.Ic announce
>  .Pq Ic IPv4 Ns | Ns Ic IPv6
>  .Pq Ic none Ns | Ns Ic unicast Ns | Ns Ic vpn
>  .Xc
> 

-- 
:wq Claudio



Re: bgpd: unbreak route origin validation

2019-03-31 Thread Claudio Jeker
On Fri, Mar 22, 2019 at 04:47:46PM +0100, Denis Fondras wrote:
> ROV has been broken since the configuration reload changes.

I don't like this. The goal is to not use memcpy for struct bgpd_config
because of all the pointer it holds. It feels like a field is missing in
copy_config(). Will look into this later.
 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.466
> diff -u -p -r1.466 rde.c
> --- rde.c 13 Mar 2019 14:35:39 -  1.466
> +++ rde.c 22 Mar 2019 15:36:41 -
> @@ -769,7 +769,7 @@ rde_dispatch_imsg_parent(struct imsgbuf 
>   fatal(NULL);
>   SIMPLEQ_INIT(newdomains);
>   nconf = new_config();
> - copy_config(nconf, imsg.data);
> + memcpy(nconf, imsg.data, sizeof(struct bgpd_config));
>  
>   for (rid = 0; rid < rib_size; rid++) {
>   if (!rib_valid(rid))
> @@ -2899,7 +2899,7 @@ rde_reload_done(void)
>   roa_old = conf->rde_roa;
>   as_sets_old = conf->as_sets;
>  
> - copy_config(conf, nconf);
> + memcpy(conf, nconf, sizeof(struct bgpd_config));
>   SIMPLEQ_INIT(>rde_prefixsets);
>   SIMPLEQ_INIT(>rde_originsets);
>   SIMPLEQ_CONCAT(>rde_prefixsets, >rde_prefixsets);
> 

-- 
:wq Claudio