Index | Thread | Search

From:
Emiel Kollof <emiel@kollof.nl>
Subject:
Re: another yubikey diff
To:
tech@openbsd.org
Date:
Sat, 23 Aug 2025 01:06:48 +0200

Download raw body.

Thread
  • Emiel Kollof:

    another yubikey diff

  • Brandon Mercer schreef op 2025-08-22 16:56:
    > On Fri, Aug 22, 2025, at 10:42 AM, Theo de Raadt wrote:
    >> Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
    >> 
    >> > > From: "Theo de Raadt" <deraadt@openbsd.org>
    >> > > Date: Fri, 22 Aug 2025 08:31:19 -0600
    >> > >
    >> > > Why not invert this with a "donotconnect" variable, then your diff
    >> > > shrinks significantly.
    >> >
    >> > Not really; the struct wkbddev_attach_args is typically allocated on
    >> > the stack, without an explicit memset, so the new member must be set.
    >> 
    >> So change all those stack allocations to = { 0 }
    >> 
    >> And change one driver to set .noconnect = 1;
    >> 
    >> 
    >> Making the default noconnect is going to explode someone's head later
    >> on when they write a new kbd driver.
    > 
    > My reply has nothing to do with the diff and more to do with a 
    > particular use case. My typical usage is to use my OTP to sign into my 
    > machine upon boot. If I have to fiddle with wsconsctl in order to use 
    > the yubikey OTP, then my initial sign on requires me to sign in first. 
    > This makes me lean towards fixing the yubikey tools so it's easier to 
    > reprogram the default behavior of slot one not to spam OTP's on each 
    > press. I do agree that their tooling is arduous at very best and this 
    > default behavior is prohibitive.
    
    So my idea and patch to add a sysctl (which you can add in 
    /etc/sysctl.conf)
    isn't that crazy. Instead of fiddling with wsconsctl, you set a sysctl 
    and
    reinsert your device and it magically works again, without having to log 
    in.
    
    Adding a sysctl might be a kill-a-mosquito-with-a-nuke solution, but it 
    is
    effective. And yes, better tools for yubikeys would be preferable, but 
    it
    is what it is.
    
    I attached my patch again in case people missed it.
    
    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 12:38:51 -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 12:38:51 -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 12:38:51 -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.
    +	 * as a keyboard by default. Set sysctl to enable (grep for it)
     	 */
    -	if (uha->uaa->vendor == USB_VENDOR_YUBICO)
    +	if (!yubico_sucks && uha->uaa->vendor == USB_VENDOR_YUBICO)
     		return (UMATCH_NONE);
     
     	if (UHIDEV_CLAIM_MULTIPLE_REPORTID(uha))
    Index: lib/libc/sys/sysctl.2
    ===================================================================
    RCS file: /cvs/src/lib/libc/sys/sysctl.2,v
    diff -u -p -r1.66 sysctl.2
    --- lib/libc/sys/sysctl.2	6 Aug 2025 16:50:53 -0000	1.66
    +++ lib/libc/sys/sysctl.2	20 Aug 2025 12:38:51 -0000
    @@ -49,7 +49,7 @@ consists of integers, strings, and table
     Information may be retrieved and set using the
     .Xr sysctl 8
     utility;
    -the variable names used by this utility are given here in parentheses.
    +The variable names used by this utility are given here in parentheses.
     .Pp
     Unless explicitly noted below,
     .Fn sysctl
    @@ -537,6 +537,7 @@ information.
     .It Dv KERN_WATCHDOG Ta "node" Ta "not applicable"
     .It Dv KERN_WITNESS Ta "node" Ta "not applicable"
     .It Dv KERN_WXABORT Ta "integer" Ta "yes"
    +.It Dv KERN_YUBICO_SUCKS Ta "integer" Ta "yes"
     .El
     .Bl -tag -width "123456"
     .It Dv KERN_ALLOWDT Pq Va kern.allowdt
    @@ -1251,7 +1252,10 @@ The same as 2, but also drop into the ke
     Generate an abort,
     rather than returning an error,
     on W^X violation.
    -.El
    +.It Dv KERN_YUBICO_SUCKS Pq Va kern.yubico_sucks
    +Setting this to 1 enables Yubico keyboard OTP.
    +Reinsert your key after enabling.
    +Disabled by default until Yubico fixes their tooling and documentation.
     .Ss CTL_MACHDEP
     The set of variables defined is architecture dependent.
     Most architectures define at least the following variables.
    
  • Emiel Kollof:

    another yubikey diff