fix: sign raw bytes (#1332)

* fix: sign raw bytes * cleanup * fix signing
novasamatech · Oct 6, 2022 · 01b5902 · 01b5902
1 parent b4a44c8
commit 01b5902
Show file tree

Hide file tree

Showing 9 changed files with 124 additions and 59 deletions.
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
diff --git a/rust/db_handling/src/db_transactions.rs b/rust/db_handling/src/db_transactions.rs
@@ -844,7 +844,7 @@ pub struct TrDbColdSign {
 ///
 /// Messages contain SCALE-encoded text messages.
 #[cfg(feature = "signer")]
-#[derive(Debug, Decode, Encode)]
+#[derive(Debug, Decode, Encode, Clone)]
 pub enum SignContent {
     /// `53xx00` or `53xx02` transaction
     Transaction {

diff --git a/rust/navigator/src/tests.rs b/rust/navigator/src/tests.rs
@@ -137,14 +137,20 @@ fn signature_is_good(transaction_hex: &str, signature_hex: &str) -> bool {
             sp_core::ed25519::Pair::verify(&signature, &message, &public)
         }
         "5301" => {
-            assert!(
-                signature_hex.starts_with("01"),
-                "Signature in sr25519 should start with `01`."
-            );
-            let into_signature: [u8; 64] = hex::decode(&signature_hex[2..])
-                .unwrap()
-                .try_into()
-                .unwrap();
+            let into_signature: [u8; 64] = match &transaction_hex[4..6] {
+                "03" => hex::decode(&signature_hex).unwrap().try_into().unwrap(), // raw message
+                _ => {
+                    assert!(
+                        signature_hex.starts_with("01"),
+                        "Signature in sr25519 should start with `01`."
+                    );
+                    hex::decode(&signature_hex[2..])
+                        .unwrap()
+                        .try_into()
+                        .unwrap()
+                }
+            };
+
             let signature = sp_core::sr25519::Signature::from_raw(into_signature);
             let into_public: [u8; 32] = hex::decode(&transaction_hex[6..70])
                 .unwrap()
@@ -5601,9 +5607,10 @@ fn flow_test_1() {
 
     // let's scan a text message
     do_action(Action::NavbarScan, "", "").unwrap().unwrap();
-    let message_hex = "5301033efeca331d646d8a2986374bb3bb8d6e9e3cfcdd7c45c2b69104fab5d61d3f34f5064c6f72656d20697073756d20646f6c6f722073697420616d65742c20636f6e73656374657475722061646970697363696e6720656c69742c2073656420646f20656975736d6f642074656d706f7220696e6369646964756e74207574206c61626f726520657420646f6c6f7265206d61676e6120616c697175612e20557420656e696d206164206d696e696d2076656e69616d2c2071756973206e6f737472756420657865726369746174696f6e20756c6c616d636f206c61626f726973206e69736920757420616c697175697020657820656120636f6d6d6f646f20636f6e7365717561742e2044756973206175746520697275726520646f6c6f7220696e20726570726568656e646572697420696e20766f6c7570746174652076656c697420657373652063696c6c756d20646f6c6f726520657520667567696174206e756c6c612070617269617475722e204578636570746575722073696e74206f6363616563617420637570696461746174206e6f6e2070726f6964656e742c2073756e7420696e2063756c706120717569206f666669636961206465736572756e74206d6f6c6c697420616e696d20696420657374206c61626f72756d2ee143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e";
-    let card_text = "4c6f72656d20697073756d20646f6c6f722073697420616d65742c20636f6e73656374657475722061646970697363696e6720656c69742c2073656420646f20656975736d6f642074656d706f7220696e6369646964756e74207574206c61626f726520657420646f6c6f7265206d61676e6120616c697175612e20557420656e696d206164206d696e696d2076656e69616d2c2071756973206e6f737472756420657865726369746174696f6e20756c6c616d636f206c61626f726973206e69736920757420616c697175697020657820656120636f6d6d6f646f20636f6e7365717561742e2044756973206175746520697275726520646f6c6f7220696e20726570726568656e646572697420696e20766f6c7570746174652076656c697420657373652063696c6c756d20646f6c6f726520657520667567696174206e756c6c612070617269617475722e204578636570746575722073696e74206f6363616563617420637570696461746174206e6f6e2070726f6964656e742c2073756e7420696e2063756c706120717569206f666669636961206465736572756e74206d6f6c6c697420616e696d20696420657374206c61626f72756d2e".to_string();
-    let action = do_action(Action::TransactionFetched, message_hex, "")
+    let card_text = hex::encode(b"uuid-abcd");
+    let sign_msg = hex::encode(b"<Bytes>uuid-abcd</Bytes>");
+    let message_hex = format!("5301033efeca331d646d8a2986374bb3bb8d6e9e3cfcdd7c45c2b69104fab5d61d3f34{}e143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e", sign_msg);
+    let action = do_action(Action::TransactionFetched, &message_hex, "")
         .unwrap()
         .unwrap();
     let expected_action = ActionResult {
@@ -5679,7 +5686,7 @@ fn flow_test_1() {
     */
 
     assert!(
-        signature_is_good(message_hex, &signature_hex),
+        signature_is_good(&message_hex, &signature_hex),
         "Produced bad signature: \n{}",
         signature_hex
     );
@@ -5702,7 +5709,7 @@ fn flow_test_1() {
                         timestamp: String::new(),
                         events: vec![Event::MessageSigned {
                             sign_message_display: SignMessageDisplay {
-                                message: String::from_utf8(hex::decode(&card_text).unwrap())
+                                message: String::from_utf8(hex::decode(&sign_msg).unwrap())
                                     .unwrap(),
                                 network_name: "westend".to_string(),
                                 signed_by: VerifierValue::Standard {
@@ -5748,7 +5755,7 @@ fn flow_test_1() {
                 events: vec![MEventMaybeDecoded {
                     event: Event::MessageSigned {
                         sign_message_display: SignMessageDisplay {
-                            message: String::from_utf8(hex::decode(&card_text).unwrap()).unwrap(),
+                            message: String::from_utf8(hex::decode(&sign_msg).unwrap()).unwrap(),
                             network_name: "westend".to_string(),
                             signed_by: VerifierValue::Standard {
                                 m: MultiSigner::Sr25519(
@@ -6307,9 +6314,7 @@ fn flow_test_1() {
                     message: Some(vec![TransactionCard {
                         index: 0,
                         indent: 0,
-                        card: Card::TextCard {
-                            f: card_text.clone(),
-                        },
+                        card: Card::TextCard { f: card_text },
                     }]),
                     ..Default::default()
                 },
@@ -6442,7 +6447,7 @@ fn flow_test_1() {
     };
     let network_genesis_hash =
         H256::from_str("e143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e").unwrap();
-    let message = String::from_utf8(hex::decode(&card_text).unwrap()).unwrap();
+    let message = String::from_utf8(hex::decode(&sign_msg).unwrap()).unwrap();
     let expected_action = ActionResult {
         screen_label: String::new(),
         back: false,
@@ -6685,7 +6690,7 @@ fn flow_test_1() {
                         timestamp: String::new(),
                         events: vec![Event::MessageSigned {
                             sign_message_display: SignMessageDisplay {
-                                message: String::from_utf8_lossy(&hex::decode(&card_text).unwrap())
+                                message: String::from_utf8_lossy(&hex::decode(&sign_msg).unwrap())
                                     .to_string(),
                                 network_name: "westend".to_string(),
                                 signed_by: verifier_value,

diff --git a/rust/transaction_parsing/Cargo.toml b/rust/transaction_parsing/Cargo.toml
@@ -18,6 +18,7 @@ sled = "0.34.6"
 sp-core = {git = "https://github.com/paritytech/substrate", default-features = false, features = ["full_crypto"]}
 sp-runtime = {git = "https://github.com/paritytech/substrate", default-features = false}
 thiserror = "1.0.37"
+nom = "7.1.1"
 
 [dev-dependencies]
 pretty_assertions = "1"

diff --git a/rust/transaction_parsing/src/error.rs b/rust/transaction_parsing/src/error.rs
@@ -577,6 +577,12 @@ pub enum Error {
         /// network supported encryption
         encryption: Encryption,
     },
+
+    #[error(transparent)]
+    NotUtf8(#[from] std::str::Utf8Error),
+
+    #[error("Parser error: {0}")]
+    ParserError(String),
 }
 
 fn display_parsing_errors(network_name: &str, errors: &[(u32, parser::Error)]) -> String {

diff --git a/rust/transaction_parsing/src/message.rs b/rust/transaction_parsing/src/message.rs
@@ -6,9 +6,13 @@ use definitions::{
     keyring::{AddressKey, NetworkSpecsKey},
     navigation::TransactionCardSet,
 };
-use parity_scale_codec::DecodeAll;
+use nom::bytes::complete::{tag, take_until};
 use parser::cards::ParserCard;
 use std::path::Path;
+use std::str;
+
+use nom::sequence::preceded;
+use nom::IResult;
 
 use crate::cards::{make_author_info, Card, Warning};
 use crate::error::{Error, Result};
@@ -23,11 +27,8 @@ where
         multisigner_msg_genesis_encryption(data_hex)?;
     let network_specs_key = NetworkSpecsKey::from_parts(&genesis_hash, &encryption);
 
-    // this is a standard decoding of String, with utf8 conversion;
-    // processing input vec![20, 104, 101, 3, 108, 111] will not throw error at element `3`,
-    // it will result in output `helo` instead, length, however, is still correct, 5.
-    // note that some invisible symbols may thus sneak into the message;
-    let message = String::decode_all(&mut &message_vec[..])?;
+    let message = str::from_utf8(&message_vec)?.to_owned();
+    let display_msg = strip_bytes_tag(&message)?;
 
     // initialize index and indent
     let mut index: u32 = 0;
@@ -39,7 +40,7 @@ where
             match try_get_address_details(&db_path, &address_key)? {
                 Some(address_details) => {
                     if address_details.network_id.contains(&network_specs_key) {
-                        let message_card = Card::ParserCard(&ParserCard::Text(message.to_string()))
+                        let message_card = Card::ParserCard(&ParserCard::Text(display_msg))
                             .card(&mut index, indent);
                         let sign = TrDbColdSign::generate(
                             SignContent::Message(message),
@@ -136,3 +137,40 @@ where
         }
     }
 }
+
+fn bytes_prefix(i: &str) -> IResult<&str, &str> {
+    tag("<Bytes>")(i)
+}
+
+fn take_until_suffix(i: &str) -> IResult<&str, &str> {
+    take_until("</Bytes>")(i)
+}
+
+fn strip_bytes_tag(message: &str) -> Result<String> {
+    let mut parser = preceded(bytes_prefix, take_until_suffix);
+    let (_, payload) = parser(message).map_err(|e| Error::ParserError(format!("{:?}", e)))?;
+    Ok(payload.to_owned())
+}
+
+#[cfg(test)]
+mod tests {
+
+    use super::*;
+
+    #[test]
+    fn parse_bytes_msg() {
+        let result = strip_bytes_tag("<Bytes>uuid-1234</Bytes>");
+        assert!(result.is_ok());
+        assert_eq!(result.unwrap(), "uuid-1234");
+    }
+
+    #[test]
+    fn parse_bytes_err() {
+        let result = strip_bytes_tag("<Bytes>uuid-1234");
+        assert!(result.is_err());
+        assert_eq!(
+            result.unwrap_err().to_string(),
+            "Parser error: Error(Error { input: \"uuid-1234\", code: TakeUntil })"
+        );
+    }
+}
diff --git a/rust/transaction_parsing/src/tests.rs b/rust/transaction_parsing/src/tests.rs
@@ -2381,9 +2381,9 @@ fn parse_transaction_8_error_on_parsing() {
 fn parse_msg_1() {
     let dbname = "for_tests/parse_msg_1";
     populate_cold(dbname, Verifier { v: None }).unwrap();
-    let line = "530103d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27df5064c6f72656d20697073756d20646f6c6f722073697420616d65742c20636f6e73656374657475722061646970697363696e6720656c69742c2073656420646f20656975736d6f642074656d706f7220696e6369646964756e74207574206c61626f726520657420646f6c6f7265206d61676e6120616c697175612e20557420656e696d206164206d696e696d2076656e69616d2c2071756973206e6f737472756420657865726369746174696f6e20756c6c616d636f206c61626f726973206e69736920757420616c697175697020657820656120636f6d6d6f646f20636f6e7365717561742e2044756973206175746520697275726520646f6c6f7220696e20726570726568656e646572697420696e20766f6c7570746174652076656c697420657373652063696c6c756d20646f6c6f726520657520667567696174206e756c6c612070617269617475722e204578636570746575722073696e74206f6363616563617420637570696461746174206e6f6e2070726f6964656e742c2073756e7420696e2063756c706120717569206f666669636961206465736572756e74206d6f6c6c697420616e696d20696420657374206c61626f72756d2ee143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e";
-
-    let text = "4c6f72656d20697073756d20646f6c6f722073697420616d65742c20636f6e73656374657475722061646970697363696e6720656c69742c2073656420646f20656975736d6f642074656d706f7220696e6369646964756e74207574206c61626f726520657420646f6c6f7265206d61676e6120616c697175612e20557420656e696d206164206d696e696d2076656e69616d2c2071756973206e6f737472756420657865726369746174696f6e20756c6c616d636f206c61626f726973206e69736920757420616c697175697020657820656120636f6d6d6f646f20636f6e7365717561742e2044756973206175746520697275726520646f6c6f7220696e20726570726568656e646572697420696e20766f6c7570746174652076656c697420657373652063696c6c756d20646f6c6f726520657520667567696174206e756c6c612070617269617475722e204578636570746575722073696e74206f6363616563617420637570696461746174206e6f6e2070726f6964656e742c2073756e7420696e2063756c706120717569206f666669636961206465736572756e74206d6f6c6c697420616e696d20696420657374206c61626f72756d2e".to_string();
+    let sign_msg = hex::encode(b"<Bytes>uuid-abcd</Bytes>");
+    let text = hex::encode(b"uuid-abcd");
+    let line = format!("530103d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d{}e143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e", sign_msg);
 
     let set_expected = TransactionCardSet {
         message: Some(vec![TransactionCard {
@@ -2405,7 +2405,7 @@ fn parse_msg_1() {
     };
 
     let network_info_known = westend_spec();
-    let action = produce_output(line, dbname);
+    let action = produce_output(&line, dbname);
 
     if let TransactionAction::Sign {
         content: set,
@@ -2430,18 +2430,19 @@ fn parse_msg_2() {
     let dbname = "for_tests/parse_msg_2";
     populate_cold(dbname, Verifier { v: None }).unwrap();
     // sneaking one extra byte in the text body
-    let line = "530103d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27df5064c6f72656d20697073756d20646f6c6f722073697420616d65742c20636f6e73656374657475722061646970697363696e6720656c69742c2073656420646f20656975736d6f642074656d706f7220696e6369646964756e74207574206c61626f726520657420646f6c6f7265206d61676e6120616c697175612e20557420656e696d206164206d696e696d2076656e69616d2c2071756973206e6f737472756420657865726369746174696f6e20756c6c616d636f206c61626f726973206e69736920757420616c697175697020657820656120636f6d6d6f646f20636f6e7365717561742e2044756973206175746520697275726520646f6c6f7220696e20726570726568656e646572697420696e20766f6c7570746174652076656c697420657373652063696c6c756d20646f6c6f726520657520667567696174206e756c6c612070617269617475722e204578636570746575722073696e74206f6363616563617420637570696461746174206e6f6e2070726f6964656e742c2073756e7420696e2063756c706120717569206f666669636961206465736572756e74206d6f6c6c697420616e696d20696420657374206c6c61626f72756d2ee143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e";
+    let sign_msg = hex::encode(b"<Bytes>uuid-abcd");
+    let line = format!("530103d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d{}e143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e", sign_msg);
     let set_expected = TransactionCardSet {
         error: Some(vec![TransactionCard {
             index: 0,
             indent: 0,
             card: Card::ErrorCard {
-                f: "Bad input data. Received message could not be read. Input buffer has still data left after decoding!".to_string(),
+                f: "Bad input data. Parser error: Error(Error { input: \"uuid-abcd\", code: TakeUntil })".to_string(),
             },
         }]),
         ..Default::default()
     };
-    let action = produce_output(line, dbname);
+    let action = produce_output(&line, dbname);
     if let TransactionAction::Read { r: set } = action {
         assert_eq!(set, set_expected);
     } else {

diff --git a/rust/transaction_signing/src/sign_transaction.rs b/rust/transaction_signing/src/sign_transaction.rs
@@ -18,7 +18,7 @@ pub(crate) fn create_signature(
     user_comment: &str,
     database_name: &str,
     checksum: u32,
-) -> Result<MultiSignature> {
+) -> Result<String> {
     let sign = TrDbColdSign::from_storage(database_name, checksum)?;
     let pwd = {
         if sign.has_pwd() {
@@ -27,11 +27,12 @@ pub(crate) fn create_signature(
             None
         }
     };
-    let content_vec = match sign.content() {
+    let content = sign.content().to_owned();
+    let content_vec = match &content {
         SignContent::Transaction { method, extensions } => {
             [method.to_vec(), extensions.to_vec()].concat()
         }
-        SignContent::Message(a) => a.encode(),
+        SignContent::Message(a) => a.as_bytes().to_vec(),
     };
 
     // For larger transactions, their hash should be signed instead; this is not implemented
@@ -44,7 +45,8 @@ pub(crate) fn create_signature(
         }
     };
     let mut full_address = seed_phrase.to_owned() + &sign.path();
-    match sign_as_address_key(&content_vec, &sign.multisigner(), &full_address, pwd) {
+    let signature = match sign_as_address_key(&content_vec, &sign.multisigner(), &full_address, pwd)
+    {
         Ok(s) => {
             full_address.zeroize();
             sign.apply(false, user_comment, database_name)?;
@@ -59,7 +61,20 @@ pub(crate) fn create_signature(
                 Err(e)
             }
         }
-    }
+    }?;
+
+    let encoded = match &content {
+        SignContent::Transaction {
+            method: _,
+            extensions: _,
+        } => hex::encode(signature.encode()),
+        SignContent::Message(_) => match signature {
+            MultiSignature::Sr25519(a) => hex::encode(a),
+            MultiSignature::Ed25519(a) => hex::encode(a),
+            MultiSignature::Ecdsa(a) => hex::encode(a),
+        },
+    };
+    Ok(encoded)
 }
 
 pub fn create_signature_png(
@@ -69,16 +84,13 @@ pub fn create_signature_png(
     database_name: &str,
     checksum: u32,
 ) -> Result<Vec<u8>> {
-    let hex_result = hex::encode(
-        create_signature(
-            seed_phrase,
-            pwd_entry,
-            user_comment,
-            database_name,
-            checksum,
-        )?
-        .encode(),
-    );
+    let hex_result = create_signature(
+        seed_phrase,
+        pwd_entry,
+        user_comment,
+        database_name,
+        checksum,
+    )?;
     let qr_data = png_qr_from_string(&hex_result, DataType::Regular)?;
     Ok(qr_data)
 }