* Fixed a bug in block validation where the writecap was not

  being validated.
* Many small formatting changes.

crates/btnode/src/crypto/mod.rs

@@ -7,7 +7,7 @@ use super::*;
 use openssl::{
     error::ErrorStack,
     encrypt::{Encrypter as OsslEncrypter, Decrypter as OsslDecrypter},
-    pkey::{PKey, Public as OsslPublic, Private as OsslPrivate, HasPublic, HasPrivate},
+    pkey::{PKey, HasPublic, HasPrivate},
     symm::{Cipher, encrypt as openssl_encrypt, decrypt as openssl_decrypt},
     rand::rand_bytes,
     rsa::{Rsa, Padding as OpensslPadding},
@@ -43,6 +43,8 @@ pub enum Error {
     KeyVariantUnsupported,
     BlockNotEncrypted,
     InvalidHashFormat,
+    InvalidSignature,
+    WritecapAuthzErr(WritecapAuthzErr),
     Serde(serde_block_tree::Error),
     Io(std::io::Error),
     Custom(Box<dyn std::fmt::Debug>)
@@ -63,13 +65,21 @@ impl Display for Error {
             Error::KeyVariantUnsupported => write!(f, "unsupported key variant"),
             Error::BlockNotEncrypted => write!(f, "block was not encrypted"),
             Error::InvalidHashFormat => write!(f, "invalid format"),
-            Error::Serde(serde_err) => serde_err.fmt(f),
-            Error::Io(io_err) => io_err.fmt(f),
+            Error::InvalidSignature => write!(f, "invalid signature"),
+            Error::WritecapAuthzErr(err) => err.fmt(f),
+            Error::Serde(err) => err.fmt(f),
+            Error::Io(err) => err.fmt(f),
             Error::Custom(cus) => cus.fmt(f),
         }
     }
 }
 
+impl From<WritecapAuthzErr> for Error {
+    fn from(err: WritecapAuthzErr) -> Self {
+        Error::WritecapAuthzErr(err)
+    }
+}
+
 impl From<ErrorStack> for Error {
     fn from(error: ErrorStack) -> Error {
         Error::custom(error)
@@ -303,8 +313,8 @@ pub trait Scheme: for<'de> Deserialize<'de> + Serialize
     fn as_enum(self) -> SchemeKind;
     fn message_digest(&self) -> MessageDigest;
     fn padding(&self) -> Option<OpensslPadding>;
-    fn public_from_der(self, der: &[u8]) -> Result<PKey<OsslPublic>>;
-    fn private_from_der(self, der: &[u8]) -> Result<PKey<OsslPrivate>>;
+    fn public_from_der(self, der: &[u8]) -> Result<PKey<Public>>;
+    fn private_from_der(self, der: &[u8]) -> Result<PKey<Private>>;
     fn generate(self) -> Result<AsymKeyPair<Self::Kind>>;
 }
 
@@ -337,13 +347,13 @@ impl Scheme for Encrypt {
         }
     }
 
-    fn public_from_der(self, der: &[u8]) -> Result<PKey<OsslPublic>> {
+    fn public_from_der(self, der: &[u8]) -> Result<PKey<Public>> {
         match self {
             Encrypt::RsaEsOaep(inner) => inner.public_from_der(der),
         }
     }
 
-    fn private_from_der(self, der: &[u8]) -> Result<PKey<OsslPrivate>> {
+    fn private_from_der(self, der: &[u8]) -> Result<PKey<Private>> {
         match self {
             Encrypt::RsaEsOaep(inner) => inner.private_from_der(der),
         }
@@ -387,13 +397,13 @@ impl Scheme for Sign {
         }
     }
 
-    fn public_from_der(self, der: &[u8]) -> Result<PKey<OsslPublic>> {
+    fn public_from_der(self, der: &[u8]) -> Result<PKey<Public>> {
         match self {
             Sign::RsaSsaPss(inner) => inner.public_from_der(der),
         }
     }
 
-    fn private_from_der(self, der: &[u8]) -> Result<PKey<OsslPrivate>> {
+    fn private_from_der(self, der: &[u8]) -> Result<PKey<Private>> {
         match self {
             Sign::RsaSsaPss(inner) => inner.private_from_der(der),
         }
@@ -451,12 +461,12 @@ impl Scheme for RsaEsOaep {
         Some(OpensslPadding::PKCS1_OAEP)
     }
 
-    fn public_from_der(self, der: &[u8]) -> Result<PKey<OsslPublic>> {
-        Ok(PKey::public_key_from_der(der)?)
+    fn public_from_der(self, der: &[u8]) -> Result<PKey<Public>> {
+        Ok(PKey::public_key_from_der(der)?.conv_pub())
     }
 
-    fn private_from_der(self, der: &[u8]) -> Result<PKey<OsslPrivate>> {
-        Ok(PKey::private_key_from_der(der)?)
+    fn private_from_der(self, der: &[u8]) -> Result<PKey<Private>> {
+        Ok(PKey::private_key_from_der(der)?.conv_priv())
     }
 
     fn generate(self) -> Result<AsymKeyPair<Self::Kind>> {
@@ -491,12 +501,12 @@ impl Scheme for RsaSsaPss {
         Some(OpensslPadding::PKCS1_PSS)
     }
 
-    fn public_from_der(self, der: &[u8]) -> Result<PKey<OsslPublic>> {
-        Ok(PKey::public_key_from_der(der)?)
+    fn public_from_der(self, der: &[u8]) -> Result<PKey<Public>> {
+        Ok(PKey::public_key_from_der(der)?.conv_pub())
     }
 
-    fn private_from_der(self, der: &[u8]) -> Result<PKey<OsslPrivate>> {
-        Ok(PKey::private_key_from_der(der)?)
+    fn private_from_der(self, der: &[u8]) -> Result<PKey<Private>> {
+        Ok(PKey::private_key_from_der(der)?.conv_priv())
     }
 
     fn generate(self) -> Result<AsymKeyPair<Self::Kind>> {
@@ -568,18 +578,14 @@ pub type AsymKeyPub<S> = AsymKey<Public, S>;
 
 impl<S: Scheme> AsymKey<Public, S> {
     pub(crate) fn new(scheme: S, der: &[u8]) -> Result<AsymKey<Public, S>> {
-        let pkey = scheme.public_from_der(der)?.conv_pub();
+        let pkey = scheme.public_from_der(der)?;
         Ok(AsymKey { scheme, pkey })
     }
-
-    fn to_der(&self) -> Result<Vec<u8>> {
-        Ok(self.pkey.public_key_to_der()?)
-    }
 }
 
 impl<S: Scheme> AsymKey<Private, S> {
     pub(crate) fn new(scheme: S, der: &[u8]) -> Result<AsymKey<Private, S>> {
-        let pkey = scheme.private_from_der(der)?.conv_priv();
+        let pkey = scheme.private_from_der(der)?;
         Ok(AsymKey { scheme, pkey })
     }
 }
@@ -639,22 +645,11 @@ impl<S: Scheme> PartialEq for AsymKey<Public, S> {
 
 impl Owned for AsymKey<Public, Sign> {
     fn owner_of_kind(&self, kind: HashKind) -> Principal {
-        match kind {
-            HashKind::Sha2_256 => {
-                let mut buf = [0; 32];
-                let der = self.to_der().unwrap();
-                let bytes = hash(MessageDigest::sha256(), der.as_slice()).unwrap();
-                buf.copy_from_slice(&*bytes);
-                Principal(Hash::Sha2_256(buf))
-            },
-            HashKind::Sha2_512 => {
-                let mut buf = [0; 64];
-                let der = self.to_der().unwrap();
-                let bytes = hash(MessageDigest::sha512(), der.as_slice()).unwrap();
-                buf.copy_from_slice(&*bytes);
-                Principal(Hash::Sha2_512(buf))
-            }
-        }
+        let der = self.pkey.public_key_to_der().unwrap();
+        let bytes = hash(kind.into(), der.as_slice()).unwrap();
+        let mut hash_buf = Hash::from(kind);
+        hash_buf.as_mut().copy_from_slice(&bytes);
+        Principal(hash_buf)
     }
 }
 
@@ -710,7 +705,7 @@ impl Signer for AsymKey<Private, Sign> {
 }
 
 impl Verifier for AsymKey<Public, Sign> {
-    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<bool> {
+    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<()> {
         let mut verifier = OsslVerifier::new(self.scheme.message_digest(), &self.pkey)?;
         if let Some(padding) = self.scheme.padding() {
             verifier.set_rsa_padding(padding)?;
@@ -718,7 +713,12 @@ impl Verifier for AsymKey<Public, Sign> {
         for part in parts {
             verifier.update(part)?;
         }
-        Ok(verifier.verify(signature)?)
+        if verifier.verify(signature)? {
+            Ok(())
+        }
+        else {
+            Err(Error::InvalidSignature)
+        }
     }
 }
 
@@ -760,7 +760,7 @@ impl Signer for AsymKeyPair<Sign> {
 }
 
 impl Verifier for AsymKeyPair<Sign> {
-    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<bool> {
+    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<()> {
         self.public.verify(parts, signature)
     }
 }
@@ -784,7 +784,7 @@ impl ConcreteCreds {
 }
 
 impl Verifier for ConcreteCreds {
-    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<bool> {
+    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<()> {
         self.sign.verify(parts, signature)
     }
 }
@@ -801,7 +801,11 @@ impl Owned for ConcreteCreds {
     }
 }
 
-impl CredsPub for ConcreteCreds {}
+impl CredsPub for ConcreteCreds {
+    fn public_sign(&self) -> &AsymKey<Public, Sign> {
+        &self.sign.public
+    }
+}
 
 impl Signer for ConcreteCreds {
     fn sign<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I) -> Result<Signature> {
@@ -817,11 +821,7 @@ impl Decrypter for ConcreteCreds {
 
 impl CredsPriv for ConcreteCreds {}
 
-impl Creds for ConcreteCreds {
-    fn public(&self) -> &AsymKey<Public, Sign> {
-        &self.sign.public
-    }
-}
+impl Creds for ConcreteCreds {}
 
 pub(crate) trait Encrypter {
     fn encrypt(&self, slice: &[u8]) -> Result<Vec<u8>>;
@@ -836,19 +836,20 @@ pub(crate) trait Signer {
 }
 
 pub(crate) trait Verifier {
-    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<bool>;
+    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<()>;
 }
 
 /// Trait for types which can be used as public credentials.
-pub(crate) trait CredsPub: Verifier + Encrypter + Owned {}
+pub(crate) trait CredsPub: Verifier + Encrypter + Owned {
+    /// Returns a reference to the public signing key which can be used to verify signatures.
+    fn public_sign(&self) -> &AsymKey<Public, Sign>;
+}
 
 /// Trait for types which contain private credentials.
 pub(crate) trait CredsPriv: Decrypter + Signer {}
 
 /// Trait for types which contain both public and private credentials.
-pub(crate) trait Creds: CredsPriv + CredsPub {
-    fn public(&self) -> &AsymKey<Public, Sign>;
-}
+pub(crate) trait Creds: CredsPriv + CredsPub {}
 
 /// A trait for types which store credentials.
 pub(crate) trait CredStore {
@@ -942,9 +943,8 @@ fn get_body(block: &Block) -> Result<&[u8]> {
 }
 
 pub(crate) fn sign_block<K: Signer>(
-    block: &mut Block, writecap: Writecap, priv_key: &K
+    block: &mut Block, priv_key: &K
 ) -> Result<()> {
-    block.writecap = writecap;
     let body = get_body(block)?;
     let sig_input = HeaderSigInput::from(&*block);
     let header = to_vec(&sig_input)?;
@@ -953,9 +953,10 @@ pub(crate) fn sign_block<K: Signer>(
     Ok(())
 }
 
-pub(crate) fn verify_block(block: &Block) -> Result<bool> {
+pub(crate) fn verify_block(block: &Block) -> Result<()> {
+    verify_writecap(&block.writecap, &block.path)?;
     let body = get_body(block)?;
-    let sig_input = HeaderSigInput::from(&*block);
+    let sig_input = HeaderSigInput::from(block);
     let header = to_vec(&sig_input)?;
     let parts = [header.as_slice(), body].into_iter();
     block.writecap.signing_key.verify(parts, block.signature.as_slice())
@@ -988,52 +989,46 @@ pub(crate) fn sign_writecap<K: Signer>(writecap: &mut Writecap, priv_key: &K) ->
 
 /// The types of errors which can occur when verifying a writecap chain is authorized to write to
 /// a given path.
-#[derive(Debug, PartialEq)]
-pub(crate) enum WritecapAuthzErr {
+#[derive(Debug, PartialEq, Display)]
+pub enum WritecapAuthzErr {
     /// The chain is not valid for use on the given path.
     UnauthorizedPath,
     /// At least one writecap in the chain is expired.
     Expired,
     /// The given writecaps do not actually form a chain.
     NotChained,
-    /// There is an invalid signature in the chain.
-    InvalidSignature,
     /// The principal the root writecap was issued to does not own the given path.
     RootDoesNotOwnPath,
     /// An error occurred while serializing a writecap.
     Serde(String),
-    /// A cryptographic error occurred while attempting to verify a writecap.
-    Crypto(String),
+    /// The write cap chain was too long to be validated. The value contained in this error is
+    /// the maximum allowed length.
+    ChainTooLong(usize),
 }
 
 /// Verifies that the given `Writecap` actually grants permission to write to the given `Path`.
-pub(crate) fn verify_writecap(
-    mut writecap: &Writecap, path: &Path
-) -> std::result::Result<(), WritecapAuthzErr> {
+pub(crate) fn verify_writecap(mut writecap: &Writecap, path: &Path) -> Result<()> {
+    const CHAIN_LEN_LIMIT: usize = 256;
     let mut prev: Option<&Writecap> = None;
     let mut sig_input = Vec::new();
     let now = Epoch::now();
-    loop {
+    for _ in 0..CHAIN_LEN_LIMIT {
         if !writecap.path.contains(path) {
-            return Err(WritecapAuthzErr::UnauthorizedPath);
+            return Err(WritecapAuthzErr::UnauthorizedPath.into());
         }
         if writecap.expires <= now {
-            return Err(WritecapAuthzErr::Expired);
+            return Err(WritecapAuthzErr::Expired.into());
         }
         if let Some(prev) = &prev {
             if prev.signing_key.owner_of_kind(writecap.issued_to.kind()) != writecap.issued_to {
-                return Err(WritecapAuthzErr::NotChained);
+                return Err(WritecapAuthzErr::NotChained.into());
             }
         }
         let sig = WritecapSigInput::from(writecap);
         sig_input.clear();
         write_to(&sig, &mut sig_input).map_err(|e| WritecapAuthzErr::Serde(e.to_string()))?;
-        let valid = writecap.signing_key 
-            .verify([sig_input.as_slice()].into_iter(), writecap.signature.as_slice())
-            .map_err(|e| WritecapAuthzErr::Crypto(e.to_string()))?;
-        if !valid {
-            return Err(WritecapAuthzErr::InvalidSignature);
-        }
+        writecap.signing_key 
+            .verify([sig_input.as_slice()].into_iter(), writecap.signature.as_slice())?;
         match &writecap.next {
             Some(next) => {
                 prev = Some(writecap);
@@ -1046,11 +1041,12 @@ pub(crate) fn verify_writecap(
                     return Ok(());
                 }
                 else {
-                    return Err(WritecapAuthzErr::RootDoesNotOwnPath)
+                    return Err(WritecapAuthzErr::RootDoesNotOwnPath.into())
                 }
             }
         }
     }
+    Err(WritecapAuthzErr::ChainTooLong(CHAIN_LEN_LIMIT).into())
 }
 
 #[cfg(test)]
@@ -1097,9 +1093,7 @@ mod tests {
         let header = b"About: lyrics".as_slice();
         let message = b"Everything that feels so good is bad bad bad.".as_slice();
         let signature = key.sign([header, message].into_iter())?;
-        let verified = key.verify([header, message].into_iter(), signature.as_slice())?;
-        assert_eq!(true, verified);
-        Ok(())
+        key.verify([header, message].into_iter(), signature.as_slice())
     }
 
     #[test]
@@ -1108,17 +1102,9 @@ mod tests {
         let principal = readcap.issued_to.clone();
         let mut block = make_block_with(readcap)?;
         let key = make_key_pair();
-        let writecap = Writecap {
-            issued_to: Principal(Hash::Sha2_256(PRINCIPAL)),
-            path: make_path(vec!["contacts", "emergency"]),
-            expires: Epoch(1649904316),
-            signing_key: key.public().clone(),
-            signature: Signature::Rsa(SIGNATURE),
-            next: None,
-        };
         block = encrypt_block(block, &principal, &key)?;
-        sign_block(&mut block, writecap, &key)?;
-        assert_eq!(true, verify_block(&block)?);
+        sign_block(&mut block, &key)?;
+        verify_block(&block)?;
         Ok(())
     }
 
@@ -1140,40 +1126,51 @@ mod tests {
 
     #[test]
     fn verify_writecap_valid() -> Result<()> {
-        let writecap = make_writecap()?;
-        let result = verify_writecap(&writecap, &writecap.path);
-        assert_eq!(Ok(()), result);
-        Ok(())
+        let writecap = make_writecap(vec!["apps", "verse"])?;
+        verify_writecap(&writecap, &writecap.path)
     }
 
     #[test]
     fn verify_writecap_invalid_signature() -> Result<()> {
-        let mut writecap = make_writecap()?;
+        let mut writecap = make_writecap(vec!["apps", "verse"])?;
         writecap.signature = Signature::default();
         let result = verify_writecap(&writecap, &writecap.path);
-        assert_eq!(Err(WritecapAuthzErr::InvalidSignature), result);
-        Ok(())
+        if let Err(Error::InvalidSignature) = result {
+            Ok(())
+        }
+        else {
+            Err(Error::custom(format!("unexpected result {:?}", result)))
+        }
+    }
+
+    fn assert_authz_err<T: std::fmt::Debug>(
+        expected: WritecapAuthzErr, result: Result<T>
+    ) -> Result<()> {
+        if let Err(Error::WritecapAuthzErr(actual)) = &result {
+            if *actual == expected {
+                return Ok(());
+            }
+        }
+        Err(Error::custom(format!("unexpected result: {:?}", result)))
     }
 
     #[test]
     fn verify_writecap_invalid_path_not_contained() -> Result<()> {
-        let writecap = make_writecap()?;
+        let writecap = make_writecap(vec!["apps", "verse"])?;
         let mut path = writecap.path.clone();
         path.components.pop();
         // `path` is now a superpath of `writecap.path`, thus the writecap is not authorized to
         // write to it.
         let result = verify_writecap(&writecap, &path);
-        assert_eq!(Err(WritecapAuthzErr::UnauthorizedPath), result);
-        Ok(())
+        assert_authz_err(WritecapAuthzErr::UnauthorizedPath, result)
     }
 
     #[test]
     fn verify_writecap_invalid_expired() -> Result<()> {
-        let mut writecap = make_writecap()?;
+        let mut writecap = make_writecap(vec!["apps", "verse"])?;
         writecap.expires = Epoch::now() - Duration::from_secs(1);
         let result = verify_writecap(&writecap, &writecap.path);
-        assert_eq!(Err(WritecapAuthzErr::Expired), result);
-        Ok(())
+        assert_authz_err(WritecapAuthzErr::Expired, result)
     }
 
     #[test]
@@ -1186,8 +1183,7 @@ mod tests {
         let writecap = make_writecap_trusted_by(
             root_writecap, root_key, node_principal, vec!["apps", "contacts"])?;
         let result = verify_writecap(&writecap, &writecap.path);
-        assert_eq!(Err(WritecapAuthzErr::NotChained), result);
-        Ok(())
+        assert_authz_err(WritecapAuthzErr::NotChained, result)
     }
 
     #[test]
@@ -1201,8 +1197,7 @@ mod tests {
         let writecap = make_writecap_trusted_by(
             root_writecap, root_key, node_owner, vec!["apps", "contacts"])?;
         let result = verify_writecap(&writecap, &writecap.path);
-        assert_eq!(Err(WritecapAuthzErr::RootDoesNotOwnPath), result);
-        Ok(())
+        assert_authz_err(WritecapAuthzErr::RootDoesNotOwnPath, result)
     }
 
     /// Tests that validate the dependencies of this module.

crates/btnode/src/crypto/tpm.rs

@@ -144,7 +144,8 @@ impl ContextExt for Context {
         let mut unused: Option<PersistentTpmHandle> = None;
         // We simultaneously iterate over the possible handles and the used handles, breaking when
         // we find a handle which is not equal to the current used handle, or when
-        // there are no more used handles. This works because the used handles are sorted.
+        // there are no more used handles. This works because both `used_handles` and
+        // `possible_handles` are sorted.
         for handle in possible_handles {
             let used = match used_handles.next() {
                 Some(used) => used,
@@ -200,8 +201,7 @@ impl ContextExt for Context {
             Some(key_handle) => key_handle,
             None => self.tr_from_tpm_public(tpm_handle)?.into()
         };
-        let persistent = Persistent::Persistent(
-            PersistentTpmHandle::try_from(tpm_handle)?);
+        let persistent = Persistent::Persistent(tpm_handle.try_into()?);
         self.evict_control(Provision::Owner, key_handle.into(), persistent)?;
         Ok(())
     }
@@ -214,8 +214,7 @@ impl ContextExt for Context {
             SessionType::Hmac,
             SymmetricDefinition::AES_256_CFB,
             HashingAlgorithm::Sha256,
-        )
-        ?
+        )?
         .ok_or_else(|| Error::custom("empty session handle received from TPM"))?;
 
         let (attributes, mask) = SessionAttributesBuilder::new()
@@ -334,7 +333,6 @@ impl TpmHandles {
     }
 }
 
-
 #[derive(Serialize, Deserialize)]
 struct Storage {
     cookie: Cookie,
@@ -360,8 +358,7 @@ impl Storage {
     }
 
     fn init<P: AsRef<Path>>(&self, path: P) -> Result<()> {
-        let file = OpenOptions::new().write(true).create_new(true).open(&path)
-            ?;
+        let file = OpenOptions::new().write(true).create_new(true).open(&path)?;
         // Only allow access from the current user.
         let metadata = file.metadata()?;
         let mut permissions = metadata.permissions();
@@ -391,28 +388,6 @@ impl Storage {
     }
 }
 
-#[derive(Clone, Copy)]
-enum KeyKind {
-    Sign,
-    Decrypt,
-}
-
-impl KeyKind {
-    fn sign(self) -> bool {
-        match self {
-            KeyKind::Sign => true,
-            KeyKind::Decrypt => false,
-        }
-    }
-
-    fn decrypt(self) -> bool {
-        match self {
-            KeyKind::Sign => false,
-            KeyKind::Decrypt => true,
-        }
-    }
-}
-
 impl From<HashKind> for HashingAlgorithm {
     fn from(kind: HashKind) -> Self {
         match kind {
@@ -474,8 +449,7 @@ impl<'a, S: Scheme> KeyBuilder<'a, S> {
             .with_decrypt(decrypt)
             .with_sign_encrypt(!decrypt)
             .with_restricted(false)
-            .build()
-            ?;
+            .build()?;
         let name_hashing_algorithm = HashingAlgorithm::Sha256;
         let auth_policy = Digest::empty();
         let parameters = PublicRsaParameters::new(
@@ -484,8 +458,7 @@ impl<'a, S: Scheme> KeyBuilder<'a, S> {
             Self::RSA_KEY_BITS,
             RsaExponent::try_from(Self::RSA_EXPONENT)?, 
         );
-        let unique = PublicKeyRsa::try_from(self.unique)
-            ?;
+        let unique = PublicKeyRsa::try_from(self.unique)?;
         let public = Public::Rsa {
             object_attributes,
             name_hashing_algorithm,
@@ -595,8 +568,7 @@ impl TpmCredStore {
                 None,
                 None,
                 None,
-            )
-            ?
+            )?
         };
         let public = AsymKey::try_from(result.out_public, params.scheme)?;
         Ok(KeyPair { public, private: result.key_handle })
@@ -625,12 +597,13 @@ impl TpmCredStore {
             Ok(handle) => handle,
             Err(error) => {
                 guard.context.evict_key(sign_handle, Some(creds.sign.private))?;
-                return Err(error)
+                return Err(error);
             }
         };
         let handles = TpmHandles::new(
             creds.sign.to_stored(sign_handle),
-            creds.enc.to_stored(enc_handle));
+            creds.enc.to_stored(enc_handle)
+        );
         update_storage(&mut guard.storage, handles);
         match self.save_storage(&mut guard) {
             Ok(_) => Ok(()),
@@ -680,7 +653,7 @@ impl CredStore for TpmCredStore {
         {
             let guard = self.state.read()?;
             if let Some(creds) = &guard.node_creds {
-                return Ok(creds.clone())
+                return Ok(creds.clone());
             }
         }
         self.gen_node_creds()
@@ -796,12 +769,11 @@ impl HashcheckTicketExt for HashcheckTicket {
     /// Returns the NULL Ticket of the hashcheck type, as defined in part 1 of the TPM spec,
     /// clause 4.47.
     fn null() -> HashcheckTicket {
-        let tk = TPMT_TK_HASHCHECK {
+        TPMT_TK_HASHCHECK {
             tag: HashcheckTicket::POSSIBLE_TAGS[0].into(),
             digest: Default::default(),
             hierarchy: TPM2_RH_NULL,
-        };
-        HashcheckTicket::try_from(tk).unwrap()
+        }.try_into().unwrap()
     }
 }
 
@@ -820,34 +792,12 @@ impl TpmCreds {
 
 impl Owned for TpmCreds {
     fn owner_of_kind(&self, kind: HashKind) -> Principal {
-        fn hash(creds: &TpmCreds, msg_digest: MessageDigest, mut buf: &mut [u8]) {
-            let digest = {
-                let sign_der = creds.sign.public.to_der().unwrap();
-                let enc_der = creds.enc.public.to_der().unwrap();
-                let mut hasher = Hasher::new(msg_digest).unwrap();
-                hasher.update(sign_der.as_slice()).unwrap();
-                hasher.update(enc_der.as_slice()).unwrap();
-                hasher.finish().unwrap()
-            };
-            buf.write_all(&digest).unwrap();
-        }
-        match kind {
-            HashKind::Sha2_256 => {
-                let mut buf = [0; 32];
-                hash(self, MessageDigest::sha256(), &mut buf);
-                Principal(Hash::Sha2_256(buf))
-            },
-            HashKind::Sha2_512 => {
-                let mut buf = [0; 64];
-                hash(self, MessageDigest::sha512(), &mut buf);
-                Principal(Hash::Sha2_512(buf))
-            }
-        }
+        self.sign.public.owner_of_kind(kind)
     } 
 }
 
 impl Verifier for TpmCreds {
-    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<bool> {
+    fn verify<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I, signature: &[u8]) -> Result<()> {
         self.sign.public.verify(parts, signature)
     }
 }
@@ -858,7 +808,11 @@ impl Encrypter for TpmCreds {
     }
 }
 
-impl CredsPub for TpmCreds {}
+impl CredsPub for TpmCreds {
+    fn public_sign(&self) -> &AsymKeyPub<Sign> {
+        &self.sign.public
+    }
+}
 
 impl Signer for TpmCreds {
     fn sign<'a, I: Iterator<Item=&'a [u8]>>(&self, parts: I) -> Result<Signature> {
@@ -872,6 +826,7 @@ impl Signer for TpmCreds {
             Digest::try_from(slice)?
         };
         let validation = HashcheckTicket::null();
+        // Null means the scheme used during key creation will be used.
         let scheme = SignatureScheme::Null;
 
         let sig = {
@@ -895,6 +850,7 @@ impl Signer for TpmCreds {
 impl Decrypter for TpmCreds {
     fn decrypt(&self, slice: &[u8]) -> Result<Vec<u8>> {
         let cipher_text = PublicKeyRsa::try_from(slice)?;
+        // Null means the scheme used during key creation will be used.
         let in_scheme = RsaDecryptionScheme::Null;
         let empty = [0u8; 0];
         let label = Data::try_from(empty.as_slice())?;
@@ -908,11 +864,7 @@ impl Decrypter for TpmCreds {
 
 impl CredsPriv for TpmCreds {}
 
-impl Creds for TpmCreds {
-    fn public(&self) -> &AsymKeyPub<Sign> {
-        &self.sign.public
-    }
-}
+impl Creds for TpmCreds {}
 
 trait TctiNameConfExt {
     fn default() -> TctiNameConf;
@@ -1059,7 +1011,7 @@ active_pcr_banks = sha256
                 .spawn()?;
             // TODO: This wait is flakey and system-specific. I need to find a DBus library I can
             // use to check when tpm2-abrmd has finished starting up and is accepting bus
-            //connections.
+            // connections.
             std::thread::sleep(std::time::Duration::from_millis(50));
             Ok(SwtpmHarness { dir, swtpm, abrmd, bus_name, state_path })
         }
@@ -1175,8 +1127,7 @@ active_pcr_banks = sha256
     #[test]
     fn tpm_cred_store_new() -> Result<()> {
         let harness = SwtpmHarness::new()?;
-        let dir = TempDir::new("btnode")?;
-        let cookie_path = dir.path().join("cookie.bin");
+        let cookie_path = harness.dir.path().join("cookie.bin");
         let store = TpmCredStore::new(harness.context()?, &cookie_path)?;
         let cookie = File::open(&cookie_path)?;
         let metadata = cookie.metadata()?;
@@ -1184,21 +1135,20 @@ active_pcr_banks = sha256
         // Assert that the cookie can only be read by its owner.
         assert_eq!(0o600, 0o777 & actual);
         drop(store);
-        dir.close()?;
         Ok(())
     }
 
     #[test]
     fn gen_creds() -> Result<()> {
         let harness = SwtpmHarness::new()?;
-        let dir = TempDir::new("btnode")?;
-        let cookie_path = dir.path().join("cookie.bin");
+        let cookie_path = harness.dir.path().join("cookie.bin");
         let store = TpmCredStore::new(harness.context()?, &cookie_path)?;
         store.gen_node_creds()?;
         Ok(())
     }
 
     /// Displays the numeric identifiers used by the supported hash algorithms.
+    //#[test]
     fn show_nids() {
         fn show_nid(digest: MessageDigest) {
             let nid = digest.type_();
@@ -1242,12 +1192,7 @@ active_pcr_banks = sha256
         let data: [u8; 1024] = rand_array()?;
         let parts = [data.as_slice()];
         let sig = creds.sign(parts.into_iter())?;
-        if creds.verify(parts.into_iter(), sig.as_slice())? {
-            Ok(())
-        }
-        else {
-            Err(Error::custom("signature verification failed"))
-        }
+        creds.verify(parts.into_iter(), sig.as_slice())
     }
 
     #[test]
@@ -1295,17 +1240,18 @@ active_pcr_banks = sha256
     }
 
     #[test]
-    fn persistent_handles() {
-        let harness = SwtpmHarness::new().unwrap();
-        let mut context = harness.context().unwrap();
-        let _all_handles = context.persistent_handles().unwrap();
+    fn persistent_handles() -> Result<()> {
+        let harness = SwtpmHarness::new()?;
+        let mut context = harness.context()?;
+        context.persistent_handles()?;
+        Ok(())
     }
 
     #[test]
     fn first_free_persistent() -> Result<()> {
         let harness = SwtpmHarness::new()?;
         let mut context = harness.context()?;
-        let _first = context.unused_persistent_primary_key()?;
+        context.unused_persistent_primary_key()?;
         Ok(())
     }
 
@@ -1326,7 +1272,6 @@ active_pcr_banks = sha256
         let (harness, store) = test_store()?;
         let expected = {
             let creds = store.node_creds()?;
-            // This contains a hash of the public keys in `creds`, so it strongly identifies them.
             creds.owner()
         };
         drop(store);

crates/btnode/src/serde_tests.rs

@@ -45,7 +45,7 @@ fn roundtrip_fragment() -> Result<()> {
 
 #[test]
 fn roundtrip_write_cap() -> Result<()> {
-    let expected = make_writecap()?;
+    let expected = make_writecap(vec!["nodes", "home"])?;
     let ser_result = to_vec(&expected);
     let de_result = from_vec(&ser_result?);
     assert_eq!(expected, de_result?);

crates/btnode/src/test_helpers.rs

@@ -388,11 +388,11 @@ pub(crate) fn make_path(rel_components: Vec<&str>) -> Path {
     make_path_with_owner(make_principal(), rel_components)
 }
 
-pub(crate) fn make_writecap() -> Result<Writecap> {
+pub(crate) fn make_writecap(rel_components: Vec<&str>) -> Result<Writecap> {
     let (root_writecap, root_key) = make_self_signed_writecap()?;
     let issued_to = Principal(Hash::Sha2_256(PRINCIPAL));
     make_writecap_trusted_by(
-        root_writecap, root_key, issued_to, vec!["contacts", "emergency"])
+        root_writecap, root_key, issued_to, rel_components)
 }
 
 pub(crate) fn make_writecap_trusted_by<C: Creds>(
@@ -403,7 +403,7 @@ pub(crate) fn make_writecap_trusted_by<C: Creds>(
         issued_to,
         path: make_path_with_owner(next.path.owner.clone(), path_components),
         expires: hour_hence,
-        signing_key: trusting_creds.public().clone(),
+        signing_key: trusting_creds.public_sign().clone(),
         signature: Signature::default(),
         next: Some(Box::from(next)),
     };
@@ -435,7 +435,7 @@ pub(crate) fn make_self_signed_writecap_with<C: Creds>(key: &C) -> Result<Writec
         issued_to: root_principal.clone(),
         path: make_path_with_owner(root_principal, vec![]),
         expires: hour_hence,
-        signing_key: key.public().clone(),
+        signing_key: key.public_sign().clone(),
         signature: Signature::default(),
         next: None,
     };
@@ -457,9 +457,12 @@ pub(crate) fn make_block() -> Result<Block> {
 pub(crate) fn make_block_with(readcap: Readcap) -> Result<Block> {
     let mut readcaps = HashMap::new();
     readcaps.insert(readcap.issued_to, readcap.key);
-    let writecap = make_writecap()?;
+    // Notice that the writecap path contains the block path. If this were not the case, the block
+    // would be invalid.
+    let writecap = make_writecap(vec!["apps"])?;
+    let root_writecap = writecap.next.as_ref().unwrap();
     Ok(Block {
-        path: make_path(vec!["apps", "verse"]),
+        path: make_path_with_owner(root_writecap.issued_to.clone(), vec!["apps", "verse"]),
         readcaps,
         writecap,
         body: Cryptotext::Plain(Vec::from(PAYLOAD)),