Re: unwind: log missing config file
On Mon, Nov 18, 2019 at 07:57:09PM -0700, Theo de Raadt wrote: > Florian Obser wrote: > > > On Tue, Nov 19, 2019 at 12:15:34AM +0100, Klemens Nanni wrote: > > > On Mon, Nov 18, 2019 at 10:19:47PM +0100, Klemens Nanni wrote: > > > > With that, my initial case is no longer misleading; alternatively, I > > > > can implement the dash semantic, but that's another diff. > > > Hm, that makes the default setup (no /etc/unwind.conf, empty > > > unwind_flags) always print a warning, which is ugly. > > > > It's not just ugly, it's missleading. We want unwind to be the best it > > can be *without* a config file. We don't to suggest a config file. We > > want to steer people away from pushing buttons, not towards. > > Agree strongly. > > > Btw. I'm not sure the parser can work with stdin, I think it needs to > > seek, which you can't do on stdin? But I might be mistaken on multiple > > levels. > > No, I think it only does ungetc and (nested fd's in case of include > if you do that) If it is the normal parse.y setup then it does work on stdin (at least for simple configs). I do use stdin with bgpd to validate rpki files before putting them into place. -- :wq Claudio
Re: unwind: log missing config file
Florian Obser wrote: > On Tue, Nov 19, 2019 at 12:15:34AM +0100, Klemens Nanni wrote: > > On Mon, Nov 18, 2019 at 10:19:47PM +0100, Klemens Nanni wrote: > > > With that, my initial case is no longer misleading; alternatively, I > > > can implement the dash semantic, but that's another diff. > > Hm, that makes the default setup (no /etc/unwind.conf, empty > > unwind_flags) always print a warning, which is ugly. > > It's not just ugly, it's missleading. We want unwind to be the best it > can be *without* a config file. We don't to suggest a config file. We > want to steer people away from pushing buttons, not towards. Agree strongly. > Btw. I'm not sure the parser can work with stdin, I think it needs to > seek, which you can't do on stdin? But I might be mistaken on multiple > levels. No, I think it only does ungetc and (nested fd's in case of include if you do that)
Re: unwind: log missing config file
On Tue, Nov 19, 2019 at 12:15:34AM +0100, Klemens Nanni wrote: > On Mon, Nov 18, 2019 at 10:19:47PM +0100, Klemens Nanni wrote: > > With that, my initial case is no longer misleading; alternatively, I > > can implement the dash semantic, but that's another diff. > Hm, that makes the default setup (no /etc/unwind.conf, empty > unwind_flags) always print a warning, which is ugly. It's not just ugly, it's missleading. We want unwind to be the best it can be *without* a config file. We don't to suggest a config file. We want to steer people away from pushing buttons, not towards. Btw. I'm not sure the parser can work with stdin, I think it needs to seek, which you can't do on stdin? But I might be mistaken on multiple levels. > > Please disregard the diff, perhaps I'll come up with a better idea. > I think the file doesn't exist but it's fine check is in the wrong place. It's also stupid that it doesn't fatal(!) for unwind -f nonexistent. I think this needs to be handled further up in main. Maybe. I'll have a look if I don't forget. -- I'm not entirely sure you are real.
Re: unwind: log missing config file
On Mon, Nov 18, 2019 at 10:19:47PM +0100, Klemens Nanni wrote: > With that, my initial case is no longer misleading; alternatively, I > can implement the dash semantic, but that's another diff. Hm, that makes the default setup (no /etc/unwind.conf, empty unwind_flags) always print a warning, which is ugly. Please disregard the diff, perhaps I'll come up with a better idea.
Re: ifconfig inet6 netmask
On Mon, Nov 18, 2019 at 12:04:47PM -0700, Theo de Raadt wrote: > > As our ifconfig(8) man page says we support it, and route(8) also > > supports it, I prefer fixing inet6 netmask. > > accepting a netmask also requires thinking about the behaviour in the > discontig case (surely that means detecting and preventing it). I saw this check for inet netmask in ifconfig. Setting the inet route also prevents such an address in the kernel. But if I remove all userland checks and use old SIOCSIFNETMASK interface I get inet 10.188.254.74 netmask 0xff40 broadcast 10.255.255.255 For inet6 in6_mask2len() does the check within the kernel. /sbin/ifconfig vether99 inet6 fdd7:e83e:66bc:254::74 netmask :::::4000:: ifconfig: SIOCAIFADDR: Invalid argument It should be checked early in the kernel for both families. bluhm
contiguous inet netmask check in kernel
Hi,
Although ifconfig(8) checks it already, contiguous inet netmask
should be enforced by the kernel. Currently the routing table does
not support non-contiguous netmask, but we should error out early
during interface ioctl(2).
ok?
bluhm
Index: netinet/in.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.c,v
retrieving revision 1.166
diff -u -p -r1.166 in.c
--- netinet/in.c8 Nov 2019 07:16:29 - 1.166
+++ netinet/in.c18 Nov 2019 22:50:38 -
@@ -330,8 +330,16 @@ in_ioctl(u_long cmd, caddr_t data, struc
error = EINVAL;
break;
}
+ /* do not check inet family or strict len */
+ sin = satosin(&ifr->ifr_addr);
+ if (ntohl(sin->sin_addr.s_addr) &
+ (~ntohl(sin->sin_addr.s_addr) >> 1)) {
+ /* non-contiguous netmask */
+ error = EINVAL;
+ break;
+ }
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr =
- satosin(&ifr->ifr_addr)->sin_addr.s_addr;
+ sin->sin_addr.s_addr;
break;
}
err:
@@ -404,6 +412,7 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
struct in_ifaddr *ia = NULL;
struct in_aliasreq *ifra = (struct in_aliasreq *)data;
struct sockaddr_in *sin = NULL, *dstsin = NULL, *broadsin = NULL;
+ struct sockaddr_in *masksin = NULL;
int error = 0;
int newifaddr;
@@ -440,6 +449,14 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
error = EINVAL;
break;
}
+ /* do not check inet family or strict len */
+ masksin = &ifra->ifra_mask;
+ if (ntohl(masksin->sin_addr.s_addr) &
+ (~ntohl(masksin->sin_addr.s_addr) >> 1)) {
+ /* non-contiguous netmask */
+ error = EINVAL;
+ break;
+ }
}
if ((ifp->if_flags & IFF_POINTOPOINT) &&
ifra->ifra_dstaddr.sin_family == AF_INET) {
@@ -480,10 +497,10 @@ in_ioctl_change_ifaddr(u_long cmd, caddr
sin->sin_addr.s_addr != ia->ia_addr.sin_addr.s_addr) {
needinit = 1;
}
- if (ifra->ifra_mask.sin_len) {
+ if (masksin != NULL) {
in_ifscrub(ifp, ia);
ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr =
- ifra->ifra_mask.sin_addr.s_addr;
+ masksin->sin_addr.s_addr;
needinit = 1;
}
if (dstsin != NULL) {
Re: kill VNDIOCGET60
Jeremie Courreges-Anglas wrote: > On Mon, Nov 18 2019, Benjamin Baier wrote: > > Hi, found this. > > No idea why I commented this out instead of just deleting it. We're not > keeping old ioctls in other places. Committed, thanks. yes, that was a weird style.
Re: kill VNDIOCGET60
On Mon, Nov 18 2019, Benjamin Baier wrote:
> Hi, found this.
No idea why I commented this out instead of just deleting it. We're not
keeping old ioctls in other places. Committed, thanks.
> -- Ben
>
> Index: vndioctl.h
> ===
> RCS file: /cvs/src/sys/dev/vndioctl.h,v
> retrieving revision 1.10
> diff -u -p -r1.10 vndioctl.h
> --- vndioctl.h14 Dec 2016 18:59:12 - 1.10
> +++ vndioctl.h18 Nov 2019 20:11:50 -
> @@ -75,8 +75,6 @@ struct vnd_user {
> */
> #define VNDIOCSET_IOWR('F', 0, struct vnd_ioctl) /* enable disk */
> #define VNDIOCCLR_IOW('F', 1, struct vnd_ioctl) /* disable disk */
> -/* XXX kill after 6.1 */
> -/* #define VNDIOCGET60 _IOWR('F', 2, struct vnd_user60) */
> #define VNDIOCGET_IOWR('F', 3, struct vnd_user) /* get disk info */
>
> #endif /* !_SYS_VNDIOCTL_H_ */
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
kill VNDIOCGET60
Hi, found this.
-- Ben
Index: vndioctl.h
===
RCS file: /cvs/src/sys/dev/vndioctl.h,v
retrieving revision 1.10
diff -u -p -r1.10 vndioctl.h
--- vndioctl.h 14 Dec 2016 18:59:12 - 1.10
+++ vndioctl.h 18 Nov 2019 20:11:50 -
@@ -75,8 +75,6 @@ struct vnd_user {
*/
#define VNDIOCSET _IOWR('F', 0, struct vnd_ioctl) /* enable disk */
#define VNDIOCCLR _IOW('F', 1, struct vnd_ioctl) /* disable disk */
-/* XXX kill after 6.1 */
-/* #define VNDIOCGET60 _IOWR('F', 2, struct vnd_user60) */
#define VNDIOCGET _IOWR('F', 3, struct vnd_user) /* get disk info */
#endif /* !_SYS_VNDIOCTL_H_ */
unwind: log missing config file
I gave unwind.conf(5) a read and wanted to test the following:
$ echo preference dhcp | unwind -vnf-
preference { DoT forwarder recursor dhcp stub }
First, I thought unwind had a parser bug and would ignore my preference,
but turns out it simply does not treat "-" as standard input.
Fair enough, using /dev/stdin works, but while playing around I found
that logging nonexistent config files would really help, so the following
diff does
$ unwind -vnf/nonexistent
preference { DoT forwarder recursor dhcp stub }
$ ./obj/unwind -vnf/nonexistent
/nonexistent: No such file or directory
preference { DoT forwarder recursor dhcp stub }
With that, my initial case is no longer misleading; alternatively, I
can implement the dash semantic, but that's another diff.
OK?
Index: parse.y
===
RCS file: /cvs/src/sbin/unwind/parse.y,v
retrieving revision 1.15
diff -u -p -r1.15 parse.y
--- parse.y 9 Nov 2019 16:28:10 - 1.15
+++ parse.y 18 Nov 2019 21:08:26 -
@@ -784,9 +784,9 @@ parse_config(char *filename)
file = pushfile(filename, 0);
if (file == NULL) {
+ log_warn("%s", filename);
if (errno == ENOENT)/* no config file is fine */
return (conf);
- log_warn("%s", filename);
free(conf);
return (NULL);
}
Re: XHCI support for bulk xfers >64k
On Sun, Nov 17, 2019 at 12:36:49PM +0100, Marcus Glocker wrote: > While recently testing UDL on XHCI I faced a lot of USB timeouts, and > therefore screen rendering issues. > The reason is that XHCI currently only supports single bulk transfers > up to 64k, while UDL can schedule a bulk transfer up to 1m. > > The following diff adds XHCI bulk transfer support >64k and makes UDL > work fine for me on XHCI. > > mpi@ already did an initial re-view, and I have adapted some of his > comments already in this diff. > > More testing and comments welcome. After patrick@ did feedback that the current bulk transfer code in XHCI should also work for larger transfers as is, we have found the bug which resulted in a much smaller diff :-) Attached for your reference. It just has been committed. Index: xhci.c === RCS file: /cvs/src/sys/dev/usb/xhci.c,v retrieving revision 1.106 diff -u -p -u -p -r1.106 xhci.c --- xhci.c 6 Oct 2019 17:30:00 - 1.106 +++ xhci.c 18 Nov 2019 19:32:30 - @@ -2871,7 +2871,7 @@ xhci_device_generic_start(struct usbd_xf /* If the buffer crosses a 64k boundary, we need one more. */ len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); if (len < xfer->length) - ntrb++; + ntrb = howmany(xfer->length - len, XHCI_TRB_MAXSIZE) + 1; else len = xfer->length;
Re: ifconfig inet6 netmask
Alexander Bluhm wrote: > On Mon, Nov 18, 2019 at 09:58:34AM -0700, Theo de Raadt wrote: > > how about the text "use prefixlen instead of netmask for inet6" > > Much better error message, I have changed my diff. > > But we still have to decide whether we want to fix or disable inet6 > netmask. Man page in FreeBSD says (Inet only.), NetBSD has (inet > and inet6). > > As our ifconfig(8) man page says we support it, and route(8) also > supports it, I prefer fixing inet6 netmask. accepting a netmask also requires thinking about the behaviour in the discontig case (surely that means detecting and preventing it).
Re: ifconfig inet6 netmask
On Mon, Nov 18, 2019 at 09:58:34AM -0700, Theo de Raadt wrote: > how about the text "use prefixlen instead of netmask for inet6" Much better error message, I have changed my diff. But we still have to decide whether we want to fix or disable inet6 netmask. Man page in FreeBSD says (Inet only.), NetBSD has (inet and inet6). As our ifconfig(8) man page says we support it, and route(8) also supports it, I prefer fixing inet6 netmask. bluhm
Re: ifconfig inet6 netmask
Alexander Bluhm wrote: > + if (which == MASK) > + errx(1, "inet6 needs prefixlen, not netmask"); needs? you can configure an inet6 without prefixlen, and one is inferred. how about the text "use prefixlen instead of netmask for inet6"
Re: ifconfig inet6 netmask
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.11.18 16:31:05 +0100:
> On Thu, Nov 14, 2019 at 11:14:45PM +0100, Sebastian Benoit wrote:
> > The alternative is to not allow netmask for ipv6 and only / and
> > prefixlen
> > . Why support such a crazy way of specifying the mask?
>
> We can also do it the other way around and forbid ifconfig inet6
> netmask.
sorry, thats what i meant.
> > In route we removed a few quirks like that with notice in current.html ,
> > why not here?
>
> route(8) still accepts netmask.
Yes, i wrote too fast. The quirks were problematic ordering of arguments.
> route add -inet6 1234:: -netmask :: ::1
>
> Ignoring options silently is bad. So either use or reject netmask.
> If ifconfig(8) uses it, it would be consistent to route(8).
Right. Probably better to be consistent and not break peoples configs then.
ok for both diffs with a preference for the first ;)
>
> bluhm
>
> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.343
> diff -u -p -r1.343 ifconfig.8
> --- sbin/ifconfig/ifconfig.8 10 Nov 2019 09:10:44 - 1.343
> +++ sbin/ifconfig/ifconfig.8 18 Nov 2019 13:57:18 -
> @@ -429,7 +429,7 @@ output from
> .Cm hwfeatures
> shows the maximum supported MTU.
> .It Cm netmask Ar mask
> -(inet and inet6 only)
> +(inet only)
> Specify how much of the address to reserve for subdividing
> networks into subnetworks.
> The mask includes the network part of the local address
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.414
> diff -u -p -r1.414 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 24 Oct 2019 18:54:10 - 1.414
> +++ sbin/ifconfig/ifconfig.c 18 Nov 2019 13:56:31 -
> @@ -6151,6 +6151,9 @@ in6_getaddr(const char *s, int which)
> char buf[HOST_NAME_MAX+1 + sizeof("/128")], *pfxlen;
> int error;
>
> + if (which == MASK)
> + errx(1, "inet6 needs prefixlen, not netmask");
> +
> memset(&hints, 0, sizeof(hints));
> hints.ai_family = AF_INET6;
> hints.ai_socktype = SOCK_DGRAM; /*dummy*/
>
Re: ifconfig inet6 netmask
On Thu, Nov 14, 2019 at 11:14:45PM +0100, Sebastian Benoit wrote:
> The alternative is to not allow netmask for ipv6 and only / and
> prefixlen
> . Why support such a crazy way of specifying the mask?
We can also do it the other way around and forbid ifconfig inet6
netmask.
> In route we removed a few quirks like that with notice in current.html , why
> not here?
route(8) still accepts netmask.
route add -inet6 1234:: -netmask :: ::1
Ignoring options silently is bad. So either use or reject netmask.
If ifconfig(8) uses it, it would be consistent to route(8).
bluhm
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.343
diff -u -p -r1.343 ifconfig.8
--- sbin/ifconfig/ifconfig.810 Nov 2019 09:10:44 - 1.343
+++ sbin/ifconfig/ifconfig.818 Nov 2019 13:57:18 -
@@ -429,7 +429,7 @@ output from
.Cm hwfeatures
shows the maximum supported MTU.
.It Cm netmask Ar mask
-(inet and inet6 only)
+(inet only)
Specify how much of the address to reserve for subdividing
networks into subnetworks.
The mask includes the network part of the local address
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.414
diff -u -p -r1.414 ifconfig.c
--- sbin/ifconfig/ifconfig.c24 Oct 2019 18:54:10 - 1.414
+++ sbin/ifconfig/ifconfig.c18 Nov 2019 13:56:31 -
@@ -6151,6 +6151,9 @@ in6_getaddr(const char *s, int which)
char buf[HOST_NAME_MAX+1 + sizeof("/128")], *pfxlen;
int error;
+ if (which == MASK)
+ errx(1, "inet6 needs prefixlen, not netmask");
+
memset(&hints, 0, sizeof(hints));
hints.ai_family = AF_INET6;
hints.ai_socktype = SOCK_DGRAM; /*dummy*/
Re: sysupgrade: Allow to use another directory for the sets
On Mon, Nov 18, 2019 at 01:23:27PM +0100, Renaud Allard wrote: > > > On 11/18/19 10:08 AM, Raimo Niskanen wrote: > > > A configuration parameter could be accomplished through an optional symlink > > /etc/_sysupgrade that the sysadmin can create to point at the installation's > > sysupgrade directory. The sysupgrade script would test -s /etc/_sysupgrade > > and if there is a symlink readlink -f /etc/_sysupgrade to get SETSDIR. > > > > As it was said earlier, this doesn't solve the removal issue. With your > patch, please try "ln -s /home/_sysupgrade / ; sysupgrade". > This is actually not yet in my patch. I just want to, as a first step towards either of our solutions, patch to have the /home/_sysupgrade literal in only one place in the sysupgrade script, which would facilitate patching the sysupgrade script before calling it, as a poor man's solution. Plus, the script gets cleaner. The symlink suggestion is a future patch. But I think your suggestion to instead specify the parent directory of the _syspatch directory should be sufficient to remedy the removal issue both for your command line suggestion, as for this future patch symlink suggestion. It is a pity nobody else has responded to that prefix suggestion of yours! I think the feature itself is more important than if it is implemented with a command line argument or a symlink. Other than that. What do you or anyone else think about my argument that the location of the system upgrade download directory is an installation configuration that will not change between upgrades and hence it would be nice to not have to remember the command line argument for every future sysupgrade. Best regards -- / Raimo Niskanen, Erlang/OTP, Ericsson AB
iked(8): enable udpencap with -t
Hi,
this is only a minor fix. The '-t' flag forces iked to use NAT-traversal on UDP
port 4500. Currently it enables NATT only for IKE, not for the resulting
ESP SAs. The diff enables ESP NATT when iked NATT is enforced.
ok?
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index bd22bda0255..3ffda3cd2ec 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -1075,7 +1075,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct
iked_policy *pol,
if (ntohs(port) == IKED_NATT_PORT) {
/* Enforce NAT-T on the initiator side */
log_debug("%s: enforcing NAT-T", __func__);
- req.msg_natt = sa->sa_natt = 1;
+ req.msg_natt = sa->sa_natt = sa->sa_udpencap = 1;
}
if ((len = ikev2_add_nat_detection(env, buf, &pld, &req, len))
== -1)
[PATCH] Extra lines in ssh-pkcs11-client.c file
Hello,
if I didn't overlook the function, there's extra lines in there.
Please see the patch below.
diff --git usr.bin/ssh/ssh-pkcs11-client.c usr.bin/ssh/ssh-pkcs11-client.c
index 20284d98ecf..44065df1a96 100644
--- usr.bin/ssh/ssh-pkcs11-client.c
+++ usr.bin/ssh/ssh-pkcs11-client.c
@@ -230,9 +230,6 @@ wrap_key(struct sshkey *k)
static int
pkcs11_start_helper_methods(void)
{
-if (helper_ecdsa != NULL)
-return (0);
-
int (*orig_sign)(int, const unsigned char *, int, unsigned char *,
unsigned int *, const BIGNUM *, const BIGNUM *, EC_KEY *) = NULL;
if (helper_ecdsa != NULL)
--
Kind regards,
Ville Valkonen
Re: sysupgrade: Allow to use another directory for the sets
On 11/18/19 10:08 AM, Raimo Niskanen wrote: A configuration parameter could be accomplished through an optional symlink /etc/_sysupgrade that the sysadmin can create to point at the installation's sysupgrade directory. The sysupgrade script would test -s /etc/_sysupgrade and if there is a symlink readlink -f /etc/_sysupgrade to get SETSDIR. As it was said earlier, this doesn't solve the removal issue. With your patch, please try "ln -s /home/_sysupgrade / ; sysupgrade". smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] [src] bin/ed/README - fix quote/comma
On Sun, Nov 17, 2019 at 11:08:49AM +, Raf Czlonka wrote: > On Sun, Nov 17, 2019 at 10:23:38AM GMT, Otto Moerbeek wrote: > > On Sun, Nov 17, 2019 at 08:31:00AM +, Raf Czlonka wrote: > > > > > Hi all, > > > > > > Pretty straightforward - comma snuck in inside the quoted book title. > > > > This is how I learned it. Myabe a bit old-fashinoed, but not wrong. > > > > -Otto > > Hi Otto, > > Unless I'm misunderstanding CMOS (i.e. american style), comma would > indeed be *inside* quotes, when we are actually "using" quotes, > i.e. quoting text, or quotes are a part of a boot title itself. > hi. you can use quotes in this way, and you are "using" them. i think the distinction about what they're used for is irrelevant. as i understand it, US english would have the comma inside the quotes (as they are here), and UK english probably less likely. personally i would write with the comma outside the quotes in this case. > Here, however, quotes are used *instead of* italics to represent > the book title itself, so, i.e. in print, they wouldn't be present > at all. > i don;t think that's relevant. > The below is a bibliographical reference (of sorts) so the comma > should merely separate the book title from the publisher. > > Unfortunately, the finer details of CMOS are behind a paywall so I > can't confirm that categorically. > > Chances are, I might be wrong, though. In which case, I'd appreciate > pointing me to the source :^) > since what we have is not wrong (as such), i don;t see a need to change it. jmc
Re: iwm: support 9260 devices
On Mon, Nov 18, 2019 at 04:00:29AM +0100, Mark Kettenis wrote: > > > iwm0: hw rev 0x310, fw ver 34.-1169155311.0, address 90:78:41:39:57:8d > > > > I don't know yet what's up with that. Also happens on -17 firmware. > > The number we show for -34 firmware on 8260 looks OK though. > > It's printed using %d; probably should use %u instead. Yes, exactly. jcs has fixed it in-tree.
Re: sysupgrade: Allow to use another directory for the sets
On Thu, Nov 14, 2019 at 10:32:43AM +0100, Renaud Allard wrote:
>
>
> On 11/13/19 11:51 PM, Renaud Allard wrote:
> >
> >
> > On 12/11/2019 19:02, Renaud Allard wrote:
> >>
> >>
> >> On 12/11/2019 08:29, Theo de Raadt wrote:
> >>>
> >>> Renaud, please test it for me like this:
> >>>
> >>> sysupgrade -d /
> >>>
> >>> This interface is dangerously incorrect.
> >>>
> >>
> >> What about this one?
> >
> > ping.
> >
> > I haven't seen any reply on the prefix based patch, but what about also
> > making -k (Keep the files in /home/_sysupgrade) implicit in case -d is
> > used?
> >
>
> Here is a patch which disables the rm (enables -k) when -d is used
>
> This will require a little bit more work from the admin side, but at
> least there is no real danger of removal of wrong files.
>
> Any comment?
I too have a need for this kind of feature, but in my use case it is our
lab that has NFS auto mounted /home/* directories, so /home is a mount
point for the automounter.
We could start with a minimalistic patch to clean up that /home/_sysupgrade
is mentioned in 3 places in the sysupgrade script. After fixing that the
sysadmin need only change the line SETSDIR=/home/_sysupgrade to change the
sysupgrade directory, which could be a non-documented way to do it.
To implement a more supported feature I think that the sysupgrade
directory is a property of the installation; a specific installation
will always use the same sysupgrade directory. Hence this should be
a configuration parameter instead of a command line argument.
A configuration parameter could be accomplished through an optional symlink
/etc/_sysupgrade that the sysadmin can create to point at the installation's
sysupgrade directory. The sysupgrade script would test -s /etc/_sysupgrade
and if there is a symlink readlink -f /etc/_sysupgrade to get SETSDIR.
Does that sound all right?
Anyway, here is my minimalistic patch for script cleanup; the changes for
mkdir is there to detect if e.g an unexpected umask, or incorrect owner of
the parent directory creates a directory with wrong properties:
Index: usr.sbin/sysupgrade/sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.25
diff -u -u -r1.25 sysupgrade.sh
--- usr.sbin/sysupgrade/sysupgrade.sh 28 Sep 2019 17:30:07 - 1.25
+++ usr.sbin/sysupgrade/sysupgrade.sh 14 Nov 2019 13:27:34 -
@@ -119,6 +119,7 @@
URL=${MIRROR}/${NEXT_VERSION}/${ARCH}/
fi
+[[ -e ${SETSDIR} ]] || mkdir -p ${SETSDIR}
if [[ -e ${SETSDIR} ]]; then
eval $(stat -s ${SETSDIR})
[[ $st_uid -eq 0 ]] ||
@@ -127,8 +128,6 @@
ug_err "${SETSDIR} needs to be owned by root:wheel"
[[ $st_mode -eq 040755 ]] ||
ug_err "${SETSDIR} is not a directory with permissions 0755"
-else
- mkdir -p ${SETSDIR}
fi
cd ${SETSDIR}
@@ -185,7 +184,7 @@
cat <<__EOT >/auto_upgrade.conf
Location of sets = disk
-Pathname to the sets = /home/_sysupgrade/
+Pathname to the sets = ${SETSDIR}/
Set name(s) = done
Directory does not contain SHA256.sig. Continue without verification = yes
__EOT
@@ -193,7 +192,7 @@
if ! ${KEEP}; then
CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
cat <<__EOT > /etc/rc.firsttime
-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}
__EOT
fi
> Index: sysupgrade.8
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
> retrieving revision 1.10
> diff -u -p -r1.10 sysupgrade.8
> --- sysupgrade.8 3 Oct 2019 12:43:58 - 1.10
> +++ sysupgrade.8 14 Nov 2019 09:29:15 -
> @@ -24,6 +24,7 @@
> .Nm
> .Op Fl fkn
> .Op Fl r | s
> +.Op Fl d Ar directory
> .Op Ar installurl
> .Sh DESCRIPTION
> .Nm
> @@ -48,6 +49,14 @@ triggering a one-shot upgrade using the
> .Pp
> The options are as follows:
> .Bl -tag -width Ds
> +.It Fl d Ar directory
> +Choose the prefix of the
> +.Ar directory
> +in which the sets will be downloaded.
> +_sysupgrade will be appended to that name.
> +Default is
> +.Pa /home .
> +This will also implicitely force -k flag.
> .It Fl f
> Force an already applied upgrade.
> The default is to upgrade to latest snapshot only if available.
> Index: sysupgrade.sh
> ===
> RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
> retrieving revision 1.32
> diff -u -p -r1.32 sysupgrade.sh
> --- sysupgrade.sh 11 Nov 2019 18:26:52 - 1.32
> +++ sysupgrade.sh 14 Nov 2019 09:29:15 -
> @@ -25,7 +25,6 @@ umask 0022
> export PATH=/usr/bin:/bin:/usr/sbin:/sbin
>
> ARCH=$(uname -m)
> -SETSDIR=/home/_sysupgrade
>
> ug_err()
> {
> @@ -34,7 +33,7 @@ ug_err()
>
> usage()
> {
> - ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
> + ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-d directory] [installurl]"
> }
>
Re: ospfd: type p2p
On 17/11/2019 13:44, Remi Locherer wrote: > Yes, I'll send a separate diff for that later. > > OK for the new diff? Works for me. G
