Index | Thread | Search

From:
Stuart Henderson <stu@spacehopper.org>
Subject:
Re: patch: stop login_yubikey(8) leaking OTP data to syslog
To:
Emiel Kollof <emiel@kollof.nl>
Cc:
tech@openbsd.org
Date:
Wed, 20 Aug 2025 11:05:09 +0100

Download raw body.

Thread
this is a much wider problem than one specific device, there are other
usb keys that have both fido and "fake keyboard" otp buttons on the same
device like yubikeys, also we have problems with some UPS that require
users to build modified kernels,

let me show you how a different os handles this:

https://man.freebsd.org/cgi/man.cgi?usbconfig

specifically add_quirk here, I'm not saying we should necessarily have
the same, but this covers the situation where some users want one type
of behaviour from some device, and others want different behaviour.

though none of this helps with the actual problem that AIUI is really
what prompted the "disable attaching kbd" commit: the difficulty of
using the vendor's original management tools (to disable otp, or swap
it to the "long press" slot) - for that, implementing hidraw(4) might
be the best option as it would allow using the current vendor config
tool (rather than the old one yubikey-personalisation-gui which uses
libusb and is very awkward to get working on OpenBSD) - though there is
still a question of which uids get access to that (it feels somewhat
similar to the cases of microphones or cameras)



On 2025/08/20 09:42, Emiel Kollof wrote:
> Emiel Kollof schreef op 2025-08-19 11:24:
> 
> > I also petition to revert this, or to make this a sysctl
> > knob that defaults
> > to disabled so at least people that do want it can at least
> > turn it back
> > on and have to do so knowingly.
> 
> Attached is a patch that does exactly this.
> 
> Cheers,
> Emiel

> Index: sys/sys/sysctl.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/sysctl.h,v
> diff -u -p -r1.246 sysctl.h
> --- sys/sys/sysctl.h	31 Jul 2025 09:05:11 -0000	1.246
> +++ sys/sys/sysctl.h	20 Aug 2025 07:41:08 -0000
> @@ -193,7 +193,8 @@ struct ctlname {
>  #define	KERN_VIDEO		89	/* struct: video properties */
>  #define	KERN_CLOCKINTR		90	/* node: clockintr */
>  #define	KERN_AUTOCONF_SERIAL	91	/* int: kernel device tree state serial */
> -#define	KERN_MAXID		92	/* number of valid kern ids */
> +#define	KERN_YUBICO_SUCKS	92	/* int: Yubico is a horrible company */
> +#define	KERN_MAXID		93	/* number of valid kern ids */
>  
>  #define	CTL_KERN_NAMES { \
>  	{ 0, 0 }, \
> @@ -288,6 +289,7 @@ struct ctlname {
>  	{ "video", CTLTYPE_STRUCT }, \
>   	{ "clockintr", CTLTYPE_NODE }, \
>  	{ "autoconf_serial", CTLTYPE_INT }, \
> +	{ "yubico_sucks", CTLTYPE_INT }, \
>  }
>  
>  /*
> Index: sys/kern/kern_sysctl.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> diff -u -p -r1.482 kern_sysctl.c
> --- sys/kern/kern_sysctl.c	6 Aug 2025 14:00:33 -0000	1.482
> +++ sys/kern/kern_sysctl.c	20 Aug 2025 07:41:08 -0000
> @@ -344,6 +344,7 @@ extern int nosuidcoredump;
>  extern int maxlocksperuid;
>  extern int uvm_wxabort;
>  extern int global_ptrace;
> +extern int yubico_sucks;
>  
>  const struct sysctl_bounded_args kern_vars[] = {
>  	{KERN_OSREV, &openbsd, SYSCTL_INT_READONLY},
> @@ -397,6 +398,7 @@ const struct sysctl_bounded_args kern_va
>  	{KERN_GLOBAL_PTRACE, &global_ptrace, 0, 1},
>  #endif
>  	{KERN_AUTOCONF_SERIAL, &autoconf_serial, SYSCTL_INT_READONLY},
> +	{KERN_YUBICO_SUCKS, &yubico_sucks, 0, 1},
>  };
>  
>  int
> @@ -709,6 +711,7 @@ kern_sysctl(int *name, u_int namelen, vo
>  	case KERN_NETLIVELOCKS:
>  	case KERN_GLOBAL_PTRACE:
>  	case KERN_AUTOCONF_SERIAL:
> +	case KERN_YUBICO_SUCKS:
>  #endif /* SMALL_KERNEL */
>  	case KERN_OSREV:
>  	case KERN_MAXPARTITIONS:
> Index: sys/dev/usb/ukbd.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/ukbd.c,v
> diff -u -p -r1.91 ukbd.c
> --- sys/dev/usb/ukbd.c	14 Aug 2025 14:39:44 -0000	1.91
> +++ sys/dev/usb/ukbd.c	20 Aug 2025 07:41:08 -0000
> @@ -135,6 +135,14 @@ struct ukbd_softc {
>  #endif
>  };
>  
> +/* Disable yubico usb keys by default. Because of
> + *
> + * ccccccccccbkkrjuubeufctukdrlcejhlkidctfenjfr
> + *
> + * Enable with sysctl kern.yubico_sucks=1 and reinsert key
> + */
> +int     yubico_sucks = 0;
> +
>  void	ukbd_cngetc(void *, u_int *, int *);
>  void	ukbd_cnpollc(void *, int);
>  void	ukbd_cnbell(void *, u_int, u_int, u_int);
> @@ -196,12 +204,12 @@ ukbd_match(struct device *parent, void *
>  	int size;
>  	void *desc;
>  
> -	/*
> -	 * Most Yubikey have OTP enabled by default, and the feature
> -	 * is difficult to disable.  Policy decision: Don't attach
> -	 * as a keyboard.
> -	 */
> -	if (uha->uaa->vendor == USB_VENDOR_YUBICO)
> +        /*
> +         * Most Yubikey have OTP enabled by default, and the feature
> +         * is difficult to disable.  Policy decision: Don't attach
> +         * as a keyboard by default. Set sysctl to enable (grep for it)
> +         */
> +	if (!yubico_sucks && uha->uaa->vendor == USB_VENDOR_YUBICO)
>  		return (UMATCH_NONE);
>  
>  	if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))