fix(webauthn): address largeBlob write/delete review findings

Preserve foreign largeBlobArray entries by their exact bytes on
read-modify-write rather than re-serialising them, which also drops a
redundant re-encode of the new entry. Treat a valid-trailer but
unparseable array as a fail-safe error rather than clobbering it. Satisfy
the production clippy::indexing_slicing denies in the write path. Refuse
the U2F downgrade for largeBlob write and delete.

Adds tests for nonce uniqueness across writes, the pinUvAuthParam
protocol-1 path, the JSON write input, the PIN-protected write token
ceremony under UV=Discouraged, foreign-blob survival across write and
delete, the single-allowCredential guard, and shrink-on-replace.

Author: AlfioEmanueleFresta

libwebauthn-tests/tests/large_blob.rs

@@ -13,7 +13,7 @@ use libwebauthn::proto::ctap2::{
     Ctap2PublicKeyCredentialUserEntity,
 };
 use libwebauthn::transport::{Channel, ChannelSettings, Device};
-use libwebauthn::webauthn::WebAuthn;
+use libwebauthn::webauthn::{Error, PlatformError, WebAuthn};
 use libwebauthn::UvUpdate;
 use libwebauthn_tests::virt::get_virtual_device;
 use rand::{thread_rng, Rng};
@@ -301,7 +301,7 @@ fn ga_request(
 #[test(tokio::test)]
 async fn test_webauthn_large_blob_write_then_read_returns_blob() {
     let mut device = get_virtual_device();
-    let mut channel = device.channel().await.unwrap();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
     let challenge: [u8; 32] = thread_rng().gen();
 
     let state_recv = channel.get_ux_update_receiver();
@@ -350,7 +350,7 @@ async fn test_webauthn_large_blob_write_then_read_returns_blob() {
 #[test(tokio::test)]
 async fn test_webauthn_large_blob_write_replaces_existing_entry() {
     let mut device = get_virtual_device();
-    let mut channel = device.channel().await.unwrap();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
     let challenge: [u8; 32] = thread_rng().gen();
 
     let state_recv = channel.get_ux_update_receiver();
@@ -403,7 +403,7 @@ async fn test_webauthn_large_blob_write_replaces_existing_entry() {
 #[test(tokio::test)]
 async fn test_webauthn_large_blob_delete_without_existing_entry_reports_false() {
     let mut device = get_virtual_device();
-    let mut channel = device.channel().await.unwrap();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
     let challenge: [u8; 32] = thread_rng().gen();
 
     let state_recv = channel.get_ux_update_receiver();
@@ -436,7 +436,7 @@ async fn test_webauthn_large_blob_delete_without_existing_entry_reports_false()
 #[test(tokio::test)]
 async fn test_webauthn_large_blob_delete_removes_entry() {
     let mut device = get_virtual_device();
-    let mut channel = device.channel().await.unwrap();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
     let challenge: [u8; 32] = thread_rng().gen();
 
     let state_recv = channel.get_ux_update_receiver();
@@ -489,3 +489,240 @@ async fn test_webauthn_large_blob_delete_removes_entry() {
 
     update_handle.await.unwrap();
 }
+
+async fn write_large_blob(
+    channel: &mut libwebauthn::transport::hid::channel::HidChannel<'_>,
+    credential: &Ctap2PublicKeyCredentialDescriptor,
+    challenge: &[u8; 32],
+    blob: Vec<u8>,
+) {
+    let resp = channel
+        .webauthn_get_assertion(&ga_request(
+            credential,
+            challenge,
+            GetAssertionLargeBlobExtension::Write(blob),
+        ))
+        .await
+        .expect("write assertion should succeed");
+    assert_eq!(
+        resp.assertions[0]
+            .unsigned_extensions_output
+            .as_ref()
+            .and_then(|u| u.large_blob.as_ref())
+            .and_then(|lb| lb.written),
+        Some(true),
+        "largeBlob.written should be true"
+    );
+}
+
+async fn read_large_blob(
+    channel: &mut libwebauthn::transport::hid::channel::HidChannel<'_>,
+    credential: &Ctap2PublicKeyCredentialDescriptor,
+    challenge: &[u8; 32],
+) -> Option<Vec<u8>> {
+    let resp = channel
+        .webauthn_get_assertion(&ga_request(
+            credential,
+            challenge,
+            GetAssertionLargeBlobExtension::Read,
+        ))
+        .await
+        .expect("read assertion should succeed");
+    resp.assertions[0]
+        .unsigned_extensions_output
+        .as_ref()
+        .and_then(|u| u.large_blob.as_ref())
+        .and_then(|lb| lb.blob.clone())
+}
+
+/// Two largeBlob-capable credentials coexist on one authenticator. A's write
+/// (replace) and delete must leave B's foreign entry intact (CTAP 2.2 §6.10.6).
+#[test(tokio::test)]
+async fn test_webauthn_large_blob_foreign_entry_survives_write_and_delete() {
+    let mut device = get_virtual_device();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
+    let challenge: [u8; 32] = thread_rng().gen();
+
+    let state_recv = channel.get_ux_update_receiver();
+    // 2 registrations + 3 writes + 4 reads + 1 delete = 10 ceremonies.
+    let update_handle = tokio::spawn(handle_updates_n(state_recv, 10));
+
+    let cred_a = register_with_large_blob(&mut channel, "alice", &challenge).await;
+    let cred_b = register_with_large_blob(&mut channel, "bob", &challenge).await;
+
+    let blob_a = b"alice's blob".to_vec();
+    let blob_b = b"bob's blob must survive".to_vec();
+
+    write_large_blob(&mut channel, &cred_a, &challenge, blob_a.clone()).await;
+    write_large_blob(&mut channel, &cred_b, &challenge, blob_b.clone()).await;
+
+    assert_eq!(
+        read_large_blob(&mut channel, &cred_a, &challenge)
+            .await
+            .as_deref(),
+        Some(blob_a.as_slice()),
+        "A reads back its own blob"
+    );
+    assert_eq!(
+        read_large_blob(&mut channel, &cred_b, &challenge)
+            .await
+            .as_deref(),
+        Some(blob_b.as_slice()),
+        "B reads back its own blob"
+    );
+
+    let blob_a2 = b"alice's replacement blob, longer than the first".to_vec();
+    write_large_blob(&mut channel, &cred_a, &challenge, blob_a2.clone()).await;
+    assert_eq!(
+        read_large_blob(&mut channel, &cred_b, &challenge)
+            .await
+            .as_deref(),
+        Some(blob_b.as_slice()),
+        "B's blob survives A's replace"
+    );
+
+    let del_resp = channel
+        .webauthn_get_assertion(&ga_request(
+            &cred_a,
+            &challenge,
+            GetAssertionLargeBlobExtension::Delete,
+        ))
+        .await
+        .expect("delete A should succeed");
+    assert_eq!(
+        del_resp.assertions[0]
+            .unsigned_extensions_output
+            .as_ref()
+            .and_then(|u| u.large_blob.as_ref())
+            .and_then(|lb| lb.written),
+        Some(true),
+        "delete A reports written=true"
+    );
+    assert_eq!(
+        read_large_blob(&mut channel, &cred_b, &challenge)
+            .await
+            .as_deref(),
+        Some(blob_b.as_slice()),
+        "B's blob survives A's delete"
+    );
+
+    update_handle.await.unwrap();
+}
+
+/// WebAuthn L3 §10.1.5: largeBlob.write/delete requires exactly one allowCredentials
+/// entry. The platform guard in get_assertion_fido2 rejects a write with two (or zero)
+/// allowed credentials as NotSupported, before any CTAP traffic.
+#[test(tokio::test)]
+async fn test_webauthn_large_blob_write_requires_single_allow_credential() {
+    let mut device = get_virtual_device();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
+    let challenge: [u8; 32] = thread_rng().gen();
+
+    let state_recv = channel.get_ux_update_receiver();
+    // Two registrations, one PresenceRequired each. The rejected writes emit none.
+    let update_handle = tokio::spawn(handle_updates_n(state_recv, 2));
+
+    let cred_a = register_with_large_blob(&mut channel, "alice", &challenge).await;
+    let cred_b = register_with_large_blob(&mut channel, "bob", &challenge).await;
+
+    let mut two = ga_request(
+        &cred_a,
+        &challenge,
+        GetAssertionLargeBlobExtension::Write(b"blob".to_vec()),
+    );
+    two.allow = vec![cred_a.clone(), cred_b];
+    let err = channel
+        .webauthn_get_assertion(&two)
+        .await
+        .expect_err("write with two allowCredentials must be rejected");
+    assert_eq!(err, Error::Platform(PlatformError::NotSupported));
+
+    let mut none = ga_request(
+        &cred_a,
+        &challenge,
+        GetAssertionLargeBlobExtension::Write(b"blob".to_vec()),
+    );
+    none.allow = vec![];
+    let err = channel
+        .webauthn_get_assertion(&none)
+        .await
+        .expect_err("write with empty allowCredentials must be rejected");
+    assert_eq!(err, Error::Platform(PlatformError::NotSupported));
+
+    update_handle.await.unwrap();
+}
+
+/// Replacing a blob with a strictly smaller one shrinks the stored array: the
+/// read-modify-write rebuild must drop the larger entry entirely so the read
+/// returns exactly the smaller blob (no stale trailing bytes).
+#[test(tokio::test)]
+async fn test_webauthn_large_blob_write_replaces_with_smaller_blob() {
+    let mut device = get_virtual_device();
+    let mut channel = device.channel(ChannelSettings::default()).await.unwrap();
+    let challenge: [u8; 32] = thread_rng().gen();
+
+    let state_recv = channel.get_ux_update_receiver();
+    // MakeCredential + write(large) + read + write(small) + read = 5 PresenceRequired updates.
+    let update_handle = tokio::spawn(handle_updates_n(state_recv, 5));
+
+    let credential = register_with_large_blob(&mut channel, "erin", &challenge).await;
+
+    let large = b"compressible largeBlob payload ".repeat(20);
+    let small = b"tiny".to_vec();
+
+    channel
+        .webauthn_get_assertion(&ga_request(
+            &credential,
+            &challenge,
+            GetAssertionLargeBlobExtension::Write(large.clone()),
+        ))
+        .await
+        .expect("large write");
+
+    let read_large = channel
+        .webauthn_get_assertion(&ga_request(
+            &credential,
+            &challenge,
+            GetAssertionLargeBlobExtension::Read,
+        ))
+        .await
+        .expect("read after large write");
+    let blob_large = read_large.assertions[0]
+        .unsigned_extensions_output
+        .as_ref()
+        .and_then(|u| u.large_blob.as_ref())
+        .and_then(|lb| lb.blob.as_ref())
+        .expect("blob present after large write");
+    assert_eq!(blob_large.as_slice(), large.as_slice());
+
+    channel
+        .webauthn_get_assertion(&ga_request(
+            &credential,
+            &challenge,
+            GetAssertionLargeBlobExtension::Write(small.clone()),
+        ))
+        .await
+        .expect("small write");
+
+    let read_small = channel
+        .webauthn_get_assertion(&ga_request(
+            &credential,
+            &challenge,
+            GetAssertionLargeBlobExtension::Read,
+        ))
+        .await
+        .expect("read after small write");
+    let blob_small = read_small.assertions[0]
+        .unsigned_extensions_output
+        .as_ref()
+        .and_then(|u| u.large_blob.as_ref())
+        .and_then(|lb| lb.blob.as_ref())
+        .expect("blob present after small write");
+    assert_eq!(
+        blob_small.as_slice(),
+        small.as_slice(),
+        "smaller write replaced larger"
+    );
+
+    update_handle.await.unwrap();
+}

libwebauthn/src/ops/webauthn/get_assertion.rs

@@ -214,7 +214,7 @@ impl FromIdlModel<PublicKeyCredentialRequestOptionsJSON> for GetAssertionRequest
             .and_then(|e| e.large_blob.as_ref())
         {
             // L3 §10.1.5: largeBlob without read=true or support is a no-op, not an error.
-            Some(lb) if lb.support.is_none() && lb.read != Some(true) => None,
+            Some(lb) if lb.support.is_none() && lb.read != Some(true) && lb.write.is_none() => None,
             Some(lb) => Some(GetAssertionLargeBlobExtension::try_from(lb.clone())?),
             None => None,
         };
@@ -353,8 +353,8 @@ impl TryFrom<LargeBlobInputJson> for GetAssertionLargeBlobExtension {
                 "largeBlob.support is only valid at registration".to_string(),
             ));
         }
-        // WebAuthn L3 §10.1.5: read and write are mutually exclusive.
-        if value.read == Some(true) && value.write.is_some() {
+        // WebAuthn L3 §10.1.5: read and write present together is an error.
+        if value.read.is_some() && value.write.is_some() {
             return Err(GetAssertionPrepareError::NotSupported(
                 "largeBlob.read and largeBlob.write are mutually exclusive".to_string(),
             ));
@@ -604,6 +604,15 @@ impl DowngradableRequest<Vec<SignRequest>> for GetAssertionRequest {
             return false;
         }
 
+        if matches!(
+            self.extensions.as_ref().and_then(|e| e.large_blob.as_ref()),
+            Some(GetAssertionLargeBlobExtension::Write(_))
+                | Some(GetAssertionLargeBlobExtension::Delete)
+        ) {
+            debug!("Not downgradable: largeBlob write/delete requires FIDO2");
+            return false;
+        }
+
         true
     }
 
@@ -1409,6 +1418,7 @@ mod tests {
             GetAssertionLargeBlobExtension::try_from(LargeBlobInputJson {
                 support: None,
                 read: Some(true),
+                write: None,
             })
             .unwrap(),
             GetAssertionLargeBlobExtension::Read
@@ -1417,6 +1427,7 @@ mod tests {
             GetAssertionLargeBlobExtension::try_from(LargeBlobInputJson {
                 support: Some("required".to_string()),
                 read: Some(true),
+                write: None,
             }),
             Err(GetAssertionPrepareError::NotSupported(_))
         ));
@@ -1453,4 +1464,28 @@ mod tests {
         .expect("largeBlob.read=false must be a no-op, not an error");
         assert!(req.extensions.and_then(|e| e.large_blob).is_none());
     }
+
+    #[test]
+    fn large_blob_json_write_input_and_mutual_exclusion() {
+        use crate::ops::webauthn::idl::get::LargeBlobInputJson;
+
+        let blob = b"blob to write".to_vec();
+        assert_eq!(
+            GetAssertionLargeBlobExtension::try_from(LargeBlobInputJson {
+                support: None,
+                read: None,
+                write: Some(Base64UrlString::from(blob.clone())),
+            })
+            .unwrap(),
+            GetAssertionLargeBlobExtension::Write(blob)
+        );
+        assert!(matches!(
+            GetAssertionLargeBlobExtension::try_from(LargeBlobInputJson {
+                support: None,
+                read: Some(true),
+                write: Some(Base64UrlString::from(b"x".to_vec())),
+            }),
+            Err(GetAssertionPrepareError::NotSupported(_))
+        ));
+    }
 }