Browse Source

* Fixed the test flakiness in btfuse by using a lazy unmount.
* Removed the brotli crate from btlib.
* Changed FsClient::read to be consistent with the other FsClient methods.
* Changed IssuedProcRec.addr to be an IpAddr because the creds in the
record determine the port to use.

Matthew Carr 2 years ago
parent
commit
5d08568cc0

+ 0 - 37
Cargo.lock

@@ -26,21 +26,6 @@ dependencies = [
  "memchr",
 ]
 
-[[package]]
-name = "alloc-no-stdlib"
-version = "2.0.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3"
-
-[[package]]
-name = "alloc-stdlib"
-version = "0.2.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "94fb8275041c72129eb51b7d0322c29b8387a0386127718b096429201a5d6ece"
-dependencies = [
- "alloc-no-stdlib",
-]
-
 [[package]]
 name = "android_system_properties"
 version = "0.1.5"
@@ -189,27 +174,6 @@ version = "1.3.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a"
 
-[[package]]
-name = "brotli"
-version = "3.3.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a1a0b1dbcc8ae29329621f8d4f0d835787c1c38bb1401979b49d13b0b305ff68"
-dependencies = [
- "alloc-no-stdlib",
- "alloc-stdlib",
- "brotli-decompressor",
-]
-
-[[package]]
-name = "brotli-decompressor"
-version = "2.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "59ad2d4653bf5ca36ae797b1f4bb4dbddb60ce49ca4aed8a2ce4829f60425b80"
-dependencies = [
- "alloc-no-stdlib",
- "alloc-stdlib",
-]
-
 [[package]]
 name = "btfproto"
 version = "0.1.0"
@@ -289,7 +253,6 @@ dependencies = [
  "anyhow",
  "base64-url",
  "bcder",
- "brotli",
  "btserde",
  "bytes",
  "chrono",

+ 0 - 8
README.md

@@ -61,14 +61,6 @@ Note that even if you run the tool in a subdirectory, the report will still be s
 of the repository.
 Please do not commit the coverage report.
 
-## Test Flakiness in btfuse
-The tests in btfuse are currently suffering from an issue where they randomly fail with the error
-`fusermount exited with a non-zero status: 1`. This doesn't happen every time, but it does
-happen annoyingly often. The issue is that the `fusermount` program is being used to umount
-the test file system, and it fails if the kernel is still accessing this file system when it's
-called. In the future I'd like to solve this by adopting (or writing) a different FUSE library,
-perhaps one which just wraps `libfuse`.
-
 ## License
 Copyright 2023 Delease, LLC. The software contained in this repository is licensed under the
 GNU Affero General Public License Version 3 or later. A copy of this license is provided in the

+ 2 - 6
crates/btfproto-tests/src/local_fs_tests.rs

@@ -13,7 +13,7 @@ use btlib::{
     AuthzAttrs, BlockError, BlockPath, IssuedProcRec,
 };
 use std::{
-    net::{IpAddr, Ipv6Addr, SocketAddr},
+    net::{IpAddr, Ipv6Addr},
     path::PathBuf,
     sync::Arc,
 };
@@ -80,14 +80,10 @@ impl LocalFsTest {
             .writecap()
             .ok_or(BlockError::MissingWritecap)
             .unwrap();
-        let node_bind_path = node_writecap.bind_path();
         let fs = LocalFs::new_empty(path, 0, root_creds, ModeAuthorizer).unwrap();
 
         let proc_rec = IssuedProcRec {
-            addr: SocketAddr::new(
-                IpAddr::V6(Ipv6Addr::LOCALHOST),
-                node_bind_path.port().unwrap(),
-            ),
+            addr: IpAddr::V6(Ipv6Addr::LOCALHOST),
             pub_creds: node_creds.concrete_pub(),
             writecap: node_writecap.to_owned(),
             authz_attrs: AuthzAttrs {

+ 15 - 2
crates/btfproto/src/client.rs

@@ -173,13 +173,26 @@ impl<T: Transmitter> FsClient<T> {
         self.tx.call(msg, extractor!(Open)).await?
     }
 
-    pub async fn read<R, Fut, F>(&self, read: Read, callback: F) -> Result<R>
+    pub async fn read<R, Fut, F>(
+        &self,
+        inode: Inode,
+        handle: Handle,
+        offset: u64,
+        size: u64,
+        callback: F,
+    ) -> Result<R>
     where
         F: 'static + Send + FnOnce(ReadReply<'_>) -> Fut,
         Fut: Send + Future<Output = R>,
     {
+        let msg = FsMsg::Read(Read {
+            inode,
+            handle,
+            offset,
+            size,
+        });
         let callback = ExtractRead::new(callback);
-        self.tx.call(FsMsg::Read(read), callback).await?
+        self.tx.call(msg, callback).await?
     }
 
     pub async fn write(

+ 3 - 7
crates/btfproto/src/local_fs.rs

@@ -21,7 +21,7 @@ use std::{
     fmt::{Display, Formatter},
     fs::File,
     io::{self, Read as IoRead, Seek, SeekFrom, Write as IoWrite},
-    net::{IpAddr, Ipv6Addr, SocketAddr},
+    net::{IpAddr, Ipv6Addr},
     path::{Path, PathBuf},
     sync::{
         atomic::{AtomicU64, Ordering},
@@ -555,10 +555,7 @@ mod private {
             let root_principal = writecap.root_principal();
             if fs.creds.principal() != root_principal {
                 let proc_rec = IssuedProcRec {
-                    addr: SocketAddr::new(
-                        IpAddr::V6(Ipv6Addr::LOCALHOST),
-                        writecap.bind_path().port()?,
-                    ),
+                    addr: IpAddr::V6(Ipv6Addr::LOCALHOST),
                     pub_creds: fs.creds.concrete_pub(),
                     writecap: writecap.to_owned(),
                     authz_attrs: AuthzAttrs {
@@ -666,7 +663,6 @@ mod private {
         ) -> Result<Accessor<FileBlock<C>>> {
             let block = BlockOpenOptions::new()
                 .with_creds(creds)
-                .with_compress(false)
                 .with_encrypt(true)
                 .with_inner(file)
                 .with_block_path(block_path)
@@ -970,7 +966,7 @@ mod private {
         /// Retrieves the authorization attributes for the principal identified by the given path.
         /// If the principal is not associated with a valid process record, then an [Err] is
         /// returned.
-        /// # Warning
+        /// ## Warning
         /// If this method is called while a lock for any component on the given path is held, then
         /// a deadlock may occur. It's safest to call this method when _no_ locks are held.
         fn authz_attrs(&self, from: &Arc<BlockPath>) -> Result<AuthzAttrs> {

+ 21 - 57
crates/btfsd/src/main.rs

@@ -81,11 +81,7 @@ mod tests {
     use btlib_tests::TpmCredStoreHarness;
     use btmsg::{BlockAddr, Transmitter};
     use btserde::from_slice;
-    use std::{
-        future::ready,
-        net::{Ipv4Addr, SocketAddr},
-        time::Duration,
-    };
+    use std::{future::ready, net::Ipv4Addr, time::Duration};
     use swtpm_harness::SwtpmHarness;
     use tempdir::TempDir;
 
@@ -174,14 +170,8 @@ mod tests {
             .unwrap();
         let WriteReply { written, .. } = client.write(inode, handle, 0, EXPECTED).await.unwrap();
         assert_eq!(EXPECTED.len() as u64, written);
-        let msg = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: EXPECTED.len() as u64,
-        };
         let actual = client
-            .read(msg, |reply| {
+            .read(inode, handle, 0, EXPECTED.len() as u64, |reply| {
                 let mut buf = Vec::with_capacity(EXPECTED.len());
                 buf.extend_from_slice(reply.data);
                 ready(buf)
@@ -222,14 +212,8 @@ mod tests {
             .open(inode, FlagValue::ReadOnly.into())
             .await
             .unwrap();
-        let msg = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: EXPECTED.len() as u64,
-        };
         let actual = client
-            .read(msg, |reply| {
+            .read(inode, handle, 0, EXPECTED.len() as u64, |reply| {
                 let mut buf = Vec::with_capacity(EXPECTED.len());
                 buf.extend_from_slice(reply.data);
                 ready(buf)
@@ -312,14 +296,10 @@ mod tests {
             .open(inode, FlagValue::ReadOnly.into())
             .await
             .unwrap();
-        let msg = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: EXPECTED.len() as u64,
-        };
         let actual = client
-            .read(msg, |reply| ready(reply.data.to_owned()))
+            .read(inode, handle, 0, EXPECTED.len() as u64, |reply| {
+                ready(reply.data.to_owned())
+            })
             .await
             .unwrap();
 
@@ -552,14 +532,10 @@ mod tests {
             .allocate(inode, handle, EXPECTED.len() as u64)
             .await
             .unwrap();
-        let msg = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: EXPECTED.len() as u64,
-        };
         let actual = client
-            .read(msg, |reply| ready(Vec::from(reply.data)))
+            .read(inode, handle, 0, EXPECTED.len() as u64, |reply| {
+                ready(Vec::from(reply.data))
+            })
             .await
             .unwrap();
 
@@ -588,14 +564,10 @@ mod tests {
             .allocate(inode, handle, EXPECTED.len() as u64)
             .await
             .unwrap();
-        let msg = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: EXPECTED.len() as u64,
-        };
         let actual = client
-            .read(msg, |reply| ready(Vec::from(reply.data)))
+            .read(inode, handle, 0, EXPECTED.len() as u64, |reply| {
+                ready(Vec::from(reply.data))
+            })
             .await
             .unwrap();
 
@@ -662,7 +634,7 @@ mod tests {
             .unwrap();
         creds.set_writecap(writecap);
         let expected = IssuedProcRec {
-            addr: SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), 52982),
+            addr: IpAddr::V6(Ipv6Addr::LOCALHOST),
             pub_creds: creds.concrete_pub(),
             writecap: creds.writecap().unwrap().to_owned(),
             authz_attrs: AuthzAttrs {
@@ -684,14 +656,10 @@ mod tests {
             .open(inode, FlagValue::ReadOnly.into())
             .await
             .unwrap();
-        let read = Read {
-            inode,
-            handle,
-            offset: 0,
-            size: entry.attr.size,
-        };
         let record = client
-            .read(read, |reply| ready(from_slice::<ProcRec>(reply.data)))
+            .read(inode, handle, 0, entry.attr.size, |reply| {
+                ready(from_slice::<ProcRec>(reply.data))
+            })
             .await
             .unwrap()
             .unwrap();
@@ -716,7 +684,7 @@ mod tests {
             .unwrap();
         creds.set_writecap(writecap);
         let expected = IssuedProcRec {
-            addr: SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), 52982),
+            addr: IpAddr::V6(Ipv6Addr::LOCALHOST),
             pub_creds: creds.concrete_pub(),
             writecap: creds.writecap().unwrap().to_owned(),
             authz_attrs: AuthzAttrs {
@@ -753,14 +721,10 @@ mod tests {
             .open(record_inode, FlagValue::ReadOnly.into())
             .await
             .unwrap();
-        let read = Read {
-            inode: record_inode,
-            handle: record_handle,
-            offset: 0,
-            size: entry.attr.size,
-        };
         let record = client
-            .read(read, |reply| ready(from_slice::<ProcRec>(reply.data)))
+            .read(record_inode, record_handle, 0, entry.attr.size, |reply| {
+                ready(from_slice::<ProcRec>(reply.data))
+            })
             .await
             .unwrap()
             .unwrap();
@@ -797,7 +761,7 @@ mod tests {
             creds
         };
         let expected = IssuedProcRec {
-            addr: SocketAddr::new(IpAddr::V6(Ipv6Addr::LOCALHOST), 52982),
+            addr: IpAddr::V6(Ipv6Addr::LOCALHOST),
             pub_creds: user_creds.concrete_pub(),
             writecap: user_creds.writecap().unwrap().to_owned(),
             authz_attrs: AuthzAttrs {

+ 1 - 1
crates/btfuse/src/main.rs

@@ -143,7 +143,7 @@ mod test {
             .to_str()
             .expect("failed to convert mnt_path to `str`");
         let code = std::process::Command::new(PROG)
-            .args(["-u", mnt_path])
+            .args(["-z", "-u", mnt_path])
             .status()
             .expect("waiting for exit status failed")
             .code()

+ 0 - 1
crates/btlib/Cargo.toml

@@ -21,7 +21,6 @@ tss-esapi-sys = "0.3.0"
 foreign-types = "0.3.1"
 zeroize = { version = "1.5.7", features = ["zeroize_derive"] }
 static_assertions = "1.1.0"
-brotli = "3.3.4"
 fuse-backend-rs = "0.9.6"
 libc = "0.2.137"
 env_logger = { version = "0.9.0" }

+ 6 - 92
crates/btlib/src/lib.rs

@@ -21,7 +21,6 @@ extern crate static_assertions;
 extern crate lazy_static;
 
 use ::log::error;
-use brotli::{CompressorWriter, Decompressor};
 use btserde::{read_from, write_to};
 use fuse_backend_rs::abi::fuse_abi::{stat64, Attr};
 use positioned_io::{ReadAt, Size, WriteAt};
@@ -33,7 +32,7 @@ use std::{
     fmt::{self, Display, Formatter},
     hash::Hash as Hashable,
     io::{self, Read, Seek, SeekFrom, Write},
-    net::SocketAddr,
+    net::IpAddr,
     ops::{Add, Sub},
     os::unix::prelude::MetadataExt,
     time::{Duration, SystemTime},
@@ -953,7 +952,6 @@ pub struct BlockOpenOptions<T, C> {
     inner: T,
     creds: C,
     encrypt: bool,
-    compress: bool,
     block_path: Option<BlockPath>,
 }
 
@@ -963,7 +961,6 @@ impl BlockOpenOptions<(), ()> {
             inner: (),
             creds: (),
             encrypt: true,
-            compress: true,
             block_path: Default::default(),
         }
     }
@@ -975,7 +972,6 @@ impl<T, C> BlockOpenOptions<T, C> {
             inner,
             creds: self.creds,
             encrypt: self.encrypt,
-            compress: self.compress,
             block_path: self.block_path,
         }
     }
@@ -985,7 +981,6 @@ impl<T, C> BlockOpenOptions<T, C> {
             inner: self.inner,
             creds,
             encrypt: self.encrypt,
-            compress: self.compress,
             block_path: self.block_path,
         }
     }
@@ -995,11 +990,6 @@ impl<T, C> BlockOpenOptions<T, C> {
         self
     }
 
-    pub fn with_compress(mut self, compress: bool) -> Self {
-        self.compress = compress;
-        self
-    }
-
     pub fn with_block_path(mut self, block_path: BlockPath) -> Self {
         self.block_path = Some(block_path);
         self
@@ -1031,54 +1021,6 @@ impl Default for BlockOpenOptions<(), ()> {
     }
 }
 
-impl<T: Write> Decompose<T> for CompressorWriter<T> {
-    fn into_inner(self) -> T {
-        self.into_inner()
-    }
-}
-
-impl<T: Read> Decompose<T> for Decompressor<T> {
-    fn into_inner(self) -> T {
-        self.into_inner()
-    }
-}
-
-#[derive(Clone)]
-pub struct BrotliParams {
-    buf_sz: usize,
-    quality: u32,
-    window_sz: u32,
-}
-
-impl BrotliParams {
-    pub fn new(buf_sz: usize, quality: u32, window_sz: u32) -> BrotliParams {
-        BrotliParams {
-            buf_sz,
-            quality,
-            window_sz,
-        }
-    }
-}
-
-impl<T: Write> TryCompose<T, CompressorWriter<T>> for BrotliParams {
-    type Error = crate::Error;
-    fn try_compose(self, inner: T) -> Result<CompressorWriter<T>> {
-        Ok(CompressorWriter::new(
-            inner,
-            self.buf_sz,
-            self.quality,
-            self.window_sz,
-        ))
-    }
-}
-
-impl<T: Read> TryCompose<T, Decompressor<T>> for BrotliParams {
-    type Error = crate::Error;
-    fn try_compose(self, inner: T) -> Result<Decompressor<T>> {
-        Ok(Decompressor::new(inner, self.buf_sz))
-    }
-}
-
 /// An envelopment of a key, which is tagged with the principal who the key is meant for.
 #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)]
 pub struct Readcap {
@@ -1190,7 +1132,7 @@ impl Fragment {
 /// Structure for keeping track of server information in a directory.
 pub struct ServerRecord {
     /// The most up-to-date address for this server.
-    pub addr: SocketAddr,
+    pub addr: IpAddr,
     /// The public credentials for this server.
     pub pub_creds: ConcretePub,
 }
@@ -1208,7 +1150,7 @@ pub struct AuthzAttrs {
 
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 pub struct IssuedProcRec {
-    pub addr: SocketAddr,
+    pub addr: IpAddr,
     pub pub_creds: ConcretePub,
     pub writecap: Writecap,
     pub authz_attrs: AuthzAttrs,
@@ -1218,7 +1160,7 @@ pub struct IssuedProcRec {
 /// Structure stored in process blocks for keeping track of process credentials and location.
 pub enum ProcRec {
     Requested {
-        addr: SocketAddr,
+        addr: IpAddr,
         pub_creds: ConcretePub,
     },
     Valid(IssuedProcRec),
@@ -1496,45 +1438,17 @@ impl<F: FnOnce()> Drop for DropTrigger<F> {
 #[cfg(test)]
 mod tests {
     use btserde::{from_vec, to_vec};
-    use std::{
-        fs::OpenOptions,
-        io::{Cursor, Write},
-        path::PathBuf,
-    };
+    use std::{fs::OpenOptions, io::Write, path::PathBuf};
     use tempdir::TempDir;
 
     use super::*;
     use crate::{
         crypto::{ConcreteCreds, CredsPriv},
         sectored_buf::SectoredBuf,
-        test_helpers::{
-            node_creds, read_check, root_creds, write_fill, SectoredCursor, SECTOR_SZ_DEFAULT,
-        },
+        test_helpers::{node_creds, root_creds, SectoredCursor, SECTOR_SZ_DEFAULT},
         Cursor as PioCursor,
     };
 
-    #[test]
-    fn brotli_compress_decompress() {
-        const SECT_SZ: usize = SECTOR_SZ_DEFAULT;
-        const SECT_CT: usize = 16;
-        let params = BrotliParams::new(SECT_SZ, 8, 20);
-        let mut memory = Cursor::new([0u8; SECT_SZ * SECT_CT]);
-        {
-            let write: CompressorWriter<_> = params
-                .clone()
-                .try_compose(&mut memory)
-                .expect("compose for write failed");
-            write_fill(write, SECT_SZ, SECT_CT);
-        }
-        memory.seek(SeekFrom::Start(0)).expect("seek failed");
-        {
-            let read: Decompressor<_> = params
-                .try_compose(&mut memory)
-                .expect("compose for read failed");
-            read_check(read, SECT_SZ, SECT_CT);
-        }
-    }
-
     /// Tests that the `BlockMetaBody` struct has an updated secrets struct after it is modified
     /// in the `access_secrets` method.
     #[test]