fido_init() not being called

[Damien Miller]

On Sat, 2 May 2020, Brian Candler wrote:

> Hello, just raising a minor issue in sk-usbhid.c
> 
> At the moment, fido_init() is not being called except when SK_DEBUG is 
> enabled (which normally isn't):
> 
> #ifdef SK_DEBUG
>          fido_init(FIDO_DEBUG);
> #endif
> 
> That exists in two places.  I mentioned this in passing on the libfido2 
> tracker and the response is:
> 
> https://github.com/Yubico/libfido2/issues/166#issuecomment-622991151
> 
> "Regarding OpenSSH and fido_init(), OpenSSH should be calling 
> fido_init(). There are currently no ill effects from not calling it, but 
> that might change in the future."
> 
> So I'd suggest changing to something like:
> 
> #ifdef SK_DEBUG
>      fido_init(FIDO_DEBUG);
> #else
>      fido_init(0);
> #endif
> 
> - or define a new macro with value 0 or FIDO_DEBUG as appropriate.

Good idea. We're also missing fido_init() for the download-resident-keys
case.


diff --git a/sk-usbhid.c b/sk-usbhid.c
index 0060877..548f9cc 100644
--- a/sk-usbhid.c
+++ b/sk-usbhid.c
@@ -51,6 +51,12 @@
 
 /* #define SK_DEBUG 1 */
 
+#ifdef SK_DEBUG
+#define SSH_FIDO_INIT_ARG	FIDO_DEBUG
+#else
+#define SSH_FIDO_INIT_ARG	0
+#endif
+
 #define MAX_FIDO_DEVICES	256
 
 /* Compatibility with OpenSSH 1.0.x */
@@ -453,9 +459,8 @@ sk_enroll(uint32_t alg, const uint8_t *challenge, size_t challenge_len,
 	int r;
 	char *device = NULL;
 
-#ifdef SK_DEBUG
-	fido_init(FIDO_DEBUG);
-#endif
+	fido_init(SSH_FIDO_INIT_ARG);
+
 	if (enroll_response == NULL) {
 		skdebug(__func__, "enroll_response == NULL");
 		goto out;
@@ -743,9 +748,7 @@ sk_sign(uint32_t alg, const uint8_t *data, size_t datalen,
 	int ret = SSH_SK_ERR_GENERAL;
 	int r;
 
-#ifdef SK_DEBUG
-	fido_init(FIDO_DEBUG);
-#endif
+	fido_init(SSH_FIDO_INIT_ARG);
 
 	if (sign_response == NULL) {
 		skdebug(__func__, "sign_response == NULL");
@@ -989,6 +992,8 @@ sk_load_resident_keys(const char *pin, struct sk_option **options,
 	*rksp = NULL;
 	*nrksp = 0;
 
+	fido_init(SSH_FIDO_INIT_ARG);
+
 	if (check_sign_load_resident_options(options, &device) != 0)
 		goto out; /* error already logged */
 	if (device != NULL) {