pkg/57919: patching away -Werror in security/libfido2 broke it on NetBSD

[campbell+netbsd%mumble.net@localhost]

>Number:         57919
>Category:       pkg
>Synopsis:       patching away -Werror in security/libfido2 broke it on NetBSD
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 09 22:10:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        pkgsrc-2023Q3, pkgsrc-2023Q4, pkgsrc-HEAD
>Organization:
The NetFIDO Foundation
>Environment:
LP64 BSD
>Description:
security/libfido2/patches/patch-CMakeLists.txt patches away all uses of -Werror, including this one:

+ try_compile(HAVE_POSIX_IOCTL
+     "${CMAKE_CURRENT_BINARY_DIR}/posix_ioctl_check.o"
+     "${CMAKE_CURRENT_SOURCE_DIR}/openbsd-compat/posix_ioctl_check.c"
+-    COMPILE_DEFINITIONS "-Werror -Woverflow -Wsign-conversion")
++    COMPILE_DEFINITIONS "-Woverflow -Wsign-conversion")

Unfortunately, libfido2 relies on this for correctness.  It tries to compile posix_ioctl_check.c with -Werror in order to detect whether ioctl takes int (as in POSIX) or unsigned long (as in BSD) for the command/request:

$ cat openbsd-compat/posix_ioctl_check.c
#include <sys/ioctl.h>

int
posix_ioctl_check(int fd)
{
	return ioctl(fd, -1, 0);
}

On a BSD system where ioctl takes unsigned long instead of int, the compiler might warn about passing -1 here -- and the libfido2 build relies on upgrading such a warning to an error to detect this.  Without that warning, it assumes ioctl takes int.

And on systems where it thinks ioctl takes int, libfido2 _explicitly casts_ all the input arguments to int before passing them.  Which causes sign-extension, on NetBSD, so that, for example, USB_HID_SET_RAW = 0x80046802 gets converted to (unsigned long)(int)USB_HID_SET_RAW = 0xffffffff80046802, which is not an ioctl that uhid(4) recognizes so it fails with EINVAL and libfido2 is completely broken in pkgsrc for interacting with fido2 devices.
>How-To-Repeat:
Plug in a USB FIDO device, so that it appears at uhid0; then run:

$ echo credential challenge | openssl sha256 -binary | base64 > cred_param
$ echo relying party >> cred_param
$ echo user name >> cred_param
$ dd if=/dev/urandom bs=1 count=32 | base64 >> cred_param
$ fido2-cred -M -i cred_param /dev/uhid0 | fido2-cred -V -o cred

Expected result: silent success, and ktrace shows:

kdump:
 13581  13581 fido2-cred CALL  ioctl(4,USB_HID_SET_RAW,0x7f7fff228c0c)
kdump -n:
 13581  13581 fido2-cred CALL  ioctl(4,0x80046802,0x7f7fff228c0c)

Actual result: fails with:
fido2-cred: fido_dev_open /dev/uhid0: FIDO_ERR_INTERNAL
fido2-cred: input error

And ktrace shows:

kdump:
 21174  21174 fido2-cred CALL  ioctl(4,_IOW('h',0x2,0x4),0x7f7fff1d8d1c)
kdump -n:
 21174  21174 fido2-cred CALL  ioctl(4,0xffffffff80046802,0x7f7fff1d8d1c)

(kdump(1) showing _IOW('h',0x2,0x4) for the sign-extended one is wrong; see https://gnats.NetBSD.org/57918 for that problem.
>Fix:
Yes, please!

- Maybe just unpatch out that -Werror.
- Maybe find a better way to do this detection.
- Maybe patch the NetBSD code to not use the IOCTL_REQ macro that conditionally casts to int.