Re: pfctl should allow administrator to flush _anchors

2019-03-25 Thread Ted Unangst
Alexandr Nedvedicky wrote:
> it is, however -Fall operates on main ruleset only. -Fall also does
> not reset limits and timeouts. Hence my first idea was to introduce
> '-FNuke', which kills all rulesets and tables.
> 
> I don't want to change behaviour of existing option ('-Fall'), therefore
> I'm in favor to introduce a new option. Either '-FNuke' or '-U' works
> for me. I'm the most concerned about flushing all rulesets.

Is the existing behavior intentional or an oversight? I don't know when I
would want to use -Fall, but keep the old timeouts, and depend on that. I'd
guess most people using -Fall are keeping old timeout only by happen stance,
and not because they desire that.

In any case, if you're seeking input on the name, something like -Freset says
to me that it resets pf back to its initial state.



New driver apupgio(4): interface for LEDs and button of APU1. (Advices needed)

2019-03-25 Thread Rafael Neves
Hi tech@,

I wrote a driver to control the APU1 LEDs and read the state of
the pushbutton switch, like skgpio(4).

I would like some reviews and hints about some project decisions 
that I have to make. I am cooking a diff to support APU{2,3,4},
but it is an extension of this code.

Setup:
Machine: APU 1 (dmesg with path applied bellow).
GPIOs: Pushbutton Switch (GPIO 187), LED1 (GPIO 189), LED2 (GPIO 190),
LED3 (GPIO 191).

Implementation: One gpio(4) for pushbutton (gpio0) and other for the
LEDs (gpio1).

apugpio0 at isa0
gpio0 at apugpio0 (You can read, but can't write.)
gpio1 at apugpio0 (You can read and write.)



Questions:

1. It attaches at isabus using iomem. Is it okay?

Kettenis said (here:
https://marc.info/?l=openbsd-tech=147023610319035=2) that isa
drivers is a risk to probe after hibernate, but is it relevant with
these machines, and considering that it address is fixed and known?
 Alternatives are:
 + I could add the code directly to piixpm(4): gpio0 at
   piixpm(4).
 + Make piixpm(4) like a pcib(4), so I can attach apugpio
   to it (gpio0 at apupgio at piixpm). I think it is 
   intrusive.


2. It has two handlers to map the non contiguous addresses.

It has two handlers, one for the button and the other for the LEDs
(their addresses are contiguous). I did this because I assume that we
must not map GPIOs that we won't use, because we don't know what these
GPIOs does, and if another driver bus_space_map(9) it too the kernel
will panic(4).

Is this approach correct?


3. Is the bus_space_unmap(9) necessary when some of the mappings fails?

The driver is intended to work as a whole (button and leds) or doesn't
work at all. The bus_space_unmap(9) lines in case of failed maps are
necessary? If so, if the second bus_space_map(9) fails, I must unmap
the first one in apugpio_match(), like I am doing inside the attach 
function apugpio_attach().


4. I allow only reading from button, not setting to.

I allow only reading to button, because I think that doesn't make sense to set
its value, and when I tried it the GPIO does not hold the state that I
set. Is that ok?

5. Are there a preferred ordering between the LEDs gpio(4) and the switches one?

Now the button attaches first, but to me it is perfectly natural that
the LEDs, that people may use more, get the gpio0 driver. Any thoughts
about it?


6. Any other comments or suggestions?



Regards,
Rafael


If you have an APU1 and want to test the patch, before using
the gpio(4) devices trough gpioctl(8), you must configure them
before the securelevel is raised. For example, with the 
/etc/rc.securelevel below:


#!/bin/sh
#
# $OpenBSD: rc.securelevel,v 1.3 2014/07/14 10:15:33 ajacoutot Exp $
#
# site-specific startup actions, daemons, and other things which
# can be done BEFORE your system goes into securemode.  For actions
# which should be done AFTER your system has gone into securemode
# please see /etc/rc.local.
#

/usr/sbin/gpioctl -q gpio0 0 set

/usr/sbin/gpioctl -q gpio1 0 set
/usr/sbin/gpioctl -q gpio1 1 set
/usr/sbin/gpioctl -q gpio1 2 set



dmesg (with patch):
OpenBSD 6.5-beta (GENERIC.MP) #24: Mon Mar 25 00:34:29 WAT 2019
rne...@orus.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 4246003712 (4049MB)
avail mem = 4106997760 (3916MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xdf16d820 (7 entries)
bios0: vendor coreboot version "4.0" date 07/08/2014
bios0: PC Engines APU
acpi0 at bios0: rev 0
acpi0: sleep states S0 S1 S3 S4 S5
acpi0: tables DSDT FACP SPCR HPET APIC HEST SSDT SSDT SSDT
acpi0: wakeup devices AGPB(S4) HDMI(S4) PBR4(S4) PBR5(S4) PBR6(S4) PBR7(S4) 
PE20(S4) PE21(S4) PE22(S4) PE23(S4) PIBR(S4) UOH1(S3) UOH2(S3) UOH3(S3) 
UOH4(S3) UOH5(S3) [...]
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD G-T40E Processor, 1000.15 MHz, 14-02-00
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,MWAIT,SSSE3,CX16,POPCNT,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,IBS,SKINIT,ITSC
cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line 
16-way L2 cache
cpu0: 8 4MB entries fully associative
cpu0: DTLB 40 4KB entries fully associative, 8 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 199MHz
cpu0: mwait min=64, max=64, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD G-T40E Processor, 1000.00 MHz, 14-02-00
cpu1: 

Re: pfctl should allow administrator to flush _anchors

2019-03-25 Thread Theo de Raadt
Alexandr Nedvedicky  wrote:

> how about making the '-U' (or whatever name we agree) undocumented. We can
> also make the option available if pfctl will get compiled with 'DEBUG'
> option (assuming we are doing regress on debug bits anyway).

no, it should be documented.

but few people will use it, and that's fine



Re: pfctl should allow administrator to flush _anchors

2019-03-25 Thread Alexandr Nedvedicky
Hello,

> > > > Isn't -U pretty close to -Fall ?
> > > > 
> > > 
> > > it is, however -Fall operates on main ruleset only. -Fall also does
> > > not reset limits and timeouts. Hence my first idea was to introduce
> > > '-FNuke', which kills all rulesets and tables.
> > > 
> > > I don't want to change behaviour of existing option ('-Fall'), 
> > > therefore
> > > I'm in favor to introduce a new option. Either '-FNuke' or '-U' works
> > > for me. I'm the most concerned about flushing all rulesets.
> > > 
> > > Also making "pfctl -a '_1/_2' -Fr" to remove PF 'private' rulesets 
> > > works
> > > for me. Actually this is the most important thing I'd like to achieve.
> > 
> > whatever gets done here, the initial-raw-state-forcing should be 1 
> > operation.
> > not multiple operations acting on aspects of pf.
> > 
> > I think if it is multiple operations, people won't ever get comfortable
> > using it.
> 
> Not sure about that: I wont be comfortable anyway, as it can cause all sorts
> of problems on a running system.
> 
> When i reset things to the boot state, i would expect thats not a simple
> thing and not without issues.
> 
> I consider this as a cleanup op, most useful for regress tests, developers
> testing stuff etc. In normal sysadmin work i never needed it.

I think this is a good point. I don't expect experienced admins, who
maintain production systems to use an unconfigure operation. It's indeed
more useful for regression tests and people who are configuring their PF
in sandbox/testing environment.

how about making the '-U' (or whatever name we agree) undocumented. We can
also make the option available if pfctl will get compiled with 'DEBUG'
option (assuming we are doing regress on debug bits anyway).

sashan



Re: ospfd: Warn when the router ID changes during config reload

2019-03-25 Thread Jeremie Courreges-Anglas
On Mon, Mar 25 2019, Remi Locherer  wrote:

[...]

> This works and it makes sense to me.
>
> The log message is a bit lengthy compared to other log messages produced
> by ospfd. Maybe something like this: "router-id changed: restart required"

Yep, fine with me.

> But the patch is also OK remi@ as it is now.

ok jca@ for your wording instead

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: pfctl should allow administrator to flush _anchors

2019-03-25 Thread Sebastian Benoit
Theo de Raadt(dera...@openbsd.org) on 2019.03.24 10:22:25 -0600:
> Alexandr Nedvedicky  wrote:
> 
> > On Sun, Mar 24, 2019 at 09:51:13AM +0100, Denis Fondras wrote:
> > > On Sun, Mar 24, 2019 at 09:24:34AM +0100, Alexandr Nedvedicky wrote:
> > > > I think all the above calls for a new standalone option, which I named 
> > > > as
> > > > 'Unconfigure'.  Patch below suggest unconfigure behavior for PF.
> > > > Doing 'pfctl -U' will bring PF back to its initial state (e.g. right 
> > > > before
> > > > pf.conf got processed during the system boot). In case of PF the 
> > > > proposed -U
> > > > will do following:
> > > > - remove all rulesets and tables
> > > > - remove all states and source nodes
> > > > - remove all OS fingerprints
> > > > - set all limits, timeouts and options to their defaults
> > > > 
> > > 
> > > Isn't -U pretty close to -Fall ?
> > > 
> > 
> > it is, however -Fall operates on main ruleset only. -Fall also does
> > not reset limits and timeouts. Hence my first idea was to introduce
> > '-FNuke', which kills all rulesets and tables.
> > 
> > I don't want to change behaviour of existing option ('-Fall'), therefore
> > I'm in favor to introduce a new option. Either '-FNuke' or '-U' works
> > for me. I'm the most concerned about flushing all rulesets.
> > 
> > Also making "pfctl -a '_1/_2' -Fr" to remove PF 'private' rulesets works
> > for me. Actually this is the most important thing I'd like to achieve.
> 
> whatever gets done here, the initial-raw-state-forcing should be 1 operation.
> not multiple operations acting on aspects of pf.
> 
> I think if it is multiple operations, people won't ever get comfortable
> using it.

Not sure about that: I wont be comfortable anyway, as it can cause all sorts
of problems on a running system.

When i reset things to the boot state, i would expect thats not a simple
thing and not without issues.

I consider this as a cleanup op, most useful for regress tests, developers
testing stuff etc. In normal sysadmin work i never needed it.



unbound 1.9.1

2019-03-25 Thread Stuart Henderson
this has a couple of fixes, nothing particularly major. we already
had the fix for "#4225: clients seem to erroneously receive no answer
with DNS-over-TLS and qname-minimisation" backported.

working here, anyone else want to test?

Index: doc/Changelog
===
RCS file: /cvs/src/usr.sbin/unbound/doc/Changelog,v
retrieving revision 1.31
diff -u -p -r1.31 Changelog
--- doc/Changelog   8 Feb 2019 10:29:08 -   1.31
+++ doc/Changelog   25 Mar 2019 20:42:08 -
@@ -1,5 +1,85 @@
-5 February 2019: Wouter
-   - Fix tls-ciphers spelling in example.conf
+1 March 2019: Wouter
+   - output forwarder log in ssl_req_order test.
+
+28 February 2019: Wouter
+   - Remove memory leak on pythonmod python2 script file init.
+   - Remove swig gcc8 python function cast warnings, they are ignored.
+   - Print correct module that failed when module-config is wrong.
+
+27 February 2019: Wouter
+   - Fix #4229: Unbound man pages lack information, about access-control
+ order and local zone tags, and elements in views.
+   - Fix #14: contrib/unbound.init: Fix wrong comparison judgment
+ before copying.
+   - Fix for python module on Windows, fix fopen.
+
+25 February 2019: Wouter
+   - Fix #4227: pair event del and add for libevent for tcp_req_info.
+
+21 February 2019: Wouter
+   - Fix the error for unknown module in module-config is understandable,
+ and explains it was not compiled in and where to see the list.
+   - In example.conf explain where to put cachedb module in module-config.
+   - In man page and example config explain that most modules have to
+ be listed at the start of module-config.
+
+20 February 2019: Wouter
+   - Fix pythonmod include and sockaddr_un ifdefs for compile on
+ Windows, and for libunbound.
+
+18 February 2019: Wouter
+   - Print query name with ip_ratelimit exceeded log lines.
+   - Spaces instead of tabs in that log message.
+   - Print query name and IP address when domain rate limit exceeded.
+
+14 February 2019: Wouter
+   - Fix capsforid canonical sort qsort callback.
+
+11 February 2019: Wouter
+   - Note default for module-config in man page.
+   - Fix recursion lame test for qname minimisation asked queries,
+ that were not present in the set of prepared answers.
+   - Fix #13: Remove left-over requirements on OpenSSL >= 1.1.0 for
+ cert name matching, from man page.
+   - make depend, with newer gcc, nicer layout.
+
+7 February 2019: Wouter
+   - Fix #4206: OpenSSL 1.0.2 hostname verification for FreeBSD 11.2.
+   - Fix that qname minimisation does not skip a label when missing
+ nameserver targets need to be fetched.
+   - Fix #4225: clients seem to erroneously receive no answer with
+ DNS-over-TLS and qname-minimisation.
+
+4 February 2019: Wouter
+   - Fix that log-replies prints the correct name for local-alias
+ names, for names that have a CNAME in local-data configuration.
+ It logs the original query name, not the target of the CNAME.
+   - Add local-zone type inform_redirect, which logs like type inform,
+ and redirects like type redirect.
+   - Perform canonical sort for 0x20 capsforid compare of replies,
+ this sorts rrsets in the authority and additional section before
+ comparison, so that out of order rrsets do not cause failure.
+
+31 January 2019: Wouter
+   - Set ub_ctx_set_tls call signature in ltrace config file for
+ libunbound in contrib/libunbound.so.conf.
+   - improve documentation for tls-service-key and forward-first.
+   - #10: fixed pkg-config operations, PKG_PROG_PKG_CONFIG moved out of
+ conditional section, fixes systemd builds, from Enrico Scholz.
+   - #9: For openssl 1.0.2 use the CRYPTO_THREADID locking callbacks,
+ still supports the set_id_callback previous API.  And for 1.1.0
+ no locking callbacks are needed.
+   - #8: Fix OpenSSL without ENGINE support compilation.
+   - Wipe TLS session key data from memory on exit.
+
+30 January 2019: Ralph
+   - Fix case in which query timeout can result in marking delegation
+ as edns_lame_known.
+
+29 January 2019: Wouter
+   - Fix spelling of tls-ciphers in example.conf.in.
+   - Fix #4224: auth_xfr_notify.rpl test broken due to typo
+   - Fix locking for libunbound context setup with broken port config.
 
 28 January 2019: Wouter
- ub_ctx_set_tls call for libunbound that enables DoT for the machines
@@ -8,7 +88,9 @@
- List example config for root zone copy locally hosted with auth-zone
  as suggested from draft-ietf-dnsop-7706-bis-02.  But with updated
  B root address.
-   - set version to 1.9.0 for release.
+   - set version to 1.9.0 for release.  And this was released with the
+ spelling 

Re: chflagsat(2): fix function argument

2019-03-25 Thread Jeremie Courreges-Anglas
On Mon, Mar 25 2019, Anton Lindqvist  wrote:
> Hi,
> My guess is that flag actually refers to atflags.
>
> Comments? OK?

ok jca@

> Index: chflags.2
> ===
> RCS file: /cvs/src/lib/libc/sys/chflags.2,v
> retrieving revision 1.27
> diff -u -p -r1.27 chflags.2
> --- chflags.2 19 Jan 2015 15:54:11 -  1.27
> +++ chflags.2 25 Mar 2019 20:38:23 -
> @@ -113,12 +113,12 @@ in the
>  .Fa fd
>  parameter, the current working directory is used.
>  If
> -.Fa flag
> +.Fa atflags
>  is also zero, the behavior is identical to a call to
>  .Fn chflags .
>  .Pp
>  The
> -.Fa flag
> +.Fa atflags
>  argument is the bitwise OR of zero or more of the following values:
>  .Pp
>  .Bl -tag -width AT_SYMLINK_NOFOLLOW -offset indent -compact
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: chflagsat(2): fix function argument

2019-03-25 Thread Todd C . Miller
On Mon, 25 Mar 2019 21:44:45 +0100, Anton Lindqvist wrote:

> My guess is that flag actually refers to atflags.

Yes, looks like copy pasta from fchmodat.  OK millert@

 - todd



Re: ospfd: Warn when the router ID changes during config reload

2019-03-25 Thread Remi Locherer
On Mon, Mar 25, 2019 at 02:43:26PM +0100, Jeremie Courreges-Anglas wrote:
> On Sun, Mar 24 2019, Mitchell Krome  wrote:
> > On 24/03/2019 7:23 am, Theo de Raadt wrote:
> >> Sebastian Benoit  wrote:
> >> 
> >>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000:
>  Was messing around with ospf and got myself into a situation where the
>  router ID's were the same on two boxes because I only did a reload on
>  one of them when I changed the loopback IP's.
> >>>
> >>> Thats sub optimal i believe...
> >>>
>  This adds a warning when reloading if the router ID changes (there was
>  already a comment saying as much). Same patch can probably be applied to
>  ospf6d if people think it's useful.
> 
> ospf6d currently doesn't support config reloads at all.  It might be
> worth adding an XXX comment there.
> 
> >>> I think it would be better to abort the reload if the router-id is 
> >>> changed,
> >>> i.e. not load the new config at all.
> >> 
> >> That's the right approach in all our other daemons:
> >> 
> >> if the configuration change cannot be installed correctly, consider
> >> it invalid and abort.  Someone would need to write code to make it
> >> valid..
> >> 
> >
> > That makes sense. I checked the manuals for the routers I use at work
> > and they also required the ospf process to be restarted for the config
> > to take effect after changing the router id.
> >
> > I moved the check up into ospf_reload because it doesn't make sense
> > sending the config to all the children if we know we're going to abort.
> 
> Your patch was mangled (long line wrapped) but the changes looked good.
> Here's an updated version which tweaks punctuation and case (to match
> the router-id keyword).  Works for me in my simple test setup.
> 
> Comments/oks?

This works and it makes sense to me.

The log message is a bit lengthy compared to other log messages produced
by ospfd. Maybe something like this: "router-id changed: restart required"

But the patch is also OK remi@ as it is now.

> 
> 
> Index: ospfd.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.105
> diff -u -p -r1.105 ospfd.c
> --- ospfd.c   15 Jan 2019 22:18:10 -  1.105
> +++ ospfd.c   25 Mar 2019 13:33:43 -
> @@ -642,6 +642,13 @@ ospf_reload(void)
>   if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
>   return (-1);
>  
> + /* Abort the reload if rtr_id changed */
> + if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) {
> + log_warnx("router-id changed in new configuration, "
> + "this requires ospfd to be restarted.");
> + return (-1);
> + }
> +
>   /* send config to childs */
>   if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
>   return (-1);
> @@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st
>   struct redistribute *r;
>   int  rchange = 0;
>  
> - /* change of rtr_id needs a restart */
>   conf->flags = xconf->flags;
>   conf->spf_delay = xconf->spf_delay;
>   conf->spf_hold_time = xconf->spf_hold_time;
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



chflagsat(2): fix function argument

2019-03-25 Thread Anton Lindqvist
Hi,
My guess is that flag actually refers to atflags.

Comments? OK?

Index: chflags.2
===
RCS file: /cvs/src/lib/libc/sys/chflags.2,v
retrieving revision 1.27
diff -u -p -r1.27 chflags.2
--- chflags.2   19 Jan 2015 15:54:11 -  1.27
+++ chflags.2   25 Mar 2019 20:38:23 -
@@ -113,12 +113,12 @@ in the
 .Fa fd
 parameter, the current working directory is used.
 If
-.Fa flag
+.Fa atflags
 is also zero, the behavior is identical to a call to
 .Fn chflags .
 .Pp
 The
-.Fa flag
+.Fa atflags
 argument is the bitwise OR of zero or more of the following values:
 .Pp
 .Bl -tag -width AT_SYMLINK_NOFOLLOW -offset indent -compact



Re: ospfd: Warn when the router ID changes during config reload

2019-03-25 Thread Sebastian Benoit
ok

Jeremie Courreges-Anglas(j...@wxcvbn.org) on 2019.03.25 14:43:26 +0100:
> On Sun, Mar 24 2019, Mitchell Krome  wrote:
> > On 24/03/2019 7:23 am, Theo de Raadt wrote:
> >> Sebastian Benoit  wrote:
> >> 
> >>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000:
>  Was messing around with ospf and got myself into a situation where the
>  router ID's were the same on two boxes because I only did a reload on
>  one of them when I changed the loopback IP's.
> >>>
> >>> Thats sub optimal i believe...
> >>>
>  This adds a warning when reloading if the router ID changes (there was
>  already a comment saying as much). Same patch can probably be applied to
>  ospf6d if people think it's useful.
> 
> ospf6d currently doesn't support config reloads at all.  It might be
> worth adding an XXX comment there.
> 
> >>> I think it would be better to abort the reload if the router-id is 
> >>> changed,
> >>> i.e. not load the new config at all.
> >> 
> >> That's the right approach in all our other daemons:
> >> 
> >> if the configuration change cannot be installed correctly, consider
> >> it invalid and abort.  Someone would need to write code to make it
> >> valid..
> >> 
> >
> > That makes sense. I checked the manuals for the routers I use at work
> > and they also required the ospf process to be restarted for the config
> > to take effect after changing the router id.
> >
> > I moved the check up into ospf_reload because it doesn't make sense
> > sending the config to all the children if we know we're going to abort.
> 
> Your patch was mangled (long line wrapped) but the changes looked good.
> Here's an updated version which tweaks punctuation and case (to match
> the router-id keyword).  Works for me in my simple test setup.
> 
> Comments/oks?
> 
> 
> Index: ospfd.c
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
> retrieving revision 1.105
> diff -u -p -r1.105 ospfd.c
> --- ospfd.c   15 Jan 2019 22:18:10 -  1.105
> +++ ospfd.c   25 Mar 2019 13:33:43 -
> @@ -642,6 +642,13 @@ ospf_reload(void)
>   if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
>   return (-1);
>  
> + /* Abort the reload if rtr_id changed */
> + if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) {
> + log_warnx("router-id changed in new configuration, "
> + "this requires ospfd to be restarted.");
> + return (-1);
> + }
> +
>   /* send config to childs */
>   if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
>   return (-1);
> @@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st
>   struct redistribute *r;
>   int  rchange = 0;
>  
> - /* change of rtr_id needs a restart */
>   conf->flags = xconf->flags;
>   conf->spf_delay = xconf->spf_delay;
>   conf->spf_hold_time = xconf->spf_hold_time;
> 
> 
> -- 
> jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE
> 



Re: ssl(8): change example key size to 4096 bits

2019-03-25 Thread Jason McIntyre
On Sat, Mar 23, 2019 at 11:30:15PM -0600, Randy Hartman wrote:
> ssl(8) man page changed example key size from 2048 to 4096
> in 2012 but reverted after two days.  smtpd.conf(5)'s man page
> and acme-client both use 4096.  is it time to un-revert?
> 

fixed, thanks.
jmc

> Index: ssl.8
> ===
> RCS file: /cvs/src/share/man/man8/ssl.8,v
> retrieving revision 1.66
> diff -u -p -u -p -r1.66 ssl.8
> --- ssl.8 8 Jul 2017 17:52:44 -   1.66
> +++ ssl.8 24 Mar 2019 04:41:31 -
> @@ -65,13 +65,13 @@ To support HTTPS transactions in
>  .Xr httpd 8
>  you will need to generate an RSA certificate.
>  .Bd -literal -offset indent
> -# openssl genrsa -out /etc/ssl/private/server.key 2048
> +# openssl genrsa -out /etc/ssl/private/server.key 4096
>  .Ed
>  .Pp
>  Or, if you wish the key to be encrypted with a passphrase that you will
>  have to type in when starting servers
>  .Bd -literal -offset indent
> -# openssl genrsa -aes256 -out /etc/ssl/private/server.key 2048
> +# openssl genrsa -aes256 -out /etc/ssl/private/server.key 4096
>  .Ed
>  .Pp
>  The next step is to generate a Certificate Signing Request (CSR) which is
> 



Re: divert(4): increment divs_errors on if_get failure

2019-03-25 Thread Martin Pieuchot
On 24/03/19(Sun) 20:16, Lawrence Teo wrote:
> This diff modifies divert_packet() to increment the divs_errors counter
> on if_get failure so that users will be aware of it via netstat(1).

If if_get(9) fails that means the interface no longer exists.  Is it an
error?  Or a delayed free?  Did we write an incorrect interface index?

> The same thing is done for divert6_packet().

Is it the correct things to do?

> While here, it also modifies divert_output() to move m_freem(m) below
> divstat_inc(divs_errors).  This is purely for consistency since all the
> other divert_* functions increment the divstat counters before m_freem.
> divert6_output() already does this so there is no need to make the same
> change there.
> 
> ok?
> 
> 
> Index: netinet/ip_divert.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_divert.c,v
> retrieving revision 1.61
> diff -u -p -r1.61 ip_divert.c
> --- netinet/ip_divert.c   4 Feb 2019 21:40:52 -   1.61
> +++ netinet/ip_divert.c   24 Mar 2019 23:39:58 -
> @@ -163,8 +163,8 @@ divert_output(struct inpcb *inp, struct 
>   return (error);
>  
>  fail:
> - m_freem(m);
>   divstat_inc(divs_errors);
> + m_freem(m);
>   return (error ? error : EINVAL);
>  }
>  
> @@ -199,6 +199,7 @@ divert_packet(struct mbuf *m, int dir, u
>  
>   ifp = if_get(m->m_pkthdr.ph_ifidx);
>   if (ifp == NULL) {
> + divstat_inc(divs_errors);
>   m_freem(m);
>   return (0);
>   }
> Index: netinet6/ip6_divert.c
> ===
> RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
> retrieving revision 1.59
> diff -u -p -r1.59 ip6_divert.c
> --- netinet6/ip6_divert.c 4 Feb 2019 21:40:52 -   1.59
> +++ netinet6/ip6_divert.c 24 Mar 2019 23:39:03 -
> @@ -205,6 +205,7 @@ divert6_packet(struct mbuf *m, int dir, 
>  
>   ifp = if_get(m->m_pkthdr.ph_ifidx);
>   if (ifp == NULL) {
> + div6stat_inc(div6s_errors);
>   m_freem(m);
>   return (0);
>   }
> 



Re: diff: reboot(8): document the -l option

2019-03-25 Thread Theo de Raadt
When you see a comment like this, it means it is intentionally
undocumented.

It is the current mechanism shutdown uses.  It could use a different
mechanism in the future.  It is done this way so that noone else depends
on it.  If you document this user-visible, they will come to depend on
it, then we cannot be agile and change the mechanism later.

I don't think we don't want that, so no.

If you persist at going through the tree you'll find about 20 other
instances of this.  It is a fairly common pattern.

-   case 'l':   /* Undocumented; used by shutdown. */


> Hi,
> 
> The following diff adds missing documentation for the -l option of
> reboot(8) as its done in NetBSD.
> 
> Bye,
> Jan
> 
> Index: reboot.8
> ===
> RCS file: /cvs/src/sbin/reboot/reboot.8,v
> retrieving revision 1.50
> diff -u -p -r1.50 reboot.8
> --- reboot.8  3 Sep 2016 14:25:05 -   1.50
> +++ reboot.8  25 Mar 2019 14:12:48 -
> @@ -30,7 +30,7 @@
>  .\"
>  .\"  @(#)reboot.88.1 (Berkeley) 6/9/93
>  .\"
> -.Dd $Mdocdate: September 3 2016 $
> +.Dd $Mdocdate: March 25 2019 $
>  .Dt REBOOT 8
>  .Os
>  .Sh NAME
> @@ -39,9 +39,9 @@
>  .Nd stopping and restarting the system
>  .Sh SYNOPSIS
>  .Nm halt
> -.Op Fl dnpq
> +.Op Fl dlnpq
>  .Nm reboot
> -.Op Fl dnq
> +.Op Fl dlnq
>  .Sh DESCRIPTION
>  The
>  .Nm halt
> @@ -70,6 +70,10 @@ capturing the state of a corrupted or mi
>  See
>  .Xr savecore 8
>  for information on how to recover this dump.
> +.It Fl l
> +Suppress sending a message via
> +.Xr syslog 3
> +before halting or restarting.
>  .It Fl n
>  Prevent file system cache from being flushed.
>  This option should probably not be used.
> Index: reboot.c
> ===
> RCS file: /cvs/src/sbin/reboot/reboot.c,v
> retrieving revision 1.38
> diff -u -p -r1.38 reboot.c
> --- reboot.c  22 Aug 2017 00:30:16 -  1.38
> +++ reboot.c  25 Mar 2019 14:03:43 -
> @@ -94,7 +94,7 @@ main(int argc, char *argv[])
>   case 'd':
>   howto |= RB_DUMP;
>   break;
> - case 'l':   /* Undocumented; used by shutdown. */
> + case 'l':
>   lflag = 1;
>   break;
>   case 'n':
> @@ -272,7 +272,7 @@ restart:
>  void
>  usage(void)
>  {
> - fprintf(stderr, "usage: %s [-dn%sq]\n", __progname,
> + fprintf(stderr, "usage: %s [-dln%sq]\n", __progname,
>   dohalt ? "p" : "");
>   exit(1);
>  }
> 



Re: [patch] Re: Possible sasyncd memory leak ?

2019-03-25 Thread Otto Moerbeek
On Sat, Mar 23, 2019 at 06:07:02PM +0100, Michał Koc wrote:

> ... [snip]

This is almost good. You might fold host_ip() into net_set_sa(). the
double malloc and copy isn't really needed.

-Otto



diff: reboot(8): document the -l option

2019-03-25 Thread Jan Klemkow
Hi,

The following diff adds missing documentation for the -l option of
reboot(8) as its done in NetBSD.

Bye,
Jan

Index: reboot.8
===
RCS file: /cvs/src/sbin/reboot/reboot.8,v
retrieving revision 1.50
diff -u -p -r1.50 reboot.8
--- reboot.83 Sep 2016 14:25:05 -   1.50
+++ reboot.825 Mar 2019 14:12:48 -
@@ -30,7 +30,7 @@
 .\"
 .\"@(#)reboot.88.1 (Berkeley) 6/9/93
 .\"
-.Dd $Mdocdate: September 3 2016 $
+.Dd $Mdocdate: March 25 2019 $
 .Dt REBOOT 8
 .Os
 .Sh NAME
@@ -39,9 +39,9 @@
 .Nd stopping and restarting the system
 .Sh SYNOPSIS
 .Nm halt
-.Op Fl dnpq
+.Op Fl dlnpq
 .Nm reboot
-.Op Fl dnq
+.Op Fl dlnq
 .Sh DESCRIPTION
 The
 .Nm halt
@@ -70,6 +70,10 @@ capturing the state of a corrupted or mi
 See
 .Xr savecore 8
 for information on how to recover this dump.
+.It Fl l
+Suppress sending a message via
+.Xr syslog 3
+before halting or restarting.
 .It Fl n
 Prevent file system cache from being flushed.
 This option should probably not be used.
Index: reboot.c
===
RCS file: /cvs/src/sbin/reboot/reboot.c,v
retrieving revision 1.38
diff -u -p -r1.38 reboot.c
--- reboot.c22 Aug 2017 00:30:16 -  1.38
+++ reboot.c25 Mar 2019 14:03:43 -
@@ -94,7 +94,7 @@ main(int argc, char *argv[])
case 'd':
howto |= RB_DUMP;
break;
-   case 'l':   /* Undocumented; used by shutdown. */
+   case 'l':
lflag = 1;
break;
case 'n':
@@ -272,7 +272,7 @@ restart:
 void
 usage(void)
 {
-   fprintf(stderr, "usage: %s [-dn%sq]\n", __progname,
+   fprintf(stderr, "usage: %s [-dln%sq]\n", __progname,
dohalt ? "p" : "");
exit(1);
 }



Re: ospfd: Warn when the router ID changes during config reload

2019-03-25 Thread Jeremie Courreges-Anglas
On Sun, Mar 24 2019, Mitchell Krome  wrote:
> On 24/03/2019 7:23 am, Theo de Raadt wrote:
>> Sebastian Benoit  wrote:
>> 
>>> Mitchell Krome(mitchellkr...@gmail.com) on 2019.03.23 20:27:17 +1000:
 Was messing around with ospf and got myself into a situation where the
 router ID's were the same on two boxes because I only did a reload on
 one of them when I changed the loopback IP's.
>>>
>>> Thats sub optimal i believe...
>>>
 This adds a warning when reloading if the router ID changes (there was
 already a comment saying as much). Same patch can probably be applied to
 ospf6d if people think it's useful.

ospf6d currently doesn't support config reloads at all.  It might be
worth adding an XXX comment there.

>>> I think it would be better to abort the reload if the router-id is changed,
>>> i.e. not load the new config at all.
>> 
>> That's the right approach in all our other daemons:
>> 
>> if the configuration change cannot be installed correctly, consider
>> it invalid and abort.  Someone would need to write code to make it
>> valid..
>> 
>
> That makes sense. I checked the manuals for the routers I use at work
> and they also required the ospf process to be restarted for the config
> to take effect after changing the router id.
>
> I moved the check up into ospf_reload because it doesn't make sense
> sending the config to all the children if we know we're going to abort.

Your patch was mangled (long line wrapped) but the changes looked good.
Here's an updated version which tweaks punctuation and case (to match
the router-id keyword).  Works for me in my simple test setup.

Comments/oks?


Index: ospfd.c
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
retrieving revision 1.105
diff -u -p -r1.105 ospfd.c
--- ospfd.c 15 Jan 2019 22:18:10 -  1.105
+++ ospfd.c 25 Mar 2019 13:33:43 -
@@ -642,6 +642,13 @@ ospf_reload(void)
if ((xconf = parse_config(conffile, ospfd_conf->opts)) == NULL)
return (-1);
 
+   /* Abort the reload if rtr_id changed */
+   if (ospfd_conf->rtr_id.s_addr != xconf->rtr_id.s_addr) {
+   log_warnx("router-id changed in new configuration, "
+   "this requires ospfd to be restarted.");
+   return (-1);
+   }
+
/* send config to childs */
if (ospf_sendboth(IMSG_RECONF_CONF, xconf, sizeof(*xconf)) == -1)
return (-1);
@@ -693,7 +700,6 @@ merge_config(struct ospfd_conf *conf, st
struct redistribute *r;
int  rchange = 0;
 
-   /* change of rtr_id needs a restart */
conf->flags = xconf->flags;
conf->spf_delay = xconf->spf_delay;
conf->spf_hold_time = xconf->spf_hold_time;


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: MPLSv6 2/2 : bgpd diff

2019-03-25 Thread Denis Fondras
On Sun, Mar 24, 2019 at 10:03:15PM +1300, Richard Procter wrote:
> The ldpd issue might merit a hint in the man page as I found it difficult to 
> diagnose 
> as a newbie (see attached patch), and the man page, while not wrong, threw me 
> by 
> stating that GTSM is mandatory for LDPv6; it is now but wasn’t in the past. 
> 
> ok for the patch?
> 

Thank you very much for testing and the report.

I like the idea of being more specific in the manual.
OK denis@, I would however remove the parenthesis.

> Are there any other tests you’d be interested in while I have the machine 
> configured? 
> 
> best, 
> Richard. 
> 
> Here’s the version info of JUNOS I was using (note the git -dirty commit 
> (!!!)) 
> 
> uname -a: FreeBSD mx480-lab-re0 JNPR-10.3-20171207.04b87e3_buil FreeBSD 
> JNPR-10.3-20171207.04b87e3_builder_stable_10 #0 r356532+04b87e3(HEAD)-dirty: 
> Thu Dec  7 09:13:19 PST 2017 
> buil...@basith.juniper.net:/volume/build/junos/occam/freebsd/stable_10/20171116.200501_builder_stable_10.04b87e3/obj/amd64/juniper/kernels/JNPR-AMD64-PRD/kernel
>   amd64
> 
> Model: mx480
> Family: junos
> Junos: 17.2R2-S2.1
> 
> RFC5036 (2007) "LDP Specification"
> 
> RFC6720 (2012) "The Generalized TTL Security Mechanism (GTSM) for LDP” - 1 
> >> GTSM specifies that "it SHOULD NOT be enabled by default in order to
> >> remain backward compatible with the unmodified protocol" (see
> >> Section 3 of [RFC5082]).
> >> 
> >> This document specifies a "built-in dynamic GTSM capability negotiation" 
> >> for
> >> LDP to suggest the use of GTSM.  GTSM will be used as specified in this
> >> document provided both peers on an LDP session can detect each others' 
> >> support
> >> for GTSM procedures and agree to use it.  That is, the desire to use GTSM
> >> (i.e., its negotiation mechanics) is enabled by default without any 
> >> configuration.
> 
> RFC7552 (2015) "Updates to LDP for IPv6” - 5.1
> >> Also, the LDP Link Hello packets MUST have their IPv6 Hop Limit set
> >> to 255, be checked for the same upon receipt (before any LDP-specific
> >> processing), and be handled as specified in Section 3 of [RFC5082].
> 
> 
> cvs server: Diffing .
> Index: ldpd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ldpd/ldpd.conf.5,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 ldpd.conf.5
> --- ldpd.conf.5 23 Jan 2019 02:02:04 -  1.37
> +++ ldpd.conf.5 24 Mar 2019 08:36:14 -
> @@ -177,7 +177,8 @@ and RFC 7552 (for the IPv6 address-famil
>  Since GTSM is mandatory for LDPv6, the only effect of disabling GTSM for the
>  IPv6 address-family is that
>  .Xr ldpd 8
> -will not check the incoming packets' hop limit.
> +will not check the incoming packets' hop limit. (This may be necessary to 
> +interoperate with implementations lacking RFC 7552 (2015) compliance.)   
>  Outgoing packets will still be sent using a hop limit of 255 to guarantee
>  interoperability.
>  .Pp
> 
>