From 30fb782525fffb66ce8473ebb353844dcb9e84d3 Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Wed, 29 Dec 2021 09:42:46 +0100
Subject: [PATCH 1/4] adds extract_* functions to CBOR library

---
 libraries/cbor/src/values.rs              | 268 +++++++++++++++++++++-
 libraries/opensk/src/ctap/data_formats.rs |  47 +---
 2 files changed, 278 insertions(+), 37 deletions(-)

diff --git a/libraries/cbor/src/values.rs b/libraries/cbor/src/values.rs
index a293bffd..ffdc4e4c 100644
--- a/libraries/cbor/src/values.rs
+++ b/libraries/cbor/src/values.rs
@@ -105,6 +105,84 @@ impl Value {
             Value::Simple(_) => 7,
         }
     }
+
+    pub fn extract_unsigned(self) -> Option<u64> {
+        match self {
+            Value::Unsigned(unsigned) => Some(unsigned),
+            _ => None,
+        }
+    }
+
+    pub fn extract_integer(self) -> Option<i64> {
+        match self {
+            Value::Unsigned(unsigned) => {
+                if unsigned <= core::i64::MAX as u64 {
+                    Some(unsigned as i64)
+                } else {
+                    None
+                }
+            }
+            Value::Negative(signed) => Some(signed),
+            _ => None,
+        }
+    }
+
+    pub fn extract_byte_string(self) -> Option<Vec<u8>> {
+        match self {
+            Value::ByteString(byte_string) => Some(byte_string),
+            _ => None,
+        }
+    }
+
+    pub fn extract_text_string(self) -> Option<String> {
+        match self {
+            Value::TextString(text_string) => Some(text_string),
+            _ => None,
+        }
+    }
+
+    pub fn extract_array(self) -> Option<Vec<Value>> {
+        match self {
+            Value::Array(array) => Some(array),
+            _ => None,
+        }
+    }
+
+    pub fn extract_map(self) -> Option<Vec<(Value, Value)>> {
+        match self {
+            Value::Map(map) => Some(map),
+            _ => None,
+        }
+    }
+
+    pub fn extract_tag(self) -> Option<(u64, Value)> {
+        match self {
+            Value::Tag(tag, value) => Some((tag, *value)),
+            _ => None,
+        }
+    }
+
+    pub fn extract_bool(self) -> Option<bool> {
+        match self {
+            Value::Simple(SimpleValue::FalseValue) => Some(false),
+            Value::Simple(SimpleValue::TrueValue) => Some(true),
+            _ => None,
+        }
+    }
+
+    pub fn extract_null(self) -> Option<()> {
+        match self {
+            Value::Simple(SimpleValue::NullValue) => Some(()),
+            _ => None,
+        }
+    }
+
+    pub fn extract_undefined(self) -> Option<()> {
+        match self {
+            Value::Simple(SimpleValue::Undefined) => Some(()),
+            _ => None,
+        }
+    }
 }
 
 impl Ord for Value {
@@ -285,9 +363,197 @@ where
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::{cbor_array, cbor_bool, cbor_bytes, cbor_int, cbor_map, cbor_tagged, cbor_text};
+    use crate::{
+        cbor_array, cbor_bool, cbor_bytes, cbor_bytes_lit, cbor_int, cbor_map, cbor_null,
+        cbor_tagged, cbor_text, cbor_undefined, cbor_unsigned,
+    };
     use alloc::vec;
 
+    #[test]
+    fn test_extract_unsigned() {
+        assert_eq!(cbor_int!(1).extract_unsigned(), Some(1));
+        assert_eq!(cbor_int!(-1).extract_unsigned(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_unsigned(), None);
+        assert_eq!(cbor_text!("").extract_unsigned(), None);
+        assert_eq!(cbor_array![].extract_unsigned(), None);
+        assert_eq!(cbor_map! {}.extract_unsigned(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_unsigned(), None);
+        assert_eq!(cbor_bool!(false).extract_unsigned(), None);
+    }
+
+    #[test]
+    fn test_extract_unsigned_limits() {
+        assert_eq!(
+            cbor_unsigned!(core::u64::MAX).extract_unsigned(),
+            Some(core::u64::MAX)
+        );
+        assert_eq!(
+            cbor_unsigned!((core::i64::MAX as u64) + 1).extract_unsigned(),
+            Some((core::i64::MAX as u64) + 1)
+        );
+        assert_eq!(
+            cbor_int!(core::i64::MAX).extract_unsigned(),
+            Some(core::i64::MAX as u64)
+        );
+        assert_eq!(cbor_int!(123).extract_unsigned(), Some(123));
+        assert_eq!(cbor_int!(0).extract_unsigned(), Some(0));
+        assert_eq!(cbor_int!(-123).extract_unsigned(), None);
+        assert_eq!(cbor_int!(core::i64::MIN).extract_unsigned(), None);
+    }
+
+    #[test]
+    fn test_extract_integer() {
+        assert_eq!(cbor_int!(1).extract_integer(), Some(1));
+        assert_eq!(cbor_int!(-1).extract_integer(), Some(-1));
+        assert_eq!(cbor_bytes!(vec![]).extract_integer(), None);
+        assert_eq!(cbor_text!("").extract_integer(), None);
+        assert_eq!(cbor_array![].extract_integer(), None);
+        assert_eq!(cbor_map! {}.extract_integer(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_integer(), None);
+        assert_eq!(cbor_bool!(false).extract_integer(), None);
+    }
+
+    #[test]
+    fn test_extract_integer_limits() {
+        assert_eq!(cbor_unsigned!(core::u64::MAX).extract_integer(), None);
+        assert_eq!(
+            cbor_unsigned!((core::i64::MAX as u64) + 1).extract_integer(),
+            None
+        );
+        assert_eq!(
+            cbor_int!(core::i64::MAX).extract_integer(),
+            Some(core::i64::MAX)
+        );
+        assert_eq!(cbor_int!(123).extract_integer(), Some(123));
+        assert_eq!(cbor_int!(0).extract_integer(), Some(0));
+        assert_eq!(cbor_int!(-123).extract_integer(), Some(-123));
+        assert_eq!(
+            cbor_int!(core::i64::MIN).extract_integer(),
+            Some(core::i64::MIN)
+        );
+    }
+
+    #[test]
+    fn test_extract_byte_string() {
+        assert_eq!(cbor_int!(1).extract_byte_string(), None);
+        assert_eq!(cbor_int!(-1).extract_byte_string(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_byte_string(), Some(Vec::new()));
+        assert_eq!(
+            cbor_bytes_lit!(b"bar").extract_byte_string(),
+            Some(b"bar".to_vec())
+        );
+        assert_eq!(cbor_text!("").extract_byte_string(), None);
+        assert_eq!(cbor_array![].extract_byte_string(), None);
+        assert_eq!(cbor_map! {}.extract_byte_string(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_byte_string(), None);
+        assert_eq!(cbor_bool!(false).extract_byte_string(), None);
+    }
+
+    #[test]
+    fn test_extract_text_string() {
+        assert_eq!(cbor_int!(1).extract_text_string(), None);
+        assert_eq!(cbor_int!(-1).extract_text_string(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_text_string(), None);
+        assert_eq!(cbor_text!("").extract_text_string(), Some(String::new()));
+        assert_eq!(cbor_text!("s").extract_text_string(), Some("s".to_string()));
+        assert_eq!(cbor_array![].extract_text_string(), None);
+        assert_eq!(cbor_map! {}.extract_text_string(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_text_string(), None);
+        assert_eq!(cbor_bool!(false).extract_text_string(), None);
+    }
+
+    #[test]
+    fn test_extract_array() {
+        assert_eq!(cbor_int!(1).extract_array(), None);
+        assert_eq!(cbor_int!(-1).extract_array(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_array(), None);
+        assert_eq!(cbor_text!("").extract_array(), None);
+        assert_eq!(cbor_array![].extract_array(), Some(Vec::new()));
+        assert_eq!(
+            cbor_array![cbor_int!(1)].extract_array(),
+            Some(vec![cbor_int!(1)])
+        );
+        assert_eq!(cbor_map! {}.extract_array(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_array(), None);
+        assert_eq!(cbor_bool!(false).extract_array(), None);
+    }
+
+    #[test]
+    fn test_extract_map() {
+        assert_eq!(cbor_int!(1).extract_map(), None);
+        assert_eq!(cbor_int!(-1).extract_map(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_map(), None);
+        assert_eq!(cbor_text!("").extract_map(), None);
+        assert_eq!(cbor_array![].extract_map(), None);
+        assert_eq!(cbor_map! {}.extract_map(), Some(Vec::new()));
+        assert_eq!(
+            cbor_map! {0 => 1}.extract_map(),
+            Some(vec![(cbor_int!(0), cbor_int!(1))])
+        );
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_map(), None);
+        assert_eq!(cbor_bool!(false).extract_map(), None);
+    }
+
+    #[test]
+    fn test_extract_tag() {
+        assert_eq!(cbor_int!(1).extract_tag(), None);
+        assert_eq!(cbor_int!(-1).extract_tag(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_tag(), None);
+        assert_eq!(cbor_text!("").extract_tag(), None);
+        assert_eq!(cbor_array![].extract_tag(), None);
+        assert_eq!(cbor_map! {}.extract_tag(), None);
+        assert_eq!(
+            cbor_tagged!(1, cbor_text!("s")).extract_tag(),
+            Some((1, cbor_text!("s")))
+        );
+        assert_eq!(cbor_bool!(false).extract_tag(), None);
+    }
+
+    #[test]
+    fn test_extract_bool() {
+        assert_eq!(cbor_int!(1).extract_bool(), None);
+        assert_eq!(cbor_int!(-1).extract_bool(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_bool(), None);
+        assert_eq!(cbor_text!("").extract_bool(), None);
+        assert_eq!(cbor_array![].extract_bool(), None);
+        assert_eq!(cbor_map! {}.extract_bool(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_bool(), None);
+        assert_eq!(cbor_bool!(false).extract_bool(), Some(false));
+        assert_eq!(cbor_bool!(true).extract_bool(), Some(true));
+        assert_eq!(cbor_null!().extract_bool(), None);
+        assert_eq!(cbor_undefined!().extract_bool(), None);
+    }
+
+    #[test]
+    fn test_extract_null() {
+        assert_eq!(cbor_int!(1).extract_null(), None);
+        assert_eq!(cbor_int!(-1).extract_null(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_null(), None);
+        assert_eq!(cbor_text!("").extract_null(), None);
+        assert_eq!(cbor_array![].extract_null(), None);
+        assert_eq!(cbor_map! {}.extract_null(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_null(), None);
+        assert_eq!(cbor_bool!(false).extract_null(), None);
+        assert_eq!(cbor_bool!(true).extract_null(), None);
+        assert_eq!(cbor_null!().extract_null(), Some(()));
+        assert_eq!(cbor_undefined!().extract_null(), None);
+    }
+
+    #[test]
+    fn test_extract_undefined() {
+        assert_eq!(cbor_int!(1).extract_undefined(), None);
+        assert_eq!(cbor_int!(-1).extract_undefined(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_undefined(), None);
+        assert_eq!(cbor_text!("").extract_undefined(), None);
+        assert_eq!(cbor_array![].extract_undefined(), None);
+        assert_eq!(cbor_map! {}.extract_undefined(), None);
+        assert_eq!(cbor_tagged!(1, cbor_text!("s")).extract_undefined(), None);
+        assert_eq!(cbor_bool!(false).extract_undefined(), None);
+        assert_eq!(cbor_bool!(true).extract_undefined(), None);
+        assert_eq!(cbor_null!().extract_undefined(), None);
+        assert_eq!(cbor_undefined!().extract_undefined(), Some(()));
+    }
+
     #[test]
     fn test_value_ordering() {
         assert!(cbor_int!(0) < cbor_int!(23));
diff --git a/libraries/opensk/src/ctap/data_formats.rs b/libraries/opensk/src/ctap/data_formats.rs
index d10d2408..105699a4 100644
--- a/libraries/opensk/src/ctap/data_formats.rs
+++ b/libraries/opensk/src/ctap/data_formats.rs
@@ -1163,63 +1163,38 @@ impl From<CredentialManagementSubCommandParameters> for cbor::Value {
     }
 }
 
+fn ok_or_cbor_type<T>(value_option: Option<T>) -> Result<T, Ctap2StatusCode> {
+    value_option.ok_or(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE)
+}
+
 pub fn extract_unsigned(cbor_value: cbor::Value) -> Result<u64, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::Unsigned(unsigned) => Ok(unsigned),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_unsigned())
 }
 
 pub fn extract_integer(cbor_value: cbor::Value) -> Result<i64, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::Unsigned(unsigned) => {
-            if unsigned <= core::i64::MAX as u64 {
-                Ok(unsigned as i64)
-            } else {
-                Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE)
-            }
-        }
-        cbor::Value::Negative(signed) => Ok(signed),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_integer())
 }
 
 pub fn extract_byte_string(cbor_value: cbor::Value) -> Result<Vec<u8>, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::ByteString(byte_string) => Ok(byte_string),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_byte_string())
 }
 
 pub fn extract_text_string(cbor_value: cbor::Value) -> Result<String, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::TextString(text_string) => Ok(text_string),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_text_string())
 }
 
 pub fn extract_array(cbor_value: cbor::Value) -> Result<Vec<cbor::Value>, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::Array(array) => Ok(array),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_array())
 }
 
 pub fn extract_map(
     cbor_value: cbor::Value,
 ) -> Result<Vec<(cbor::Value, cbor::Value)>, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::Map(map) => Ok(map),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_map())
 }
 
 pub fn extract_bool(cbor_value: cbor::Value) -> Result<bool, Ctap2StatusCode> {
-    match cbor_value {
-        cbor::Value::Simple(cbor::SimpleValue::FalseValue) => Ok(false),
-        cbor::Value::Simple(cbor::SimpleValue::TrueValue) => Ok(true),
-        _ => Err(Ctap2StatusCode::CTAP2_ERR_CBOR_UNEXPECTED_TYPE),
-    }
+    ok_or_cbor_type(cbor_value.extract_bool())
 }
 
 pub fn ok_or_missing<T>(value_option: Option<T>) -> Result<T, Ctap2StatusCode> {

From 35ce345ee5e440b267f1d2a00360586064635648 Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Thu, 30 Dec 2021 23:18:27 +0100
Subject: [PATCH 2/4] hides Value implementation details

---
 libraries/cbor/examples/cbor.rs       |  17 +-
 libraries/cbor/src/lib.rs             |   2 +-
 libraries/cbor/src/macros.rs          | 216 +++++++++++---------------
 libraries/cbor/src/reader.rs          |   6 +-
 libraries/cbor/src/values.rs          | 159 +++++++++++--------
 libraries/cbor/src/writer.rs          |  22 +--
 libraries/opensk/src/api/key_store.rs |  10 +-
 7 files changed, 215 insertions(+), 217 deletions(-)

diff --git a/libraries/cbor/examples/cbor.rs b/libraries/cbor/examples/cbor.rs
index 871be4e1..e881bf79 100644
--- a/libraries/cbor/examples/cbor.rs
+++ b/libraries/cbor/examples/cbor.rs
@@ -18,7 +18,7 @@
 
 extern crate alloc;
 
-use sk_cbor::values::{SimpleValue, Value};
+use sk_cbor::values::Value;
 use sk_cbor::{cbor_array, cbor_bytes, cbor_map, cbor_null, cbor_true};
 
 fn hexify(data: &[u8]) -> String {
@@ -32,17 +32,14 @@ fn hexify(data: &[u8]) -> String {
 fn main() {
     // Build a CBOR object with various different types included. Note that this
     // object is not built in canonical order.
-    let manual_object = Value::Map(vec![
+    let manual_object = Value::map(vec![
         (
-            Value::Unsigned(1),
-            Value::Array(vec![Value::Unsigned(2), Value::Unsigned(3)]),
+            Value::from(1),
+            Value::array(vec![Value::from(2), Value::from(3)]),
         ),
-        (
-            Value::TextString("tstr".to_owned()),
-            Value::ByteString(vec![1, 2, 3]),
-        ),
-        (Value::Negative(-2), Value::Simple(SimpleValue::NullValue)),
-        (Value::Unsigned(3), Value::Simple(SimpleValue::TrueValue)),
+        (Value::from("tstr".to_owned()), Value::from(vec![1, 2, 3])),
+        (Value::from(-2), Value::null_value()),
+        (Value::from(3), Value::bool_value(true)),
     ]);
 
     // Build the same object using the crate's convenience macros.
diff --git a/libraries/cbor/src/lib.rs b/libraries/cbor/src/lib.rs
index ed378f53..0cc4902c 100644
--- a/libraries/cbor/src/lib.rs
+++ b/libraries/cbor/src/lib.rs
@@ -22,5 +22,5 @@ pub mod values;
 pub mod writer;
 
 pub use self::reader::read;
-pub use self::values::{SimpleValue, Value};
+pub use self::values::Value;
 pub use self::writer::write;
diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs
index e9e48601..e437a05a 100644
--- a/libraries/cbor/src/macros.rs
+++ b/libraries/cbor/src/macros.rs
@@ -168,7 +168,7 @@ macro_rules! cbor_map {
             $(
                 _map.push(($key.into_cbor_value(), $value.into_cbor_value()));
             )*
-            $crate::values::Value::Map(_map)
+            $crate::values::Value::map(_map)
         }
     };
 }
@@ -214,7 +214,7 @@ macro_rules! cbor_map_options {
                 }
             }
             )*
-            $crate::values::Value::Map(_map)
+            $crate::values::Value::map(_map)
         }
     };
 }
@@ -223,7 +223,7 @@ macro_rules! cbor_map_options {
 #[macro_export]
 macro_rules! cbor_map_collection {
     ( $tree:expr ) => {{
-        $crate::values::Value::from($tree)
+        $crate::values::Value::map($tree)
     }};
 }
 
@@ -250,7 +250,7 @@ macro_rules! cbor_array {
             // The import is unused if the list is empty.
             #[allow(unused_imports)]
             use $crate::values::IntoCborValue;
-            $crate::values::Value::Array(vec![ $( $value.into_cbor_value(), )* ])
+            $crate::values::Value::array(vec![ $( $value.into_cbor_value(), )* ])
         }
     };
 }
@@ -260,7 +260,7 @@ macro_rules! cbor_array {
 macro_rules! cbor_array_vec {
     ( $vec:expr ) => {{
         use $crate::values::IntoCborValue;
-        $crate::values::Value::Array($vec.into_iter().map(|x| x.into_cbor_value()).collect())
+        $crate::values::Value::array($vec.into_iter().map(|x| x.into_cbor_value()).collect())
     }};
 }
 
@@ -268,7 +268,7 @@ macro_rules! cbor_array_vec {
 #[macro_export]
 macro_rules! cbor_true {
     ( ) => {
-        $crate::values::Value::Simple($crate::values::SimpleValue::TrueValue)
+        $crate::values::Value::bool_value(true)
     };
 }
 
@@ -276,7 +276,7 @@ macro_rules! cbor_true {
 #[macro_export]
 macro_rules! cbor_false {
     ( ) => {
-        $crate::values::Value::Simple($crate::values::SimpleValue::FalseValue)
+        $crate::values::Value::bool_value(false)
     };
 }
 
@@ -284,7 +284,7 @@ macro_rules! cbor_false {
 #[macro_export]
 macro_rules! cbor_null {
     ( ) => {
-        $crate::values::Value::Simple($crate::values::SimpleValue::NullValue)
+        $crate::values::Value::null_value()
     };
 }
 
@@ -292,7 +292,7 @@ macro_rules! cbor_null {
 #[macro_export]
 macro_rules! cbor_undefined {
     ( ) => {
-        $crate::values::Value::Simple($crate::values::SimpleValue::Undefined)
+        $crate::values::Value::undefined()
     };
 }
 
@@ -308,7 +308,7 @@ macro_rules! cbor_bool {
 #[macro_export]
 macro_rules! cbor_unsigned {
     ( $x:expr ) => {
-        $crate::values::Value::Unsigned($x)
+        $crate::values::Value::from($x)
     };
 }
 
@@ -324,7 +324,7 @@ macro_rules! cbor_int {
 #[macro_export]
 macro_rules! cbor_text {
     ( $x:expr ) => {
-        $crate::values::Value::TextString($x.into())
+        $crate::values::Value::from($x)
     };
 }
 
@@ -332,7 +332,7 @@ macro_rules! cbor_text {
 #[macro_export]
 macro_rules! cbor_bytes {
     ( $x:expr ) => {
-        $crate::values::Value::ByteString($x)
+        $crate::values::Value::from($x)
     };
 }
 
@@ -340,7 +340,7 @@ macro_rules! cbor_bytes {
 #[macro_export]
 macro_rules! cbor_tagged {
     ( $t:expr, $x: expr ) => {
-        $crate::values::Value::Tag($t, ::alloc::boxed::Box::new($x))
+        $crate::values::Value::tag($t, $x)
     };
 }
 
@@ -362,41 +362,41 @@ macro_rules! cbor_bytes_lit {
 
 #[cfg(test)]
 mod test {
-    use super::super::values::{SimpleValue, Value};
+    use super::super::values::Value;
     use alloc::string::String;
     use alloc::vec;
     use alloc::vec::Vec;
 
     #[test]
     fn test_cbor_simple_values() {
-        assert_eq!(cbor_true!(), Value::Simple(SimpleValue::TrueValue));
-        assert_eq!(cbor_false!(), Value::Simple(SimpleValue::FalseValue));
-        assert_eq!(cbor_null!(), Value::Simple(SimpleValue::NullValue));
-        assert_eq!(cbor_undefined!(), Value::Simple(SimpleValue::Undefined));
+        assert_eq!(cbor_true!(), Value::bool_value(true));
+        assert_eq!(cbor_false!(), Value::bool_value(false));
+        assert_eq!(cbor_null!(), Value::null_value());
+        assert_eq!(cbor_undefined!(), Value::undefined());
     }
 
     #[test]
     fn test_cbor_bool() {
-        assert_eq!(cbor_bool!(true), Value::Simple(SimpleValue::TrueValue));
-        assert_eq!(cbor_bool!(false), Value::Simple(SimpleValue::FalseValue));
+        assert_eq!(cbor_bool!(true), Value::bool_value(true));
+        assert_eq!(cbor_bool!(false), Value::bool_value(false));
     }
 
     #[test]
     fn test_cbor_int_unsigned() {
-        assert_eq!(cbor_int!(0), Value::Unsigned(0));
-        assert_eq!(cbor_int!(1), Value::Unsigned(1));
-        assert_eq!(cbor_int!(123456), Value::Unsigned(123456));
+        assert_eq!(cbor_int!(0), Value::from(0));
+        assert_eq!(cbor_int!(1), Value::from(1));
+        assert_eq!(cbor_int!(123456), Value::from(123456));
         assert_eq!(
             cbor_int!(core::i64::MAX),
-            Value::Unsigned(core::i64::MAX as u64)
+            Value::from(core::i64::MAX as u64)
         );
     }
 
     #[test]
     fn test_cbor_int_negative() {
-        assert_eq!(cbor_int!(-1), Value::Negative(-1));
-        assert_eq!(cbor_int!(-123456), Value::Negative(-123456));
-        assert_eq!(cbor_int!(core::i64::MIN), Value::Negative(core::i64::MIN));
+        assert_eq!(cbor_int!(-1), Value::from(-1));
+        assert_eq!(cbor_int!(-123456), Value::from(-123456));
+        assert_eq!(cbor_int!(core::i64::MIN), Value::from(core::i64::MIN));
     }
 
     #[test]
@@ -413,17 +413,17 @@ mod test {
             core::i64::MAX,
             core::u64::MAX,
         ];
-        let b = Value::Array(vec![
-            Value::Negative(core::i64::MIN),
-            Value::Negative(core::i32::MIN as i64),
-            Value::Negative(-123456),
-            Value::Negative(-1),
-            Value::Unsigned(0),
-            Value::Unsigned(1),
-            Value::Unsigned(123456),
-            Value::Unsigned(core::i32::MAX as u64),
-            Value::Unsigned(core::i64::MAX as u64),
-            Value::Unsigned(core::u64::MAX),
+        let b = Value::array(vec![
+            Value::from(core::i64::MIN),
+            Value::from(core::i32::MIN as i64),
+            Value::from(-123456),
+            Value::from(-1),
+            Value::from(0),
+            Value::from(1),
+            Value::from(123456),
+            Value::from(core::i32::MAX as u64),
+            Value::from(core::i64::MAX as u64),
+            Value::from(core::u64::MAX),
         ]);
         assert_eq!(a, b);
     }
@@ -442,22 +442,17 @@ mod test {
             cbor_map! {},
             cbor_map! {2 => 3},
         ];
-        let b = Value::Array(vec![
-            Value::Negative(-123),
-            Value::Unsigned(456),
-            Value::Simple(SimpleValue::TrueValue),
-            Value::Simple(SimpleValue::NullValue),
-            Value::TextString(String::from("foo")),
-            Value::ByteString(b"bar".to_vec()),
-            Value::Array(Vec::new()),
-            Value::Array(vec![Value::Unsigned(0), Value::Unsigned(1)]),
-            Value::Map(Vec::new()),
-            Value::Map(
-                [(Value::Unsigned(2), Value::Unsigned(3))]
-                    .iter()
-                    .cloned()
-                    .collect(),
-            ),
+        let b = Value::array(vec![
+            Value::from(-123),
+            Value::from(456),
+            Value::bool_value(true),
+            Value::null_value(),
+            Value::from(String::from("foo")),
+            Value::from(b"bar".to_vec()),
+            Value::array(Vec::new()),
+            Value::array(vec![Value::from(0), Value::from(1)]),
+            Value::map(Vec::new()),
+            Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()),
         ]);
         assert_eq!(a, b);
     }
@@ -465,18 +460,18 @@ mod test {
     #[test]
     fn test_cbor_array_vec_empty() {
         let a = cbor_array_vec!(Vec::<bool>::new());
-        let b = Value::Array(Vec::new());
+        let b = Value::array(Vec::new());
         assert_eq!(a, b);
     }
 
     #[test]
     fn test_cbor_array_vec_int() {
         let a = cbor_array_vec!(vec![1, 2, 3, 4]);
-        let b = Value::Array(vec![
-            Value::Unsigned(1),
-            Value::Unsigned(2),
-            Value::Unsigned(3),
-            Value::Unsigned(4),
+        let b = Value::array(vec![
+            Value::from(1),
+            Value::from(2),
+            Value::from(3),
+            Value::from(4),
         ]);
         assert_eq!(a, b);
     }
@@ -484,10 +479,10 @@ mod test {
     #[test]
     fn test_cbor_array_vec_text() {
         let a = cbor_array_vec!(vec!["a", "b", "c"]);
-        let b = Value::Array(vec![
-            Value::TextString(String::from("a")),
-            Value::TextString(String::from("b")),
-            Value::TextString(String::from("c")),
+        let b = Value::array(vec![
+            Value::from(String::from("a")),
+            Value::from(String::from("b")),
+            Value::from(String::from("c")),
         ]);
         assert_eq!(a, b);
     }
@@ -495,10 +490,10 @@ mod test {
     #[test]
     fn test_cbor_array_vec_bytes() {
         let a = cbor_array_vec!(vec![b"a", b"b", b"c"]);
-        let b = Value::Array(vec![
-            Value::ByteString(b"a".to_vec()),
-            Value::ByteString(b"b".to_vec()),
-            Value::ByteString(b"c".to_vec()),
+        let b = Value::array(vec![
+            Value::from(b"a".to_vec()),
+            Value::from(b"b".to_vec()),
+            Value::from(b"c".to_vec()),
         ]);
         assert_eq!(a, b);
     }
@@ -517,34 +512,23 @@ mod test {
             9 => cbor_map!{},
             10 => cbor_map!{2 => 3},
         };
-        let b = Value::Map(
+        let b = Value::map(
             [
-                (Value::Negative(-1), Value::Negative(-23)),
-                (Value::Unsigned(4), Value::Unsigned(56)),
+                (Value::from(-1), Value::from(-23)),
+                (Value::from(4), Value::from(56)),
+                (Value::from(String::from("foo")), Value::bool_value(true)),
+                (Value::from(b"bar".to_vec()), Value::null_value()),
+                (Value::from(5), Value::from(String::from("foo"))),
+                (Value::from(6), Value::from(b"bar".to_vec())),
+                (Value::from(7), Value::array(Vec::new())),
                 (
-                    Value::TextString(String::from("foo")),
-                    Value::Simple(SimpleValue::TrueValue),
+                    Value::from(8),
+                    Value::array(vec![Value::from(0), Value::from(1)]),
                 ),
+                (Value::from(9), Value::map(Vec::new())),
                 (
-                    Value::ByteString(b"bar".to_vec()),
-                    Value::Simple(SimpleValue::NullValue),
-                ),
-                (Value::Unsigned(5), Value::TextString(String::from("foo"))),
-                (Value::Unsigned(6), Value::ByteString(b"bar".to_vec())),
-                (Value::Unsigned(7), Value::Array(Vec::new())),
-                (
-                    Value::Unsigned(8),
-                    Value::Array(vec![Value::Unsigned(0), Value::Unsigned(1)]),
-                ),
-                (Value::Unsigned(9), Value::Map(Vec::new())),
-                (
-                    Value::Unsigned(10),
-                    Value::Map(
-                        [(Value::Unsigned(2), Value::Unsigned(3))]
-                            .iter()
-                            .cloned()
-                            .collect(),
-                    ),
+                    Value::from(10),
+                    Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()),
                 ),
             ]
             .iter()
@@ -576,34 +560,23 @@ mod test {
             18 => None::<u64>,
             10 => Some(cbor_map!{2 => 3}),
         };
-        let b = Value::Map(
+        let b = Value::map(
             [
-                (Value::Negative(-1), Value::Negative(-23)),
-                (Value::Unsigned(4), Value::Unsigned(56)),
+                (Value::from(-1), Value::from(-23)),
+                (Value::from(4), Value::from(56)),
+                (Value::from(String::from("foo")), Value::bool_value(true)),
+                (Value::from(b"bar".to_vec()), Value::null_value()),
+                (Value::from(5), Value::from(String::from("foo"))),
+                (Value::from(6), Value::from(b"bar".to_vec())),
+                (Value::from(7), Value::array(Vec::new())),
                 (
-                    Value::TextString(String::from("foo")),
-                    Value::Simple(SimpleValue::TrueValue),
+                    Value::from(8),
+                    Value::array(vec![Value::from(0), Value::from(1)]),
                 ),
+                (Value::from(9), Value::map(Vec::new())),
                 (
-                    Value::ByteString(b"bar".to_vec()),
-                    Value::Simple(SimpleValue::NullValue),
-                ),
-                (Value::Unsigned(5), Value::TextString(String::from("foo"))),
-                (Value::Unsigned(6), Value::ByteString(b"bar".to_vec())),
-                (Value::Unsigned(7), Value::Array(Vec::new())),
-                (
-                    Value::Unsigned(8),
-                    Value::Array(vec![Value::Unsigned(0), Value::Unsigned(1)]),
-                ),
-                (Value::Unsigned(9), Value::Map(Vec::new())),
-                (
-                    Value::Unsigned(10),
-                    Value::Map(
-                        [(Value::Unsigned(2), Value::Unsigned(3))]
-                            .iter()
-                            .cloned()
-                            .collect(),
-                    ),
+                    Value::from(10),
+                    Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()),
                 ),
             ]
             .iter()
@@ -616,22 +589,19 @@ mod test {
     #[test]
     fn test_cbor_map_collection_empty() {
         let a = cbor_map_collection!(Vec::<(_, _)>::new());
-        let b = Value::Map(Vec::new());
+        let b = Value::map(Vec::new());
         assert_eq!(a, b);
     }
 
     #[test]
     fn test_cbor_map_collection_foo() {
-        let a = cbor_map_collection!(vec![(Value::Unsigned(2), Value::Unsigned(3))]);
-        let b = Value::Map(vec![(Value::Unsigned(2), Value::Unsigned(3))]);
+        let a = cbor_map_collection!(vec![(Value::from(2), Value::from(3))]);
+        let b = Value::map(vec![(Value::from(2), Value::from(3))]);
         assert_eq!(a, b);
     }
 
     fn extract_map(cbor_value: Value) -> Vec<(Value, Value)> {
-        match cbor_value {
-            Value::Map(map) => map,
-            _ => panic!("Expected CBOR map."),
-        }
+        cbor_value.extract_map().unwrap()
     }
 
     #[test]
diff --git a/libraries/cbor/src/reader.rs b/libraries/cbor/src/reader.rs
index 9d2a23e3..007de738 100644
--- a/libraries/cbor/src/reader.rs
+++ b/libraries/cbor/src/reader.rs
@@ -14,7 +14,7 @@
 
 //! Functionality for deserializing CBOR data into values.
 
-use super::values::{Constants, SimpleValue, Value};
+use super::values::{Constants, SimpleValue, Value, ValueImpl};
 use crate::{
     cbor_array_vec, cbor_bytes_lit, cbor_map_collection, cbor_tagged, cbor_text, cbor_unsigned,
 };
@@ -144,7 +144,7 @@ impl<'a> Reader<'a> {
         if signed_size < 0 {
             Err(DecoderError::OutOfRangeIntegerValue)
         } else {
-            Ok(Value::Negative(-(size_value as i64) - 1))
+            Ok(Value(ValueImpl::Negative(-(size_value as i64) - 1)))
         }
     }
 
@@ -221,7 +221,7 @@ impl<'a> Reader<'a> {
             return Err(DecoderError::UnsupportedFloatingPointValue);
         }
         match SimpleValue::from_integer(size_value) {
-            Some(simple_value) => Ok(Value::Simple(simple_value)),
+            Some(simple_value) => Ok(Value(ValueImpl::Simple(simple_value))),
             None => Err(DecoderError::UnsupportedSimpleValue),
         }
     }
diff --git a/libraries/cbor/src/values.rs b/libraries/cbor/src/values.rs
index ffdc4e4c..1bd705ab 100644
--- a/libraries/cbor/src/values.rs
+++ b/libraries/cbor/src/values.rs
@@ -19,9 +19,13 @@ use alloc::string::{String, ToString};
 use alloc::vec::Vec;
 use core::cmp::Ordering;
 
+/// The CBOR data structure.
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
+pub struct Value(pub(crate) ValueImpl);
+
 /// Possible CBOR values.
 #[derive(Clone, Debug)]
-pub enum Value {
+pub(crate) enum ValueImpl {
     /// Unsigned integer value (uint).
     Unsigned(u64),
     /// Signed integer value (nint). Only 63 bits of information are used here.
@@ -42,7 +46,7 @@ pub enum Value {
 
 /// Specific simple CBOR values.
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
-pub enum SimpleValue {
+pub(crate) enum SimpleValue {
     FalseValue = 20,
     TrueValue = 21,
     NullValue = 22,
@@ -75,119 +79,146 @@ impl Value {
     /// For simplicity, this only takes i64. Construct directly for the last bit.
     pub fn integer(int: i64) -> Value {
         if int >= 0 {
-            Value::Unsigned(int as u64)
+            Value(ValueImpl::Unsigned(int as u64))
         } else {
-            Value::Negative(int)
+            Value(ValueImpl::Negative(int))
         }
     }
 
+    /// Create a CBOR array value.
+    pub fn array(a: Vec<Value>) -> Value {
+        Value(ValueImpl::Array(a))
+    }
+
+    /// Create a CBOR map value.
+    pub fn map(m: Vec<(Value, Value)>) -> Value {
+        Value(ValueImpl::Map(m))
+    }
+
+    /// Create a CBOR tagged value.
+    pub fn tag(int: u64, value: Value) -> Value {
+        Value(ValueImpl::Tag(int, Box::new(value)))
+    }
+
     /// Create a CBOR boolean simple value.
     pub fn bool_value(b: bool) -> Value {
         if b {
-            Value::Simple(SimpleValue::TrueValue)
+            Value(ValueImpl::Simple(SimpleValue::TrueValue))
         } else {
-            Value::Simple(SimpleValue::FalseValue)
+            Value(ValueImpl::Simple(SimpleValue::FalseValue))
         }
     }
 
-    /// Return the major type for the [`Value`].
-    pub fn type_label(&self) -> u8 {
-        // TODO use enum discriminant instead when stable
-        // https://github.com/rust-lang/rust/issues/60553
-        match self {
-            Value::Unsigned(_) => 0,
-            Value::Negative(_) => 1,
-            Value::ByteString(_) => 2,
-            Value::TextString(_) => 3,
-            Value::Array(_) => 4,
-            Value::Map(_) => 5,
-            Value::Tag(_, _) => 6,
-            Value::Simple(_) => 7,
-        }
+    /// Creates a null value.
+    pub fn null_value() -> Value {
+        Value(ValueImpl::Simple(SimpleValue::NullValue))
+    }
+
+    /// Creates an undefined value.
+    pub fn undefined() -> Value {
+        Value(ValueImpl::Simple(SimpleValue::Undefined))
     }
 
     pub fn extract_unsigned(self) -> Option<u64> {
         match self {
-            Value::Unsigned(unsigned) => Some(unsigned),
+            Value(ValueImpl::Unsigned(unsigned)) => Some(unsigned),
             _ => None,
         }
     }
 
     pub fn extract_integer(self) -> Option<i64> {
         match self {
-            Value::Unsigned(unsigned) => {
+            Value(ValueImpl::Unsigned(unsigned)) => {
                 if unsigned <= core::i64::MAX as u64 {
                     Some(unsigned as i64)
                 } else {
                     None
                 }
             }
-            Value::Negative(signed) => Some(signed),
+            Value(ValueImpl::Negative(signed)) => Some(signed),
             _ => None,
         }
     }
 
     pub fn extract_byte_string(self) -> Option<Vec<u8>> {
         match self {
-            Value::ByteString(byte_string) => Some(byte_string),
+            Value(ValueImpl::ByteString(byte_string)) => Some(byte_string),
             _ => None,
         }
     }
 
     pub fn extract_text_string(self) -> Option<String> {
         match self {
-            Value::TextString(text_string) => Some(text_string),
+            Value(ValueImpl::TextString(text_string)) => Some(text_string),
             _ => None,
         }
     }
 
     pub fn extract_array(self) -> Option<Vec<Value>> {
         match self {
-            Value::Array(array) => Some(array),
+            Value(ValueImpl::Array(array)) => Some(array),
             _ => None,
         }
     }
 
     pub fn extract_map(self) -> Option<Vec<(Value, Value)>> {
         match self {
-            Value::Map(map) => Some(map),
+            Value(ValueImpl::Map(map)) => Some(map),
             _ => None,
         }
     }
 
     pub fn extract_tag(self) -> Option<(u64, Value)> {
         match self {
-            Value::Tag(tag, value) => Some((tag, *value)),
+            Value(ValueImpl::Tag(tag, value)) => Some((tag, *value)),
             _ => None,
         }
     }
 
     pub fn extract_bool(self) -> Option<bool> {
         match self {
-            Value::Simple(SimpleValue::FalseValue) => Some(false),
-            Value::Simple(SimpleValue::TrueValue) => Some(true),
+            Value(ValueImpl::Simple(SimpleValue::FalseValue)) => Some(false),
+            Value(ValueImpl::Simple(SimpleValue::TrueValue)) => Some(true),
             _ => None,
         }
     }
 
     pub fn extract_null(self) -> Option<()> {
         match self {
-            Value::Simple(SimpleValue::NullValue) => Some(()),
+            Value(ValueImpl::Simple(SimpleValue::NullValue)) => Some(()),
             _ => None,
         }
     }
 
     pub fn extract_undefined(self) -> Option<()> {
         match self {
-            Value::Simple(SimpleValue::Undefined) => Some(()),
+            Value(ValueImpl::Simple(SimpleValue::Undefined)) => Some(()),
             _ => None,
         }
     }
 }
 
-impl Ord for Value {
-    fn cmp(&self, other: &Value) -> Ordering {
-        use super::values::Value::{
+impl ValueImpl {
+    /// Return the major type for the [`ValueImpl`].
+    pub fn type_label(&self) -> u8 {
+        // TODO use enum discriminant instead when stable
+        // https://github.com/rust-lang/rust/issues/60553
+        match self {
+            ValueImpl::Unsigned(_) => 0,
+            ValueImpl::Negative(_) => 1,
+            ValueImpl::ByteString(_) => 2,
+            ValueImpl::TextString(_) => 3,
+            ValueImpl::Array(_) => 4,
+            ValueImpl::Map(_) => 5,
+            ValueImpl::Tag(_, _) => 6,
+            ValueImpl::Simple(_) => 7,
+        }
+    }
+}
+
+impl Ord for ValueImpl {
+    fn cmp(&self, other: &ValueImpl) -> Ordering {
+        use super::values::ValueImpl::{
             Array, ByteString, Map, Negative, Simple, Tag, TextString, Unsigned,
         };
         let self_type_value = self.type_label();
@@ -234,16 +265,16 @@ impl Ord for Value {
     }
 }
 
-impl PartialOrd for Value {
-    fn partial_cmp(&self, other: &Value) -> Option<Ordering> {
+impl PartialOrd for ValueImpl {
+    fn partial_cmp(&self, other: &ValueImpl) -> Option<Ordering> {
         Some(self.cmp(other))
     }
 }
 
-impl Eq for Value {}
+impl Eq for ValueImpl {}
 
-impl PartialEq for Value {
-    fn eq(&self, other: &Value) -> bool {
+impl PartialEq for ValueImpl {
+    fn eq(&self, other: &ValueImpl) -> bool {
         self.cmp(other) == Ordering::Equal
     }
 }
@@ -263,7 +294,7 @@ impl SimpleValue {
 
 impl From<u64> for Value {
     fn from(unsigned: u64) -> Self {
-        Value::Unsigned(unsigned)
+        Value(ValueImpl::Unsigned(unsigned))
     }
 }
 
@@ -281,37 +312,43 @@ impl From<i32> for Value {
 
 impl From<Vec<u8>> for Value {
     fn from(bytes: Vec<u8>) -> Self {
-        Value::ByteString(bytes)
+        Value(ValueImpl::ByteString(bytes))
     }
 }
 
 impl From<&[u8]> for Value {
     fn from(bytes: &[u8]) -> Self {
-        Value::ByteString(bytes.to_vec())
+        Value(ValueImpl::ByteString(bytes.to_vec()))
+    }
+}
+
+impl From<&[u8; 0]> for Value {
+    fn from(bytes: &[u8; 0]) -> Self {
+        Value(ValueImpl::ByteString(bytes.to_vec()))
     }
 }
 
 impl From<String> for Value {
     fn from(text: String) -> Self {
-        Value::TextString(text)
+        Value(ValueImpl::TextString(text))
     }
 }
 
 impl From<&str> for Value {
     fn from(text: &str) -> Self {
-        Value::TextString(text.to_string())
+        Value(ValueImpl::TextString(text.to_string()))
     }
 }
 
 impl From<Vec<Value>> for Value {
     fn from(array: Vec<Value>) -> Self {
-        Value::Array(array)
+        Value(ValueImpl::Array(array))
     }
 }
 
 impl From<Vec<(Value, Value)>> for Value {
     fn from(map: Vec<(Value, Value)>) -> Self {
-        Value::Map(map)
+        Value(ValueImpl::Map(map))
     }
 }
 
@@ -373,7 +410,7 @@ mod test {
     fn test_extract_unsigned() {
         assert_eq!(cbor_int!(1).extract_unsigned(), Some(1));
         assert_eq!(cbor_int!(-1).extract_unsigned(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_unsigned(), None);
+        assert_eq!(cbor_bytes!(b"").extract_unsigned(), None);
         assert_eq!(cbor_text!("").extract_unsigned(), None);
         assert_eq!(cbor_array![].extract_unsigned(), None);
         assert_eq!(cbor_map! {}.extract_unsigned(), None);
@@ -405,7 +442,7 @@ mod test {
     fn test_extract_integer() {
         assert_eq!(cbor_int!(1).extract_integer(), Some(1));
         assert_eq!(cbor_int!(-1).extract_integer(), Some(-1));
-        assert_eq!(cbor_bytes!(vec![]).extract_integer(), None);
+        assert_eq!(cbor_bytes!(b"").extract_integer(), None);
         assert_eq!(cbor_text!("").extract_integer(), None);
         assert_eq!(cbor_array![].extract_integer(), None);
         assert_eq!(cbor_map! {}.extract_integer(), None);
@@ -437,7 +474,7 @@ mod test {
     fn test_extract_byte_string() {
         assert_eq!(cbor_int!(1).extract_byte_string(), None);
         assert_eq!(cbor_int!(-1).extract_byte_string(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_byte_string(), Some(Vec::new()));
+        assert_eq!(cbor_bytes!(b"").extract_byte_string(), Some(Vec::new()));
         assert_eq!(
             cbor_bytes_lit!(b"bar").extract_byte_string(),
             Some(b"bar".to_vec())
@@ -453,7 +490,7 @@ mod test {
     fn test_extract_text_string() {
         assert_eq!(cbor_int!(1).extract_text_string(), None);
         assert_eq!(cbor_int!(-1).extract_text_string(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_text_string(), None);
+        assert_eq!(cbor_bytes!(b"").extract_text_string(), None);
         assert_eq!(cbor_text!("").extract_text_string(), Some(String::new()));
         assert_eq!(cbor_text!("s").extract_text_string(), Some("s".to_string()));
         assert_eq!(cbor_array![].extract_text_string(), None);
@@ -466,7 +503,7 @@ mod test {
     fn test_extract_array() {
         assert_eq!(cbor_int!(1).extract_array(), None);
         assert_eq!(cbor_int!(-1).extract_array(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_array(), None);
+        assert_eq!(cbor_bytes!(b"").extract_array(), None);
         assert_eq!(cbor_text!("").extract_array(), None);
         assert_eq!(cbor_array![].extract_array(), Some(Vec::new()));
         assert_eq!(
@@ -482,7 +519,7 @@ mod test {
     fn test_extract_map() {
         assert_eq!(cbor_int!(1).extract_map(), None);
         assert_eq!(cbor_int!(-1).extract_map(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_map(), None);
+        assert_eq!(cbor_bytes!(b"").extract_map(), None);
         assert_eq!(cbor_text!("").extract_map(), None);
         assert_eq!(cbor_array![].extract_map(), None);
         assert_eq!(cbor_map! {}.extract_map(), Some(Vec::new()));
@@ -498,7 +535,7 @@ mod test {
     fn test_extract_tag() {
         assert_eq!(cbor_int!(1).extract_tag(), None);
         assert_eq!(cbor_int!(-1).extract_tag(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_tag(), None);
+        assert_eq!(cbor_bytes!(b"").extract_tag(), None);
         assert_eq!(cbor_text!("").extract_tag(), None);
         assert_eq!(cbor_array![].extract_tag(), None);
         assert_eq!(cbor_map! {}.extract_tag(), None);
@@ -513,7 +550,7 @@ mod test {
     fn test_extract_bool() {
         assert_eq!(cbor_int!(1).extract_bool(), None);
         assert_eq!(cbor_int!(-1).extract_bool(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_bool(), None);
+        assert_eq!(cbor_bytes!(b"").extract_bool(), None);
         assert_eq!(cbor_text!("").extract_bool(), None);
         assert_eq!(cbor_array![].extract_bool(), None);
         assert_eq!(cbor_map! {}.extract_bool(), None);
@@ -528,7 +565,7 @@ mod test {
     fn test_extract_null() {
         assert_eq!(cbor_int!(1).extract_null(), None);
         assert_eq!(cbor_int!(-1).extract_null(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_null(), None);
+        assert_eq!(cbor_bytes!(b"").extract_null(), None);
         assert_eq!(cbor_text!("").extract_null(), None);
         assert_eq!(cbor_array![].extract_null(), None);
         assert_eq!(cbor_map! {}.extract_null(), None);
@@ -543,7 +580,7 @@ mod test {
     fn test_extract_undefined() {
         assert_eq!(cbor_int!(1).extract_undefined(), None);
         assert_eq!(cbor_int!(-1).extract_undefined(), None);
-        assert_eq!(cbor_bytes!(vec![]).extract_undefined(), None);
+        assert_eq!(cbor_bytes!(b"").extract_undefined(), None);
         assert_eq!(cbor_text!("").extract_undefined(), None);
         assert_eq!(cbor_array![].extract_undefined(), None);
         assert_eq!(cbor_map! {}.extract_undefined(), None);
@@ -567,8 +604,8 @@ mod test {
         assert!(cbor_int!(-24) < cbor_int!(-1000));
         assert!(cbor_int!(-1000) < cbor_int!(-1000000));
         assert!(cbor_int!(-1000000) < cbor_int!(core::i64::MIN));
-        assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(vec![]));
-        assert!(cbor_bytes!(vec![]) < cbor_bytes!(vec![0x00]));
+        assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(b""));
+        assert!(cbor_bytes!(b"") < cbor_bytes!(vec![0x00]));
         assert!(cbor_bytes!(vec![0x00]) < cbor_bytes!(vec![0x01]));
         assert!(cbor_bytes!(vec![0x01]) < cbor_bytes!(vec![0xFF]));
         assert!(cbor_bytes!(vec![0xFF]) < cbor_bytes!(vec![0x00, 0x00]));
@@ -599,8 +636,8 @@ mod test {
         assert!(cbor_map! {0 => 0} < cbor_tagged!(2, cbor_int!(0)));
         assert!(cbor_map! {0 => 0, 0 => 0} < cbor_bool!(false));
         assert!(cbor_bool!(false) < cbor_bool!(true));
-        assert!(cbor_bool!(true) < Value::Simple(SimpleValue::NullValue));
-        assert!(Value::Simple(SimpleValue::NullValue) < Value::Simple(SimpleValue::Undefined));
+        assert!(cbor_bool!(true) < cbor_null!());
+        assert!(cbor_null!() < cbor_undefined!());
         assert!(cbor_tagged!(1, cbor_text!("s")) < cbor_tagged!(2, cbor_int!(0)));
         assert!(cbor_int!(1) < cbor_int!(-1));
         assert!(cbor_int!(1) < cbor_bytes!(vec![0x00]));
diff --git a/libraries/cbor/src/writer.rs b/libraries/cbor/src/writer.rs
index 16a0f213..e3b37547 100644
--- a/libraries/cbor/src/writer.rs
+++ b/libraries/cbor/src/writer.rs
@@ -14,7 +14,7 @@
 
 //! Functionality for serializing CBOR values into bytes.
 
-use super::values::{Constants, Value};
+use super::values::{Constants, Value, ValueImpl};
 use alloc::vec::Vec;
 
 /// Possible errors from a serialization operation.
@@ -60,25 +60,25 @@ impl<'a> Writer<'a> {
         if remaining_depth.map_or(false, |d| d < 0) {
             return Err(EncoderError::TooMuchNesting);
         }
-        let type_label = value.type_label();
-        match value {
-            Value::Unsigned(unsigned) => self.start_item(type_label, unsigned),
-            Value::Negative(negative) => self.start_item(type_label, -(negative + 1) as u64),
-            Value::ByteString(byte_string) => {
+        let type_label = value.0.type_label();
+        match value.0 {
+            ValueImpl::Unsigned(unsigned) => self.start_item(type_label, unsigned),
+            ValueImpl::Negative(negative) => self.start_item(type_label, -(negative + 1) as u64),
+            ValueImpl::ByteString(byte_string) => {
                 self.start_item(type_label, byte_string.len() as u64);
                 self.encoded_cbor.extend(byte_string);
             }
-            Value::TextString(text_string) => {
+            ValueImpl::TextString(text_string) => {
                 self.start_item(type_label, text_string.len() as u64);
                 self.encoded_cbor.extend(text_string.into_bytes());
             }
-            Value::Array(array) => {
+            ValueImpl::Array(array) => {
                 self.start_item(type_label, array.len() as u64);
                 for el in array {
                     self.encode_cbor(el, remaining_depth.map(|d| d - 1))?;
                 }
             }
-            Value::Map(mut map) => {
+            ValueImpl::Map(mut map) => {
                 map.sort_by(|a, b| a.0.cmp(&b.0));
                 let map_len = map.len();
                 map.dedup_by(|a, b| a.0.eq(&b.0));
@@ -91,11 +91,11 @@ impl<'a> Writer<'a> {
                     self.encode_cbor(v, remaining_depth.map(|d| d - 1))?;
                 }
             }
-            Value::Tag(tag, inner_value) => {
+            ValueImpl::Tag(tag, inner_value) => {
                 self.start_item(type_label, tag);
                 self.encode_cbor(*inner_value, remaining_depth.map(|d| d - 1))?;
             }
-            Value::Simple(simple_value) => self.start_item(type_label, simple_value as u64),
+            ValueImpl::Simple(simple_value) => self.start_item(type_label, simple_value as u64),
         }
         Ok(())
     }
diff --git a/libraries/opensk/src/api/key_store.rs b/libraries/opensk/src/api/key_store.rs
index de150fd1..d001fcb3 100644
--- a/libraries/opensk/src/api/key_store.rs
+++ b/libraries/opensk/src/api/key_store.rs
@@ -404,17 +404,11 @@ impl From<StoreError> for Error {
 }
 
 fn extract_byte_string(cbor_value: cbor::Value) -> Result<Vec<u8>, Error> {
-    match cbor_value {
-        cbor::Value::ByteString(byte_string) => Ok(byte_string),
-        _ => Err(Error),
-    }
+    cbor_value.extract_byte_string().ok_or(Error)
 }
 
 fn extract_map(cbor_value: cbor::Value) -> Result<Vec<(cbor::Value, cbor::Value)>, Error> {
-    match cbor_value {
-        cbor::Value::Map(map) => Ok(map),
-        _ => Err(Error),
-    }
+    cbor_value.extract_map().ok_or(Error)
 }
 
 #[cfg(test)]

From ff3ccdefa9a00332e38511cb4690b3ff37a1db49 Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Fri, 21 Jul 2023 11:01:07 +0200
Subject: [PATCH 3/4] CBOR API fixes

---
 libraries/cbor/src/macros.rs |  67 ++++++++++++++--------
 libraries/cbor/src/values.rs | 105 +++++++++++++++++++++++++++++------
 libraries/cbor/src/writer.rs |  46 +--------------
 3 files changed, 133 insertions(+), 85 deletions(-)

diff --git a/libraries/cbor/src/macros.rs b/libraries/cbor/src/macros.rs
index e437a05a..090cbd71 100644
--- a/libraries/cbor/src/macros.rs
+++ b/libraries/cbor/src/macros.rs
@@ -139,9 +139,12 @@ macro_rules! assert_sorted_keys {
 ///
 /// Keys and values are expressions and converted into CBOR Keys and Values.
 /// The syntax for these pairs is `key_expression => value_expression,`.
-/// Duplicate keys will lead to invalid CBOR, i.e. writing these values fails.
 /// Keys do not have to be sorted.
 ///
+/// # Panics
+///
+/// You may not call this function with identical keys in its argument.
+///
 /// Example usage:
 ///
 /// ```rust
@@ -308,7 +311,7 @@ macro_rules! cbor_bool {
 #[macro_export]
 macro_rules! cbor_unsigned {
     ( $x:expr ) => {
-        $crate::values::Value::from($x)
+        $crate::values::Value::unsigned($x as u64)
     };
 }
 
@@ -316,7 +319,7 @@ macro_rules! cbor_unsigned {
 #[macro_export]
 macro_rules! cbor_int {
     ( $x:expr ) => {
-        $crate::values::Value::integer($x)
+        $crate::values::Value::integer($x as i64)
     };
 }
 
@@ -324,7 +327,7 @@ macro_rules! cbor_int {
 #[macro_export]
 macro_rules! cbor_text {
     ( $x:expr ) => {
-        $crate::values::Value::from($x)
+        $crate::values::Value::text_string($x.into())
     };
 }
 
@@ -332,7 +335,7 @@ macro_rules! cbor_text {
 #[macro_export]
 macro_rules! cbor_bytes {
     ( $x:expr ) => {
-        $crate::values::Value::from($x)
+        $crate::values::Value::byte_string($x)
     };
 }
 
@@ -501,23 +504,20 @@ mod test {
     #[test]
     fn test_cbor_map() {
         let a = cbor_map! {
-            -1 => -23,
             4 => 56,
-            "foo" => true,
-            b"bar" => cbor_null!(),
             5 => "foo",
             6 => b"bar",
             7 => cbor_array![],
             8 => cbor_array![0, 1],
             9 => cbor_map!{},
             10 => cbor_map!{2 => 3},
+            -1 => -23,
+            b"bar" => cbor_null!(),
+            "foo" => true,
         };
         let b = Value::map(
             [
-                (Value::from(-1), Value::from(-23)),
                 (Value::from(4), Value::from(56)),
-                (Value::from(String::from("foo")), Value::bool_value(true)),
-                (Value::from(b"bar".to_vec()), Value::null_value()),
                 (Value::from(5), Value::from(String::from("foo"))),
                 (Value::from(6), Value::from(b"bar".to_vec())),
                 (Value::from(7), Value::array(Vec::new())),
@@ -530,6 +530,9 @@ mod test {
                     Value::from(10),
                     Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()),
                 ),
+                (Value::from(-1), Value::from(-23)),
+                (Value::from(b"bar".to_vec()), Value::null_value()),
+                (Value::from(String::from("foo")), Value::bool_value(true)),
             ]
             .iter()
             .cloned()
@@ -541,31 +544,28 @@ mod test {
     #[test]
     fn test_cbor_map_options() {
         let a = cbor_map_options! {
-            -1 => -23,
             4 => Some(56),
+            5 => "foo",
+            6 => Some(b"bar" as &[u8]),
+            7 => cbor_array![],
+            8 => Some(cbor_array![0, 1]),
+            9 => cbor_map!{},
+            10 => Some(cbor_map!{2 => 3}),
             11 => None::<String>,
-            "foo" => true,
             12 => None::<&str>,
-            b"bar" => Some(cbor_null!()),
             13 => None::<Vec<u8>>,
-            5 => "foo",
             14 => None::<&[u8]>,
-            6 => Some(b"bar" as &[u8]),
             15 => None::<bool>,
-            7 => cbor_array![],
             16 => None::<i32>,
-            8 => Some(cbor_array![0, 1]),
             17 => None::<i64>,
-            9 => cbor_map!{},
             18 => None::<u64>,
-            10 => Some(cbor_map!{2 => 3}),
+            -1 => -23,
+            b"bar" => Some(cbor_null!()),
+            "foo" => true,
         };
         let b = Value::map(
             [
-                (Value::from(-1), Value::from(-23)),
                 (Value::from(4), Value::from(56)),
-                (Value::from(String::from("foo")), Value::bool_value(true)),
-                (Value::from(b"bar".to_vec()), Value::null_value()),
                 (Value::from(5), Value::from(String::from("foo"))),
                 (Value::from(6), Value::from(b"bar".to_vec())),
                 (Value::from(7), Value::array(Vec::new())),
@@ -578,6 +578,9 @@ mod test {
                     Value::from(10),
                     Value::map([(Value::from(2), Value::from(3))].iter().cloned().collect()),
                 ),
+                (Value::from(-1), Value::from(-23)),
+                (Value::from(b"bar".to_vec()), Value::null_value()),
+                (Value::from(String::from("foo")), Value::bool_value(true)),
             ]
             .iter()
             .cloned()
@@ -691,4 +694,22 @@ mod test {
         assert_eq!(x4, Some(cbor_unsigned!(40)));
         assert_eq!(x5, None);
     }
+
+    #[test]
+    fn test_destructure_unsorted_cbor_map() {
+        let map = cbor_map! {
+            2 => 20,
+            1 => 10,
+        };
+
+        destructure_cbor_map! {
+            let {
+                1 => x1,
+                2 => x2,
+            } = extract_map(map);
+        }
+
+        assert_eq!(x1, Some(cbor_unsigned!(10)));
+        assert_eq!(x2, Some(cbor_unsigned!(20)));
+    }
 }
diff --git a/libraries/cbor/src/values.rs b/libraries/cbor/src/values.rs
index 1bd705ab..7a08d85b 100644
--- a/libraries/cbor/src/values.rs
+++ b/libraries/cbor/src/values.rs
@@ -75,6 +75,11 @@ impl Constants {
 }
 
 impl Value {
+    /// Creates a CBOR unsigned value.
+    pub fn unsigned(int: u64) -> Value {
+        Value(ValueImpl::Unsigned(int))
+    }
+
     /// Create an appropriate CBOR integer value (uint/nint).
     /// For simplicity, this only takes i64. Construct directly for the last bit.
     pub fn integer(int: i64) -> Value {
@@ -85,13 +90,35 @@ impl Value {
         }
     }
 
+    /// Creates a CBOR byte string value.
+    pub fn byte_string(bytes: Vec<u8>) -> Value {
+        Value(ValueImpl::ByteString(bytes))
+    }
+
+    /// Creates a CBOR text string value.
+    pub fn text_string(text: String) -> Value {
+        Value(ValueImpl::TextString(text))
+    }
+
     /// Create a CBOR array value.
     pub fn array(a: Vec<Value>) -> Value {
         Value(ValueImpl::Array(a))
     }
 
     /// Create a CBOR map value.
-    pub fn map(m: Vec<(Value, Value)>) -> Value {
+    ///
+    /// Keys do not have to be sorted.
+    ///
+    /// # Panics
+    ///
+    /// You may not call this function with identical keys in its argument.
+    pub fn map(mut m: Vec<(Value, Value)>) -> Value {
+        m.sort_by(|a, b| a.0.cmp(&b.0));
+        let map_len = m.len();
+        m.dedup_by(|a, b| a.0.eq(&b.0));
+        if map_len != m.len() {
+            panic!();
+        }
         Value(ValueImpl::Map(m))
     }
 
@@ -293,8 +320,26 @@ impl SimpleValue {
 }
 
 impl From<u64> for Value {
-    fn from(unsigned: u64) -> Self {
-        Value(ValueImpl::Unsigned(unsigned))
+    fn from(u: u64) -> Self {
+        Value::unsigned(u)
+    }
+}
+
+impl From<u32> for Value {
+    fn from(u: u32) -> Self {
+        Value::unsigned(u as u64)
+    }
+}
+
+impl From<u16> for Value {
+    fn from(u: u16) -> Self {
+        Value::unsigned(u as u64)
+    }
+}
+
+impl From<u8> for Value {
+    fn from(u: u8) -> Self {
+        Value::unsigned(u as u64)
     }
 }
 
@@ -310,6 +355,18 @@ impl From<i32> for Value {
     }
 }
 
+impl From<i16> for Value {
+    fn from(i: i16) -> Self {
+        Value::integer(i as i64)
+    }
+}
+
+impl From<i8> for Value {
+    fn from(i: i8) -> Self {
+        Value::integer(i as i64)
+    }
+}
+
 impl From<Vec<u8>> for Value {
     fn from(bytes: Vec<u8>) -> Self {
         Value(ValueImpl::ByteString(bytes))
@@ -348,7 +405,7 @@ impl From<Vec<Value>> for Value {
 
 impl From<Vec<(Value, Value)>> for Value {
     fn from(map: Vec<(Value, Value)>) -> Self {
-        Value(ValueImpl::Map(map))
+        Value::map(map)
     }
 }
 
@@ -406,11 +463,23 @@ mod test {
     };
     use alloc::vec;
 
+    #[test]
+    #[should_panic]
+    fn test_duplicate_map_key() {
+        let _map = cbor_map! {
+            0 => "a",
+            -1 => "c",
+            b"a" => "e",
+            "c" => "g",
+            0 => "b",
+        };
+    }
+
     #[test]
     fn test_extract_unsigned() {
         assert_eq!(cbor_int!(1).extract_unsigned(), Some(1));
         assert_eq!(cbor_int!(-1).extract_unsigned(), None);
-        assert_eq!(cbor_bytes!(b"").extract_unsigned(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_unsigned(), None);
         assert_eq!(cbor_text!("").extract_unsigned(), None);
         assert_eq!(cbor_array![].extract_unsigned(), None);
         assert_eq!(cbor_map! {}.extract_unsigned(), None);
@@ -442,7 +511,7 @@ mod test {
     fn test_extract_integer() {
         assert_eq!(cbor_int!(1).extract_integer(), Some(1));
         assert_eq!(cbor_int!(-1).extract_integer(), Some(-1));
-        assert_eq!(cbor_bytes!(b"").extract_integer(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_integer(), None);
         assert_eq!(cbor_text!("").extract_integer(), None);
         assert_eq!(cbor_array![].extract_integer(), None);
         assert_eq!(cbor_map! {}.extract_integer(), None);
@@ -474,7 +543,7 @@ mod test {
     fn test_extract_byte_string() {
         assert_eq!(cbor_int!(1).extract_byte_string(), None);
         assert_eq!(cbor_int!(-1).extract_byte_string(), None);
-        assert_eq!(cbor_bytes!(b"").extract_byte_string(), Some(Vec::new()));
+        assert_eq!(cbor_bytes!(vec![]).extract_byte_string(), Some(Vec::new()));
         assert_eq!(
             cbor_bytes_lit!(b"bar").extract_byte_string(),
             Some(b"bar".to_vec())
@@ -490,7 +559,7 @@ mod test {
     fn test_extract_text_string() {
         assert_eq!(cbor_int!(1).extract_text_string(), None);
         assert_eq!(cbor_int!(-1).extract_text_string(), None);
-        assert_eq!(cbor_bytes!(b"").extract_text_string(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_text_string(), None);
         assert_eq!(cbor_text!("").extract_text_string(), Some(String::new()));
         assert_eq!(cbor_text!("s").extract_text_string(), Some("s".to_string()));
         assert_eq!(cbor_array![].extract_text_string(), None);
@@ -503,7 +572,7 @@ mod test {
     fn test_extract_array() {
         assert_eq!(cbor_int!(1).extract_array(), None);
         assert_eq!(cbor_int!(-1).extract_array(), None);
-        assert_eq!(cbor_bytes!(b"").extract_array(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_array(), None);
         assert_eq!(cbor_text!("").extract_array(), None);
         assert_eq!(cbor_array![].extract_array(), Some(Vec::new()));
         assert_eq!(
@@ -519,7 +588,7 @@ mod test {
     fn test_extract_map() {
         assert_eq!(cbor_int!(1).extract_map(), None);
         assert_eq!(cbor_int!(-1).extract_map(), None);
-        assert_eq!(cbor_bytes!(b"").extract_map(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_map(), None);
         assert_eq!(cbor_text!("").extract_map(), None);
         assert_eq!(cbor_array![].extract_map(), None);
         assert_eq!(cbor_map! {}.extract_map(), Some(Vec::new()));
@@ -535,7 +604,7 @@ mod test {
     fn test_extract_tag() {
         assert_eq!(cbor_int!(1).extract_tag(), None);
         assert_eq!(cbor_int!(-1).extract_tag(), None);
-        assert_eq!(cbor_bytes!(b"").extract_tag(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_tag(), None);
         assert_eq!(cbor_text!("").extract_tag(), None);
         assert_eq!(cbor_array![].extract_tag(), None);
         assert_eq!(cbor_map! {}.extract_tag(), None);
@@ -550,7 +619,7 @@ mod test {
     fn test_extract_bool() {
         assert_eq!(cbor_int!(1).extract_bool(), None);
         assert_eq!(cbor_int!(-1).extract_bool(), None);
-        assert_eq!(cbor_bytes!(b"").extract_bool(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_bool(), None);
         assert_eq!(cbor_text!("").extract_bool(), None);
         assert_eq!(cbor_array![].extract_bool(), None);
         assert_eq!(cbor_map! {}.extract_bool(), None);
@@ -565,7 +634,7 @@ mod test {
     fn test_extract_null() {
         assert_eq!(cbor_int!(1).extract_null(), None);
         assert_eq!(cbor_int!(-1).extract_null(), None);
-        assert_eq!(cbor_bytes!(b"").extract_null(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_null(), None);
         assert_eq!(cbor_text!("").extract_null(), None);
         assert_eq!(cbor_array![].extract_null(), None);
         assert_eq!(cbor_map! {}.extract_null(), None);
@@ -580,7 +649,7 @@ mod test {
     fn test_extract_undefined() {
         assert_eq!(cbor_int!(1).extract_undefined(), None);
         assert_eq!(cbor_int!(-1).extract_undefined(), None);
-        assert_eq!(cbor_bytes!(b"").extract_undefined(), None);
+        assert_eq!(cbor_bytes!(vec![]).extract_undefined(), None);
         assert_eq!(cbor_text!("").extract_undefined(), None);
         assert_eq!(cbor_array![].extract_undefined(), None);
         assert_eq!(cbor_map! {}.extract_undefined(), None);
@@ -604,8 +673,8 @@ mod test {
         assert!(cbor_int!(-24) < cbor_int!(-1000));
         assert!(cbor_int!(-1000) < cbor_int!(-1000000));
         assert!(cbor_int!(-1000000) < cbor_int!(core::i64::MIN));
-        assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(b""));
-        assert!(cbor_bytes!(b"") < cbor_bytes!(vec![0x00]));
+        assert!(cbor_int!(core::i64::MIN) < cbor_bytes!(vec![]));
+        assert!(cbor_bytes!(vec![]) < cbor_bytes!(vec![0x00]));
         assert!(cbor_bytes!(vec![0x00]) < cbor_bytes!(vec![0x01]));
         assert!(cbor_bytes!(vec![0x01]) < cbor_bytes!(vec![0xFF]));
         assert!(cbor_bytes!(vec![0xFF]) < cbor_bytes!(vec![0x00, 0x00]));
@@ -632,9 +701,9 @@ mod test {
         assert!(cbor_map! {"" => 0} < cbor_map! {cbor_array![] => 0});
         assert!(cbor_map! {cbor_array![] => 0} < cbor_map! {cbor_map!{} => 0});
         assert!(cbor_map! {cbor_map!{} => 0} < cbor_map! {false => 0});
-        assert!(cbor_map! {false => 0} < cbor_map! {0 => 0, 0 => 0});
+        assert!(cbor_map! {false => 0} < cbor_map! {0 => 0, 1 => 0});
         assert!(cbor_map! {0 => 0} < cbor_tagged!(2, cbor_int!(0)));
-        assert!(cbor_map! {0 => 0, 0 => 0} < cbor_bool!(false));
+        assert!(cbor_map! {0 => 0, 1 => 0} < cbor_bool!(false));
         assert!(cbor_bool!(false) < cbor_bool!(true));
         assert!(cbor_bool!(true) < cbor_null!());
         assert!(cbor_null!() < cbor_undefined!());
diff --git a/libraries/cbor/src/writer.rs b/libraries/cbor/src/writer.rs
index e3b37547..ac291351 100644
--- a/libraries/cbor/src/writer.rs
+++ b/libraries/cbor/src/writer.rs
@@ -78,14 +78,8 @@ impl<'a> Writer<'a> {
                     self.encode_cbor(el, remaining_depth.map(|d| d - 1))?;
                 }
             }
-            ValueImpl::Map(mut map) => {
-                map.sort_by(|a, b| a.0.cmp(&b.0));
-                let map_len = map.len();
-                map.dedup_by(|a, b| a.0.eq(&b.0));
-                if map_len != map.len() {
-                    return Err(EncoderError::DuplicateMapKey);
-                }
-                self.start_item(type_label, map_len as u64);
+            ValueImpl::Map(map) => {
+                self.start_item(type_label, map.len() as u64);
                 for (k, v) in map {
                     self.encode_cbor(k, remaining_depth.map(|d| d - 1))?;
                     self.encode_cbor(v, remaining_depth.map(|d| d - 1))?;
@@ -342,42 +336,6 @@ mod test {
         assert_eq!(write_return(sorted_map), write_return(unsorted_map));
     }
 
-    #[test]
-    fn test_write_map_duplicates() {
-        let duplicate0 = cbor_map! {
-            0 => "a",
-            -1 => "c",
-            b"a" => "e",
-            "c" => "g",
-            0 => "b",
-        };
-        assert_eq!(write_return(duplicate0), None);
-        let duplicate1 = cbor_map! {
-            0 => "a",
-            -1 => "c",
-            b"a" => "e",
-            "c" => "g",
-            -1 => "d",
-        };
-        assert_eq!(write_return(duplicate1), None);
-        let duplicate2 = cbor_map! {
-            0 => "a",
-            -1 => "c",
-            b"a" => "e",
-            "c" => "g",
-            b"a" => "f",
-        };
-        assert_eq!(write_return(duplicate2), None);
-        let duplicate3 = cbor_map! {
-            0 => "a",
-            -1 => "c",
-            b"a" => "e",
-            "c" => "g",
-            "c" => "h",
-        };
-        assert_eq!(write_return(duplicate3), None);
-    }
-
     #[test]
     fn test_write_map_with_array() {
         let value_map = cbor_map! {

From 420a150d59a1801380bfcccd38020c2c46c08c99 Mon Sep 17 00:00:00 2001
From: Fabian Kaczmarczyck <kaczmarczyck@google.com>
Date: Thu, 10 Aug 2023 17:38:13 +0200
Subject: [PATCH 4/4] README adapted to API changes

---
 libraries/cbor/README.md | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/libraries/cbor/README.md b/libraries/cbor/README.md
index d71810e5..6db38152 100644
--- a/libraries/cbor/README.md
+++ b/libraries/cbor/README.md
@@ -13,41 +13,26 @@ This crate implements Concise Binary Object Representation (CBOR) from [RFC
 
 ```rust
 fn main() {
-    // Build a CBOR object with various different types included. Note that this
+    // Build a CBOR object with the crate's convenience macros. Note that this
     // object is not built in canonical order.
-    let manual_object = Value::Map(vec![
-        (
-            Value::Unsigned(1),
-            Value::Array(vec![Value::Unsigned(2), Value::Unsigned(3)]),
-        ),
-        (
-            Value::TextString("tstr".to_owned()),
-            Value::ByteString(vec![1, 2, 3]),
-        ),
-        (Value::Negative(-2), Value::Simple(SimpleValue::NullValue)),
-        (Value::Unsigned(3), Value::Simple(SimpleValue::TrueValue)),
-    ]);
-
-    // Build the same object using the crate's convenience macros.
-    let macro_object = cbor_map! {
+    let map_object = cbor_map! {
         1 => cbor_array![2, 3],
         "tstr" => cbor_bytes!(vec![1, 2, 3]),
         -2 => cbor_null!(),
         3 => cbor_true!(),
     };
 
-    assert_eq!(manual_object, macro_object);
-    println!("Object {:?}", manual_object);
+    println!("Object {:?}", map_object);
 
     // Serialize to bytes.
-    let mut manual_data = vec![];
-    sk_cbor::writer::write(manual_object, &mut manual_data);
-    let hex_manual_data = hexify(&manual_data);
+    let mut map_data = vec![];
+    sk_cbor::writer::write(map_object, &mut map_data).unwrap();
+    let hex_map_data = hex::encode(&map_data);
 
     // Serialized version is in canonical order.
-    println!("Serializes to {}", hex_manual_data);
+    println!("Serializes to {}", hex_map_data);
     assert_eq!(
-        hex_manual_data,
+        hex_map_data,
         concat!(
             "a4",         // 4-map
             "01",         // int(1) =>
@@ -63,7 +48,7 @@ fn main() {
 
     // Convert back to an object.  This is different than the original object,
     // because the map is now in canonical order.
-    let recovered_object = sk_cbor::reader::read(&manual_data).unwrap();
+    let recovered_object = sk_cbor::reader::read(&map_data).unwrap();
     println!("Deserializes to {:?}", recovered_object);
 }
 ```