From 290f90320e0a89dfab5848e15ab026baed0de5da Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin@nitrokey.com>
Date: Wed, 5 Jul 2023 23:29:39 +0200
Subject: [PATCH] Rename url to icon and ignore its content

The icon field of PublicKeyCredentialRpEntity has been removed from the
Webauthn spec.  CTAP 2.2 still requires us to parse it although we may
not store its content.  This patch:
- renames the url field to icon and keeps url as an alias for backwards
  compatibility,
- introduces an Icon helper type to make sure that we do not store any
  data for this field, and
- adds the skip_serializing attribute to the field.

Fixes: https://github.com/solokeys/ctap-types/issues/9
---
 CHANGELOG.md                 |  4 ++++
 Cargo.toml                   |  3 +++
 src/ctap2/make_credential.rs | 16 ++++++++++++++++
 src/webauthn.rs              | 30 +++++++++++++++++++++++++++---
 4 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8d1f2ca..de64e3a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Rename `url` to `icon` in `PublicKeyCredentialRpEntity` and ignore its
+  content ([#9][])
+
+[#9]: https://github.com/solokeys/ctap-types/issues/9
 
 ## [0.1.2] - 2022-03-07
 
diff --git a/Cargo.toml b/Cargo.toml
index 9f4fb93..af46a17 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -22,6 +22,9 @@ serde = { version = "1", default-features = false, features = ["derive"] }
 serde-indexed = "0.1"
 serde_repr = "0.1"
 
+[dev-dependencies]
+serde = { version = "1" }
+
 [features]
 log-all = ["cbor-smol/log-all"]
 log-none = []
diff --git a/src/ctap2/make_credential.rs b/src/ctap2/make_credential.rs
index e712125..214c322 100644
--- a/src/ctap2/make_credential.rs
+++ b/src/ctap2/make_credential.rs
@@ -226,3 +226,19 @@ pub struct PackedAttestationStatement {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub x5c: Option<Vec<Bytes<1024>, 1>>,
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn rp_entity_icon() {
+        // icon has been removed but must still be parsed
+        let cbor = b"\xa4\x01X \xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\x02\xa2bidx0make_credential_relying_party_entity.example.comdiconohttp://icon.png\x03\xa2bidX \x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1ddnamedAdam\x04\x81\xa2calg&dtypejpublic-key";
+        let _request: Request = cbor_smol::cbor_deserialize(cbor.as_slice()).unwrap();
+
+        // previously, we called it `url` and should still be able to deserialize it
+        let cbor = b"\xa4\x01X \xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\xcd\x02\xa2bidx0make_credential_relying_party_entity.example.comcurlohttp://icon.png\x03\xa2bidX \x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1d\x1ddnamedAdam\x04\x81\xa2calg&dtypejpublic-key";
+        let _request: Request = cbor_smol::cbor_deserialize(cbor.as_slice()).unwrap();
+    }
+}
diff --git a/src/webauthn.rs b/src/webauthn.rs
index 95322a2..56fa272 100644
--- a/src/webauthn.rs
+++ b/src/webauthn.rs
@@ -2,15 +2,39 @@
 
 use crate::sizes::*;
 use crate::{Bytes, String};
-use serde::{Deserialize, Serialize};
+use serde::{de::Deserializer, Deserialize, Serialize};
 
 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub struct PublicKeyCredentialRpEntity {
     pub id: String<256>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub name: Option<String<64>>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub url: Option<String<64>>,
+    /// This field has been removed in Webauthn 2 but CTAP 2.2 requires implementors to accept it.
+    ///
+    /// The content of this field must not be stored.  Therefore we use the [`Icon`][] helper type.
+    ///
+    /// See [issue #9][] for more information.
+    ///
+    /// [issue #9]: https://github.com/solokeys/ctap-types/issues/9
+    #[serde(skip_serializing, alias = "url")]
+    pub icon: Option<Icon>,
+}
+
+/// Helper type for the `icon` field of [`PublicKeyCredentialRpEntity`][].
+///
+/// This field must be parsed but not used or stored.  Therefore this wrapper type can be
+/// deserialized from a string but does not store any data.
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct Icon;
+
+impl<'de> Deserialize<'de> for Icon {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let _s: &'de str = Deserialize::deserialize(deserializer)?;
+        Ok(Self)
+    }
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]