Development practises

OpenBSD got no continuous integration system, and apparently build breakage are, according to the FAQ, happening from time to time:

Do not report source tree compilation problems, unless they persist. They are almost always your mistake, or they are being worked on as you encounter them. People working on the project are doing make build at least once per day, and usually several times per day with different architectures.

For example, recently, -current was panicking, for ~15h, between those two commits.

There is no bug tracker either, only the sendbug tool, to send emails to openbsd-bugs.

There is no mandatory public review either: apparently the best practice™ is to send a diff to openbsd-tech@, and hope that people will reply. There is no way, beside manually searching on the mailing list, to see what commit what reviewed by whom. From time to time, there is a “ok bob” in commits, to indicate that bob validated the changes, but those reviews are usually happening in private. This is completely backwards compared to the modern practises.

Apparently, in 2006, there were 114 people with the commit bit, with around 25 being slackers. There is no publicly available list of people able to directly push into the codebase.

There is a code style, but since it’s not automatically enforced, if only because there is no CI.

The VCS used is CVS, the Concurrent Versions System:

CVS is the popular version control system in the free software community, used by *BSD, many Linux projects, Netscape and others.

The last stable release was done in May 2008, and is under GPL, a license disaliked by OpenBSD. Apparently, there is OpenCVS, to be released soon, but it hasn’t seen much activity since 2017.

There is a long changelog available for each release, but on has to resort to cvs log to find in what commit a specific bug was fixed or a feature added.

Mitigations design

In 2023, in his CanSecWest talk, De Raadt said:

At least 2 attacks have manipulated mmap(2) or mprotect(2) to change a permission, perform a memory operation, and continued to control/escalation

Apparently, one of them was an iPhone exploit, likely the talk from Charlie Miller and Vincenzo Iozzo, at BlackHat 2009, about bypassing code signature, which isn’t a thing on OpenBSD. He also mentioned how iPhones are restricting syscalls to the text segment, which is also not a thing.

He said said, on slide 27, in the context of MAP_STACK and syscall protection:

Increasing exploitation difficulty is a valid strategy

This sounds like a classic case of mitigator instead of properly designed mitigations

During the talk, on the “code can only be read if it has been executed once” implementation for execute-only memory, “I don’t know if this is going to stop an attack in the future, but who knows.”

He also mentioned:

Chrome […] wants to protect the v8 flags page, so that the flags can’t be changed after startup, because that’s a very juicy target, for your JITs to go and modify, because they know where it is. So they mprotect it. Bug guess what, what if you can make your JIT go and make it writeable, you’d be able to change your flags, and carry on."

and

I want to make an observation here, that I don’t here from anybody in the attack surface out here. Quite often, when a program crashes, crashes in this way, and the BROP code starts, you’re executing inside one particular .so, okay? But if the tools that you need to use are not inside that same .so, you have to know what the other .so is. So for example, if you crash inside libcrypto, but libcrypto doesn’t allow you do to system calls, you now have to do BROP, you now have to do ROP code, to determine where your libc is, so you can do system calls from there. I don’t know why no one’s talking about this. I think people on the ROP side should think about this.

This rambling shows an worryingly haphazard comprehension of the exploitation/mitigation terminology and techniques.

When not straight up lying, when talking about the latest openssh pre-auth double-free:

I think the way they [Qualys] explained it is that they have a 33 in a million chances of succeeding at actually, getting control of the PC.

While it’s actually 1 chances in 43690 times, as Qualys said on oss-sec. He also conveniently forgot to say that OpenBSD malloc made exploitation easier, compared to the glibc one.

To the question “What was the inspiration for all this? For the immutable flag, execute-only, … Where do the ideas come from? Was there any inspiration here? “, De Raadt answered “Yeah, … I don’t know.” which is worrying at best.

Code reviews

OpenBSD claims that they have “between six and twelve members who continue to search for and fix new security holes”, but it seems that this doesn’t prevent low-hanging bugs from entering the codebase, for example:

• In August 2016, OpenBSD fixed a couple of integer overflows in its memory mapping code. This should have been detected by using something like UBSAN.
• In 2017, OpenBSD realised that it forgot to initialize variables before using them in ptrace, fcntl, and ELF binaries. This should have been caught with ASAN, trivial static analysis, or systematically initializing variables, like Windows' InitAll or clang’s -ftrivial-auto-var-init.
• In 2017, OpenBSD got the TCB mapping wrong, fixed it wrongly, and fixed it again one month later, when the original reporter highlighted the flaw.
• In August 2019, OpenBSD fixed a denial of service in OpenSMTPD, caused by a textbook misuse of vsnprintf.
• In October 2019, OpenBSD realised that their dhcp server was leaking memory over the wire, again due to unitialized memory.
• The 5^th December 2019, Qualys published multiple proof of concepts for an authentication bypass, based on an option injection issue, meaning that no OpenBSd developers thought about usernames with a leading dash.
• In December 2019, Qualys published a local privilege escalation via OpenBSD’s dynamic linker: environment variables weren’t correctly sanitized in an artificially constrained environment.
• In 2020, Qualys found an other remote code execution in OpenSMTPD present since one year: a shell injection, for which they “drew inspiration from the Morris worm (https://spaf.cerias.purdue.edu/tech-reps/823.pdf)”, the one from 1988. Interestingly, the author of OpenSTMPD wrote a blogpost about the issue, and the possible future mitigations.
• In February 2020, Maxime Villard reported a couple of severe issues in OpenBSD’s hypervisor, granting arbitrary write from guest to the host’s kernel. They weren’t correctly fixed.
• May 2021, Maxime Villard reported an other trivial guest-to-host vulnerability via a hilarious arbitrary r/w.

Security advisories

OpenBSD is publishing security issues on its Errata pages, but doesn’t provide much context nor analysis. For example, this is OpenBSD’s CVE-2019-19519 advisory:

untrusted comment: verify with openbsd-66-base.pub
RWSvK/c+cFe24Icv2ypHJly8sKJfGTilSEiQVVkDejBzxQaH/xYrNbwM/DexJBiNT0QBg1IKTeswHou4vAmT75O/s+KMmlb4swI=

OpenBSD 6.6 errata 012, December 8, 2019:

A user can log in with a different user's login class.

Apply by doing:
    signify -Vep /etc/signify/openbsd-66-base.pub -x 012_suauth.patch.sig \
        -m - | (cd /usr/src && patch -p0)

And then rebuild and install su:
    cd /usr/src/usr.bin/su
    make obj
    make
    make install

Index: usr.bin/su/su.c
===================================================================
RCS file: /var/cvs/src/usr.bin/su/su.c,v
retrieving revision 1.77.2.1
diff -u -p -r1.77.2.1 su.c
--- usr.bin/su/su.c	4 Dec 2019 09:52:22 -0000	1.77.2.1
+++ usr.bin/su/su.c	6 Dec 2019 20:46:25 -0000
@@ -172,6 +172,8 @@ main(int argc, char **argv)
 		err(1, "unveil");
 
 	for (;;) {
+		char *pw_class = class;
+
 		/* get target user, default to root unless in -L mode */
 		if (*argv) {
 			user = *argv;
@@ -207,11 +209,11 @@ main(int argc, char **argv)
 		}
 
 		/* If the user specified a login class, use it */
-		if (!class && pwd && pwd->pw_class && pwd->pw_class[0] != '\0')
-			class = strdup(pwd->pw_class);
-		if ((lc = login_getclass(class)) == NULL)
+		if (pw_class == NULL && pwd != NULL)
+			pw_class = pwd->pw_class;
+		if ((lc = login_getclass(pw_class)) == NULL)
 			auth_errx(as, 1, "no such login class: %s",
-			    class ? class : LOGIN_DEFCLASS);
+			    pw_class ? pw_class : LOGIN_DEFCLASS);
 
 		if ((ruid == 0 && !emlogin) ||
 		    verify_user(username, pwd, style, lc, as) == 0)

It doesn’t mention the CVE, the CVSS, if it’s remote or local, if and how is it possible to mitigate without patching, … also, it doesn’t provide a link to the (amazing) original report.

And this is true for all the security “erratas”: no context whatsoever.

Commit messages

OpenBSD’s commits tends to be short:

• A bit less than 50% of the commit messages (title + body) is less than 10 characters words long
• Around 75% of them are less than 20 characters words.

The previous graph can be obtained via this:

$ git clone https://github.com/openbsd/src
$ git rev-list master |
    while read sha1; do
			git show -s --format='%B' $sha1 | tr -d '\n'; echo
		done | awk '{ print NF}' | sort | uniq -c > ~/out.cvs

and the following python script:

$ cat commits_to_svg.py
import pygal
import csv
import collections

d = collections.defaultdict(list)

with open('out.cvs') as csvfile:
    for line in csv.reader(csvfile, delimiter=';'):
        k, v = int(line[1]), int(line[0])
        d[(k + 5) - k%5].append(v)

pie_chart = pygal.Pie()
pie_chart.title = 'Length of commit message in OpenBSD'
for a,b in sorted(d.items()):
    pie_chart.add(str(a), b)
pie_chart.render()
pie_chart.render_to_file('bar_chart.svg')
$

Interestingly, there are a lot of commits with a single word as message, and this is an accepted practise:

$ git log --log-size --format="%B" | \
awk '/^log size/{
  if (matches == 1) {messages[line]++; line = ""}
  matches = 0;
  if ($3 <= 10) { matches = 1}
}
{
  if (matches == 1 && $0 !~ /^log size/) {line = line tolower($0)}
}
END {
  for (line in messages){ print messages[line]": "line}
}' | \
sort -n | tail
107: tweaks;
115: spelling
117: regen.
135: indent
183: oops
249: spacing
416: knf
441: typo
1902: regen
4915: sync

Commits adding new mitigations sometimes don’t contain much information nor context, like for MAP_STACK:

commit 80e6ddab5b6b834f2d6fa7f2078e697ea58cdbf9
Author: deraadt <deraadt@openbsd.org>
Date:   Sun Feb 11 04:09:19 2018 +0000

    Add MAP_STACK flag.  Currently masked by mmap()

or the Stackclash mitigation:

commit 4ed6bfeac112229466414b94cdbd983fb8017796
Author: kettenis <kettenis@openbsd.org>
Date:   Thu May 18 18:50:32 2017 +0000

    Add a gap of 1MB between the stack and mmap spaces.
    
    ok deraadt@, millert@, stefan@

The [non-syscalls from non-marked maps one]({{ ref “/mitigations/WX_refinement.md”}}) is also interesting:

Repurpose the "syscalls must be on a writeable page" mechanism to
enforce a new policy: system calls must be in pre-registered regions.
We have discussed more strict checks than this, but none satisfy the
cost/benefit based upon our understanding of attack methods, anyways
let's see what the next iteration looks like.

This is intended to harden (translation: attackers must put extra
effort into attacking) against a mixture of W^X failures and JIT bugs
which allow syscall misinterpretation, especially in environments with
polymorphic-instruction/variable-sized instructions.  It fits in a bit
with libc/libcrypto/ld.so random relink on boot and no-restart-at-crash
behaviour, particularily for remote problems. Less effective once on-host
since someone the libraries can be read.

For static-executables the kernel registers the main program's
PIE-mapped exec section valid, as well as the randomly-placed sigtramp
page.  For dynamic executables ELF ld.so's exec segment is also
labelled valid; ld.so then has enough information to register libc's
exec section as valid via call-once msyscall(2)

For dynamic binaries, we continue to to permit the main program exec
segment because "go" (and potentially a few other applications) have
embedded system calls in the main program.  Hopefully at least go gets
fixed soon.

We declare the concept of embedded syscalls a bad idea for numerous
reasons, as we notice the ecosystem has many of
static-syscall-in-base-binary which are dynamically linked against
libraries which in turn use libc, which contains another set of
syscall stubs.  We've been concerned about adding even one additional
syscall entry point... but go's approach tends to double the entry-point
attack surface.

This was started at a nano-hackathon in Bob Beck's basement 2 weeks
ago during a long discussion with mortimer trying to hide from the SSL
scream-conversations, and finished in more comfortable circumstances
next to a wood-stove at Elk Lakes cabin with UVM scream-conversations.

ok guenther kettenis mortimer, lots of feedback from others
conversations about go with jsing tb sthen

While it explains what’s going on, it doesn’t provide much rationale: what other strict checks were considered? What were the cost/benefit ratios? What are the attack methods? Upon what are their understanding based? “Less effective once on-host” as in “a complete bypass”, since the attacker can simply use ROP, so it seems that this doesn’t add much on top of ASLR and random relink. But at least the commit provides some bits of rationale and a couple of arguments.

Anyway, this tendency to have super-small commits messages makes it tricky to understand what are those mitigations doing, against what they are defending, …

Equally worrying, OpenBSD doesn’t usually mention papers about their mitigations, either because they’re not reading it, or to claim that their ideas are brand new. For example, their ROP gadget removal process doesn’t mention Is Less Really More? Towards Better Metrics for Measuring Security Improvements Realized Through Software Debloating; MAP_STACK doesn’t mention ROPGuard; …

Some performance-critical changes don’t provide data/benchmarks, like choosing the right™ number of pools for the memory allocator:

commit e02928f11c682f13cb9c80ef478a1405cff78ee2
Author: otto <otto@openbsd.org>
Date:   Thu Jan 10 18:47:05 2019 +0000

    Move default numer of pools in the multi-threaded case to 8. Various tests
    by me and others indicate that it is the optimum.

diff --git a/lib/libc/stdlib/malloc.c b/lib/libc/stdlib/malloc.c
index 2a1bcfc8b69..e41178d5c56 100644
--- a/lib/libc/stdlib/malloc.c
+++ b/lib/libc/stdlib/malloc.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: malloc.c,v 1.258 2019/01/10 18:45:33 otto Exp $       */
+/*     $OpenBSD: malloc.c,v 1.259 2019/01/10 18:47:05 otto Exp $       */
 /*
  * Copyright (c) 2008, 2010, 2011, 2016 Otto Moerbeek <otto@drijf.net>
  * Copyright (c) 2012 Matthew Dempsky <matthew@openbsd.org>
@@ -409,7 +409,7 @@ omalloc_init(void)
        /*
         * Default options
         */
-       mopts.malloc_mutexes = 4;
+       mopts.malloc_mutexes = 8;
        mopts.malloc_junk = 1;
        mopts.malloc_cache = MALLOC_DEFAULT_CACHE;