Re: [PATCH] add ping(1)-like stats to tcpbench(1)
> On 4/05/2020, at 9:30 AM, Stuart Henderson wrote:
>
> On 2020/05/04 09:23, Richard Procter wrote:
>> I like it.
>>
>> Assuming a mention in tcpbench.1 - ok procter
>
> ok like this? text stolen from ping.
the server has an small edge case: it refuses to display stats
that don’t exist, e.g. if no data transfer has occured. Though
it will indicate receipt of SIGINFO by printing ‘\n’.
I see ping(1) in the same circumstance omits the stats line alone.
But that’s perhaps something for a later commit. I don’t think it
warrants mention in the man page.
ok procter
> Index: tcpbench.1
> ===
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.1,v
> retrieving revision 1.27
> diff -u -p -r1.27 tcpbench.1
> --- tcpbench.12 May 2020 22:00:29 - 1.27
> +++ tcpbench.13 May 2020 21:29:22 -
> @@ -82,6 +82,15 @@ Its accuracy may be increased by decreas
> .Ar interval .
> The summary bytes and duration cover the interval from transfer start
> to process exit.
> +The summary information can also be displayed while
> +.Nm
> +is running by sending it a
> +.Dv SIGINFO
> +signal (see the
> +.Cm status
> +argument of
> +.Xr stty 1
> +for more information).
> .Pp
> The options are as follows:
> .Bl -tag -width Ds
> Index: tcpbench.c
> ===
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 tcpbench.c
> --- tcpbench.c2 May 2020 22:00:29 - 1.62
> +++ tcpbench.c3 May 2020 21:29:22 -
> @@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
>* signal handler rules don't apply, libevent decouples for us
>*/
> switch (sig) {
> + case SIGINFO:
> + printf("\n");
> + wrapup(-1);
> + break;
> case SIGINT:
> printf("\n");
> wrapup(0);
> @@ -1109,7 +1113,8 @@ wrapup(int err)
> summary_display();
> }
>
> - exit(err);
> + if (err != -1)
> + exit(err);
> }
>
> int
> @@ -1126,7 +1131,7 @@ main(int argc, char **argv)
> int family = PF_UNSPEC;
> struct nlist nl[] = { { "_tcbtable" }, { "" } };
> const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
> - struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
> + struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
> struct sockaddr_un sock_un;
>
> /* Init world */
> @@ -1362,9 +1367,11 @@ main(int argc, char **argv)
> signal_set(_sigterm, SIGTERM, signal_handler, NULL);
> signal_set(_sighup, SIGHUP, signal_handler, NULL);
> signal_set(_sigint, SIGINT, signal_handler, NULL);
> + signal_set(_siginfo, SIGINFO, signal_handler, NULL);
> signal_add(_sigint, NULL);
> signal_add(_sigterm, NULL);
> signal_add(_sighup, NULL);
> + signal_add(_siginfo, NULL);
> signal(SIGPIPE, SIG_IGN);
>
> if (UDP_MODE) {
>
Re: pfctl_parser.c vs. __KAME
Hello JCA, thanks for the pointers. > about this specific piece of code in pfctl, but the "kame hack" is still > here and you really want to double check before removing such chunks in ...so pfctl is the wrong place to start with removal. thanks and regards sashan
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
On 2020/05/04 09:23, Richard Procter wrote:
> I like it.
>
> Assuming a mention in tcpbench.1 - ok procter
ok like this? text stolen from ping.
Index: tcpbench.1
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.1,v
retrieving revision 1.27
diff -u -p -r1.27 tcpbench.1
--- tcpbench.1 2 May 2020 22:00:29 - 1.27
+++ tcpbench.1 3 May 2020 21:29:22 -
@@ -82,6 +82,15 @@ Its accuracy may be increased by decreas
.Ar interval .
The summary bytes and duration cover the interval from transfer start
to process exit.
+The summary information can also be displayed while
+.Nm
+is running by sending it a
+.Dv SIGINFO
+signal (see the
+.Cm status
+argument of
+.Xr stty 1
+for more information).
.Pp
The options are as follows:
.Bl -tag -width Ds
Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.62
diff -u -p -r1.62 tcpbench.c
--- tcpbench.c 2 May 2020 22:00:29 - 1.62
+++ tcpbench.c 3 May 2020 21:29:22 -
@@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
* signal handler rules don't apply, libevent decouples for us
*/
switch (sig) {
+ case SIGINFO:
+ printf("\n");
+ wrapup(-1);
+ break;
case SIGINT:
printf("\n");
wrapup(0);
@@ -1109,7 +1113,8 @@ wrapup(int err)
summary_display();
}
- exit(err);
+ if (err != -1)
+ exit(err);
}
int
@@ -1126,7 +1131,7 @@ main(int argc, char **argv)
int family = PF_UNSPEC;
struct nlist nl[] = { { "_tcbtable" }, { "" } };
const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
- struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
+ struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
struct sockaddr_un sock_un;
/* Init world */
@@ -1362,9 +1367,11 @@ main(int argc, char **argv)
signal_set(_sigterm, SIGTERM, signal_handler, NULL);
signal_set(_sighup, SIGHUP, signal_handler, NULL);
signal_set(_sigint, SIGINT, signal_handler, NULL);
+ signal_set(_siginfo, SIGINFO, signal_handler, NULL);
signal_add(_sigint, NULL);
signal_add(_sigterm, NULL);
signal_add(_sighup, NULL);
+ signal_add(_siginfo, NULL);
signal(SIGPIPE, SIG_IGN);
if (UDP_MODE) {
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
I like it.
Assuming a mention in tcpbench.1 - ok procter
> On 4/05/2020, at 4:25 AM, Stuart Henderson wrote:
>
> Is it worth triggering this on SIGINFO? I use that often with ping(1).
>
> Index: tcpbench.c
> ===
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 tcpbench.c
> --- tcpbench.c2 May 2020 22:00:29 - 1.62
> +++ tcpbench.c3 May 2020 16:24:05 -
> @@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
>* signal handler rules don't apply, libevent decouples for us
>*/
> switch (sig) {
> + case SIGINFO:
> + printf("\n");
> + wrapup(-1);
> + break;
> case SIGINT:
> printf("\n");
> wrapup(0);
> @@ -1109,7 +1113,8 @@ wrapup(int err)
> summary_display();
> }
>
> - exit(err);
> + if (err != -1)
> + exit(err);
> }
>
> int
> @@ -1126,7 +1131,7 @@ main(int argc, char **argv)
> int family = PF_UNSPEC;
> struct nlist nl[] = { { "_tcbtable" }, { "" } };
> const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
> - struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
> + struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
> struct sockaddr_un sock_un;
>
> /* Init world */
> @@ -1362,9 +1367,11 @@ main(int argc, char **argv)
> signal_set(_sigterm, SIGTERM, signal_handler, NULL);
> signal_set(_sighup, SIGHUP, signal_handler, NULL);
> signal_set(_sigint, SIGINT, signal_handler, NULL);
> + signal_set(_siginfo, SIGINFO, signal_handler, NULL);
> signal_add(_sigint, NULL);
> signal_add(_sigterm, NULL);
> signal_add(_sighup, NULL);
> + signal_add(_siginfo, NULL);
> signal(SIGPIPE, SIG_IGN);
>
> if (UDP_MODE) {
>
[PATCH v3] Tighter pledges for ftp(1)
This prevents non-interactive invocations of ftp(1) from spawning
external commands in case they are compromised. This is a significant
security benefit for sysupgrade(8).
In the future, output files (specified with -o) can be opened before
pledge(2) is called, which will improve security further.
Sincerely,
Demi M. Obenour
Index: usr.bin/ftp/main.c
===
RCS file: /cvs/src/usr.bin/ftp/main.c,v
retrieving revision 1.131
diff -u -p -u -p -r1.131 main.c
--- usr.bin/ftp/main.c 11 Feb 2020 18:41:39 - 1.131
+++ usr.bin/ftp/main.c 3 May 2020 18:18:31 -
@@ -483,27 +483,33 @@ main(volatile int argc, char *argv[])
(void)signal(SIGWINCH, setttywidth);
if (argc > 0) {
+ char **arg;
+ int needs_exec = 0;
if (isurl(argv[0])) {
- if (pipeout) {
#ifndef SMALL
- if (pledge("stdio rpath dns tty inet proc exec
fattr",
- NULL) == -1)
- err(1, "pledge");
-#else
- if (pledge("stdio rpath dns tty inet fattr",
- NULL) == -1)
- err(1, "pledge");
+ for (arg = argv; *arg; ++arg)
+ needs_exec |= is_interactive_url(*arg);
#endif
+ if (pipeout) {
+ if (needs_exec) {
+ if (pledge("stdio rpath dns tty inet
proc exec fattr",
+ NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath dns tty inet
fattr",
+ NULL) == -1)
+ err(1, "pledge");
+ }
} else {
-#ifndef SMALL
- if (pledge("stdio rpath wpath cpath dns tty
inet proc exec fattr",
- NULL) == -1)
- err(1, "pledge");
-#else
- if (pledge("stdio rpath wpath cpath dns tty
inet fattr",
- NULL) == -1)
- err(1, "pledge");
-#endif
+ if (needs_exec) {
+ if (pledge("stdio rpath wpath cpath dns
tty inet proc exec fattr",
+ NULL) == -1)
+ err(1, "pledge");
+ } else {
+ if (pledge("stdio rpath wpath cpath dns
tty inet fattr",
+ NULL) == -1)
+ err(1, "pledge");
+ }
}
rval = auto_fetch(argc, argv, outfile);
Index: usr.bin/ftp/fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.194
diff -u -p -u -p -r1.194 fetch.c
--- usr.bin/ftp/fetch.c 22 Feb 2020 01:00:07 - 1.194
+++ usr.bin/ftp/fetch.c 3 May 2020 18:18:32 -
@@ -1601,6 +1601,32 @@ hextochar(const char *str)
return ret;
}
+#ifndef SMALL
+int
+is_interactive_url(const char *p)
+{
+ size_t urllen;
+ const char *path;
+ if (strncasecmp(p, HTTP_URL, sizeof(HTTP_URL) - 1) == 0 ||
+#ifndef NOSSL
+ strncasecmp(p, HTTPS_URL, sizeof(HTTPS_URL) - 1) == 0 ||
+#endif /* !NOSSL */
+ strncasecmp(p, FILE_URL, sizeof(FILE_URL) - 1) == 0)
+ return (0);
+ if ((urllen = strlen(p)) == 0)
+ errx(1, "empty URL not allowed");
+ if (p[urllen - 1] == '/')
+ return (1);
+ if (strncasecmp(p, FTP_URL, sizeof(FTP_URL) - 1) != 0)
+ return (0);
+ p += sizeof(FTP_URL) - 1;
+ urllen -= sizeof(FTP_URL) - 1;
+ if (urllen == 0)
+ errx(1, "empty URL not allowed");
+ return strchr(p, '/') == NULL;
+}
+#endif
+
int
isurl(const char *p)
{
Index: usr.bin/ftp/extern.h
===
RCS file: /cvs/src/usr.bin/ftp/extern.h,v
retrieving revision 1.51
diff -u -p -u -p -r1.51 extern.h
--- usr.bin/ftp/extern.h16 May 2019 12:44:17 - 1.51
+++ usr.bin/ftp/extern.h3 May 2020 18:18:32 -
@@ -88,6 +88,7 @@ char *hookup(char *, char *);
intinitconn(void);
void intr(void);
intisurl(const char *);
+intis_interactive_url(const char *);
intftp_login(const char *, char *, char *);
void lostpeer(void);
void makeargv(void);
Re: Tighter pledges for ftp(1)
On 2020-05-03 12:18, Theo de Raadt wrote: > Thanks Stuart. > > > The lesson is clear. No pledge/unveil work unless you test *ALL PROGRAM > BEHAVIOURS*. Doing less than the full testing is ... uhm, egotistical. Sorry about that. Hopefully my next version will fix it. > And it is completely normal that as the pledges and unveils harden, > the amount of test cases to discover exceeds expectation. Indeed. Sincerely, Demi signature.asc Description: OpenPGP digital signature
Re: Tighter pledges for ftp(1)
On 2020-05-03 12:13, Stuart Henderson wrote: > On 2020/05/02 20:19, Demi M. Obenour wrote: >> The following patch tightens the pledges for ftp(1). >> >> This guarantees that ftp(1) cannot spawn child processes when operating >> in batch mode, which is a significant security win. > > It breaks interactive mode (!ls, more somefile, get somefile "|rot13"), > something is wrong with how you decide that exec is needed. > > Also it complicates the code for the SMALL version used on the ramdisk > (and maybe makes the pledge weaker too, the code is no longer easy to > follow so I didn't work out for sure) The ramdisk version should be fine. The variable `needs_exec` is initialized to 0, and it is never assigned to in SMALL mode, so the stronger pledges are used. Sincerely, Demi signature.asc Description: OpenPGP digital signature
Re: pfctl_parser.c vs. __KAME
On Sun, May 03 2020, Alexandr Nedvedicky wrote:
> Hello,
Hi Sashan,
> the question has popped up while on recent code review of some Solaris
> specific
> bug fixes: do we still need a code in diff below or is it OK to proceed
> and commit the diff?
>
> The chunk below uses bytes 2 and 3 to derive a scope of link local address.
> It
> seems to me those bytes/octets are always zero in IPv6 link scope addresses
> these days, unless I'm missing something. I was not able to identify its
> counter part in current kernel, which would make octets 2 & 3 non-zero, so it
> makes me thinking it should be OK to remove the whole chunk below.
See in6_embedscope and in6_recoverscope, IN6_IS_SCOPE_EMBED etc. Dunno
about this specific piece of code in pfctl, but the "kame hack" is still
here and you really want to double check before removing such chunks in
userland.
>
> thanks and
> regards
> sashan
>
> note __KAME__ is defined, the chunk below is getting compiled currently.
>
> 8<---8<---8<--8<
> diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> index cef0aa2474f..24175de046c 100644
> --- a/sbin/pfctl/pfctl_parser.c
> +++ b/sbin/pfctl/pfctl_parser.c
> @@ -1361,21 +1361,6 @@ ifa_load(void)
> err(1, "%s: calloc", __func__);
> n->af = ifa->ifa_addr->sa_family;
> n->ifa_flags = ifa->ifa_flags;
> -#ifdef __KAME__
> - if (n->af == AF_INET6 &&
> - IN6_IS_ADDR_LINKLOCAL(&((struct sockaddr_in6 *)
> - ifa->ifa_addr)->sin6_addr) &&
> - ((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_scope_id ==
> - 0) {
> - struct sockaddr_in6 *sin6;
> -
> - sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
> - sin6->sin6_scope_id = sin6->sin6_addr.s6_addr[2] << 8 |
> - sin6->sin6_addr.s6_addr[3];
> - sin6->sin6_addr.s6_addr[2] = 0;
> - sin6->sin6_addr.s6_addr[3] = 0;
> - }
> -#endif
> n->ifindex = 0;
> if (n->af == AF_LINK)
> n->ifindex = ((struct sockaddr_dl *)
>
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
pfctl_parser.c vs. __KAME
Hello,
the question has popped up while on recent code review of some Solaris specific
bug fixes: do we still need a code in diff below or is it OK to proceed
and commit the diff?
The chunk below uses bytes 2 and 3 to derive a scope of link local address. It
seems to me those bytes/octets are always zero in IPv6 link scope addresses
these days, unless I'm missing something. I was not able to identify its
counter part in current kernel, which would make octets 2 & 3 non-zero, so it
makes me thinking it should be OK to remove the whole chunk below.
thanks and
regards
sashan
note __KAME__ is defined, the chunk below is getting compiled currently.
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index cef0aa2474f..24175de046c 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -1361,21 +1361,6 @@ ifa_load(void)
err(1, "%s: calloc", __func__);
n->af = ifa->ifa_addr->sa_family;
n->ifa_flags = ifa->ifa_flags;
-#ifdef __KAME__
- if (n->af == AF_INET6 &&
- IN6_IS_ADDR_LINKLOCAL(&((struct sockaddr_in6 *)
- ifa->ifa_addr)->sin6_addr) &&
- ((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_scope_id ==
- 0) {
- struct sockaddr_in6 *sin6;
-
- sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
- sin6->sin6_scope_id = sin6->sin6_addr.s6_addr[2] << 8 |
- sin6->sin6_addr.s6_addr[3];
- sin6->sin6_addr.s6_addr[2] = 0;
- sin6->sin6_addr.s6_addr[3] = 0;
- }
-#endif
n->ifindex = 0;
if (n->af == AF_LINK)
n->ifindex = ((struct sockaddr_dl *)
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
On Sun, May 03, 2020 at 10:31:35AM -0600, Theo de Raadt wrote: > Theo Buehler wrote: > > > On Sun, May 03, 2020 at 05:25:25PM +0100, Stuart Henderson wrote: > > > Is it worth triggering this on SIGINFO? I use that often with ping(1). > > > > I was going to suggest a similar diff but couldn't figure out why this > > breaks the stats at the end with ^C. > > An error in managing accounting state? > No, a bug in my diff, sthen's diff works fine. (it helps if one tests the correct binary) ok tb
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
Theo Buehler wrote: > On Sun, May 03, 2020 at 05:25:25PM +0100, Stuart Henderson wrote: > > Is it worth triggering this on SIGINFO? I use that often with ping(1). > > I was going to suggest a similar diff but couldn't figure out why this > breaks the stats at the end with ^C. An error in managing accounting state?
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
That's very nice.
I should mention that signal_handler() isn't an async signal handler,
since this is a libevent program using syncronous signal_set(). Therefore
the stdio and FPU use is safe.
Stuart Henderson wrote:
> Is it worth triggering this on SIGINFO? I use that often with ping(1).
>
> Index: tcpbench.c
> ===
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 tcpbench.c
> --- tcpbench.c2 May 2020 22:00:29 - 1.62
> +++ tcpbench.c3 May 2020 16:24:05 -
> @@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
>* signal handler rules don't apply, libevent decouples for us
>*/
> switch (sig) {
> + case SIGINFO:
> + printf("\n");
> + wrapup(-1);
> + break;
> case SIGINT:
> printf("\n");
> wrapup(0);
> @@ -1109,7 +1113,8 @@ wrapup(int err)
> summary_display();
> }
>
> - exit(err);
> + if (err != -1)
> + exit(err);
> }
>
> int
> @@ -1126,7 +1131,7 @@ main(int argc, char **argv)
> int family = PF_UNSPEC;
> struct nlist nl[] = { { "_tcbtable" }, { "" } };
> const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
> - struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
> + struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
> struct sockaddr_un sock_un;
>
> /* Init world */
> @@ -1362,9 +1367,11 @@ main(int argc, char **argv)
> signal_set(_sigterm, SIGTERM, signal_handler, NULL);
> signal_set(_sighup, SIGHUP, signal_handler, NULL);
> signal_set(_sigint, SIGINT, signal_handler, NULL);
> + signal_set(_siginfo, SIGINFO, signal_handler, NULL);
> signal_add(_sigint, NULL);
> signal_add(_sigterm, NULL);
> signal_add(_sighup, NULL);
> + signal_add(_siginfo, NULL);
> signal(SIGPIPE, SIG_IGN);
>
> if (UDP_MODE) {
>
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
On Sun, May 03, 2020 at 05:25:25PM +0100, Stuart Henderson wrote:
> Is it worth triggering this on SIGINFO? I use that often with ping(1).
I was going to suggest a similar diff but couldn't figure out why this
breaks the stats at the end with ^C.
>
> Index: tcpbench.c
> ===
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> retrieving revision 1.62
> diff -u -p -r1.62 tcpbench.c
> --- tcpbench.c2 May 2020 22:00:29 - 1.62
> +++ tcpbench.c3 May 2020 16:24:05 -
> @@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
>* signal handler rules don't apply, libevent decouples for us
>*/
> switch (sig) {
> + case SIGINFO:
> + printf("\n");
> + wrapup(-1);
> + break;
> case SIGINT:
> printf("\n");
> wrapup(0);
> @@ -1109,7 +1113,8 @@ wrapup(int err)
> summary_display();
> }
>
> - exit(err);
> + if (err != -1)
> + exit(err);
> }
>
> int
> @@ -1126,7 +1131,7 @@ main(int argc, char **argv)
> int family = PF_UNSPEC;
> struct nlist nl[] = { { "_tcbtable" }, { "" } };
> const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
> - struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
> + struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
> struct sockaddr_un sock_un;
>
> /* Init world */
> @@ -1362,9 +1367,11 @@ main(int argc, char **argv)
> signal_set(_sigterm, SIGTERM, signal_handler, NULL);
> signal_set(_sighup, SIGHUP, signal_handler, NULL);
> signal_set(_sigint, SIGINT, signal_handler, NULL);
> + signal_set(_siginfo, SIGINFO, signal_handler, NULL);
> signal_add(_sigint, NULL);
> signal_add(_sigterm, NULL);
> signal_add(_sighup, NULL);
> + signal_add(_siginfo, NULL);
> signal(SIGPIPE, SIG_IGN);
>
> if (UDP_MODE) {
>
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
Is it worth triggering this on SIGINFO? I use that often with ping(1).
Index: tcpbench.c
===
RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
retrieving revision 1.62
diff -u -p -r1.62 tcpbench.c
--- tcpbench.c 2 May 2020 22:00:29 - 1.62
+++ tcpbench.c 3 May 2020 16:24:05 -
@@ -213,6 +213,10 @@ signal_handler(int sig, short event, voi
* signal handler rules don't apply, libevent decouples for us
*/
switch (sig) {
+ case SIGINFO:
+ printf("\n");
+ wrapup(-1);
+ break;
case SIGINT:
printf("\n");
wrapup(0);
@@ -1109,7 +1113,8 @@ wrapup(int err)
summary_display();
}
- exit(err);
+ if (err != -1)
+ exit(err);
}
int
@@ -1126,7 +1131,7 @@ main(int argc, char **argv)
int family = PF_UNSPEC;
struct nlist nl[] = { { "_tcbtable" }, { "" } };
const char *host = NULL, *port = DEFAULT_PORT, *srcbind = NULL;
- struct event ev_sigint, ev_sigterm, ev_sighup, ev_progtimer;
+ struct event ev_sigint, ev_sigterm, ev_sighup, ev_siginfo, ev_progtimer;
struct sockaddr_un sock_un;
/* Init world */
@@ -1362,9 +1367,11 @@ main(int argc, char **argv)
signal_set(_sigterm, SIGTERM, signal_handler, NULL);
signal_set(_sighup, SIGHUP, signal_handler, NULL);
signal_set(_sigint, SIGINT, signal_handler, NULL);
+ signal_set(_siginfo, SIGINFO, signal_handler, NULL);
signal_add(_sigint, NULL);
signal_add(_sigterm, NULL);
signal_add(_sighup, NULL);
+ signal_add(_siginfo, NULL);
signal(SIGPIPE, SIG_IGN);
if (UDP_MODE) {
Re: Tighter pledges for ftp(1)
Thanks Stuart. The lesson is clear. No pledge/unveil work unless you test *ALL PROGRAM BEHAVIOURS*. Doing less than the full testing is ... uhm, egotistical. And it is completely normal that as the pledges and unveils harden, the amount of test cases to discover exceeds expectation. Stuart Henderson wrote: > On 2020/05/02 20:19, Demi M. Obenour wrote: > > The following patch tightens the pledges for ftp(1). > > > > This guarantees that ftp(1) cannot spawn child processes when operating > > in batch mode, which is a significant security win. > > It breaks interactive mode (!ls, more somefile, get somefile "|rot13"), > something is wrong with how you decide that exec is needed. > > Also it complicates the code for the SMALL version used on the ramdisk > (and maybe makes the pledge weaker too, the code is no longer easy to > follow so I didn't work out for sure). > > Note the commit log when pledge was added: > > - > Date: 2017/01/10 17:43:12 > Author: deraadt > Branch: HEAD > Tag: (none) > Log: > Pledge more strictly. This is only enabled on the ramdisk version of the > ftp(1) client, which operates only in URL mode. Not willing to spend the > time tracking piles of global variables for sub-modes, and finding all > the pledge interactions. Would rather have the install media ftp(1) as > safe as possible, immediately. > ok tb jca > > Members: > fetch.c:1.156->1.157 > - > > ..that isn't to say that it can't be improved, but it does need care as > ftp(1) is quite complicated. >
Re: Tighter pledges for ftp(1)
On 2020/05/02 20:19, Demi M. Obenour wrote: > The following patch tightens the pledges for ftp(1). > > This guarantees that ftp(1) cannot spawn child processes when operating > in batch mode, which is a significant security win. It breaks interactive mode (!ls, more somefile, get somefile "|rot13"), something is wrong with how you decide that exec is needed. Also it complicates the code for the SMALL version used on the ramdisk (and maybe makes the pledge weaker too, the code is no longer easy to follow so I didn't work out for sure). Note the commit log when pledge was added: - Date: 2017/01/10 17:43:12 Author: deraadt Branch: HEAD Tag: (none) Log: Pledge more strictly. This is only enabled on the ramdisk version of the ftp(1) client, which operates only in URL mode. Not willing to spend the time tracking piles of global variables for sub-modes, and finding all the pledge interactions. Would rather have the install media ftp(1) as safe as possible, immediately. ok tb jca Members: fetch.c:1.156->1.157 - ..that isn't to say that it can't be improved, but it does need care as ftp(1) is quite complicated.
Re: iked(8): Removing SHA1 from default transforms
I don't believe you were joking. I believe you are one of those crusadors (clapping coconuts together) who want to eliminate a few named coding-constructs even in situations where they work in a completely safe way, and furthermore are standardized as mandatory. This is probably due to lack of education in the mathematical field, and lack of other things to do. Stephan Mending wrote: > I know Theo, Tobias told me a few mails back. I was joking... > > On Sat, May 02, 2020 at 07:32:43AM -0600, Theo de Raadt wrote: > > Stephan Mending wrote: > > > > > On 02/05/2020 02:58, Theo de Raadt wrote: > > > > > > > Stephan Mending wrote: > > > > > > > >> I don't get how this could be ? > > > > then go study. > > > > > > > I think I've struck a nerve right here. I'm sorry to have caused you > > > high blood pressure by sending this diff. I do not doubt the > > > competency of you or the other developers. And yes I did not know that > > > SHA-1 is still fine to be using as MAC. That's why my name is md5 > > > collisions and not sha1 collisions, right? > > > > MD5 HMACs are fine also. HMACs are fine. > > > > It's obvious you see yourself as a crusader. I'm thinking more monty > > python, though. > > > > > >
Re: [PATCH] add ping(1)-like stats to tcpbench(1)
Hi, On 02/05/2020 12:41, richard.n.proc...@gmail.com wrote: >> More assumptions are implied here. How do I measure a 2400 bps link >> with this >> report? The precision is lost in "%.3Lf". > I highly doubt anyone needs to benchmark a 2400 bps link. The display is > in any case precise to 1000 bps. I am commonly working with 1200bps and 9600bps links in HamBSD with the kiss(4) network interface, is there any reason to *not* be flexible on the output here? Thanks, Iain. -- https://hambsd.org/
Re: [PATCH] sysupgrade
On Thu, Apr 30, 2020 at 11:19:14AM +, Kevin Chadwick wrote: > On 2020-04-30 03:28, James Jerkins wrote: > > This patch adds two new options to sysupgrade. The first option is for > > small box systems like an APU system that only has the base and manual sets > > installed. The second option is for headless systems without X11 like > > servers. > > I used to avoid installing the X sets and I found that even on e.g. a web > server > without X11 running. I would end up installing them in the end as certain > ports > would require them. Struggling to remember why I wanted to do it, to be > honest. > A notion from securing Linux perhaps. As Linux often runs things automatically > when they're installed and desktop distros run a lot by default. It is what is > in RAM, that is more important, which is very little by default on OpenBSD > (~60M). > > May be worth re-considering if it is worth the effort to save a fairly small > amount of space with potentially unexpected consequences? > > Instead to make options for specifig sets how about just possible to use upgrade.conf file. That enables to set what ever sets you may want and other options also like site sets. -- Henri Järvinen
Re: [PATCH] sysupgrade
On Sun, May 03, 2020 at 04:41:13AM -0400, Chris Bennett wrote: > The FAQ already describes exactly how to upgrade with whatever sets one > wants to. I have used that method many, many times successfully. > I can't see any reason whatsoever to turn an addon tool into anything > more than what it is. If someone can't even bother to read the FAQ, why > should they even be using OpenBSD at all? People read the FAQ and see that partial install is possible: The complete OpenBSD installation is broken up into a number of file sets: ... New users are recommended to install all of them. ... The xservXX.tgz set is rarely needed if you don't intend to run X. So no mentioning about this kind of setup being "second-class". People see that now instead of manual update with bsd.rd it is possible to use sysupgrade. Yay! People see missing pieces (sysupgrade does not support partial install) and try to patch sysupgrade because manual upgrade does allow partial install. It's kinda expected that this topic will be raised over and over again. Maybe FAQ should be adjusted to say something like: BEWARE: ANY PARTIAL INSTALL IS NOT FULLY SUPPORTED CONFIGURATION WE ARE PROVIDING NO SUPPORT FOR THAT KIND OF STUFF. Because that's what people usually get in response when they are asking in misc@ about sysupgrade and sets.
Re: [PATCH] sysupgrade
Le 03/05/2020 à 10:41, Chris Bennett a écrit : On Sun, May 03, 2020 at 04:40:44AM +0200, Stéphane Aulery wrote: a) Removing sets selection from the installer b) Supporting an upgrade of the sets already installed. The FAQ already describes exactly how to upgrade with whatever sets one wants to. I have used that method many, many times successfully. I can't see any reason whatsoever to turn an addon tool into anything more than what it is. If someone can't even bother to read the FAQ, why should they even be using OpenBSD at all? No intention to sound rude, but everything is already spelled out in detail on the website. I actually read it, and it is very good. You don't get the point. I'm againts adding a new options to sysupgrade. The problem is that there is a discourse that says: using the sets is not supported, your are on your own. And on the other side, installing a selection of sets is still possible with an explanation in the FAQ. To be consistent I would have: - Deleted the choice of sets. Case settled. - Added an explanation in FAQ 4 (in File Sets section) and the manual page of sysupgrade which says that sysupgrade only works for all sets. So you known your are on your own. No surprise. - Completed sysupgrade to install the selected sets. Regards, -- Stéphane Aulery
Re: panic ip6_pullexthdr/m_copydata NULL mbuf fix
On 03/05/20(Sun) 14:14, Martin Pieuchot wrote:
> Diff below should fix the following panic exposed by syzkaller [0]:
>
> panic+0x15c sys/kern/subr_prf.c:207
> m_copydata+0x17e m_getptr sys/kern/uipc_mbuf.c:1031 [inline]
> m_copydata+0x17e sys/kern/uipc_mbuf.c:722
> ip6_pullexthdr+0x16f sys/netinet6/ip6_input.c:1149
> ip6_savecontrol+0x373 sys/netinet6/ip6_input.c:1036
> rip6_input+0x7e7 sys/netinet6/raw_ip6.c:225
> ip_deliver+0x353 sys/netinet/ip_input.c:665
> ip6_input_if+0x17cb ip6_ours sys/netinet6/ip6_input.c:518 [inline]
> ip6_input_if+0x17cb sys/netinet6/ip6_input.c:340
> ipv6_input+0x48 sys/netinet6/ip6_input.c:171
> if_input_local+0x121 sys/net/if.c:781
> ip6_output+0xd26
> rip6_output+0x4c0 sys/netinet6/raw_ip6.c:481
> rip6_usrreq+0x5e1 sys/netinet6/raw_ip6.c:670
> sosend+0x645 sys/kern/uipc_socket.c:524
>
> It adds more sanity checks inside ip6_pullexthdr(). The diff itself
> comes from NetBSD where the same bug has been also exposed by syzkaller.
>
> I couldn't trigger the issue with the reproducer, so I couldn't test the
> effectiveness of the fix. That said it seems correct to me.
>
> [0]
> https://syzkaller.appspot.com/bug?id=a7a41a7f641844dd94c17c63f0d9ccf68884083a
Better without typo, ok?
Index: netinet6/ip6_input.c
===
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.225
diff -u -p -r1.225 ip6_input.c
--- netinet6/ip6_input.c12 Apr 2020 11:56:53 - 1.225
+++ netinet6/ip6_input.c3 May 2020 12:02:26 -
@@ -1142,11 +1142,17 @@ ip6_pullexthdr(struct mbuf *m, size_t of
}
#endif
+ if (off + sizeof(ip6e) > m->m_pkthdr.len)
+ return NULL;
+
m_copydata(m, off, sizeof(ip6e), (caddr_t));
if (nxt == IPPROTO_AH)
elen = (ip6e.ip6e_len + 2) << 2;
else
elen = (ip6e.ip6e_len + 1) << 3;
+
+ if (off + elen > m->m_pkthdr.len)
+ return NULL;
MGET(n, M_DONTWAIT, MT_DATA);
if (n && elen >= MLEN) {
panic ip6_pullexthdr/m_copydata NULL mbuf fix
Diff below should fix the following panic exposed by syzkaller [0]:
panic+0x15c sys/kern/subr_prf.c:207
m_copydata+0x17e m_getptr sys/kern/uipc_mbuf.c:1031 [inline]
m_copydata+0x17e sys/kern/uipc_mbuf.c:722
ip6_pullexthdr+0x16f sys/netinet6/ip6_input.c:1149
ip6_savecontrol+0x373 sys/netinet6/ip6_input.c:1036
rip6_input+0x7e7 sys/netinet6/raw_ip6.c:225
ip_deliver+0x353 sys/netinet/ip_input.c:665
ip6_input_if+0x17cb ip6_ours sys/netinet6/ip6_input.c:518 [inline]
ip6_input_if+0x17cb sys/netinet6/ip6_input.c:340
ipv6_input+0x48 sys/netinet6/ip6_input.c:171
if_input_local+0x121 sys/net/if.c:781
ip6_output+0xd26
rip6_output+0x4c0 sys/netinet6/raw_ip6.c:481
rip6_usrreq+0x5e1 sys/netinet6/raw_ip6.c:670
sosend+0x645 sys/kern/uipc_socket.c:524
It adds more sanity checks inside ip6_pullexthdr(). The diff itself
comes from NetBSD where the same bug has been also exposed by syzkaller.
I couldn't trigger the issue with the reproducer, so I couldn't test the
effectiveness of the fix. That said it seems correct to me.
[0]
https://syzkaller.appspot.com/bug?id=a7a41a7f641844dd94c17c63f0d9ccf68884083a
ok?
Index: netinet6/ip6_input.c
===
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.225
diff -u -p -r1.225 ip6_input.c
--- netinet6/ip6_input.c12 Apr 2020 11:56:53 - 1.225
+++ netinet6/ip6_input.c3 May 2020 12:01:59 -
@@ -1142,11 +1142,17 @@ ip6_pullexthdr(struct mbuf *m, size_t of
}
#endif
+ if (off + sizeof(ip6e) > m->m_pkthdr.len)
+ return NULL;
+
m_copydata(m, off, sizeof(ip6e), (caddr_t));
if (nxt == IPPROTO_AH)
elen = (ip6e.ip6e_len + 2) << 2;
else
elen = (ip6e.ip6e_len + 1) << 3;
+
+ if (off + elen > m->m_pkthdr.len)
+ return NULL
MGET(n, M_DONTWAIT, MT_DATA);
if (n && elen >= MLEN) {
Re: [PATCH] sysupgrade
On Sun, May 03, 2020 at 04:40:44AM +0200, Stéphane Aulery wrote: > > a) Removing sets selection from the installer > b) Supporting an upgrade of the sets already installed. > The FAQ already describes exactly how to upgrade with whatever sets one wants to. I have used that method many, many times successfully. I can't see any reason whatsoever to turn an addon tool into anything more than what it is. If someone can't even bother to read the FAQ, why should they even be using OpenBSD at all? No intention to sound rude, but everything is already spelled out in detail on the website. Chris Bennett
Re: iked(8): Removing SHA1 from default transforms
I know Theo, Tobias told me a few mails back. I was joking... On Sat, May 02, 2020 at 07:32:43AM -0600, Theo de Raadt wrote: > Stephan Mending wrote: > > > On 02/05/2020 02:58, Theo de Raadt wrote: > > > > > Stephan Mending wrote: > > > > > >> I don't get how this could be ? > > > then go study. > > > > > I think I've struck a nerve right here. I'm sorry to have caused you > > high blood pressure by sending this diff. I do not doubt the > > competency of you or the other developers. And yes I did not know that > > SHA-1 is still fine to be using as MAC. That's why my name is md5 > > collisions and not sha1 collisions, right? > > MD5 HMACs are fine also. HMACs are fine. > > It's obvious you see yourself as a crusader. I'm thinking more monty > python, though. > > >
