cryptenroll: handle FIDO2 tokens gracefully that lack requested features

Let's try to handle keys gracefully that do not implement all features
we ask for: simply turn the feature off, and continue.

This is in particular relevant since we enroll with PIN and UP by
default, and on devices that don't support that we should just work.

Replaces: #18509

diff --git a/src/cryptenroll/cryptenroll-fido2.c b/src/cryptenroll/cryptenroll-fido2.c
index 3ba7866738efcdcbe797d51afe10c1ca5242c5e4..fbf76ee586f43707a24eeabfbd14e06f65d46ef0 100644 (file)
--- a/src/cryptenroll/cryptenroll-fido2.c
+++ b/src/cryptenroll/cryptenroll-fido2.c
@@ -45,7 +45,8 @@ int enroll_fido2(
                         &cid, &cid_size,
                         &salt, &salt_size,
                         &secret, &secret_size,
-                        NULL);
+                        NULL,
+                        &lock_with);
         if (r < 0)
                 return r;
 

diff --git a/src/home/homectl-fido2.c b/src/home/homectl-fido2.c
index a2054fcf73c058e5c0d39e119f582011cde7beaf..e8e9826bb9177ed1c97097380a566222114384b1 100644 (file)
--- a/src/home/homectl-fido2.c
+++ b/src/home/homectl-fido2.c
@@ -162,7 +162,8 @@ int identity_add_fido2_parameters(
                         &cid, &cid_size,
                         &salt, &salt_size,
                         &secret, &secret_size,
-                        &used_pin);
+                        &used_pin,
+                        NULL);
         if (r < 0)
                 return r;
 

diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c
index 24c9b76b7ad611711f824b1b53007f9f13643001..4a901cd38c9008318c7272421702deafbe53eaf1 100644 (file)
--- a/src/shared/libfido2-util.c
+++ b/src/shared/libfido2-util.c
@@ -468,7 +468,8 @@ int fido2_generate_hmac_hash(
                 void **ret_cid, size_t *ret_cid_size,
                 void **ret_salt, size_t *ret_salt_size,
                 void **ret_secret, size_t *ret_secret_size,
-                char **ret_usedpin) {
+                char **ret_usedpin,
+                Fido2EnrollFlags *ret_locked_with) {
 
         _cleanup_(erase_and_freep) void *salt = NULL, *secret_copy = NULL;
         _cleanup_(fido_assert_free_wrapper) fido_assert_t *a = NULL;
@@ -503,6 +504,7 @@ int fido2_generate_hmac_hash(
          */
 
         assert(device);
+        assert((lock_with & ~(FIDO2ENROLL_PIN|FIDO2ENROLL_UP|FIDO2ENROLL_UV)) == 0);
 
         r = dlopen_libfido2();
         if (r < 0)
@@ -529,20 +531,21 @@ int fido2_generate_hmac_hash(
         if (r < 0)
                 return r;
 
-        if (!has_client_pin && FLAGS_SET(lock_with, FIDO2ENROLL_PIN))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Requested to lock with PIN, but FIDO2 device %s does not support it.",
-                                       device);
+        /* While enrolling degrade gracefully if the requested feature set isn't available, but let the user know */
+        if (!has_client_pin && FLAGS_SET(lock_with, FIDO2ENROLL_PIN)) {
+                log_notice("Requested to lock with PIN, but FIDO2 device %s does not support it, disabling.", device);
+                lock_with &= ~FIDO2ENROLL_PIN;
+        }
 
-        if (!has_up && FLAGS_SET(lock_with, FIDO2ENROLL_UP))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Locking with user presence test requested, but FIDO2 device %s does not support it.",
-                                       device);
+        if (!has_up && FLAGS_SET(lock_with, FIDO2ENROLL_UP)) {
+                log_notice("Locking with user presence test requested, but FIDO2 device %s does not support it, disabling.", device);
+                lock_with &= ~FIDO2ENROLL_UP;
+        }
 
-        if (!has_uv && FLAGS_SET(lock_with, FIDO2ENROLL_UV))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Locking with user verification requested, but FIDO2 device %s does not support it.",
-                                       device);
+        if (!has_uv && FLAGS_SET(lock_with, FIDO2ENROLL_UV)) {
+                log_notice("Locking with user verification requested, but FIDO2 device %s does not support it, disabling.", device);
+                lock_with &= ~FIDO2ENROLL_UV;
+        }
 
         c = sym_fido_cred_new();
         if (!c)
@@ -592,6 +595,9 @@ int fido2_generate_hmac_hash(
                                                "Failed to turn off FIDO2 user verification option of credential: %s", sym_fido_strerr(r));
         }
 
+        /* As per specification "up" is assumed to be implicit when making credentials, hence we don't
+         * explicitly enable/disable it here */
+
         log_info("Initializing FIDO2 credential on security token.");
 
         log_notice("%s%s(Hint: This might require verification of user presence on security token.)",
@@ -715,11 +721,75 @@ int fido2_generate_hmac_hash(
                                    emoji_enabled() ? " " : "");
         }
 
-        r = sym_fido_dev_get_assert(d, a, FLAGS_SET(lock_with, FIDO2ENROLL_PIN) ? used_pin : NULL);
-        if (r == FIDO_ERR_UP_REQUIRED && !FLAGS_SET(lock_with, FIDO2ENROLL_UP))
-                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Locking without user presence test requested, but FIDO2 device %s requires it.",
-                                       device);
+        for (;;) {
+                bool retry_with_up = false, retry_with_pin = false;
+
+                r = sym_fido_dev_get_assert(d, a, FLAGS_SET(lock_with, FIDO2ENROLL_PIN) ? used_pin : NULL);
+
+                switch (r) {
+
+                case FIDO_ERR_UP_REQUIRED:
+                        /* If the token asks for "up" when we turn off, then this might be a feature that
+                         * isn't optional. Let's enable it */
+
+                        if (!has_up)
+                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Token asks for user presence check but doesn't advertise 'up' feature.");
+
+                        if (FLAGS_SET(lock_with, FIDO2ENROLL_UP))
+                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Token asks for user presence check but was already enabled.");
+
+                        log_notice("Locking without user presence test requested, but FIDO2 device %s requires it, enabling.", device);
+                        retry_with_up = true;
+                        break;
+
+                case FIDO_ERR_UNSUPPORTED_OPTION:
+                        /* AuthenTrend ATKey.Pro says it supports "up", but if we disable it it will fail
+                         * with FIDO_ERR_UNSUPPORTED_OPTION, probably because it isn't actually
+                         * optional. Let's see if turning it on works. This is very similar to the
+                         * FIDO_ERR_UP_REQUIRED case, but since the error is so vague we implement it
+                         * slightly more defensively. */
+
+                        if (has_up && !FLAGS_SET(lock_with, FIDO2ENROLL_UP)) {
+                                log_notice("Got unsupported option error when when user presence test is turned off. Trying with user presence test turned on.");
+                                retry_with_up = true;
+                        }
+
+                        break;
+
+                case FIDO_ERR_PIN_REQUIRED:
+                        if (!has_client_pin)
+                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Token asks for client PIN check but doesn't advertise 'clientPin' feature.");
+
+                        if (FLAGS_SET(lock_with, FIDO2ENROLL_PIN))
+                                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                                       "Token asks for user client PIN check but was already enabled.");
+
+                        log_debug("Token requires PIN for assertion, enabling.");
+                        retry_with_pin = true;
+                        break;
+
+                default:
+                        break;
+                }
+
+                if (!retry_with_up && !retry_with_pin)
+                        break;
+
+                if (retry_with_up) {
+                        r = sym_fido_assert_set_up(a, FIDO_OPT_TRUE);
+                        if (r != FIDO_OK)
+                                return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to enable FIDO2 user presence test: %s", sym_fido_strerr(r));
+
+                        lock_with |= FIDO2ENROLL_UP;
+                }
+
+                if (retry_with_pin)
+                        lock_with |= FIDO2ENROLL_PIN;
+        }
+
         if (r == FIDO_ERR_ACTION_TIMEOUT)
                 return log_error_errno(SYNTHETIC_ERRNO(ENOSTR),
                                        "Token action timeout. (User didn't interact with token quickly enough.)");
@@ -751,6 +821,9 @@ int fido2_generate_hmac_hash(
         if (ret_usedpin)
                 *ret_usedpin = TAKE_PTR(used_pin);
 
+        if (ret_locked_with)
+                *ret_locked_with = lock_with;
+
         return 0;
 }
 #endif

diff --git a/src/shared/libfido2-util.h b/src/shared/libfido2-util.h
index 1b31577e06c1ba268cc3eac2df3de7525c99e26c..bcf3c73706d756e85d4496c67f9624f47c902657 100644 (file)
--- a/src/shared/libfido2-util.h
+++ b/src/shared/libfido2-util.h
@@ -106,7 +106,8 @@ int fido2_generate_hmac_hash(
                 void **ret_cid, size_t *ret_cid_size,
                 void **ret_salt, size_t *ret_salt_size,
                 void **ret_secret, size_t *ret_secret_size,
-                char **ret_usedpin);
+                char **ret_usedpin,
+                Fido2EnrollFlags *ret_locked_with);
 
 #endif