Correct bitfield bits position and manage bus endianness

yseraf · yseraf · commit 1a9cc1c10553 · 2022-05-01T12:57:43.000+02:00
While bitfield should follow exactly datasheet's information,
endianness must be handled for bus transport which is always little.

This fix converts endianness to/from bus for data to be handled
by arch endianness (big or little).

Signed-off-by: Yannick Lanz &lt;yannick.lanz@wifx.net&gt;
diff --git a/src/address.rs b/src/address.rs
@@ -68,17 +68,17 @@ bitfield! {
     #[derive(PartialEq, Clone)]
     pub struct OffsetAddress(u16);
     impl Debug;
-    u8, offset, set_offset: 10, 8;
-    u8, block, set_block: 12, 11;
+    u8, block, set_block: 4, 3;
+    u8, offset, set_offset: 2, 0;
 }
 
 bitfield! {
     #[derive(PartialEq, Clone)]
     pub struct DataAddress(u16);
     impl Debug;
-    u8, block, set_block: 3, 0;
-    u8, offset, set_offset: 10, 8;
-    u8, slot, set_slot: 14, 11;
+    u8, block, set_block: 11, 8;
+    u8, slot, set_slot: 6, 3;
+    u8, offset, set_offset: 2, 0;
 }
 
 impl From<&Address> for u16 {
diff --git a/src/command.rs b/src/command.rs
@@ -85,9 +85,9 @@ bitfield! {
     #[derive(PartialEq)]
     pub struct LockParam(u8);
     impl Debug;
-    u8, zone, set_zone: 1, 0;
-    u8, slot, set_slot: 5, 2;
     crc, set_crc: 7;
+    u8, slot, set_slot: 5, 2;
+    u8, zone, set_zone: 1, 0;
 }
 
 impl From<LockParam> for u8 {
@@ -138,7 +138,7 @@ macro_rules! put_cmd {
     ($dest:ident, $cmd:ident, $param1:expr, $param2:expr) => {
         $dest.put_u8($cmd);
         $dest.put_u8($param1);
-        $dest.put_u16($param2);
+        $dest.put_u16_le($param2);
     };
 }
 
@@ -189,7 +189,7 @@ impl EccCommand {
                 put_cmd!(bytes, ATCA_INFO, 0, 0);
             }
             Self::GenKey { key_type, slot } => {
-                put_cmd!(bytes, ATCA_GENKEY, u8::from(key_type), (*slot as u16) << 8);
+                put_cmd!(bytes, ATCA_GENKEY, u8::from(key_type), *slot as u16);
             }
             Self::Read { is_32, address } => {
                 let mut param1 = ReadWriteParam(0);
@@ -228,7 +228,7 @@ impl EccCommand {
                 let mut param1 = SignParam(0);
                 param1.set_source(source.into());
                 param1.set_external(true);
-                put_cmd!(bytes, ATCA_SIGN, u8::from(param1), (*key_slot as u16) << 8);
+                put_cmd!(bytes, ATCA_SIGN, u8::from(param1), *key_slot as u16);
             }
             Self::Ecdh { x, y, key_slot } => {
                 put_cmd!(bytes, ATCA_ECDH, 0, *key_slot as u16);
diff --git a/src/ecc.rs b/src/ecc.rs
@@ -4,7 +4,7 @@ use crate::{
     command::{EccCommand, EccResponse},
     Address, DataBuffer, Error, KeyConfig, Result, SlotConfig, Zone,
 };
-use bytes::{BufMut, Bytes, BytesMut};
+use bytes::{Buf, BufMut, Bytes, BytesMut};
 use sha2::{Digest, Sha256};
 
 pub use crate::command::KeyType;
@@ -47,8 +47,14 @@ impl Ecc {
         let bytes = self.read(false, &Address::slot_config(slot)?)?;
         let (s0, s1) = bytes.split_at(2);
         match slot & 1 == 0 {
-            true => Ok(SlotConfig::from(s0)),
-            false => Ok(SlotConfig::from(s1)),
+            true => {
+                let mut buf = s0;
+                Ok(SlotConfig::from(buf.get_u16_le()))
+            }
+            false => {
+                let mut buf = s1;
+                Ok(SlotConfig::from(buf.get_u16_le()))
+            }
         }
     }
 
@@ -59,12 +65,12 @@ impl Ecc {
         let mut new_bytes = BytesMut::with_capacity(4);
         match slot & 1 == 0 {
             true => {
-                new_bytes.put_u16(config.into());
+                new_bytes.put_u16_le(config.into());
                 new_bytes.extend_from_slice(s1);
             }
             false => {
                 new_bytes.extend_from_slice(s0);
-                new_bytes.put_u16(config.into());
+                new_bytes.put_u16_le(config.into());
             }
         }
         self.write(&slot_address, &new_bytes.freeze())
@@ -74,8 +80,14 @@ impl Ecc {
         let bytes = self.read(false, &Address::key_config(slot)?)?;
         let (s0, s1) = bytes.split_at(2);
         match slot & 1 == 0 {
-            true => Ok(KeyConfig::from(s0)),
-            false => Ok(KeyConfig::from(s1)),
+            true => {
+                let mut buf = s0;
+                Ok(KeyConfig::from(buf.get_u16_le()))
+            }
+            false => {
+                let mut buf = s1;
+                Ok(KeyConfig::from(buf.get_u16_le()))
+            }
         }
     }
 
@@ -86,12 +98,12 @@ impl Ecc {
         let mut new_bytes = BytesMut::with_capacity(4);
         match slot & 1 == 0 {
             true => {
-                new_bytes.put_u16(config.into());
+                new_bytes.put_u16_le(config.into());
                 new_bytes.extend_from_slice(s1);
             }
             false => {
                 new_bytes.extend_from_slice(s0);
-                new_bytes.put_u16(config.into());
+                new_bytes.put_u16_le(config.into());
             }
         }
         self.write(&slot_address, &new_bytes.freeze())
@@ -101,8 +113,8 @@ impl Ecc {
         let bytes = self.read(false, &Address::config(2, 5)?)?;
         let (_, s1) = bytes.split_at(2);
         match zone {
-            Zone::Config => Ok(s1[1] == 0),
-            Zone::Data => Ok(s1[0] == 0),
+            Zone::Config => Ok(s1[1] != 0x55),
+            Zone::Data => Ok(s1[0] != 0x55),
         }
     }
 
diff --git a/src/key_config.rs b/src/key_config.rs
@@ -27,6 +27,8 @@ impl From<KeyConfigType> for u8 {
     }
 }
 
+// Should probably be removed as it should manage bus endianness while
+// this responsability should not be there
 impl From<&[u8]> for KeyConfig {
     fn from(v: &[u8]) -> Self {
         let mut buf = v;
@@ -38,17 +40,15 @@ bitfield! {
     #[derive(PartialEq)]
     pub struct KeyConfig(u16);
     impl Debug;
-
-    pub u8, auth_key, set_auth_key: 3, 0;
-    pub intrusion_disable, set_intrusion_disable: 4;
-    pub u8, x509_index, set_x509_index: 7, 6;
-
-    pub private, set_private: 8;
-    pub pub_info, set_pub_info: 9;
-    pub u8, from into KeyConfigType, key_type, set_key_type: 12, 10;
-    pub lockable, set_is_lockable: 13;
-    pub req_random, set_req_random: 14;
-    pub req_auth, set_req_auth: 15;
+    pub u8, x509_index, set_x509_index: 15,14;
+    pub intrusion_disable, set_intrusion_disable: 12;
+    pub u8, auth_key, set_auth_key:11, 8;
+    pub req_auth, set_req_auth: 7;
+    pub req_random, set_req_random: 6;
+    pub lockable, set_is_lockable: 5;
+    pub u8, from into KeyConfigType, key_type, set_key_type: 4, 2;
+    pub pub_info, set_pub_info: 1;
+    pub private, set_private: 0;
 }
 
 impl From<u16> for KeyConfig {
diff --git a/src/slot_config.rs b/src/slot_config.rs
@@ -223,15 +223,17 @@ bitfield! {
     #[derive(PartialEq)]
     pub struct SlotConfig(u16);
     impl Debug;
-    pub secret, set_secret: 15;
-    pub encrypt_read, set_encrypt_read: 14;
-    pub limited_use, set_limited_use: 13;
-    pub no_mac, set_no_mac: 12;
-    pub u8, from into ReadKey, read_key, set_read_key: 11, 8;
-    u8, _write_config, _set_write_config: 7, 4;
-    pub u8, write_key, set_write_key: 3, 0;
+    u8, _write_config, _set_write_config: 15, 12;
+    pub u8, write_key, set_write_key: 11, 8;
+    pub secret, set_secret: 7;
+    pub encrypt_read, set_encrypt_read: 6;
+    pub limited_use, set_limited_use: 5;
+    pub no_mac, set_no_mac: 4;
+    pub u8, from into ReadKey, read_key, set_read_key: 3, 0;
 }
 
+// Should probably be removed as it should manage bus endianness while
+// this responsability should not be there
 impl From<&[u8]> for SlotConfig {
     fn from(v: &[u8]) -> Self {
         let mut buf = v;
