Index | Thread | Search

From:
Emiel Kollof <emiel@kollof.nl>
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 14:42:02 +0200

Download raw body.

Thread
Kirill A. Korinsky schreef op 2025-08-20 11:57:

[snip]

>> Attached is a patch that does exactly this.
>> 
> 
> You have mised a part of sysctl(2), haven't you?

Attached is a revised patch that adds manpage changes. I also fixed
a tab/newline mixup in ukbd.c.

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.