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) {