Does openssl know about ROP?

2023-01-29 Thread Theo de Raadt
I was reading some openssl source code, in particular the x86 assembly
language files (which accelerate some crypto operations), and I find
many cases where data tables are intentionally inserted into text (code)
segments, and those tables include the byte value 0xC3.

By intentional, I mean there's a comment, don't let me judge the tone of it:

&set_label("AES_Td",64);# Yes! I keep it in the code segment!

And then a little bit later (these macros expand to placing .long into
the .text segment)

&_data_word(0x50a7f451, 0x5365417e, 0xc3a4171a, 0x965e273a);
  ^^
It doesn't do it once.  It does it nearly a hundred times.

Here's another:

&_data_word(0x02752fc3, 0x12f04c81, 0xa397468d, 0xc6f9d36b);
^^
Can I do anything dangerous with that byte?

Does anyone on the internet know if a C3 byte in the code segment of a
program is safe?

I am very curious (if anyone knows..)

I did not look carefully for other specific byte sequences in code
which carry potential danger.

Far be it from me to suggest that the security experts over there in
OpenSSL land are unaware of modern exploitation methods!  Very far from
that, very very far.

Surely I am wrong, all these bytes must be very very safe, these
numerous 0xc3 bytes (and the bytes preceeding them) must have been
reviewed repeatedly very carefully by the developers paid with the money
they received after their heartbleed error -- to make sure these
specific 0xc3 "instructions" (and the "instructions" precedeing them)
are safe.

Please let me know, I want to sleep better at night.



remove APM_IOC_NEXTEVENT mentions

2023-01-29 Thread joshua stein
The APM_IOC_NEXTEVENT ioctl was ripped out from i386 in 2001 in 
favor of a kqueue interface, but apm.4 on various arches still claim 
it's supported or may be in the future which is unlikely.

https://github.com/openbsd/src/commit/1e8a3e096b6ef01dcdf4f881eac1b86b1bafe06d


Index: share/man/man4/man4.amd64/apm.4
===
RCS file: /cvs/src/share/man/man4/man4.amd64/apm.4,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 apm.4
--- share/man/man4/man4.amd64/apm.4 31 Mar 2022 17:27:21 -  1.9
+++ share/man/man4/man4.amd64/apm.4 30 Jan 2023 05:01:25 -
@@ -103,44 +103,6 @@ The
 .Va minutes_left
 value contains the estimated number of minutes of battery life
 remaining.
-.It Dv APM_IOC_NEXTEVENT
-.Pq Li "struct apm_event_info"
-The APM driver stores up to
-.Dv APM_NEVENTS
-events.
-This was defined as 16 at the time this documentation was written.
-If the event list is full when a new event is detected, the new event is lost.
-.Dv APM_IOC_NEXTEVENT
-ioctl returns the next event on the list or
-.Er EAGAIN
-if the event list is empty.
-The format of the returned event is:
-.Bd -literal -offset indent
-struct apm_event_info {
-   u_int type;
-   u_int index;
-   u_int spare[8];
-};
-.Ed
-where
-.Va index
-is a sequential count of events that can be used to check if any
-events were lost and
-.Va type
-is one of:
-.Bl -tag -width Ds -offset indent -compact
-.It Dv APM_STANDBY_REQ
-.It Dv APM_SUSPEND_REQ
-.It Dv APM_NORMAL_RESUME
-.It Dv APM_CRIT_RESUME
-.It Dv APM_BATTERY_LOW
-.It Dv APM_POWER_CHANGE
-.It Dv APM_UPDATE_TIME
-.It Dv APM_CRIT_SUSPEND_REQ
-.It Dv APM_USER_STANDBY_REQ
-.It Dv APM_USER_SUSPEND_REQ
-.It Dv APM_SYS_STANDBY_RESUME
-.El
 .It Dv APM_IOC_DEV_CTL
 .Pq Li "struct apm_ctl"
 Allows an application to directly set the
Index: share/man/man4/man4.arm64/apm.4
===
RCS file: /cvs/src/share/man/man4/man4.arm64/apm.4,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 apm.4
--- share/man/man4/man4.arm64/apm.4 31 Mar 2022 17:27:21 -  1.5
+++ share/man/man4/man4.arm64/apm.4 30 Jan 2023 05:01:25 -
@@ -112,44 +112,6 @@ The
 .Va minutes_left
 value contains the estimated number of minutes of battery life
 remaining.
-.It Dv APM_IOC_NEXTEVENT
-.Pq Li "struct apm_event_info"
-The APM driver stores up to
-.Dv APM_NEVENTS
-events.
-This was defined as 16 at the time this documentation was written.
-If the event list is full when a new event is detected, the new event is lost.
-.Dv APM_IOC_NEXTEVENT
-ioctl returns the next event on the list or
-.Er EAGAIN
-if the event list is empty.
-The format of the returned event is:
-.Bd -literal -offset indent
-struct apm_event_info {
-   u_int type;
-   u_int index;
-   u_int spare[8];
-};
-.Ed
-where
-.Va index
-is a sequential count of events that can be used to check if any
-events were lost and
-.Va type
-is one of:
-.Bl -tag -width Ds -offset indent -compact
-.It Dv APM_STANDBY_REQ
-.It Dv APM_SUSPEND_REQ
-.It Dv APM_NORMAL_RESUME
-.It Dv APM_CRIT_RESUME
-.It Dv APM_BATTERY_LOW
-.It Dv APM_POWER_CHANGE
-.It Dv APM_UPDATE_TIME
-.It Dv APM_CRIT_SUSPEND_REQ
-.It Dv APM_USER_STANDBY_REQ
-.It Dv APM_USER_SUSPEND_REQ
-.It Dv APM_SYS_STANDBY_RESUME
-.El
 .It Dv APM_IOC_DEV_CTL
 .Em NOT YET SUPPORTED on arm64 .
 .Pp
Index: share/man/man4/man4.i386/apm.4
===
RCS file: /cvs/src/share/man/man4/man4.i386/apm.4,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 apm.4
--- share/man/man4/man4.i386/apm.4  31 Mar 2022 17:27:21 -  1.35
+++ share/man/man4/man4.i386/apm.4  30 Jan 2023 05:01:25 -
@@ -137,44 +137,6 @@ The
 .Va minutes_left
 value contains the estimated number of minutes of battery life
 remaining.
-.It Dv APM_IOC_NEXTEVENT
-.Pq Li "struct apm_event_info"
-The APM driver stores up to
-.Dv APM_NEVENTS
-events.
-This was defined as 16 at the time this documentation was written.
-If the event list is full when a new event is detected, the new event is lost.
-.Dv APM_IOC_NEXTEVENT
-ioctl returns the next event on the list or
-.Er EAGAIN
-if the event list is empty.
-The format of the returned event is:
-.Bd -literal -offset indent
-struct apm_event_info {
-   u_int type;
-   u_int index;
-   u_int spare[8];
-};
-.Ed
-where
-.Va index
-is a sequential count of events that can be used to check if any
-events were lost and
-.Va type
-is one of:
-.Bl -tag -width Ds -offset indent -compact
-.It Dv APM_STANDBY_REQ
-.It Dv APM_SUSPEND_REQ
-.It Dv APM_NORMAL_RESUME
-.It Dv APM_CRIT_RESUME
-.It Dv APM_BATTERY_LOW
-.It Dv APM_POWER_CHANGE
-.It Dv APM_UPDATE_TIME
-.It Dv APM_CRIT_SUSPEND_REQ
-.It Dv APM_USER_STANDBY_REQ
-.It Dv APM_USER_SUSPEND_REQ
-.It Dv APM_SYS_STANDBY_RESUME
-.El
 .It Dv APM_IOC_DEV_CTL
 .Pq Li "struct apm_ctl"
 Allows an application to directly set the
Index: share/man/man4/man4.loongson/apm.4
===

Replace selwakeup() with KNOTE() in tun(4) and tap(4)

2023-01-29 Thread Visa Hankala
Replace selwakeup() with KNOTE() in tun(4) and tap(4).

This patch makes the tun(4) and tap(4) event filters MP-safe.

This is similar to the change that just got committed to pppac(4)
and pppx(4). However, tun(4) and tap(4) can be destroyed abruptly,
so klist_invalidate() has to be kept in tun_clone_destroy().

The selwakeup() call in tun_dev_close() can be removed. If the device
is closed peacefully, the klists get cleared automatically and waiters
notified before the close routine is invoked. On abrupt detach,
klist_invalidate() in tun_clone_destroy() should clear any lingering
knotes.

OK?

Index: net/if_tun.c
===
RCS file: src/sys/net/if_tun.c,v
retrieving revision 1.237
diff -u -p -r1.237 if_tun.c
--- net/if_tun.c2 Jul 2022 08:50:42 -   1.237
+++ net/if_tun.c30 Jan 2023 03:32:36 -
@@ -47,13 +47,14 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 #include 
@@ -78,8 +79,9 @@
 struct tun_softc {
struct arpcom   sc_ac;  /* ethernet common data */
 #define sc_if  sc_ac.ac_if
-   struct selinfo  sc_rsel;/* read select */
-   struct selinfo  sc_wsel;/* write select (not used) */
+   struct mutexsc_mtx;
+   struct klistsc_rklist;  /* knotes for read */
+   struct klistsc_wklist;  /* knotes for write (unused) */
SMR_LIST_ENTRY(tun_softc)
sc_entry;   /* all tunnel interfaces */
int sc_unit;
@@ -125,22 +127,28 @@ int   tun_init(struct tun_softc *);
 void   tun_start(struct ifnet *);
 intfilt_tunread(struct knote *, long);
 intfilt_tunwrite(struct knote *, long);
+intfilt_tunmodify(struct kevent *, struct knote *);
+intfilt_tunprocess(struct knote *, struct kevent *);
 void   filt_tunrdetach(struct knote *);
 void   filt_tunwdetach(struct knote *);
 void   tun_link_state(struct ifnet *, int);
 
 const struct filterops tunread_filtops = {
-   .f_flags= FILTEROP_ISFD,
+   .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
.f_attach   = NULL,
.f_detach   = filt_tunrdetach,
.f_event= filt_tunread,
+   .f_modify   = filt_tunmodify,
+   .f_process  = filt_tunprocess,
 };
 
 const struct filterops tunwrite_filtops = {
-   .f_flags= FILTEROP_ISFD,
+   .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE,
.f_attach   = NULL,
.f_detach   = filt_tunwdetach,
.f_event= filt_tunwrite,
+   .f_modify   = filt_tunmodify,
+   .f_process  = filt_tunprocess,
 };
 
 SMR_LIST_HEAD(tun_list, tun_softc);
@@ -220,6 +228,9 @@ tun_create(struct if_clone *ifc, int uni
ifp = &sc->sc_if;
snprintf(ifp->if_xname, sizeof(ifp->if_xname),
"%s%d", ifc->ifc_name, unit);
+   mtx_init(&sc->sc_mtx, IPL_NET);
+   klist_init_mutex(&sc->sc_rklist, &sc->sc_mtx);
+   klist_init_mutex(&sc->sc_wklist, &sc->sc_mtx);
ifp->if_softc = sc;
 
/* this is enough state for tun_dev_open to work with */
@@ -275,6 +286,8 @@ tun_create(struct if_clone *ifc, int uni
return (0);
 
 exists:
+   klist_free(&sc->sc_rklist);
+   klist_free(&sc->sc_wklist);
free(sc, M_DEVBUF, sizeof(*sc));
return (EEXIST);
 }
@@ -284,7 +297,6 @@ tun_clone_destroy(struct ifnet *ifp)
 {
struct tun_softc*sc = ifp->if_softc;
dev_tdev;
-   int  s;
 
KERNEL_ASSERT_LOCKED();
 
@@ -314,10 +326,11 @@ tun_clone_destroy(struct ifnet *ifp)
/* wait for device entrypoints to finish */
refcnt_finalize(&sc->sc_refs, "tundtor");
 
-   s = splhigh();
-   klist_invalidate(&sc->sc_rsel.si_note);
-   klist_invalidate(&sc->sc_wsel.si_note);
-   splx(s);
+   klist_invalidate(&sc->sc_rklist);
+   klist_invalidate(&sc->sc_wklist);
+
+   klist_free(&sc->sc_rklist);
+   klist_free(&sc->sc_wklist);
 
if (ISSET(sc->sc_flags, TUN_LAYER2))
ether_ifdetach(ifp);
@@ -488,7 +501,6 @@ tun_dev_close(dev_t dev, struct proc *p)
ifq_purge(&ifp->if_snd);
 
CLR(sc->sc_flags, TUN_ASYNC);
-   selwakeup(&sc->sc_rsel);
sigio_free(&sc->sc_sigio);
 
if (!ISSET(sc->sc_flags, TUN_DEAD)) {
@@ -654,7 +666,10 @@ tun_wakeup(struct tun_softc *sc)
if (sc->sc_reading)
wakeup(&sc->sc_if.if_snd);
 
-   selwakeup(&sc->sc_rsel);
+   mtx_enter(&sc->sc_mtx);
+   KNOTE(&sc->sc_rklist, 0);
+   mtx_leave(&sc->sc_mtx);
+
if (sc->sc_flags & TUN_ASYNC)
pgsigio(&sc->sc_sigio, SIGIO, 0);
 }
@@ -960,7 +975,6 @@ tun_dev_kqfilter(dev_t dev, struct knote
struct ifnet*i

Re: Preferred TERM for pkg_add

2023-01-29 Thread Theo de Raadt
Marc Espie  wrote:

> Actual patches to pkg_add (if need be) welcome
> 
> The switch to not using full term length was actually done at Theo's
> request.
> 
> I don't remember the details, but Theo's rationale was quite
> reasonable and made lots of sense.
> 
> (specifically, using the full line length would be cool, but there
> were lots of fringe cases where it would fuck the display up for
> no reason).

printing on the last character of the screen has nasty consequences on
some terminal types, unless the entire input&output is fully controlled to
ensure in input character doesn't become a canonical output character (ie.
the way that a visual text or pager works).

avoid it unless absolutely neccessay.



Re: xonly status

2023-01-29 Thread Greg Steuck
"Theo de Raadt"  writes:

... removed the comprehensive summary of the effort.

> The major application problems have been reasonably isolated, and after
> 3 weeks they are mostly handled, or we know where the remaining problems
> lie:
>
> - libcrypto assembly functions with incorrect data placement
> - some ffi variations with the same problem
> - managing v8's embedded code blob
> - a few languages with very strange machine-dependent optimizations
>
> That is mostly managed in ports now, mostly by fixing the code, or if it
> is too difficult or intentionally obscure the specific programs can be
> linked --no-exec-only.  Maybe someone from the ports team can reply to
> this mail to say where things are at.

GHC (Glasgow Haskell Compiler) is one of the languages with unusual
machine code generation strategies. For now lang/ghc uses
--no-exec-only.  I'm exploring other options with upstream at
https://gitlab.haskell.org/ghc/ghc/-/issues/22782

Thanks
Greg



Re: Preferred TERM for pkg_add

2023-01-29 Thread Marc Espie
Actual patches to pkg_add (if need be) welcome

The switch to not using full term length was actually done at Theo's
request.

I don't remember the details, but Theo's rationale was quite
reasonable and made lots of sense.

(specifically, using the full line length would be cool, but there
were lots of fringe cases where it would fuck the display up for
no reason).



Re: Fix handling of ed addresses consisting only of separators

2023-01-29 Thread Sören Tempel
Hi,

I've been using this patch for the past 6 months and haven't noticed any
regressions. However, during this timespan I became aware of additional
POSIX compatibility issues in OpenBSD ed: `3  2p` as well as
addresses with interleaved `,` and `;` separators (e.g. `1,;`) are also
evaluated incorrectly.

These additional compatibility issues are unrelated to the issue fixed
by the proposed patch. Is there a general interest in improving the
POSIX compatibility of OpenBSD ed? If so, what would need to be done in
order to get the proposed patch committed?

Greetings,
Sören

Sören Tempel  wrote:
> PING.
> 
> I have executed the ed tests (available in bin/ed/test) with this patch
> applied and the proposed patch doesn't seem to introduce any new
> regressions.
> 
> If the patch needs to be revised further, please let me know. The only
> thing I can think of right now is that it might make sense to rename the
> `recur` variable to `sawseparator` or something along those lines.
> 
> Greetings,
> Sören
> 
> Sören Tempel  wrote:
> > Hello,
> > 
> > The POSIX specification of ed(1) includes a table in the rationale
> > section which (among others) mandates the following address handling
> > rules [1]:
> > 
> >  Address Addr1 Addr2
> >  ,,  $ $
> >  ;;  $ $
> > 
> > Unfortunately, OpenBSD does not correctly handle these two example
> > addresses. As an example, consider the following `,,p` print command:
> > 
> > printf "a\nfoo\nbar\nbaz\n.\n,,p\nQ\n" | ed
> > 
> > This should only print the last line (as `,,p` should expand to `$,$p`)
> > but instead prints all lines with OpenBSD ed. This seems to be a
> > regression introduced by Jerome Frgagic in 2017 [2]. Prior to this
> > change, `,,p` as well as `;;p` correctly printed the last line. The
> > aforementioned change added a recursive invocation of next_addr() which
> > does not update the local first variable (causing the second address
> > separator to be mistaken for the first). The patch below fixes this by
> > tracking recursive invocations of next_addr() via an additional function
> > parameter.
> > 
> > See also: https://austingroupbugs.net/view.php?id=1582
> > 
> > [1]: 
> > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ed.html#tag_20_38_18
> > [2]: 
> > https://github.com/openbsd/src/commit/b4c905bbf3932a50011ce3436b1e22acc742cfeb
> > 
> > diff --git bin/ed/main.c bin/ed/main.c
> > index 231d021ef19..ca1aeb102b3 100644
> > --- bin/ed/main.c
> > +++ bin/ed/main.c
> > @@ -64,7 +64,7 @@ void signal_hup(int);
> >  void signal_int(int);
> >  void handle_winch(int);
> >  
> > -static int next_addr(void);
> > +static int next_addr(int);
> >  static int check_addr_range(int, int);
> >  static int get_matching_node_addr(regex_t *, int);
> >  static char *get_filename(int);
> > @@ -296,7 +296,7 @@ extract_addr_range(void)
> >  
> > addr_cnt = 0;
> > first_addr = second_addr = current_addr;
> > -   while ((addr = next_addr()) >= 0) {
> > +   while ((addr = next_addr(0)) >= 0) {
> > addr_cnt++;
> > first_addr = second_addr;
> > second_addr = addr;
> > @@ -328,7 +328,7 @@ extract_addr_range(void)
> >  
> >  /*  next_addr: return the next line address in the command buffer */
> >  static int
> > -next_addr(void)
> > +next_addr(int recur)
> >  {
> > char *hd;
> > int addr = current_addr;
> > @@ -382,11 +382,15 @@ next_addr(void)
> > case '%':
> > case ',':
> > case ';':
> > -   if (first) {
> > +   if (first && !recur) {
> > ibufp++;
> > addr_cnt++;
> > second_addr = (c == ';') ? current_addr : 1;
> > -   if ((addr = next_addr()) < 0)
> > +
> > +   // If the next address is omitted (e.g. "," or 
> > ";") or
> > +   // if it is also a separator (e.g. ",," or 
> > ";;") then
> > +   // return the last address (as per the omission 
> > rules).
> > +   if ((addr = next_addr(1)) < 0)
> > addr = addr_last;
> > break;
> > }



Re: OpenBSD perl 5.36.0 - Call for Testing

2023-01-29 Thread Marc Espie
One thing that's going to be very useful IMO
is newer function bindings.

Us perlers don't really care, matching $@
after the sub is cool.

But having off-the-mill function "declarations"
that look a lot like function declarations in
other more common languages

(e.g.
sub f($a, $b)
)

might help *a lot* in getting people to read perl code.

Gonna try that one out for sure! 



Re: xonly status

2023-01-29 Thread Theo de Raadt
The two main test programs being used for xonly assessment, as
attachments.  They will eventually head towards src/regress but for
now, people can play with them.

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int	main(int argc, char *argv[]);

void *s_ldso(void);
void *s_mmap_xz(void);
void *s_mmap_x(void);
void *s_mmap_nrx(void);
void *s_mmap_nwx(void);
void *s_mmap_xnwx(void);

struct readable {
	char *name;
	void *(*setup)(void);
	int isfn;
	void *addr;
	int uu, ku;
	int skip;
} readables[] = {
	{ "ld.so",		s_ldso, 1,			},
	{ "mmap xz",		s_mmap_xz, 0,			},
	{ "mmap x",		s_mmap_x, 0,			},
	{ "mmap nrx",		s_mmap_nrx, 0,			},
	{ "mmap nwx",		s_mmap_nwx, 0,			},
	{ "mmap xnwx",		s_mmap_xnwx, 0,			},
	{ "main",		NULL, 1,	&main		},
	{ "libc unmapped?",	NULL, 1,	&wcstol		},
	{ "libc mapped",	NULL, 1,	&mmap		}
};

jmp_buf fail;

void
sigsegv(int _unused)
{
	longjmp(fail, 1);
}

void *
s_mmap_xz(void)
{
	return mmap(NULL, getpagesize(), PROT_EXEC,
	MAP_ANON | MAP_PRIVATE, -1, 0);
	/* no data written. tests read-fault of an unbacked exec-only page */
}

void *
s_mmap_x(void)
{
	char *addr;

	addr = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE,
	MAP_ANON | MAP_PRIVATE, -1, 0);
	explicit_bzero(addr, getpagesize());
	mprotect(addr, getpagesize(), PROT_EXEC);
	return addr;
}

void *
s_mmap_nrx(void)
{
	char *addr;

	addr = mmap(NULL, getpagesize(), PROT_NONE,
	MAP_ANON | MAP_PRIVATE, -1, 0);
	mprotect(addr, getpagesize(), PROT_READ | PROT_WRITE);
	explicit_bzero(addr, getpagesize());
	mprotect(addr, getpagesize(), PROT_EXEC);
	return addr;
}

void *
s_mmap_nwx(void)
{
	char *addr;

	addr = mmap(NULL, getpagesize(), PROT_NONE,
	MAP_ANON | MAP_PRIVATE, -1, 0);
	mprotect(addr, getpagesize(), PROT_WRITE);
	explicit_bzero(addr, getpagesize());
	mprotect(addr, getpagesize(), PROT_EXEC);
	return addr;
}

void *
s_mmap_xnwx(void)
{
	char *addr;

	addr = mmap(NULL, getpagesize(), PROT_EXEC,
	MAP_ANON | MAP_PRIVATE, -1, 0);
	mprotect(addr, getpagesize(), PROT_NONE);
	mprotect(addr, getpagesize(), PROT_WRITE);
	explicit_bzero(addr, getpagesize());
	mprotect(addr, getpagesize(), PROT_EXEC);
	return addr;
}

void *
s_ldso(void)
{
	void *handle, *dlopenp;

	handle = dlopen("ld.so", RTLD_NOW);
	if (handle == NULL)
		*(volatile char *)0 = 0;
	dlopenp = dlsym(handle, "dlopen");
	return dlopenp;
}

void
s_table()
{
	int i;

	for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) {

		if (setjmp(fail) == 0) {
			if (readables[i].setup)
readables[i].addr = readables[i].setup();
		} else
			readables[i].skip = 1;
#ifdef __hppa__
		/* hppa ptable headers point at the instructions */
		if (readables[i].isfn)
			readables[i].addr = (void *)*(u_int *)
			((u_int)readables[i].addr & ~3);
#endif
	}
}

int
main(int argc, char *argv[])
{
	int p[2], i;

	signal(SIGSEGV, sigsegv);
	signal(SIGBUS, sigsegv);

	s_table();

	for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) {
		struct readable *r = &readables[i];
		char c;

		if (r->skip)
			continue;
		pipe(p);
		fcntl(p[0], F_SETFL, O_NONBLOCK);

		if (write(p[1], r->addr, 1) == 1 && read(p[0], &c, 1) == 1)
			r->ku = 1;

		if (setjmp(fail) == 0) {
			volatile int x = *(int *)(r->addr);
			r->uu = 1;
		}

		close(p[0]);
		close(p[1]);
	}

	printf("%-16s  userland   kernel\n", "");	
	for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) {
		struct readable *r = &readables[i];

		if (r->skip)
			printf("%-16s  %-10s %-10s\n", r->name,
			"skipped", "skipped");
		else
			printf("%-16s  %-10s %-10s\n", r->name,
			r->uu ? "readable" : "unreadable",
			r->ku ? "readable" : "unreadable");
	}
}
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#defineroundup(x, y)   x)+((y)-1))/(y))*(y))

int pgsize;
jmp_buf fail;

void
sigsegv(int _unused)
{
longjmp(fail, 1);
}

int
callback(struct dl_phdr_info *info, size_t size, void *cookie)
{
	Elf_Addr start, end, p;
	Elf_Off len;
	void *test;
	int i, count, total, col;

	/* Find all text segments */
	for (i = 0; i < info->dlpi_phnum; i++) {
		if (info->dlpi_phdr[i].p_type == PT_LOAD &&
		info->dlpi_phdr[i].p_flags & PF_X) {
			start = info->dlpi_addr + info->dlpi_phdr[i].p_vaddr;
			len = info->dlpi_phdr[i].p_memsz;
			end = start + len;
			printf("%s %llx-%llx (%llx, %lld pg) prot %s\n\t",
			info->dlpi_name,
			(uint64_t)start, (uint64_t)end, (uint64_t)len,
			((uint64_t)len + pgsize) / pgsize,
			(info->dlpi_phdr[i].p_flags & PF_R) ? "RX" : "X");

			test = malloc(pgsize);
			count = total = col = 0;

			/* attempt to read the entire text segment */
			for (p = start; p < end; p += pgsize) {
total++;
if (setjmp(fail) == 0) {
	memcpy(test, (char *)p, pgsize);
	printf("y");
	count++;
} else {
	printf("n");
}
if (++col % 64 == 0)
	printf("\n\t");
			}
			printf("\n");
			printf("\tread %d pages of

xonly status

2023-01-29 Thread Theo de Raadt
We've made good progress in the xonly effort so here's a small summary.

architectures crossed over completely

arm64 - X bit without implied R in mmu
riscv64  - X bit without implied R in mmu
amd64 - using hardware 'PKU' feature
powerpc64 - using feature similar to PKU
hppa - using gateway feature

architectures completed in the kernel, but needing ld.bfd work

sparc64 - sun4u using split software TLB, sun4v cannot do this
octeon - newer mips cpu have a read-inhibit bit

in progress:

macppc - powerpc G5 cpus, using a feature similar to PKU

machines which cannot do this
landisk
sparc64 (sun4v)
i386
alpha
arm (32bit)
macppc G4 cpus
older mips64
amd64 cpu without PKU (but we have some ideas)

A test program has been written which checks a variety of page mappings.
Below, "userland" refers to the program directly accessing it's own memory.
"kernel" refers to a program trying to write() the memory onto a pipe.
It used to look like this:

  userland   kernel
ld.so readable   readable
mmap xz   unreadable unreadable
mmap xreadable   readable  
mmap nrx  readable   readable  
mmap nwx  readable   readable  
mmap xnwx readable   readable  
main  readable   readable
libc unmapped?readable   readable
libc mapped   readable   readable

On machines with fixes, it now looks like this:

  userland   kernel
ld.so unreadable unreadable
mmap xz   unreadable unreadable
mmap xunreadable unreadable  
mmap nrx  unreadable unreadable  
mmap nwx  unreadable unreadable  
mmap xnwx unreadable unreadable  
main  unreadable unreadable
libc unmapped?unreadable unreadable
libc mapped   unreadable unreadable

On machines lacking hardware enforcement ability, there is a diff coming
which wraps the kernel copyin() and copyinstr() with some checks.  2 or 4
important code address spaces (in the main program, signal trampoline,
ld.so, and libc.so) are added to a very short list, and checked ahead of
time.  This short list needs no locking because these address ranges are
immutable (meaning, no address space changes can be made).  This check is
very quick and results in the following:

  userland   kernel
ld.so readable   unreadable
mmap xz   unreadable unreadable
mmap xreadable   readable  
mmap nrx  readable   readable  
mmap nwx  readable   readable  
mmap xnwx readable   readable  
main  readable   unreadable
libc unmapped?readable   unreadable
libc mapped   readable   unreadable

That blocks the classic "BROP" attack method of trying to write the
text segments out a socket for offline gadget study.

I want to bring attention to the "mmap xz" line.  In the test program
this is a new page mapped PROT_EXEC which has never been faulted.  It
contains no code, and no code is run there, so the first pagefault that
happens against it is the testing PROT_READ page fault, and the
higher-level VM code says no way -- this page can only be faulted with a
PROT_EXEC request.  That leads to a surprising behaviour, and another
test program was written which tries to see what text pages may actually
be read from userland.

On a machine with proper hardware support, you cannot read any of the
text pages.

./foo7b 445a7754960-445a7755190 (830, 1 pg) prot X
n
read 0 pages of 1
cannot read the whole
/usr/lib/libc.so.96.5 4484b4139b0-4484b4b9c30 (a6280, 167 pg) prot X


nnn
read 0 pages of 167
cannot read the whole
/usr/libexec/ld.so 4480810c000-44808117666 (b666, 12 pg) prot X

read 0 pages of 12
cannot read the whole
/usr/libexec/ld.so 4480810a000-4480810c000 (2000, 3 pg) prot RX
nn
read 0 pages of 2
cannot read the whole

The surprise is what happens when this is tried on a machine which
has no hardware enforcement, like i386

./foo7b 1a67c670-1a67cea0 (830, 1 pg) prot RX
n
read 0 pages of 1
cannot read the whole
/usr/lib/libc.so.96.5 d746400-d7daf70 (94b70, 149 pg) prot X
yyynynnyyyyy
yyyyynnynnyynnynnnyyynnyynyy
yyyyn
read 97 pages of 149
cannot read the whole
/usr/libexec/ld.so 3683000-368d82f (a82f, 11 pg) prot X
nyy
read 10 pages of 11
cannot read the whole
/usr/libexec/ld.so 3681000-3683000 (2000, 3 pg) prot RX
nn
read 0 pages of 2
cannot rea

Re: strptime.c

2023-01-29 Thread Todd C . Miller
Unfortunately we cannot use strtonum(3) here since there may be
non-digit characters following the number.  So, strtoll(3)
it is then.

 - todd

Index: lib/libc/time/strptime.c
===
RCS file: /cvs/src/lib/libc/time/strptime.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 strptime.c
--- lib/libc/time/strptime.c12 May 2019 12:49:52 -  1.30
+++ lib/libc/time/strptime.c29 Jan 2023 15:13:53 -
@@ -29,8 +29,10 @@
  */
 
 #include 
+#include 
+#include 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -72,8 +74,8 @@ static const int mon_lengths[2][MONSPERY
 { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }
 };
 
-static int _conv_num64(const unsigned char **, int64_t *, int64_t, int64_t);
 static int _conv_num(const unsigned char **, int *, int, int);
+static int epoch_to_tm(const unsigned char **, struct tm *);
 static int leaps_thru_end_of(const int y);
 static char *_strptime(const char *, const char *, struct tm *, int);
 static const u_char *_find_string(const u_char *, int *, const char * const *,
@@ -338,15 +340,10 @@ literal:
if (!(_conv_num(&bp, &tm->tm_sec, 0, 60)))
return (NULL);
break;
-   case 's':   /* Seconds since epoch */
-   {
-   int64_t i64;
-   if (!(_conv_num64(&bp, &i64, 0, INT64_MAX)))
-   return (NULL);
-   if (!gmtime_r(&i64, tm))
-   return (NULL);
-   fields = 0x; /* everything */
-   }
+   case 's':   /* Seconds since epoch. */
+   if (!(epoch_to_tm(&bp, tm)))
+   return (NULL);
+   fields = 0x; /* everything */
break;
case 'U':   /* The week of year, beginning on sunday. */
case 'W':   /* The week of year, beginning on monday. */
@@ -610,26 +607,27 @@ _conv_num(const unsigned char **buf, int
 }
 
 static int
-_conv_num64(const unsigned char **buf, int64_t *dest, int64_t llim, int64_t 
ulim)
+epoch_to_tm(const unsigned char **buf, struct tm *tm)
 {
-   int result = 0;
-   int64_t rulim = ulim;
-
-   if (**buf < '0' || **buf > '9')
-   return (0);
-
-   /* we use rulim to break out of the loop when we run out of digits */
-   do {
-   result *= 10;
-   result += *(*buf)++ - '0';
-   rulim /= 10;
-   } while ((result * 10 <= ulim) && rulim && **buf >= '0' && **buf <= 
'9');
-
-   if (result < llim || result > ulim)
-   return (0);
-
-   *dest = result;
-   return (1);
+   int saved_errno = errno;
+   int ret = 0;
+   time_t secs;
+   char *ep;
+
+   errno = 0;
+   secs = strtoll(*buf, &ep, 10);
+   if (*buf == (unsigned char *)ep)
+   goto done;
+   if (secs < 0 ||
+   secs == LLONG_MAX && errno == ERANGE)
+   goto done;
+   if (localtime_r(&secs, tm) == NULL)
+   goto done;
+   ret = 1;
+done:
+   *buf = ep;
+   errno = saved_errno;
+   return (ret);
 }
 
 static const u_char *



bgpd: remove ASDOT support

2023-01-29 Thread Job Snijders
ASDOT started out as sort of a joke, but unfortunately gained some
popularity in the 2010s with the rise of 4-byte ASNs and some people
thinking "cute, I can write my longish number in a shorter exotic
notation".

Then, many operators came to realize that using a '.' (dot) in places
which used to be simple integers is quite annoying (for example when
regular expressions also are in play), and more fundamentally: why go
through the trouble of using a complicated syntax when you can just
write number itself?

Perhaps time to bring ASDOT to the gardenshed? :-)

Kind regards,

Job

Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.230
diff -u -p -r1.230 bgpd.conf.5
--- bgpd.conf.5 24 Jan 2023 14:13:11 -  1.230
+++ bgpd.conf.5 29 Jan 2023 13:53:02 -
@@ -127,17 +127,9 @@ for Latin America and the Caribbean
 for Europe, the Middle East, and parts of Asia
 .El
 .Pp
-The AS numbers 64512 \(en 65534 are designated for private use.
+The AS numbers 64496 \(en 65534 and 42 \(en 4294967294 are designated
+for private use.
 The AS number 23456 is reserved and should not be used.
-4-byte AS numbers may be specified in either the ASPLAIN format:
-.Bd -literal -offset indent
-AS 196618
-.Ed
-.Pp
-or in the older ASDOT format:
-.Bd -literal -offset indent
-AS 3.10
-.Ed
 .Pp
 .It Ic connect-retry Ar seconds
 Set the number of seconds to wait before attempting to re-open
@@ -1991,7 +1983,7 @@ Communities are encoded as
 .Ar as-number : Ns Ar local .
 Four-octet encoding is used if the
 .Ar as-number
-is bigger than 65535 or if the AS_DOT encoding is used.
+is bigger than 65535.
 IPv4 Address Specific Extended Communities are encoded as
 .Ar IP : Ns Ar local .
 Opaque Extended Communities are encoded with a single numeric value.
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.440
diff -u -p -r1.440 parse.y
--- parse.y 24 Jan 2023 14:13:11 -  1.440
+++ parse.y 29 Jan 2023 13:53:02 -
@@ -242,7 +242,7 @@ typedef struct {
 %token NE LE GE XRANGE LONGER MAXLEN MAX
 %token   STRING
 %token   NUMBER
-%typeasnumber as4number as4number_any optnumber
+%typeasnumber as4number optnumber
 %typeespah family safi restart origincode nettype
 %typeyesno inout restricted expires enforce
 %typevalidity aspa_validity
@@ -297,39 +297,7 @@ asnumber   : NUMBER{
}
}
 
-as4number  : STRING{
-   const char  *errstr;
-   char*dot;
-   uint32_t uvalh = 0, uval;
-
-   if ((dot = strchr($1,'.')) != NULL) {
-   *dot++ = '\0';
-   uvalh = strtonum($1, 0, USHRT_MAX, &errstr);
-   if (errstr) {
-   yyerror("number %s is %s", $1, errstr);
-   free($1);
-   YYERROR;
-   }
-   uval = strtonum(dot, 0, USHRT_MAX, &errstr);
-   if (errstr) {
-   yyerror("number %s is %s", dot, errstr);
-   free($1);
-   YYERROR;
-   }
-   free($1);
-   } else {
-   yyerror("AS %s is bad", $1);
-   free($1);
-   YYERROR;
-   }
-   if (uvalh == 0 && (uval == AS_TRANS || uval == 0)) {
-   yyerror("AS %u is reserved and may not be used",
-   uval);
-   YYERROR;
-   }
-   $$ = uval | (uvalh << 16);
-   }
-   | asnumber {
+as4number  : asnumber {
if ($1 == AS_TRANS || $1 == 0) {
yyerror("AS %u is reserved and may not be used",
(uint32_t)$1);
@@ -339,38 +307,6 @@ as4number  : STRING{
}
;
 
-as4number_any  : STRING{
-   const char  *errstr;
-   char*dot;
-   uint32_t uvalh = 0, uval;
-
-   if ((dot = strchr($1,'.')) != NULL) {
-   *dot++ = '\0';
-   uvalh = strtonum($1, 0, USHRT_MAX, &errstr);
-  

npppd(8): remove "pipex" option

2023-01-29 Thread Vitaliy Makkoveev
Hi,

While switchind pppx(4) and pppac(4) from selwakeup() to KNOTE(9), I
found npppd(8) doesn't create pppx interface with "pipex no" in
npppd.conf, but successfully connects the client. So packets don't flow.
However, the pppac(4) has no this problem, because corresponding pppac
interface always created when npppd(8) opened device node.

In fact, npppd(8) will not work with pppx(4) interfaces without pipex(4)
support. Otherwise npppd(8) should create pppx(4) sessions with not
pipex(4) specific PIPEXASESSION ioctl(2) command.

I propose to remove "pipex" option from npppd(8). We already have
"net.pipex.enable" sysctl MIB to control pipex behaviour. In the case
then "net.pipex.enable" is set to 0, pipex(4) sessions will be always
created, but the traffic will go outside pipex(4) layer.

The "ifdef USE_NPPPD_PIPEX" left as is. If we decide to remove them, I
will do this with the next diffs.

Please note, we never have complains about the problem described above,
so I doubt someone uses npppd(8) with "pipex no" in the npppd.conf(5).

I tested both pppac(4) and pppx(4) cases with both "net.pipex.enable=1"
and "net.pipex.enable=0".

Index: usr.sbin/npppd/npppd/npppd.c
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.c,v
retrieving revision 1.53
diff -u -p -r1.53 npppd.c
--- usr.sbin/npppd/npppd/npppd.c1 Jul 2022 09:57:24 -   1.53
+++ usr.sbin/npppd/npppd/npppd.c29 Jan 2023 11:04:30 -
@@ -235,14 +235,12 @@ npppd_get_npppd()
 int
 npppd_init(npppd *_this, const char *config_file)
 {
-   int  i, status = -1, value;
+   int  i, status = -1;
const char  *pidpath0;
FILE*pidfp = NULL;
struct tunnconf *tunn;
struct ipcpconf *ipcpconf;
struct ipcpstat *ipcpstat;
-   int  mib[] = { CTL_NET, PF_PIPEX, PIPEXCTL_ENABLE };
-   size_t   size;
 
memset(_this, 0, sizeof(npppd));
 #ifndefNO_ROUTE_FOR_POOLED_ADDRESS
@@ -294,17 +292,6 @@ npppd_init(npppd *_this, const char *con
if ((status = npppd_reload_config(_this)) != 0)
return status;
 
-   TAILQ_FOREACH(tunn, &_this->conf.tunnconfs, entry) {
-   if (tunn->pipex) {
-   size = sizeof(value);
-   if (!sysctl(mib, nitems(mib), &value, &size, NULL, 0)
-   && value == 0)
-   log_printf(LOG_WARNING,
-   "pipex(4) is disabled by sysctl");
-   break;
-   }
-   }
-
if ((_this->map_user_ppp = hash_create(
(int (*) (const void *, const void *))strcmp, str_hash,
NPPPD_USER_HASH_SIZ)) == NULL) {
@@ -1052,7 +1039,6 @@ npppd_ppp_pipex_enable(npppd *_this, npp
 
NPPPD_ASSERT(ppp != NULL);
NPPPD_ASSERT(ppp->phy_context != NULL);
-   NPPPD_ASSERT(ppp->use_pipex != 0);
 
pipex_setup_common(ppp, &req);
 
Index: usr.sbin/npppd/npppd/npppd.conf.5
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v
retrieving revision 1.30
diff -u -p -r1.30 npppd.conf.5
--- usr.sbin/npppd/npppd/npppd.conf.5   31 Mar 2022 17:27:30 -  1.30
+++ usr.sbin/npppd/npppd/npppd.conf.5   29 Jan 2023 11:04:30 -
@@ -349,19 +349,6 @@ the address assigned by
 for the link.
 The default value is
 .Dq no .
-.It Ic pipex Ar yes | no
-Specify whether
-.Xr npppd 8
-uses
-.Xr pipex 4 .
-The default is
-.Dq yes .
-The
-.Xr sysctl 8
-variable
-.Va net.pipex.enable
-should also be enabled to use
-.Xr pipex 4 .
 .It Ic debug-dump-pktin Ar protocol ...
 If this option is specified,
 .Xr npppd 8
Index: usr.sbin/npppd/npppd/npppd.h
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.h,v
retrieving revision 1.19
diff -u -p -r1.19 npppd.h
--- usr.sbin/npppd/npppd/npppd.h12 Aug 2017 11:20:34 -  1.19
+++ usr.sbin/npppd/npppd/npppd.h29 Jan 2023 11:04:30 -
@@ -133,8 +133,6 @@ struct tunnconf {
bool   ingress_filter;
intcallnum_check;
 
-   bool   pipex;
-
u_int  debug_dump_pktin;
u_int  debug_dump_pktout;
 };
Index: usr.sbin/npppd/npppd/parse.y
===
RCS file: /cvs/src/usr.sbin/npppd/npppd/parse.y,v
retrieving revision 1.25
diff -u -p -r1.25 parse.y
--- usr.sbin/npppd/npppd/parse.y15 Oct 2021 15:01:28 -  1.25
+++ usr.sbin/npppd/npppd/parse.y29 Jan 2023 11:04:30 -
@@ -125,7 +125,7 @@ typedef struct {
 %token L2TP_HELLO_INTERVAL L2TP_HELLO_TIMEOUT L2TP_ACCEPT_DIALIN
 %token MPPE MPPE_KEY_LENGTH MPPE_KEY_STATE
 %token IDLE_TIMEOUT TCP_M