Merge #606

606: Problem: full client flow not tested with "better" mock encryption+tx-query (CRO-510) r=tomtau a=tomtau

Solution:
- still WIP -- didn't turn it on yet, as it'll require modifications in integration test deployments etc.
- there were small problems discovered in full flow testing that should be fixed in this PR:
1) dummy signer didn't follow the cryptographic padding convention, so the estimate fee would sometimes be wrong
2) client couldn't connect to IP addresses, as they are not valid hostnames (hostname required by the TLS library)
3) tx-query randomly failed some requests (witness on CI as well) -- probably due to trying to parse the whole allocated buffer (rather than just the actual request length slice)


Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>

chain-tx-enclave/tx-query/enclave/src/lib.rs

@@ -100,8 +100,8 @@ fn handle_decryption_request(
     }
 
     match tls.read(&mut plain) {
-        Ok(_) => {
-            if let Ok(dr) = DecryptionRequest::decode(&mut plain.as_slice()) {
+        Ok(l) => {
+            if let Ok(dr) = DecryptionRequest::decode(&mut &plain.as_slice()[0..l]) {
                 if dr
                     .verify(&Secp256k1::verification_only(), challenge)
                     .is_err()
@@ -269,7 +269,7 @@ pub extern "C" fn run_server(socket_fd: c_int) -> sgx_status_t {
     let mut tls = rustls::Stream::new(&mut sess, &mut conn);
     let mut plain = vec![0; ENCRYPTION_REQUEST_SIZE];
     match tls.read(&mut plain) {
-        Ok(_) => match TxQueryInitRequest::decode(&mut plain.as_slice()) {
+        Ok(l) => match TxQueryInitRequest::decode(&mut &plain.as_slice()[0..l]) {
             Ok(TxQueryInitRequest::Encrypt(req)) => handle_encryption_request(&mut tls, *req),
             Ok(TxQueryInitRequest::DecryptChallenge) => handle_decryption_request(&mut tls, plain),
             _ => {

client-core/src/cipher/default.rs

@@ -35,16 +35,24 @@ fn get_tls_config() -> Arc<rustls::ClientConfig> {
 #[derive(Debug, Clone)]
 pub struct DefaultTransactionObfuscation {
     tqe_address: String,
-    tqe_hostname: String,
+    tqe_hostname: webpki::DNSName,
 }
 
 impl DefaultTransactionObfuscation {
     /// tqe_address: connection string <HOST/IP:PORT>
     /// tqe_hostname: expected hostname (e.g. localhost in testing)
     pub fn new(tqe_address: String, tqe_hostname: String) -> Self {
+        // one may just write an ip address instead of a domain name, which isn't a valid DNS name
+        // so there's a default case
+        // TODO: should TQE enforce valid domain names,
+        // as some of the infra may not be assigned a domain name
+        // and the TLS checking is augmented with attestation anyway?
+        let dns_name = webpki::DNSNameRef::try_from_ascii_str(&tqe_hostname)
+            .unwrap_or_else(|_| webpki::DNSNameRef::try_from_ascii_str("localhost").unwrap())
+            .to_owned();
         DefaultTransactionObfuscation {
             tqe_address,
-            tqe_hostname,
+            tqe_hostname: dns_name,
         }
     }
 }
@@ -56,12 +64,7 @@ impl TransactionObfuscation for DefaultTransactionObfuscation {
         private_key: &PrivateKey,
     ) -> Result<Vec<Transaction>> {
         let client_config = get_tls_config();
-        let dns_name = webpki::DNSNameRef::try_from_ascii_str(&self.tqe_hostname).chain(|| {
-            (
-                ErrorKind::InvalidInput,
-                format!("Invalid TQE hostname: {}", self.tqe_hostname),
-            )
-        })?;
+        let dns_name = self.tqe_hostname.as_ref();
         let mut sess = rustls::ClientSession::new(&client_config, dns_name);
 
         let mut conn = TcpStream::connect(&self.tqe_address).chain(|| {
@@ -139,12 +142,7 @@ impl TransactionObfuscation for DefaultTransactionObfuscation {
 
     fn encrypt(&self, transaction: SignedTransaction) -> Result<TxAux> {
         let client_config = get_tls_config();
-        let dns_name = webpki::DNSNameRef::try_from_ascii_str(&self.tqe_hostname).chain(|| {
-            (
-                ErrorKind::InvalidInput,
-                format!("Invalid TQE hostname: {}", self.tqe_hostname),
-            )
-        })?;
+        let dns_name = self.tqe_hostname.as_ref();
         let mut sess = rustls::ClientSession::new(&client_config, dns_name);
 
         let mut conn = TcpStream::connect(&self.tqe_address).chain(|| {

client-core/src/signer/dummy_signer.rs

@@ -21,11 +21,8 @@ impl DummySigner {
     fn pad_payload(&self, plain_txaux: PlainTxAux) -> Vec<u8> {
         let unit = 16_usize;
         let plain_payload_len = plain_txaux.encode().len();
-        if plain_payload_len % unit == 0 {
-            vec![0; plain_payload_len]
-        } else {
-            vec![0; plain_payload_len + unit - plain_payload_len % unit]
-        }
+        // PKCS7 padding -- if the len is multiple of the block length, it'll still be padded
+        vec![0; plain_payload_len + unit - plain_payload_len % unit]
     }
 
     /// Creates a mock merkletree