Re: [patch] Sort of fix for game "phantasia"

2023-09-15 Thread William Ahern
On Sat, Sep 16, 2023 at 05:26:26AM +0300, S V wrote:
> Maybe I understand you wrong, but I didn't return any setgid to code,
> just adding permissions to /var/games/phantasia/* files, or does it
> count as "setgid"?

I believe he means the SGID bit (S_ISGID in ) on the
/usr/games/phantasia executable[1], or possibly on the /var/games/phantasia
directory[2]. What seems to be left unstated is that your patch,

  + chmod ugo+rw ${DESTDIR}/var/games/phantasia/*

making the files writable by all is unacceptable; so unacceptable that he's
not even acknowledging it, skipping ahead several steps and responding to a
hypothetical future question.

I would think the proper solution here is to simply add your user(s) to the
games group. But given the warning in the man page, I assume there's more
going on here. Perhaps phantasia isn't ensuring that files it creates have
the correct permission bits or group ownership.

[1] This has the same effect as calling setgid, and people use setgid to
refer to both cases.[3] Not many people use "SGID".

[2] The SGID bit on a directory causes files created in that directory to
inherit the directory's GID, rather than the effective GID of the calling
process. In effect it's similar to a process calling setgid (setegid) before
creating the file.[3] SGID on directories is a BSD-specific feature that I
really like, but perhaps there's a caveat I'm missing. The one practical
caveat I'm aware of is that inheriting the GID isn't useful if the new file
isn't group writable, and the default umask of 0022 masks the S_IWGRP bit.
When using SGID on a shared Git repository, for example, you need to
explictly configure Git to ensure files and directories are group writable.

[3] But a supplementary GID doesn't grant the ability to setgid or setegid
on that GID; in practice it's only root who can change a process' effective
GID, which is why the SGID bit exists.



Re: autopledge

2023-06-02 Thread William Ahern
On Fri, Jun 02, 2023 at 04:24:31PM +0100, Leah Rowe wrote:
> 
> Hi everyone,
> 
> I had an interesting idea for OpenBSD. Haven't tried it yet. I'm
> wondering what other people think of it? The idea is, thus:
> 
> 1) Do execution tracing and just run a program. Do everything possible
> in it to the fullest extent feasible and get an entire log of the
> trace. OpenBSD can do tracing:

> 2) Write a program that scans for all system calls in the trace,
> suggesting what pledge promises to use. See:
> 
> https://man.openbsd.org/pledge.2
> 
> I call this idea "autopledge".


OpenBSD once had a tool like this as part of its systrace sandboxing
facility, in the form of the -A option argument:

  -AAutomatically generate a policy that allows every operation the
application executes. The created policy functions as a base that
can be refined.

See https://man.openbsd.org/OpenBSD-5.9/systrace.1#A

OpenBSD has already been down this road. It turned out that not only was the
notion, "if we just made it easier to autogenerate a sandbox configuration,
more people would use it", wrong--more people did not--it was based on
faulty premises. This real-world experience is what led to pledge and
unveil, and why you'll find little interest in a tool predicated on reducing
the need for a piece of software to be thoughtfully and deliberately
refactored. Rather, the point of pledge and unveil is to make that
deliberate refactoring as pleasant and minimal as is practicable.



Re: cleanup vmm_start_vm, simplifying fd cleanup

2023-04-10 Thread William Ahern
On Fri, Apr 07, 2023 at 11:45:41PM -0700, Philip Guenther wrote:
> On Fri, Apr 7, 2023 at 9:44 AM Dave Voutila  wrote:
> ...
> 
> > Touch longer, but won't generate ktrace noise by blind calls to close(2)
> > and also accounts for the other error conditions (EINTR, EIO).
> >
> > For EIO, not sure yet how we want to handle it other than log it.
> >
> > For EINTR, we want to account for that race and make sure we retry since
> > the vmm process is long-lived and could inadvertently keep things like
> > tty fds or disk image fds open after the guest vm terminates.
> >
> 
> So, this is an area where
>  * the current POSIX standard leaves the behavior unspecified
>  * everyone (except HP-UX) made close() *always* invalidate the fd, even if
> it fails with EINTR

macOS is also an exception--even messier exception. Ans worse, rather than
fix things one way or another, they simply added close$NOCANCEL. See, e.g.,
https://chromium.googlesource.com/chromium/src/base/+/refs/heads/main/mac/close_nocancel.cc

In the case of macOS, the root issue is POSIX thread cancellation, the gift
that keeps on giving.



Re: sigwaitinfo(2) and sigtimedwait(2)

2021-10-07 Thread William Ahern
On Sun, Sep 26, 2021 at 02:36:02PM +0200, Mark Kettenis wrote:
> > Date: Fri, 24 Sep 2021 19:36:21 +0200
> > From: Rafael Sadowski 
> > 
> > I'm trying to port the more KDE stuff so my question is from porter
> > perspective.
> > 
> > I need sigwaitinfo(2)/sigtimedwait(2) and I found both functions in
> > lib/libc/gen/sigwait.c with the comment "need kernel to fill in more
> > siginfo_t bits first". Is the comment still up to date? If no, is it
> > possible to unlock the functions?
> 
> Still true.  These functions are somewhat underspecified by POSIX so
> it isn't really obvious whatadditional bits need to be filled in.
> Having examples of code that use these interfaces from ports could
> help with that.

Not in ports, but in a Lua Unix bindings module I ended up writing an
incomplete sigtimedwait emulation for OpenBSD and macOS targets. It's not
thread-safe and lacks siginfo support, but sufficed for the most pressing
use case, which was to provide a standard (non-kqueue, non-signalfd), simple
(i.e. robust to subtle race conditions that might leave a process hung or
drop a signal on the floor) mechanism to receive and *clear* signals in Lua
code without having to deal with catching and invoking signal handlers
asynchronously in Lua or otherwise writing some complex, fiddly mechanism
for bridging the language and runtime divide.

By emulating sigtimedwait I in fact ended up writing a fiddly and incomplete
mechanism for bridging C and Lua runtime behaviors, but only because
sigtimedwait didn't exist on OpenBSD or macOS; from the perspective of Lua
code relying on POSIX APIs, it still made sense.

At least in my case a sigtimedwait lacking siginfo support would have been
infinitely better than no sigtimedwait at all. It's the ability to clear a
signal without using a signal handler, while also being able to specify a
timeout so you don't accidentally end up hung in sigwait, that was most
important. Technically sigpending+sigwait would suffice for non-blocking
polling (potential thread races notwithstanding), but when looking at POSIX
APIs sigtimedwait is the most obvious solution for that as well as some more
complex scenarios.



Re: POSIX_C_SOURCE 200809L, XOPEN_SOURCE 700 and bsd_locale_fallbacks errors

2021-04-21 Thread William Ahern
On Tue, Apr 13, 2021 at 09:06:12PM +0200, Mark Kettenis wrote:
> > Date: Tue, 13 Apr 2021 19:36:26 +0200
> > From: Rafael Sadowski 
> > 
> > Based on my cmake pull-request(1) to fix the cmake build on OpenBSD, the
> > following question has arisen which is worth analysing?
> > 
> > "It seems OpenBSD has a strange behavior because macro _POSIX_C_SOURCE is a
> > standard!  @sizeofvoid What are the errors raised if _POSIX_C_SOURCE or
> > _XOPEN_SOURCE are defined?" -- Marc Chevrier
> > 
> > [1]: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6000
> > 
> > The following code includes the if-defs from cmake with a simple sstream
> > include.
> > 
> > $ cat define_test.cxx
> > #if !defined(_WIN32) && !defined(__sun)
> > // POSIX APIs are needed
> > // NOLINTNEXTLINE(bugprone-reserved-identifier)
> > #  define _POSIX_C_SOURCE 200809L
> > #endif
> > #if defined(__OpenBSD__) || defined(__FreeBSD__) || defined(__NetBSD__)
> > // For isascii
> > // NOLINTNEXTLINE(bugprone-reserved-identifier)
> > #  define _XOPEN_SOURCE 700
> > #endif
> > 
> > #include 
> > int main () { return 0; }
> > 
> > $ clang++ -std=c++17 define_test.cxx  # also with c++11/14
> > In file included from define_test.cxx:16:
> > In file included from /usr/include/c++/v1/sstream:173:
> > In file included from /usr/include/c++/v1/ostream:140:
> > In file included from /usr/include/c++/v1/locale:207:
> > /usr/include/c++/v1/__bsd_locale_fallbacks.h:122:17: error: use of 
> > undeclared identifier 'vasprintf'; did you mean 'vsprintf'?
> > int __res = vasprintf(__s, __format, __va);
> > ^
> > /usr/include/c++/v1/cstdio:124:9: note: 'vsprintf' declared here
> > using ::vsprintf;
> > ^
> > In file included from define_test.cxx:16:
> > In file included from /usr/include/c++/v1/sstream:173:
> > In file included from /usr/include/c++/v1/ostream:140:
> > In file included from /usr/include/c++/v1/locale:207:
> > /usr/include/c++/v1/__bsd_locale_fallbacks.h:122:27: error: cannot 
> > initialize a parameter of type 'char *' with an lvalue of type 'char **'
> > int __res = vasprintf(__s, __format, __va);
> >   ^~~
> > /usr/include/stdio.h:269:21: note: passing argument to parameter here
> > int  vsprintf(char *, const char *, __va_list);
> > ^
> > 2 errors generated
> > 
> > Looks like, if "_XOPEN_SOURCE 700" or "_POSIX_C_SOURCE 200809L" is defined 
> > we
> > run in this compile error. The question is, is that deliberate?
> 
> This is a libc++ issue that isn't really OpenBSD-specific.  It would
> happen on Linux as well if you use libc++.
> 
> To fix this we need to implement .
> 
> That said, OpenBSD provides the POSIX APIs by default.  I believe
> FreeBSD and NetBSD don't.  So your proposed fix makes sense.

FreeBSD, NetBSD, and macOS also have a default environment that exposes all
APIs, modulo any conflicting prototypes.[1] It's been this way for at least
10 years. Ditto for DragonflyBSD and even Minix, IIRC. It's only AIX
(_ALL_SOURCE), Solaris (__EXTENSIONS__), and Linux/glibc (_GNU_SOURCE,
_DEFAULT_SOURCE) that require explicitly exposing a complete "native"
environment. I'm not even sure about AIX (my Lua Unix module doesn't bother
setting it), but I can't access my AIX dev shell at the moment.

IME, fiddling with _XOPEN_SOURCE, _POSIX_C_SOURCE, or similar is not only
unnecessary, but it *creates* more headaches. On most systems they're
effectively *subtractive*, not additive. Once you explicitly specify a
standard you enter portability hell as there's no consistent way to then
make visible other APIs--many of which are de facto standard and universally
available--that most programs invariably use. IOW, it's only when you
specify a standard that you end up maintaining a nightmarish preprocessor
feature flag ladder.

[1] glibc's strerror_r is the only example I can think of where a default
native environment exposes patently POSIX-incompatible types.



Re: [PATCH v2] tee: Add -q, --quiet, --silent option to not write to stdout

2021-01-21 Thread William Ahern
On Fri, Jan 22, 2021 at 12:12:58AM +0100, Alejandro Colomar wrote:
> This is useful for using tee to just write to a file,
> at the end of a pipeline,
> without having to redirect to /dev/null.

> @@ -93,6 +98,7 @@ Copy standard input to each FILE, and also to standard 
> output.\n\
>  "), stdout);
>fputs (_("\
>-pdiagnose errors writing to non pipes\n\
> +  -q, --quiet, --silent don't write to standard output\n\
>--output-error[=MODE]   set behavior on write error.  See MODE below\n\
>  "), stdout);
>fputs (HELP_OPTION_DESCRIPTION, stdout);
> @@ -130,6 +136,7 @@ main (int argc, char **argv)
>  
>append = false;
>ignore_interrupts = false;
> +  quiet = false;
>  
>while ((optc = getopt_long (argc, argv, "aip", long_options, NULL)) != -1)

'q' missing from optstring

>  {
> @@ -151,6 +158,10 @@ main (int argc, char **argv)
>  output_error = output_error_warn_nopipe;
>break;
>  
> +case 'q':
> +  quiet = true;
> +  break;
> +
>  case_GETOPT_HELP_CHAR;
>  
>  case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);



Re: getopt.3 bugs section

2021-01-08 Thread William Ahern
On Fri, Jan 08, 2021 at 05:29:31PM -0600, Edgar Pettijohn wrote:
> In the BUGS section for the getopt(3) manual it mentions not using
> single digits for options. I know spamd uses -4 and -6 there are
> probably others. Should they be changed? Or is the manual mistaken?
> 

That section seems ambiguous (especially in light of your post) on whether
using digits is wrong, or abusing digit support for multi-digit number
option support is wrong. But I think the intention is the latter.

FWIW, POSIX explicitly permits digits:

  Guideline 3:
Each option name should be a single alphanumeric character (the alnum
character classification) from the portable character set. The -W
(capital-W) option shall be reserved for vendor options.

Multi-digit options should not be allowed.

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02

Guideline 3 applies to getopt(3) because,

  The getopt() function... shall follow Utility Syntax Guidelines 3, 4, 5,
  6, 7, 9, and 10 in XBD Utility Syntax Guidelines.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/getopt.html



Re: New EVFILT_EXCEPT for POLLPRI & POLLRDBAND

2020-06-16 Thread William Ahern
On Tue, Jun 16, 2020 at 06:18:13AM -0600, Todd C. Miller wrote:
> On Tue, 16 Jun 2020 12:48:58 +0200, Martin Pieuchot wrote:
> 
> > The diff below implements DragonFly's approach of adding a new kind of
> > filter, EVFILT_EXCEPT, to report such conditions.  This extends the
> > existing kqueue interface which is questionable.  On the one hand this
> > allows userland programs to use kevent(2) to check for this conditions.
> > One the other hand this is not supported by any other BSD and thus non
> > standard.
> 
> Actually, it looks like macOS uses EVFILT_EXCEPT too.  They were
> the first OS to implement poll in terms of kqueue as far as I know.
> I don't think there is a problem extended kqueue with EVFILT_EXCEPT.

macOS also has EV_OOBAND as an analog to EV_EOF, but the addition caught
people by surprise:

  https://bugs.chromium.org/p/chromium/issues/detail?id=437642
  https://nvd.nist.gov/vuln/detail/CVE-2015-1105

I would guess EV_OOBAND was intended to behave like POLLRDBAND, and
considering that POLLRDBAND is in POLLIN it made some sense to signal
EV_OOBAND by default. Likewis for EV_EOF, which is also always signaled,
though ignoring it is benign. It all seems logical (notwithstanding the
inability to mask it), but only if that was the behavior from day 1.
Surprising people with it doesn't seem like a good idea.



Re: Dead peer detection in iked

2020-05-07 Thread William Ahern
On Thu, May 07, 2020 at 01:54:13PM +0200, Stephan Mending wrote:
> Hi *, 
> I was wondering why there is no dead peer detection implemented for iked ?
>
> Is it just due to lack of time ? Or are there good reasons to dismiss
> directly implemented dpd in iked ?
> 
> Because technically one has the option to just use ifstated. 
> 
> I'm just being curios here. 

AFAICT iked implements this behavior in ikev2.c:ikev2_ike_sa_alive, but it
seems that before OpenBSD 6.7 it didn't send probes on completely idle SAs
See https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sbin/iked/ikev2.c#rev1.214

Hopefully with 6.7 I can finally remove my ifstated ping checks.



Re: [PATCH] sysupgrade

2020-04-30 Thread William Ahern
On Thu, Apr 30, 2020 at 11:19:14AM +, Kevin Chadwick wrote:

> 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.

Often there's a no_x11 FLAVOR, but avoiding that extra work in ports
maintenance is part of the motivation for deprecating non-X installations.
IIRC, ports maintainers have for years made it clear that providing and
maintaining no_x11 FLAVORs isn't a priority.

> Struggling to remember why I wanted to do it, to be honest.

Because until relatively recently X was installed sgid root. But that
was fixed for 6.5:

  
https://cvsweb.openbsd.org/xenocara/xserver/Makefile.bsd-wrapper?rev=1.67=text/x-cvsweb-markup



Re: posix_openpt: allow O_CLOEXEC

2020-02-05 Thread William Ahern
On Wed, Feb 05, 2020 at 05:48:41PM -0700, Todd C. Miller wrote:
> On Wed, 05 Feb 2020 15:47:37 -0600, joshua stein wrote:
> 
> > The spec says the behavior of anything other than O_RDWR and 
> > O_NOCTTY is unspecified, but FreeBSD allows passing O_CLOEXEC.
> 
> OK, but the manual needs to specify that O_CLOEXEC support is an
> extension.  E.g., under STANDARDS:
> 
> The ability to use
> .Dv O_CLOEXEC
> is an extension to that standard.
> 
>  - todd

FWIW, it looks like a future standard version/revision will add O_CLOEXEC to
posix_openpt. See https://www.austingroupbugs.net/view.php?id=411 (grep for
posix_openpt, which is included among many other similar modifications).

I haven't yet figured out the rules for that group and how things wind up in
the published standard; what counts as a defect and what counts as a new
feature. I'm a little surprised it's not already in the -2017 revision.
IIRC, some other changes related to O_CLOEXEC/FD_CLOEXEC consistency made it
into -2017 and earlier revisions.



Re: make kevent(2) (a bit) mpsafe

2019-05-01 Thread William Ahern
On Wed, May 01, 2019 at 04:35:02PM +1000, David Gwynne wrote:
> i originally came at this from the other side, where i wanted to run
> kqueue_enqueue and _dequeue without the KERNEL_LOCK, but that implied
> making kqueue_scan use the mutex too, which allowed the syscall to
> become less locked.
> 
> it assumes that the existing locking in kqueue_scan is in the right
> place, it just turns it into a mutex instead of KERNEL_LOCK with
> splhigh. it leaves the kqueue_register code under KERNEL_LOCK, but if
> you're not making changes with kevent then this should be a win.
> 
> there's an extra rwlock around the kqueue_scan call. this protects the
> kq_head list from having multiple marker structs attached to it. that is
> an extremely rare situation, ie, you'd have to have two threads execute
> kevent on the same kq fd concurrently, but that never happens. right?

FWIW, in Linux-land a shared event descriptor with edge-triggered events is
a not uncommon pattern for multithreaded dispatch loops as it removes the
need for userspace locking. kqueue supports the same pattern and some
portable event loops likely mix threads and shared event descriptors this
way. epoll and kqueue (and Solaris Ports, for that matter) are similar
enough that a thin wrapper doesn't even need to explicitly support this
pattern for it to be available to an application.

I don't see any reason to optimize for it at the moment though.[1] That lock
doesn't appear to change semantics, just serializes threads waiting on the
queue, right? Even if the order that threads are awoken changes it shouldn't
impact correctness.

[1] Or ever. Edge-triggered events and shared mutable data are individually
brittle, difficult to maintain design choices; combining them is just asking
for trouble. I've seen projects go down this path and then switch to oneshot
events instead of edge-triggered events to "solve" thread race bugs, which
can result in worse performance than a classic select loop. For example, in
low-latency and/or high load scenarios where the total number of syscalls
constantly rearming a descriptor is greater. Linux still doesn't have
batched updates, AFAIK, and they're prohibitively difficult to implement for
a multithreaded, lock-free dispatch loop, anyhow, so it's not a common
optimization.



dhcpd domain-search patch

2019-02-26 Thread William Ahern
systemd's dhcp client doesn't accept the hack of putting multiple,
space-separated search domains in the domain-name option. The following
patch parses option domain-search as a list of host names and uses
dn_comp(3) from libc to compress the list for the on-wire option value.

Example dhcpd.conf usage:

  option domain-search openbsd.org, example.com, a.example.com;

Index: confpars.c
===
RCS file: /cvs/src/usr.sbin/dhcpd/confpars.c,v
retrieving revision 1.33
diff -u -p -r1.33 confpars.c
--- confpars.c  24 Apr 2017 14:58:36 -  1.33
+++ confpars.c  27 Feb 2019 07:23:41 -
@@ -43,7 +43,9 @@
 
 #include 
 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1207,6 +1209,12 @@ parse_option_param(FILE *cfile, struct g
tree = tree_concat(tree, tree_const(
buf, (cprefix + 7) / 8));
break;
+   case 'Z':
+   t = parse_domain_and_comp(cfile, uniform);
+   if (!t)
+   return;
+   tree = tree_concat(tree, t);
+   break;
default:
log_warnx("Bad format %c in "
"parse_option_param.", *fmt);
@@ -1467,4 +1475,82 @@ parse_address_range(FILE *cfile, struct 
 
/* Create the new address range. */
new_address_range(low, high, subnet, dynamic);
+}
+
+static void push_domain_list(char ***domains, size_t *count, char *domain)
+{
+   *domains = reallocarray(*domains, *count + 1, sizeof **domains);
+   if (!*domains)
+   fatalx("Can't allocate domain list");
+
+   (*domains)[*count] = domain;
+   ++*count;
+}
+
+static void
+free_domain_list(char **domains, size_t count)
+{
+   for (size_t i = 0; i < count; i++)
+   free(domains[i]);
+   free(domains);
+}
+
+struct tree *
+parse_domain_and_comp(FILE *cfile, int uniform)
+{
+   char **domains = NULL;
+   size_t count = 0;
+   unsigned char *buf = NULL;
+   size_t bufsiz = 0, bufn = 0;
+   unsigned char **bufptrs = NULL;
+   struct tree *rv = NULL;
+   int token;
+
+   do {
+   char *domain;
+
+   domain = parse_host_name(cfile);
+   if (!domain)
+   goto error;
+   push_domain_list(, , domain);
+   /*
+* openbsd.org normally compresses to [7]openbsd[3]org[0]. 
+* +2 to string length provides space for leading and
+* trailing (root) prefix lengths not already accounted for
+* by dots, and also provides sufficient space for pointer
+* compression.
+*/
+   bufsiz = bufsiz + 2 + strlen(domain);
+   token = peek_token(NULL, cfile);
+   if (token == ',')
+   token = next_token(NULL, cfile);
+   } while (uniform && token == ',');
+
+   buf = malloc(bufsiz);
+   if (!buf)
+   fatalx("Can't allocate compressed domain buffer");
+   bufptrs = calloc(count + 1, sizeof *bufptrs);
+   if (!bufptrs)
+   fatalx("Can't allocate compressed pointer list");
+   bufptrs[0] = buf;
+   
+   /* dn_comp takes an int for the output buffer size */
+   if (!(bufsiz <= INT_MAX))
+   fatalx("Size of compressed domain buffer too large");
+   for (size_t i = 0; i < count; i++) {
+   int n;
+
+   /* see bufsiz <= INT_MAX assertion, above */
+   n = dn_comp(domains[i], [bufn], bufsiz - bufn, bufptrs, 
[count + 1]);
+   if (n == -1)
+   fatalx("Can't compress domain");
+   bufn += (size_t)n;
+   }
+
+   rv = tree_const(buf, bufn);
+error:
+   free_domain_list(domains, count);
+   free(buf);
+   free(bufptrs);
+   return rv;
 }
Index: dhcp.h
===
RCS file: /cvs/src/usr.sbin/dhcpd/dhcp.h,v
retrieving revision 1.10
diff -u -p -r1.10 dhcp.h
--- dhcp.h  21 Jan 2014 03:07:51 -  1.10
+++ dhcp.h  27 Feb 2019 07:23:41 -
@@ -171,6 +171,7 @@ struct dhcp_packet {
 #define DHO_NDS_SERVERS85
 #define DHO_NDS_TREE_NAME  86
 #define DHO_NDS_CONTEXT87
+#define DHO_DOMAIN_SEARCH  119
 #define DHO_CLASSLESS_STATIC_ROUTES121
 #define DHO_TFTP_CONFIG_FILE   144
 #define DHO_VOIP_CONFIGURATION_SERVER  150
Index: dhcpd.h
===
RCS file: /cvs/src/usr.sbin/dhcpd/dhcpd.h,v
retrieving revision 1.66
diff -u -p -r1.66 dhcpd.h
--- dhcpd.h 4 Aug 2017 02:01:46 

Re: bgpd, protability and sockaddr sa_len

2019-02-15 Thread William Ahern
On Fri, Feb 15, 2019 at 03:07:15PM +0100, Claudio Jeker wrote:
> Another diff to ease portability of bgpd. The sa_len field in struct
> sockaddr does not exist on Linux so instead of using it pass a length to
> the function (e.g. like bind(2) and connect(2) and do the same when
> passing around struct sockaddr_storage in the listener case.
> The remaining sa_len, sin_len, sin6_len and ss_len usages are in very
> OpenBSD specific files.
> 
> -- 
> :wq Claudio
> 
> Index: bgpd.h
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.369
> diff -u -p -r1.369 bgpd.h
> --- bgpd.h15 Feb 2019 11:38:06 -  1.369
> +++ bgpd.h15 Feb 2019 13:51:28 -
> @@ -220,11 +220,12 @@ struct bgpd_addr {
>  #define  LISTENER_LISTENING  0x02
>  
>  struct listen_addr {
> - TAILQ_ENTRY(listen_addr) entry;
> - struct sockaddr_storage  sa;
> - int  fd;
> - enum reconf_action   reconf;
> - u_int8_t flags;
> + TAILQ_ENTRY(listen_addr)entry;
> + struct sockaddr_storage sa;
> + int fd;
> + enum reconf_action  reconf;
> + socklen_t   sa_len;
> + u_int8_tflags;
>  };

What's the use of maintaining and passing around sa_len if the sa member is
a fixed size? (Well, other than being a more straightforward patch.)

AFAIK the only variably sized sockaddr structure is sockaddr_un. Domain
socket paths can be longer than what sockaddr_un (or sockaddr_storage) can
nominally fit, but if the sa member is a fixed size then it's irrelevant and
you can always derive the sa object size from .sa_family or .sun_family +
.sun_path.



Re: Virtio 1.0 for the kernel

2019-01-11 Thread William Ahern
On Fri, Jan 11, 2019 at 10:43:25AM +0100, Stefan Fritsch wrote:
 
> /* only used for sizeof, not actually allocated */
> extern struct virtio_pci_common_cfg ccfg;
 
> #define CREAD(sc, memb)  _cread(sc, \
> offsetof(struct virtio_pci_common_cfg, memb), sizeof(ccfg.memb))
> 
> The compiler should optimize this to the same code as the complicated 
> macro above. You think this variant is acceptable?

Maybe I'm missing something, but these are the more idiomatic constructs

  sizeof ((struct virtio_pci_common_cfg *)0)->memb
  sizeof ((struct virtio_pci_common_cfg){ 0 }).memb



Re: binutils: build with LLVM 6.0.0

2018-03-15 Thread William Ahern
On Thu, Mar 15, 2018 at 05:23:24PM +0100, Patrick Wildt wrote:
> Hi,
> 
> LLVM 6.0.0 does now complain of code does computation on NULL pointers,
> which apparently binutils makes use of.  I think we can teach binutils
> to stop doing that.
> 
> Is my C foo correct?  Feedback?

Both (type *)0 - 1 and (type *)-1 rely on undefined behavior, but
_different_ undefined behavior. Even presuming the former was reliable, is
the latter?

FWIW, the two expressions also evaluate to different values,
0xfffc vs 0x. That's not a problem by itself but
suggests possible signedness (or whatever you call the analogous issue for
pointer arithmetic) pitfalls.

Maybe it's safer to use the address of abfd, which should be shared even if
archive_symbol_lookup is a function pointer to an external instantiation? I
originally thought to use a static global singleton, but the macros and
function pointers made me doubt the caller and callee are always from the
same object. Using abfd is simpler and I don't see where abfd might be
wrapped or proxied.

> Patrick
> 
> diff --git a/gnu/usr.bin/binutils-2.17/bfd/elflink.c 
> b/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> index a6d17dcb4d9..aa010f9d0b2 100644
> --- a/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> +++ b/gnu/usr.bin/binutils-2.17/bfd/elflink.c
> @@ -4517,7 +4517,7 @@ _bfd_elf_archive_symbol_lookup (bfd *abfd,
>len = strlen (name);
>copy = bfd_alloc (abfd, len);
>if (copy == NULL)
> -return (struct elf_link_hash_entry *) 0 - 1;
> +return (struct elf_link_hash_entry *)-1;
>  
>first = p - name + 1;
>memcpy (copy, name, first);
> @@ -4629,7 +4629,7 @@ elf_link_add_archive_symbols (bfd *abfd, struct 
> bfd_link_info *info)
>   }
>  
> h = archive_symbol_lookup (abfd, info, symdef->name);
> -   if (h == (struct elf_link_hash_entry *) 0 - 1)
> +   if (h == (struct elf_link_hash_entry *)-1)
>   goto error_return;
>  
> if (h == NULL)
> diff --git a/gnu/usr.bin/binutils-2.17/include/obstack.h 
> b/gnu/usr.bin/binutils-2.17/include/obstack.h
> index 88c2a264adc..8839c48e95f 100644
> --- a/gnu/usr.bin/binutils-2.17/include/obstack.h
> +++ b/gnu/usr.bin/binutils-2.17/include/obstack.h
> @@ -123,7 +123,7 @@ extern "C" {
>  #endif
>  
>  #ifndef __INT_TO_PTR
> -# define __INT_TO_PTR(P) ((P) + (char *) 0)
> +# define __INT_TO_PTR(P) ((char *)(P))
>  #endif
>  
>  /* We need the type of the resulting object.  If __PTRDIFF_TYPE__ is



Re: [patch] spamd-setup.c

2018-03-07 Thread William Ahern
On Wed, Mar 07, 2018 at 05:17:59PM -0600, Edgar Pettijohn wrote:
> This looks like a good place for reallocarray. Yes?
> 
> Index: spamd-setup.c
> ===
> RCS file: /cvs/src/libexec/spamd-setup/spamd-setup.c,v
> retrieving revision 1.50
> diff -u -p -u -r1.50 spamd-setup.c
> --- spamd-setup.c7 Jul 2017 00:10:15 -1.50
> +++ spamd-setup.c7 Mar 2018 23:14:00 -
> @@ -363,7 +363,7 @@ fix_quoted_colons(char *buf)
>  char *newbuf, last;
> 
>  /* Allocate enough space for a buf of all colons (impossible). */
> -newbuf = malloc(2 * strlen(buf) + 1);
> +newbuf = reallocarray(NULL, 2, strlen(buf) + 1);
>  if (newbuf == NULL)
>  return (NULL);
>  last = '\0';

FWIW, the old code evaluates as

  (2 * strlen(buf)) + 1

but the new code evaluates as

  2 * (strlen(buf) + 1)



Re: [PATCH] Fix Comment Line Unfolding in make/lowparse.c

2018-02-27 Thread William Ahern
On Fri, Feb 23, 2018 at 12:27:14PM -0800, William Ahern wrote:
> The routine skip_empty_lines_and_read_char() is an optimization to skip over
> blocks of comment lines. When it reads an unescaped '#' it uses the helper
> routine skip_to_end_of_line(). But skip_to_end_of_line() doesn't fold lines
> as it should and like its parent caller does. (See patch at end of message.)

FWIW, I meant skip_to_end_of_line() doesn't _unfold_ lines as it should. I
rephrased the subject line in case anyone ignored this patch thinking I was
submitting a patch to reformat comment lines in lowparse.c itself.

Is there any interest in accepting this patch? Would there be any interest
in a patch which rewrote skip_empty_lines_and_read_char itself? I think it
could be substantially simplified using the same, simple state machine I
used to fix skip_to_end_of_lines. The performance benefit of the current
approach seems dubious relative to the complexity, and especially
considering its (wrongly) predicated on the ability to avoid parsing escape
sequences past '#' comments without violating the syntax rules. I just
figured it wasn't worth fixing what wasn't broken as long as I could easily
fix what was broken.


> Index: lowparse.c
> ===
> RCS file: /cvs/src/usr.bin/make/lowparse.c,v
> retrieving revision 1.35
> diff -u -r1.35 lowparse.c
> --- lowparse.c21 Oct 2016 16:12:38 -  1.35
> +++ lowparse.c23 Feb 2018 20:02:59 -
> @@ -247,20 +247,21 @@
>  static int
>  skip_to_end_of_line(void)
>  {
> - if (current->F) {
> - if (current->end - current->ptr > 1)
> - current->ptr = current->end - 1;
> - if (*current->ptr == '\n')
> - return *current->ptr++;
> - return EOF;
> - } else {
> - int c;
> + int escaped = 0, c;
>  
> - do {
> - c = read_char();
> - } while (c != '\n' && c != EOF);
> - return c;
> + while (EOF != (c = read_char())) {
> + if (escaped) {
> + if (c == '\n')
> + current->origin.lineno++;
> + escaped = 0;
> + } else if (c == '\\') {
> + escaped = 1;
> + } else if (c == '\n') {
> + break;
> + }
>   }
> +
> + return c;
>  }
>  
>  



[PATCH] Folding of Comment Lines in make/lowparse.c

2018-02-23 Thread William Ahern
The routine skip_empty_lines_and_read_char() is an optimization to skip over
blocks of comment lines. When it reads an unescaped '#' it uses the helper
routine skip_to_end_of_line(). But skip_to_end_of_line() doesn't fold lines
as it should and like its parent caller does. (See patch at end of message.)

I found this bug perusing the BearSSL source code.

  25 # The lines below are a horrible hack that nonetheless works. On a
  26 # "make" utility compatible with Single Unix v4 (this includes GNU and
  27 # BSD make), the '\' at the end of a command line counts as an escape
  28 # for the newline character, so the next line is still a comment.
  29 # However, Microsoft's nmake.exe (that comes with Visual Studio) does
  30 # not interpret the final '\' that way in a comment. The end result is
  31 # that when using nmake.exe, this will include "mk/Win.mk", whereas
  32 # GNU/BSD make will include "mk/Unix.mk".
  33 
  34 # \
  35 !ifndef 0 # \
  36 !include mk/NMake.mk # \
  37 !else
  38 .POSIX:
  39 include mk/SingleUnix.mk
  40 # Extra hack for OpenBSD make.
  41 ifndef: all
  42 0: all
  43 endif: all
  44 # \
  45 !endif

  -- https://bearssl.org/gitweb/?p=BearSSL;a=blob;f=Makefile;hb=9dc62112

It's definitely a bug. POSIX says,

  There are three kinds of comments: blank lines, empty lines, and a
   ( '#' ) and all following characters up to the first
  unescaped  character.

and

  When an escaped  (one preceded by a ) is found
  anywhere in the makefile except in a command line, an include line, or a
  line immediately preceding an include line, it shall be replaced, along
  with any leading white space on the following line, with a single .

Fortunately POSIX leaves unspecified what happens to an escaped newline
preceding an include line. My patch should be the end of the
matter unless there's some other bug lurking. Here's a test case,

  # \
  broken:;@echo broken
  fixed:;@echo fixed

and the patch,

Index: lowparse.c
===
RCS file: /cvs/src/usr.bin/make/lowparse.c,v
retrieving revision 1.35
diff -u -r1.35 lowparse.c
--- lowparse.c  21 Oct 2016 16:12:38 -  1.35
+++ lowparse.c  23 Feb 2018 20:02:59 -
@@ -247,20 +247,21 @@
 static int
 skip_to_end_of_line(void)
 {
-   if (current->F) {
-   if (current->end - current->ptr > 1)
-   current->ptr = current->end - 1;
-   if (*current->ptr == '\n')
-   return *current->ptr++;
-   return EOF;
-   } else {
-   int c;
+   int escaped = 0, c;
 
-   do {
-   c = read_char();
-   } while (c != '\n' && c != EOF);
-   return c;
+   while (EOF != (c = read_char())) {
+   if (escaped) {
+   if (c == '\n')
+   current->origin.lineno++;
+   escaped = 0;
+   } else if (c == '\\') {
+   escaped = 1;
+   } else if (c == '\n') {
+   break;
+   }
}
+
+   return c;
 }
 
 



Re: Why the executable file type is also "DYN", not "EXEC"?

2017-10-05 Thread William Ahern
On Wed, Oct 04, 2017 at 04:17:32PM +0800, Nan Xiao wrote:
> Hi all,
> 
> I find the type of executable file format on OpenBSD is "DYN", not
> "EXEC":

> Is there any special consideration for it? Thanks very much in advance!
> 

Because it was built as a position-independent executable (PIE). See
https://www.openbsd.org/papers/asiabsdcon2015-pie-slides.pdf and
https://blogs.cisco.com/security/how_was_this_executable_built and
https://en.wikipedia.org/wiki/Position-independent_code#Position-independent_executables



Re: enum unsigned or not?

2017-08-31 Thread William Ahern
On Thu, Aug 31, 2017 at 02:08:07PM +0200, Otto Moerbeek wrote:
> Hi,
> 
> /usr/src/usr.sbin/sasyncd/carp.c:157:12: warning: comparison of
> unsigned enum expression < 0 is always false [-Wtautological-compare]
> if (state < 0 || state > FAIL)
> ~ ^ ~
> /usr/src/usr.sbin/sasyncd/carp.c:166:20: warning: comparison of
> unsigned enum expression < 0 is always false [-Wtautological-compare]
> if (current_state < 0 || current_state > FAIL) {
> ~ ^ ~
> 
> this warning is a tiny bit interesting. A compiler is free to choose
> the type of the enum, as long as it can represent all given values.
> So another compiler might choose not to make it unsigned. So I came up
> with this fix that is not depending on the signedness of the type. But
> most of the time avoiding enum is better, I suppose.

It's free to choose the integer type of the enum, but enumeration members
(i.e. the constant identifiers) have int type.

  The identifiers in an enumerator list are declared as constants that have
  type int and may appear wherever such are permitted.

  C11 (N1570) 6.7.2.2p3.

Furthermore, the defining expression of the constant must be representable
as an int. 6.7.2.2p2.

I've always vascillated about which operand to cast, and to which type, when
silencing compilers' annoying warnings. But now that I've read the section
more closely I think I'll just cast the operand with enum type to int, all
things being equal.



Re: httpd/libtls: TLS client certificate revocation checking

2017-04-04 Thread William Ahern
On Sat, Apr 01, 2017 at 07:10:35PM +1030, Jack Burton wrote:
> On Fri, 31 Mar 2017 13:03:44 -0700
> William Ahern <will...@25thandclement.com> wrote:

> > Basically, anything short of passing through the entire certificate
> > is going to be severely limiting and frustrating, to the point of
> > uselessness. And as a practical matter, parsing out and passing
> > through a subset of certificate data is likely going to require more
> > complex code than just passing the entire certificate to the
> > application.
> 
> Thanks William. That's an interesting idea.
> 
> Yes, I can see how having the raw client certificate available to
> fastcgi responders would be useful. That would solve a few more
> problems for my use case too and would provide the ultimate in
> flexibility. And it's not difficult to implement.
> 
> My initial gut feeling was that adding a tls_peer_serial() function to
> libtls would be less intrusive (and more in keeping with the rest of
> the tls_peer_* functions) than adding a tls_peer_cert() along the lines
> you're suggesting.

Converting a cert to a PEM string should be as simple as:

  BIO *bio = BIO_new(BIO_s_mem()); /* See NOTE 1 */
  if (!bio)
goto sslerror;
  if (!PEM_write_bio_X509(bio, crt))
goto sslerror;
  char *pem;
  long pemlen = BIO_get_mem_data(bio, );
  assert(pemlen >= 0); /* < 0 shouldn't be possible */
  ... /* copy pem string */
  BIO_free(bio); /* See NOTE 1 */

compare that to getting the serial:
  
  BIGNUM *bn = BN_new();
  if (!bn)
goto sslerror;
  ASN1_INTEGER *i;
  if (!(i = X509_get_serialNumber(crt))) /* See NOTE 2 */
goto noserial;
  if (!ASN1_INTEGER_to_BN(i, bn))
goto sslerror;
  char *serial = BN_bn2dec(serial);
  if (!serial)
goto sslerror;
  ... /* copy serial string */
  BN_free(bn);


NOTE 1: I usually keep a BIO buffer around as a singleton, using BIO_reset
instead of BIO_free.

NOTE 2: I'd try X509_get0_serialNumber unless the const-ness is troublesome.
I'm cribbing from my own code, originally written for OpenSSL 0.9.8, which
lacks X509_get0_serialNumber.



Re: httpd/libtls: TLS client certificate revocation checking

2017-03-31 Thread William Ahern
On Thu, Mar 30, 2017 at 10:31:06PM +1030, Jack Burton wrote:

> Personally, I'm leaning towards either local CRL file checking in
> httpd (with minimal changes to libtls), or passing through enough data
> to the let the fastcgi responders take whichever approach they want.

In all my experience with client certificates (whether with browsers, IPSec,
or custom applications), I've usually relied on the ability to embed data in
the client certificate, such as domains, e-mail addresses, UUIDs, etc. That
often entirely removed the need to do database queries from application code
or to maintain and expose any centralized data store (except the CRL, of
course) to network-facing nodes.

In other words, client certificates not only solve the authentication
problem, but can also help solve the authorization problem, and in many
cases solve it completely.

My $0.02: client certificates aren't particularly useful unless the
certificate itself is made available to the application. Knowing that a
certificate has a valid signature is only part of the equation, and not even
the most useful part. Unlike server certificates, however, there's no single
authorization check (i.e. matching the embedded domain name to the queried
URL) that can be implemented by middle-ware. The semantics of embedded data
are specific to a service and often ad hoc.

Basically, anything short of passing through the entire certificate is going
to be severely limiting and frustrating, to the point of uselessness. And as
a practical matter, parsing out and passing through a subset of certificate
data is likely going to require more complex code than just passing the
entire certificate to the application.



Re: OpenIKED Keepalive Broken

2016-08-12 Thread William Ahern
On Fri, Aug 12, 2016 at 09:56:41PM +0200, fRANz wrote:
> On Sat, Aug 6, 2016 at 2:18 AM, William Ahern
> <will...@25thandclement.com> wrote:

> > isakmpd unconditionally sends NAT-T keepalive messages every 30 seconds,
> > whereas iked's ikev2_ike_sa_alive only sends a keepalive message iff
> > `!foundin && foundout`. But that presumes that the SA initiator is also the
> > initiator of traffic, which definitely isn't the case in my situation, and
> > seems dubious and unreliable even for real road warriors.
> 
> ...
> 
> > I'd be happy to create a proper patch if someone could explain the purpose
> > of the conditional logic. I wouldn't want to accidentally break something.
> >
> > I also wouldn't mind making the keepalive interval configurable--rather than
> > a compile-time constant--so users could deal with NAT gateways which
> > aggressively flush state.
> 
> Hello William,
> I did the same switch (from isakmpd to iked) with a lot of problems,
> maybe the same that you're reported.
> Did you receive any feedback from OpenBSD staff, catching the occasion
> of the 6.0 release ready to go?
> Regards,
> -f

No feedback, yet, but soon after posting I realized a few things:

1) My hack makes the tunnel much more stable, but not nearly as stable as
for isakmpd. I think it's because with isakmpd both peers are sending a
keepalive every 30 seconds, whereas I only applied the hack I posted to the
active, behind-the-NAT peer. See point #3, below.

2) The logic of ikev2_ike_sa_alive is intended, I think, to preserve the
limited lifetime of SAs. Otherwise by unconditionally sending a keepalive
and not distinguishing keepalive traffic, the SA might never expire. I'm not
sure why iked isn't using the standard NAT-T keepalive message format and
protocol like isakmpd does. AFAICT it's still defined by IKEv2. Maybe iked
is using a hybrid keepalive/dead peer detetion solution, but the author
forgot to account for different effective behavior in some scenarios; or
maybe it was just more expedient than implementing NAT-T keepalive messages.
Figuring that out will probably help me answer what a proper solution looks
like.

3) I'm fairly certain the keepalive interval should be configurable. The
default UDP NAT state expiration on OpenBSD, for example, is 30 seconds. The
compile-time constant interval for keepalives in isakmpd and iked is also 30
seconds--the recommended period in the RFCs. Over time it's inevitable for a
peer's keepalive packet to miss the window for preserving NAT state,
especially considering that the peer's and router's timers are going to be
synchronized. That would explain why even with isakmpd I still need a
cronjob to ping the passive host. And it explains why isakmpd is more stable
than my hacked iked--isakmpd sends keepalives independently from both sides,
so you have two shots at making the NAT expiration window. iked's keepalive
is a round-trip message; also two packets, but the timing of the first
packet is all that matters.

Of course, the NAT state could expire before an IKE keepalive for many
reasons, but a keepalive interval at least a few seconds less than the
router's NAT expiration should keep the connection stable for longer periods
of time. And rather than having a cron job run every minute, the SA child
lifetime could be set to something smallish. If and when NAT state does
expire, the active peer behind NAT will rekey the SA within a tolerable
period, reestablishing NAT state and restoring reverse traffic. Lowering
both keepalive and child SA lifetime should make these types of tunnels much
more stable and reliable, without recourse to external hacks.



OpenIKED Keepalive Broken

2016-08-05 Thread William Ahern
The logic of ikev2_ike_sa_alive presumes too much, and breaks NAT traversal
when the _actual_ initiator of real traffic (as opposed to merely being the
SA initiator) is behind NAT.

Background: I currently have an IPSec tunnel configured to provide access to
a corporate network from a remote office. The SA initiator peer on the
corporate nework uses DHCP for its address and is behind a NAT gateway. The
passive peer at the remote office has a static routable address. Currently
I'm using isakmpd and everything has been working relatively fine.

I've been wanting to move to IKEv2 (and iked) for various reasons. But
whenever I tried using iked, the tunnel kept becoming unresponsive almost
immediately. I've spent a couple of days, approximately 200 miles of travel,
and a good deal of my reserve of patience trying to figure out what _I_ was
doing wrong or what idiotic firewall rules IT had setup with their fancy new
deep-packet filtering equipment. (Avoiding IT and DevOps and their
fascination with complex firewall and routing rules being a major reason for
managing my own tunnel.) But now I realize the problem was with iked, and
perhaps another classic case of premature optimization/specialization.

isakmpd unconditionally sends NAT-T keepalive messages every 30 seconds,
whereas iked's ikev2_ike_sa_alive only sends a keepalive message iff
`!foundin && foundout`. But that presumes that the SA initiator is also the
initiator of traffic, which definitely isn't the case in my situation, and
seems dubious and unreliable even for real road warriors.

Everything worked again with this simple patch:

  diff -u -p -r1.128 ikev2.c
  --- ikev2.c 22 Oct 2015 15:55:18 -  1.128
  +++ ikev2.c 6 Aug 2016 00:02:55 -
  @@ -3270,7 +3270,7 @@ ikev2_ike_sa_alive(struct iked *env, voi
  }
   
  /* send probe if any outging SA has been used, but no incoming SA */
  -   if (!foundin && foundout) {
  +   if (1 || (!foundin && foundout)) {

I'd be happy to create a proper patch if someone could explain the purpose
of the conditional logic. I wouldn't want to accidentally break something.

I also wouldn't mind making the keepalive interval configurable--rather than
a compile-time constant--so users could deal with NAT gateways which
aggressively flush state.



Re: Request for Funding our Electricity

2014-01-17 Thread William Ahern
On Fri, Jan 17, 2014 at 11:32:41PM +, Miod Vallat wrote:
 And it's not full emulator if it doesn't emulate the
  bugs.
 
 It's almost bedtime in Europe. Do you mind if I tell you a bedtime
 story?
 
 Years ago, a (back then) successful company selling high-end Unix-based
 workstations, having been designing its own systems and core components
 for years, started designing a new generation of workstations.
snip
 Assuming someone would write an emulator for that particular system:
 - if the ``unreliable read'' behaviour is not emulated, according to
   your logic, it's a bug in the emulator, which has to be fixed.
 - if the behaviour is emulated, how can we know it is correctly
   emulated, since even the designers of the chip did not spend enough
   time tracking down the exact conditions leading to the misbehaviour
   (and which bogus value would be put on the data bus).
 
 You may argue that, since the kernel has a workaround for this issue,
 this is a moot point. But if some developer has a better idea for the
 kernel heuristic, how can the new code be tested, if not on the real
 hardware?
 

The problem with this story is that the purported reasons for supporting old
architectures is to shake out bugs. How do the bugs get shaken out? By
exercising shared, core functionality in distinctive ways.

Idiosyncracies such as the above are not the type of thing that helps shake
out core bugs.

So there are two ways to resolve this discrepency: either it simply makes
more sense to shift to emulated environments for older hardware; or one of
the primary reasons also includes actually running on creaky, old
hardware--the coolness factor.

I suspect the coolness factor looms large. And there's nothing wrong with
that. OTOH, there's a strong case to be made for simply inventing crazy
architectures out of whole cloth and writing an emulator for them.



Re: Request for Funding our Electricity

2014-01-17 Thread William Ahern
On Fri, Jan 17, 2014 at 07:33:01PM -0700, Theo de Raadt wrote:
   You may argue that, since the kernel has a workaround for this issue,
   this is a moot point. But if some developer has a better idea for the
   kernel heuristic, how can the new code be tested, if not on the real
   hardware?
   
  
  The problem with this story is that the purported reasons for supporting old
  architectures is to shake out bugs. How do the bugs get shaken out? By
  exercising shared, core functionality in distinctive ways.
  
  Idiosyncracies such as the above are not the type of thing that helps shake
  out core bugs.
 
 You've missed the point.
 
 These idiosyncracies must be stepped over, so that we can have working
 platforms different from x86, to then go discover the core bugs!
 
 Luckily we have people in our group who support such other
 architectures in our tree, to give us this capability.
 
 Let's face it.  OpenBSD has this as a bug reducing mechanism
 available, and most other systems do not anymore, having decided to
 chase only the market-chosen architectures.  It is a true many-eyes
 machined solution.
 
 What other community has users who commonly run upstream software on
 64-bit big-endian strict alignment platform with register windows
 adjusting the frames in odd ways, or 32-bit big-endian ones with mutex
 alignment requirements, or a pile of other requirements.
 
 Quite frankly, I am not alone in being sick of people who don't use
 emulators, stepping in to tell we should use emulators.

I do use emulators, specifically for ARM, because it's just easier for me.
And one of my co-workers is a contributor to the Hercules emulator.
 
 Finally, we have people who want to work on those architectures.  You
 prefer they quit?

No, I don't prefer they quit. I donate to OpenBSD because you guys do the
hard work. And the golden rule of open source is that he who does the work
gets to make the decisions about how he's going to go about doing that work.

So, please don't misunderstand me. I'm not questioning why you guys use so
much power with old hardware. I'm not writing the code, so it's not my place
to question. And while emulators might, arguably, be more efficient in some
abstract sense, what matters is how the work is being done today. And if you
say using real hardware is easier for your workflow, so be it.

And, FWIW, I love the idea of a CD subscription service. I often end up
forgetting to buy a CD. I upgrade most of my systems remotely (with a 13
year track record of never losing a machine--thanks!), so I never have to
actually use the CD.



Re: Request for Funding our Electricity

2014-01-17 Thread William Ahern
On Fri, Jan 17, 2014 at 08:38:05PM -0700, Theo de Raadt wrote:
  I do use emulators, specifically for ARM, because it's just easier for me.
  And one of my co-workers is a contributor to the Hercules emulator.
 
 Then you know it is not sufficient for our needs, yet we keep getting
 the same message from some people.  The emulators are too slow, or they
 need to be run on super fast xeons and suddenly draw even more power.
 The suggestion is totally out of touch.

I don't know that personally. I do believe that the particular anecdote I
replied to is an insufficient premise to support the avowed need mentioned
in your ruBSD talk, namely the ability to stress core services like memory
management in diverse ways.

But I'm content taking your word for it. And I'm not trying to argue with
you. Obviously the issue is far more complex than an interview and anecdote
let on.

   Finally, we have people who want to work on those architectures.  You
   prefer they quit?
  
  No, I don't prefer they quit.
 
 But you've instructed us to power the machines off and move to emulators.

I never argued any such thing.

  So, please don't misunderstand me. I'm not questioning why you guys use so
  much power with old hardware.
 
 It is not a lot of power; that is a myth.

It is a lot of power considering that my modern, 4-core Haswell Xeon 1U
servers draw less than 50W at maximum load. I used to run OpenBSD on Sparc
and Alpha, and they drew more power than that at idle.

But that's beside the point, because I'm not attacking OpenBSD's
infrastructure setup.

snip 
  I'm not writing the code, so it's not my place to question.
 
 You said it yourself, it is not your place to question.  Yet, you that
 is precisely what you are doing.

I disagree. I merely made a point about an anecdote. I apologize if my quip
about coolness factor struck a nerve.

  And, FWIW, I love the idea of a CD subscription service. I often end up
  forgetting to buy a CD. I upgrade most of my systems remotely (with a 13
  year track record of never losing a machine--thanks!), so I never have to
  actually use the CD.
 
 Why do you need a subscription?  You can go order the ones you are
 missing (right now), and even save postage since a whole bunch fill
 arrive at once.  There is no need to setup the additional overhead of
 managing subscriptions, for people like you.

 Wow, so many crazy suggstions.

I never suggested a CD service. Somebody else suggested it and I
thought--apparently erroneously--that it received a favorable comment from
someone on the OpenBSD team.

In any event I just discovered the monthly donation subscription on the
Foundation website and have signed up for a $20 monthly donation. So the CD
subscription is less of a useful idea than it initially appeared.



Re: mandoc strlcat

2013-05-23 Thread William Ahern
On Thu, May 23, 2013 at 05:05:45PM -0400, Ted Unangst wrote:
 I was looking at mandoc and noticed it has too many strlcats (a common
 affliction affecting quite a few programs.) It's faster and simpler to
 use snprintf.

In glibc snprintf has a memory allocation failure mode. I'm curious: is
OpenBSD committed to avoiding extensions (locale features, etc) which might
trigger allocation failure?



Re: upstream vendors and why they can be really harmful

2012-11-24 Thread William Ahern
On Thu, Nov 22, 2012 at 01:27:46PM -0430, Andres Perera wrote:
 On Thu, Nov 22, 2012 at 11:58 AM, Kevin Chadwick ma1l1i...@yahoo.co.uk 
 wrote:
  On Thu, 22 Nov 2012 09:30:41 -0430
  Andres Perera wrote:
 
  i'm not sure how using js for configuration files, as opposed to using
  a language commonly deployed for the same purpose, such as lua,
  presents an innate constraint on security.
 
  Firstly the article mentioned JIT preventing true randomisation.
 
  Secondly pulling in JS as a dependency even on servers is rediculous and
  is a language very familiar to attackers and unfamiliar to many users.
  It would be especially, shall we say kind to attackers utilising rop
  attacks.
 
 but jit isn't irreparably interleaved with js
 
 am i compromising by running luajit in interpreter mode instead of the
 reference implementation

Almost certainly, yes. And that's not a slight against Mike Pall's skills.
Complexity is the enemy of security, every time. Merely disabling a feature
doesn't remove its footprint from the code base.

And have you read any of those codebases? The reference Lua implementation
is a model of clear programming, IMO.

, moreover, would that imply that lua the language is insecure or is the
 specific implementation at fault?

Lua-the-language is designed with implementation details in mind. When an
implementation details become intolerably complex, they consider removing
the feature (e.g. the new generational collector in 5.2).

 
 why would the runtime be attractive for rop? what configuration vm
 needs syscalls that would be attractive to an attacker that can change
 the address of a jump? does the runtime really need to open sockets,
 or spawn processes? (i'm not even talking about languages)

Those syscalls are accessible in the run time environment, whether or not
they're intentionally bound. And that's all that matters, at the end of the
day. If intentions drove run time safety, there would never be attacks
against real-world code.



Re: upstream vendors and why they can be really harmful

2012-11-07 Thread William Ahern
On Tue, Nov 06, 2012 at 06:24:58PM -0200, Daniel Bolgheroni wrote:
 On Tue, Nov 06, 2012 at 01:38:32PM +0100, Marc Espie wrote:
 
  It's also quickly turning Posix and Unix into a travesty: either you have
  the linux goodies, or you don't. And if you don't, you can forget anything
  modern...
 
 This IS the main problem.

The BSDs definitely need to find a way to join the inner circle at the Open
Group. Or maybe there are already BSD developers there. It's hard to know as
an outsider because the process is so opaque.



Re: Goodbye to you my file descriptor

2012-10-30 Thread William Ahern
On Tue, Oct 30, 2012 at 11:57:05AM -0400, Okan Demirmen wrote:
 On Tue, Oct 30, 2012 at 11:53 AM, Christiano F. Haesbaert
 haesba...@haesbaert.org wrote:
  On 30 October 2012 16:52, Christiano F. Haesbaert
  haesba...@haesbaert.org wrote:
  On 30 October 2012 16:45, Okan Demirmen o...@demirmen.com wrote:
  On Tue, Oct 30, 2012 at 10:32 AM, Christiano F. Haesbaert
  haesba...@haesbaert.org wrote:
  On 30 October 2012 15:03, Christiano F. Haesbaert
  haesba...@haesbaert.org wrote:
snip
  That should be an access(2) call.
 
 
  or stat(2) due to tctu.
 
  I believe in that case it would be the same, since there is still a
  window between stat(2)/access(2) and open(2).
 
  I mean, considering he would open/stat/close and open again.
 
 I didn't actually look at the code; I just noticed the words
 permission and access(2) and hit reply :)
 

Perhaps you meant fstat? Looking at the code, it doesn't look like there's
any way to fix the TOCTOU issue without resorting to a complete overhaul,
and instead using the openat() family of calls. OTOH, it looks like the
permission check is just for sanity--early failure.



Re: SIIG 4S PCIe 4-port Serial Card

2012-10-30 Thread William Ahern
On Fri, Oct 19, 2012 at 10:54:33PM +0100, Stuart Henderson wrote:
snip
 My original mail about the chip (including diff) is at
 http://marc.info/?l=openbsd-techm=126446213208560w=2, there is
 some problem I was unable to track down which prevents things from
 working when the port is set to the correct frequency, but setting
 to half the frequency gets a usable port, you just need to half
 the desired baud rate of the serial port (e.g. cu -s 4800 -l cua03).
 
 Diff there won't apply directly as things have changed since then.
 
 The pcidevs change might apply with patch but is simple enough to
 hand-apply; after doing this, run make in sys/dev/pci to update
 the pcidevs.h/pcidevs_data.h files.
 
 The pucdata.c table has changed format since then to save memory,
 something like the below should be about right, you can't get the
 speed bang-on with the new table format (rather than recording
 actual speed it uses shorthand to allow multipliers or powers-of-two
 of the standard COM port UART frequency 1.8432MHz; the exact needed
 speed can't be stored in this notation but using a multiplier of 17
 should be close enough for other ports to lock onto it - if not
 then try 18). This version is untested though as I am lacking
 machines with PCIe slots that I can use for testing.

The patch worked as advertised and unedited. Many thanks.

I have a few days before this thing goes into the colo, so I'm free to
experiment a little. If you have any ideas you want to test out, feel free
to send me a patch.



SIIG 4S PCIe 4-port Serial Card

2012-10-19 Thread William Ahern
The puc(4) man page lists the SIIG Cyber 4S PCI as supported. I just
inserted a SIIG Cyber 4S PCIe. I figured it would look the same as the PCI
card, considering that the new chip is named OXPCIe954, similar to the old
OXPCI954.

But obviously that was hopelessly naive. Instead I get:

vendor Oxford, unknown product 0xc208 (class communications
subclass serial, rev 0x00) at pci13 dev 0 function 0 not configured

It looks like the vendor id is PCI_VENDOR_OXFORD2, and not PCI_VENDOR_SIIG.
Any pointers on what data structure and defines I should try adding? It
looks like the actual PUC driver is simple enough, I just need the card to
be recognized.

I'm running stock OpenBSD 5.1/amd64 on a Soekris net6501.