[17krishna8]

MakeCredential crops the user name, display name and icon to 64 bytes before storing a discoverable credential, with a comment explaining that this is done because the input is user provided. The updateUserInformation subcommand writes the same fields back without that crop.

A platform holding a credMgmt token can therefore update a resident credential with a much longer user name. The serialized credential then exceeds the maximum value size of the persistent store (1023 bytes with the default page size), and the whole subcommand fails with CTAP2_ERR_VENDOR_INTERNAL_ERROR, even though

• WebAuthn explicitly allows the authenticator to truncate these strings to a limit of at least 64 bytes, and
• the same input would have been accepted by MakeCredential thanks to its truncation.

Measured with the test environment (which uses the real store): a stored credential is about 123 bytes, and a user.name of 900 bytes or more already makes the update fail on hardware.

Apply the same 64-byte truncation as MakeCredential, so the stored credential stays within its expected size budget.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[gemini-code-assist]

Code Review

This pull request introduces truncation of user-provided fields (user_name, user_display_name, and user_icon) to 64 bytes during credential updates to ensure they fit in storage while preserving UTF-8 boundaries, accompanied by a new unit test. Feedback recommends optimizing memory usage for this embedded environment by performing the string truncation in-place using String::truncate instead of allocating new strings, which also renders the ToString import unnecessary.

[17krishna8]

The truncation now happens in place with a local truncate_to_char_boundary(&mut String, ..) helper, and the ToString import is gone. The strings are owned at that point, so no allocation is needed.

[coveralls]

Coverage is 97.357% — 17krishna8:fix/update-user-info-truncation into google:develop. No base build found for google:develop.

[17krishna8]

Thanks for the review! Summary of the update:

• Rebased onto develop, resolving the conflict with the new key_type check (the truncation now sits after check_rp_id_permissions).
• Removed the local truncate_to_char_boundary and imported the existing helper from ctap/mod.rs instead. The call sites now use the same .map(|s| truncate_to_char_boundary(&s, 64).to_string()) style as MakeCredential.
• The misplaced doc comment went away together with the removed function.

Formatting, clippy and the unit tests pass locally, including the new truncation test.

[kaczmarczyck]

Hmm, maybe the Gemini review was inaccurate here. My own agent just suggested a similar change to this:

--- a/libraries/opensk/src/ctap/storage.rs
+++ b/libraries/opensk/src/ctap/storage.rs
@@ -20,7 +20,7 @@ use crate::ctap::data_formats::{
 };
 use crate::ctap::status_code::{Ctap2StatusCode, CtapResult};
 use crate::env::Env;
-use alloc::string::String;
+use alloc::string::{String, ToString};
 use alloc::vec::Vec;
 use core::convert::TryFrom;
 #[cfg(feature = "config_command")]
@@ -144,9 +144,21 @@ pub fn update_credential<E: Env>(
     user: PublicKeyCredentialUserEntity,
 ) -> CtapResult<()> {
     let (key, mut credential) = find_credential_item(env, credential_id)?;
-    credential.user_name = user.user_name;
-    credential.user_display_name = user.user_display_name;
-    credential.user_icon = user.user_icon;
+    if user.user_id != credential.user_handle {
+        return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
+    }
+    credential.user_name = user
+        .user_name
+        .filter(|s| !s.is_empty())
+        .map(|s| super::truncate_to_char_boundary(&s, 64).to_string());
+    credential.user_display_name = user
+        .user_display_name
+        .filter(|s| !s.is_empty())
+        .map(|s| super::truncate_to_char_boundary(&s, 64).to_string());
+    credential.user_icon = user
+        .user_icon
+        .filter(|s| !s.is_empty())
+        .map(|s| super::truncate_to_char_boundary(&s, 64).to_string());
     let value = serialize_credential(credential)?;
     env.persist().write_credential_bytes(key, &value)
 }

For your inspiration.