WIP: remove unneeded code, mark my TODOs for simpler discovery

Signed-off-by: alt3r 3go <[email protected]>

src/config.rs

@@ -44,15 +44,15 @@ cfg_if::cfg_if! {
 }
 pub const MAX_SHORT_DATA_LENGTH: usize = 128;
 
-// TODO: Do we want better keylength granularity here?
+// TODO:alt3r-3go: Do we want better keylength granularity here?
 #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))]
 pub const MAX_SIGNATURE_LENGTH: usize = 512;
 #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))]
-// TODO: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize.
+// TODO:alt3r-3go: We use PKCS#8 DER format, this value was found empirically for 2K keys. Need to generalize.
 pub const MAX_KEY_MATERIAL_LENGTH: usize = 1217;
 #[cfg(any(feature = "rsa2k-pkcs", feature = "rsa3k", feature = "rsa4k"))]
-// TODO: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH.
-//       Double-check this is okay.
+// TODO:alt3r-3go: This is due to the fact that KEY_MATERIAL_LENGTH is now bigger than MESSAGE_LENGTH.
+//                 Double-check this is okay.
 pub const MAX_MESSAGE_LENGTH: usize = MAX_KEY_MATERIAL_LENGTH;
 
 

src/mechanisms/rsa2kpkcs.rs

@@ -12,38 +12,6 @@ use crate::error::Error;
 use crate::service::*;
 use crate::types::*;
 
-// #[inline(never)]
-// fn load_public_key(keystore: &mut impl Keystore, key_id: &KeyId)
-//     -> Result<rsa::RsaPublicKey, Error> {
-
-//     //TODO: The key size should better be defined somewhere instead of hardcoding
-//     let public_bytes: [u8; 512] = keystore
-//         .load_key(key::Secrecy::Public, Some(key::Kind::Rsa2k), &key_id)?
-//         .material.as_slice()
-//         .try_into()
-//         .map_err(|_| Error::InternalError)?;
-
-//     // let public_key = salty::signature::PublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?;
-//     let public_key = rsa::RsaPublicKey::try_from(&public_bytes).map_err(|_| Error::InternalError)?;
-
-//     Ok(public_key)
-// }
-
-// #[inline(never)]
-// fn load_keypair(keystore: &mut impl Keystore, key_id: &KeyId)
-//     -> Result<salty::Keypair, Error> {
-
-//     let seed: [u8; 32] = keystore
-//         .load_key(key::Secrecy::Secret, Some(key::Kind::Rsa2k), &key_id)?
-//         .material.as_slice()
-//         .try_into()
-//         .map_err(|_| Error::InternalError)?;
-
-//     let keypair = salty::signature::Keypair::from(&seed);
-//     // hprintln!("seed: {:?}", &seed).ok();
-//     Ok(keypair)
-// }
-
 #[cfg(feature = "rsa2k-pkcs")]
 impl DeriveKey for super::Rsa2kPkcs
 {
@@ -168,8 +136,8 @@ impl SerializeKey for super::Rsa2kPkcs
             .material;
 
         let serialized_key = match request.format {
-            // TODO: There are "Der" and "Asn1Der" commented out in KeySerialization enum,
-            //       should those be used instead?
+            // TODO:alt3r-3go: There are "Der" and "Asn1Der" commented out in KeySerialization enum,
+            //                 should those be used instead?
             KeySerialization::Raw => {
                 let mut serialized_key = Message::new();
                 serialized_key.extend_from_slice(&priv_key_der).map_err(|_| Error::InternalError)?;
@@ -216,12 +184,12 @@ impl Sign for super::Rsa2kPkcs
             .expect("Failed to deserialize an RSA 2K private key from PKCS#8 DER");
 
         // RSA lib takes in a hash value to sign, not raw data.
-        // TODO: Do we assume we get digest into this function, or we calculate it ourselves?
+        // TODO:alt3r-3go: Do we assume we get digest into this function, or we calculate it ourselves?
         // use sha2::digest::Digest;
         // let digest_to_sign: [u8; 32] = sha2::Sha256::digest(&request.message).into();
 
-        // TODO: there's also .sign_blinded(), which is supposed to protect the private key from timing side channels,
-        // but requires an RNG instance - decide if we want to always use it.
+        // TODO:alt3r-3go: There's also .sign_blinded(), which is supposed to protect the private key from timing side channels,
+        //                 but requires an RNG instance - decide if we want to always use it.
         use rsa::padding::PaddingScheme;
         use rsa::hash::Hash;
         let native_signature = priv_key
@@ -251,7 +219,7 @@ impl Verify for super::Rsa2kPkcs
             return Err(Error::InvalidSerializationFormat);
         }
 
-        // TODO: This must not be a hardcoded magic number, need to generalize
+        // TODO:alt3r-3go: This must not be a hardcoded magic number, need to generalize
         if request.signature.len() != 256 {
             return Err(Error::WrongSignatureLength);
         }

src/store/keystore.rs

@@ -147,8 +147,8 @@ impl<P: Platform> Keystore for ClientKeystore<P> {
 
         let location = self.location(secrecy, id).ok_or(Error::NoSuchKey)?;
 
-        //TODO: This should better be defined in some way, instead of hardcoding.
-        //      I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea?
+        //TODO:alt3r-3go: This should better be defined in some way, instead of hardcoding.
+        //                I've tried referring to MAX_SERIALIZED_KEY_LENGTH, is this a good idea?
         #[cfg(not(feature = "rsa2k-pkcs"))]
         let bytes: Bytes<128> = store::read(self.store, location, &path)?;
         #[cfg(feature = "rsa2k-pkcs")]

tests/rsa2kpkcs.rs

@@ -9,17 +9,17 @@ use trussed::types::KeySerialization;
 use trussed::types::Location::*;
 use trussed::types::StorageAttributes;
 
-// TODO: Looks like the test infra is not supposed to be used with several tests like below -
-//       right now it randomly fails with SIGSERV on either of the two, when run together,
-//       but never when run separately. Need to investigate and fix.
+// TODO:alt3r-3go: Looks like the test infra is not supposed to be used with several tests like below -
+//                 right now it randomly fails with SIGSERV on either of the two, when run together,
+//                 but never when run separately. Need to investigate and fix.
 
 #[test]
 fn rsa2kpkcs_generate_key() {
     client::get(|client| {
         let sk = syscall!(client.generate_rsa2kpkcs_private_key(Internal)).key;
 
         // This assumes we don't ever get a key with ID 0
-        // TODO: make sure the above always holds or find a better way to check for success
+        // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success
         assert_ne!(sk, KeyId::from_special(0));
     })
 }
@@ -31,7 +31,7 @@ fn rsa2kpkcs_derive_key() {
         let pk = syscall!(client.derive_rsa2kpkcs_public_key(sk, Volatile)).key;
 
         // This assumes we don't ever get a key with ID 0
-        // TODO: make sure the above always holds or find a better way to check for success
+        // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success
         assert_ne!(pk, KeyId::from_special(0));
     })
 }
@@ -67,7 +67,7 @@ fn rsa2kpkcs_deserialize_key() {
         let deserialized_key_id = syscall!(client.deserialize_rsa2kpkcs_key(&serialized_key, KeySerialization::Raw, location)).key;
 
         // This assumes we don't ever get a key with ID 0
-        // TODO: make sure the above always holds or find a better way to check for success
+        // TODO:alt3r-3go: make sure the above always holds or find a better way to check for success
         assert_ne!(deserialized_key_id, KeyId::from_special(0));
     })
 }