unix: use libc stubs for OpenBSD pledge+unveil

For both compatibility and security concerns, it is preferrable to
perform system calls through the libc stubs rather than syscall(2).
Besides making programs resilient to changing system call numbers, it
removes an unnecessary use of the syscall(2) call which, if found by
an attacker, could be abused to perform any system call.

As OpenBSD 6.2 is the oldest supported version of OpenBSD, remove the
handling of pledge on versions prior to this release.

PledgeExecpromises is no longer usable on 6.2, as the execpromises is
treated as required (unlike Pledge, where on 6.2 it is allowed but
must be empty).  The empty execpromises string is no longer converted
to a nil pointer on 6.2.  This fixes an out-of-bounds read where, on
6.2, an empty string would be passed to the deprecated pledge(2) API,
which would interpret the pointer as an array of strings.

Author: jrick

unix/pledge_openbsd.go

@@ -8,8 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"syscall"
-	"unsafe"
 )
 
 // Pledge implements the pledge syscall.
@@ -20,42 +18,46 @@ import (
 // execpromises must be empty when Pledge is called on OpenBSD
 // releases predating 6.3, otherwise an error will be returned.
 //
+// On OpenBSD 6.3 and later, an empty execpromises removes all
+// execpromises from the process.  Use PledgePromises to only set
+// promises without setting execpromises.
+//
 // For more information see pledge(2).
 func Pledge(promises, execpromises string) error {
 	maj, min, err := majmin()
 	if err != nil {
 		return err
 	}
 
-	err = pledgeAvailable(maj, min, execpromises)
+	// OS support for execpromises is required only when execpromises is not
+	// the empty string.
+	err = supportsExecpromises(maj, min, execpromises != "")
 	if err != nil {
 		return err
 	}
 
-	pptr, err := syscall.BytePtrFromString(promises)
+	_promises, err := BytePtrFromString(promises)
 	if err != nil {
 		return err
 	}
 
-	// This variable will hold either a nil unsafe.Pointer or
-	// an unsafe.Pointer to a string (execpromises).
-	var expr unsafe.Pointer
+	// This variable will hold either a nil pointer or a pointer to the
+	// NUL-terminated execpromises string.
+	var _execpromises *byte
 
-	// If we're running on OpenBSD > 6.2, pass execpromises to the syscall.
-	if maj > 6 || (maj == 6 && min > 2) {
-		exptr, err := syscall.BytePtrFromString(execpromises)
+	// If we're running on OpenBSD >= 6.3, pass execpromises to the syscall.
+	// While an empty execpromises string is required by this API on
+	// OpenBSD <= 6.2 and has no effect, the empty execpromises string
+	// removes all execpromises on OpenBSD >= 6.3.
+	if maj > 6 || (maj == 6 && min >= 3) {
+		exptr, err := BytePtrFromString(execpromises)
 		if err != nil {
 			return err
 		}
-		expr = unsafe.Pointer(exptr)
-	}
-
-	_, _, e := syscall.Syscall(SYS_PLEDGE, uintptr(unsafe.Pointer(pptr)), uintptr(expr), 0)
-	if e != 0 {
-		return e
+		_execpromises = exptr
 	}
 
-	return nil
+	return pledge(_promises, _execpromises)
 }
 
 // PledgePromises implements the pledge syscall.
@@ -64,62 +66,39 @@ func Pledge(promises, execpromises string) error {
 //
 // For more information see pledge(2).
 func PledgePromises(promises string) error {
-	maj, min, err := majmin()
-	if err != nil {
-		return err
-	}
-
-	err = pledgeAvailable(maj, min, "")
-	if err != nil {
-		return err
-	}
-
-	// This variable holds the execpromises and is always nil.
-	var expr unsafe.Pointer
-
-	pptr, err := syscall.BytePtrFromString(promises)
+	_promises, err := BytePtrFromString(promises)
 	if err != nil {
 		return err
 	}
 
-	_, _, e := syscall.Syscall(SYS_PLEDGE, uintptr(unsafe.Pointer(pptr)), uintptr(expr), 0)
-	if e != 0 {
-		return e
-	}
-
-	return nil
+	return pledge(_promises, nil)
 }
 
 // PledgeExecpromises implements the pledge syscall.
 //
 // This changes the execpromises and leaves the promises untouched.
 //
+// The pledge syscall does not accept execpromises on OpenBSD releases
+// before 6.3.
+//
 // For more information see pledge(2).
 func PledgeExecpromises(execpromises string) error {
 	maj, min, err := majmin()
 	if err != nil {
 		return err
 	}
 
-	err = pledgeAvailable(maj, min, execpromises)
+	err = supportsExecpromises(maj, min, true)
 	if err != nil {
 		return err
 	}
 
-	// This variable holds the promises and is always nil.
-	var pptr unsafe.Pointer
-
-	exptr, err := syscall.BytePtrFromString(execpromises)
+	_execpromises, err := BytePtrFromString(execpromises)
 	if err != nil {
 		return err
 	}
 
-	_, _, e := syscall.Syscall(SYS_PLEDGE, uintptr(pptr), uintptr(unsafe.Pointer(exptr)), 0)
-	if e != 0 {
-		return e
-	}
-
-	return nil
+	return pledge(nil, _execpromises)
 }
 
 // majmin returns major and minor version number for an OpenBSD system.
@@ -145,17 +124,12 @@ func majmin() (major int, minor int, err error) {
 	return
 }
 
-// pledgeAvailable checks for availability of the pledge(2) syscall
-// based on the running OpenBSD version.
-func pledgeAvailable(maj, min int, execpromises string) error {
-	// If OpenBSD <= 5.9, pledge is not available.
-	if (maj == 5 && min != 9) || maj < 5 {
-		return fmt.Errorf("pledge syscall is not available on OpenBSD %d.%d", maj, min)
-	}
-
+// supportsExecpromises checks for availability of the execpromises argument to
+// the pledge(2) syscall based on the running OpenBSD version.
+func supportsExecpromises(maj, min int, required bool) error {
 	// If OpenBSD <= 6.2 and execpromises is not empty,
 	// return an error - execpromises is not available before 6.3
-	if (maj < 6 || (maj == 6 && min <= 2)) && execpromises != "" {
+	if (maj < 6 || (maj == 6 && min <= 2)) && required {
 		return fmt.Errorf("cannot use execpromises on OpenBSD %d.%d", maj, min)
 	}
 

unix/syscall_openbsd.go

@@ -314,6 +314,8 @@ func Uname(uname *Utsname) error {
 //sys	readlen(fd int, buf *byte, nbuf int) (n int, err error) = SYS_READ
 //sys	writelen(fd int, buf *byte, nbuf int) (n int, err error) = SYS_WRITE
 //sys	utimensat(dirfd int, path string, times *[2]Timespec, flags int) (err error)
+//sys	pledge(promises *byte, execpromises *byte) (err error)
+//sys	unveil(path *byte, flags *byte) (err error)
 
 /*
  * Unimplemented

unix/unveil_openbsd.go

@@ -4,39 +4,24 @@
 
 package unix
 
-import (
-	"syscall"
-	"unsafe"
-)
-
 // Unveil implements the unveil syscall.
 // For more information see unveil(2).
 // Note that the special case of blocking further
 // unveil calls is handled by UnveilBlock.
 func Unveil(path string, flags string) error {
-	pathPtr, err := syscall.BytePtrFromString(path)
+	_path, err := BytePtrFromString(path)
 	if err != nil {
 		return err
 	}
-	flagsPtr, err := syscall.BytePtrFromString(flags)
+	_flags, err := BytePtrFromString(flags)
 	if err != nil {
 		return err
 	}
-	_, _, e := syscall.Syscall(SYS_UNVEIL, uintptr(unsafe.Pointer(pathPtr)), uintptr(unsafe.Pointer(flagsPtr)), 0)
-	if e != 0 {
-		return e
-	}
-	return nil
+	return unveil(_path, _flags)
 }
 
 // UnveilBlock blocks future unveil calls.
 // For more information see unveil(2).
 func UnveilBlock() error {
-	// Both pointers must be nil.
-	var pathUnsafe, flagsUnsafe unsafe.Pointer
-	_, _, e := syscall.Syscall(SYS_UNVEIL, uintptr(pathUnsafe), uintptr(flagsUnsafe), 0)
-	if e != 0 {
-		return e
-	}
-	return nil
+	return unveil(nil, nil)
 }

unix/zsyscall_openbsd_386.go

@@ -667,3 +667,13 @@ TEXT libc_utimensat_trampoline<>(SB),NOSPLIT,$0-0
 	JMP	libc_utimensat(SB)
 GLOBL	·libc_utimensat_trampoline_addr(SB), RODATA, $4
 DATA	·libc_utimensat_trampoline_addr(SB)/4, $libc_utimensat_trampoline<>(SB)
+
+TEXT libc_pledge_trampoline<>(SB),NOSPLIT,$0-0
+	JMP	libc_pledge(SB)
+GLOBL	·libc_pledge_trampoline_addr(SB), RODATA, $4
+DATA	·libc_pledge_trampoline_addr(SB)/4, $libc_pledge_trampoline<>(SB)
+
+TEXT libc_unveil_trampoline<>(SB),NOSPLIT,$0-0
+	JMP	libc_unveil(SB)
+GLOBL	·libc_unveil_trampoline_addr(SB), RODATA, $4
+DATA	·libc_unveil_trampoline_addr(SB)/4, $libc_unveil_trampoline<>(SB)

unix/zsyscall_openbsd_386.s

@@ -667,3 +667,13 @@ TEXT libc_utimensat_trampoline<>(SB),NOSPLIT,$0-0
 	JMP	libc_utimensat(SB)
 GLOBL	·libc_utimensat_trampoline_addr(SB), RODATA, $8
 DATA	·libc_utimensat_trampoline_addr(SB)/8, $libc_utimensat_trampoline<>(SB)
+
+TEXT libc_pledge_trampoline<>(SB),NOSPLIT,$0-0
+	JMP	libc_pledge(SB)
+GLOBL	·libc_pledge_trampoline_addr(SB), RODATA, $8
+DATA	·libc_pledge_trampoline_addr(SB)/8, $libc_pledge_trampoline<>(SB)
+
+TEXT libc_unveil_trampoline<>(SB),NOSPLIT,$0-0
+	JMP	libc_unveil(SB)
+GLOBL	·libc_unveil_trampoline_addr(SB), RODATA, $8
+DATA	·libc_unveil_trampoline_addr(SB)/8, $libc_unveil_trampoline<>(SB)