fix: Revert a failed decryption

[Dustin-Ray]

Decryptions are performed in place, meaning a failed decryption due to an incorrect password leaves Message.data in an unstable state. To revert, we simply XOR again with the bad key. This should restore the encryption to the original state.

[Dustin-Ray]

This issue is simple. If tag verification fails on decryption, then XOR bad decryption with bad password and it should revert to original state.

[Dustin-Ray]

In fact, anything with Replace in the documentation should be reviewed for this issue as well.

[Dustin-Ray]

Here's the affected code in ops/pw_decrypt, for example: (some details omitted for brevity)

    fn pw_decrypt(&mut self, pw: &[u8], d: u64) {
        ...
        ...
        let new_t = &kmac_xof(ka, &self.msg, 512, "SKA", d);
        self.op_result = Some(self.digest.as_mut().unwrap() == new_t);
    }

Should be modified to something like: (Psuedocode)

   // optionally add a result wrapper and get points for this issue as well :D
    fn pw_decrypt(&mut self, pw: &[u8], d: u64) -> Result<String> {
        ...
        ...
        // heres where we take a hash of the resulting decryption
        // we will compare it to the original hash, if they dont match, 
        // something has gone terribly wrong
        let new_t = &kmac_xof(ka, &self.msg, 512, "SKA", d);
        if self.digest != new_t {
            // we failed to decrypt, so
            // heres where you XOR the bad PW back into the failed decryption:
            Err(xor_bytes(&mut self.msg, &m)); 
        }  
    }

Check out issue https://github.com/drcapybara/capyCRYPT/issues/27 for more info on how to use the Result wrapper

[Dustin-Ray]

Why does this work? This issue assumes you know how XOR (i.e. addition modulo 2) works. Suppose you have a message: 11111111 and you XOR it with the original (but incorrect) key 11111111, then you get as a result 00000000:

11111111 11111111 xor = 00000000

If you XOR the result with the original key, you get:

11111111 00000000 xor = 11111111

i.e. now you are back at the original message. Easy!

[Dustin-Ray]

This issue also affects ops/key_encrpyt but the fix is identical. A good solution could refactor this check into a new function, but for simplicity, we can just fix them both independently for now.

[Hyunggilwoo]

@drcapybara Does the ops/pw_decrypt function still exist? I see ops/key_encrypt function, but not the former function. I will look at ops/key_encrypt to further understand the problem, until I see if ops/pw_decrypt needs to be revised.

[Hyunggilwoo]

After reading the CONTRIBUTING.md, I recognize that all of the tests need to be passed. I ran cargo test, and the documentation tests still need to pass. It looks like, the documentation for sha3_decrypt needs to have an assert statement to compare if the original_message == encrypted_test_decrypted_with_password.

Right now, the Message struct does not have an fn equal, so I wonder if this needs to be implemented first.

[Hyunggilwoo]

In the symmetric encryption specification, we are told to accept if, and only if, t’ = t

[Hyunggilwoo]

Document test for fn key_encrypt does not pass the assertion.
The return type for fn key_encrypt is Result<KeyPair, OperationError>, but the key_encrypt and key_decrypt methods give me the following error:

no field pub_key on type Result<KeyPair, OperationError>, despite the documents on both key_encrypt and key_decrypt use KeyPair struct, which has the fields, pub_key and priv_key.

[Dustin-Ray]

After reading the CONTRIBUTING.md, I recognize that all of the tests need to be passed. I ran cargo test, and the documentation tests still need to pass. It looks like, the documentation for sha3_decrypt needs to have an assert statement to compare if the original_message == encrypted_test_decrypted_with_password.

Right now, the Message struct does not have an fn equal, so I wonder if this needs to be implemented first.

Great observation, so Message looks like this:

#[derive(Debug)]
/// Message type for which cryptographic traits are defined.
pub struct Message {
    pub msg: Box<Vec<u8>>,
    pub d: Option<SecParam>,
    pub sym_nonce: Option<Vec<u8>>,
    pub asym_nonce: Option<ExtendedPoint>,
    pub digest: Result<Vec<u8>, OperationError>,
    pub op_result: Result<(), OperationError>,
    pub sig: Option<Signature>,
}

to compare messages for equality, you are looking for Message.msg which is a heap pointer (a Box) to some data. This contains the actual contents of the message. This prevents us from copying it around, which is a problem when the message is very large.

[Dustin-Ray]

Document test for fn key_encrypt does not pass the assertion. The return type for fn key_encrypt is Result<KeyPair, OperationError>, but the key_encrypt and key_decrypt methods give me the following error:

no field pub_key on type Result<KeyPair, OperationError>, despite the documents on both key_encrypt and key_decrypt use KeyPair struct, which has the fields, pub_key and priv_key.

The documents are completely out of date since the Result was added to the return. The usage examples throughout this library need to be updated actually. Feel free to open an issue for this or I will at some point. Basically, there are many places where the library would panic, and now, we return an error instead. This is similar to

try {} catch {}

in java.

[Dustin-Ray]

@drcapybara Does the ops/pw_decrypt function still exist? I see ops/key_encrypt function, but not the former function. I will look at ops/key_encrypt to further understand the problem, until I see if ops/pw_decrypt needs to be revised.

Yes but we now have added AES encryption thanks to @HLRichardson-Git. So the symmetric encryption functions are:

sha3_encrypt
sha3_decrypt
aes_encrypt (cbc/ctr)
aes_decrypt (cbc/ctr)

each of these functions actually throw an error if the decryption fails, for instance:

Err(OperationError::SHA3DecryptionFailure)

This means that if you have the password and can detect and decryption failure, you can run ke_ka on it to extend the key and get everything all fixed up.

Last note is that youve mentioned several different items in this issue. The focus here remains on reverting a failed decryption, so lets focus on that here. If you would like a quick and easy introduction to rust and this library, I would recommend:

1. opening a new issue about the documentation updates
2. fixing the documentation to have correct usage examples

This will show you how everything works, and then this issue might be easier and more straightforward to solve.

[Hyunggilwoo]

Would there be a problem with using the clone of the message, when we need to revert from a failed decryption?

As I am writing a unit test to compare the equality of an original message to the decrypted message, I foresee that I would have to add trait PartialEq, Clone for all structs that are incorporated inside Message (including SecParam).

Adding all of these traits for the sake of writing a unit test, does not seem safe in the long run.

I want to see if I can only compare the necessary fields.

[Dustin-Ray]

Would there be a problem with using the clone of the message, when we need to revert from a failed decryption?

yeah we definitely dont want to do that because it would copy the message on the heap. the library should be able to process any amount of data and so we do not want to copy anywhere. you successfully fixed this by simply xoring the bad password back into the message! thanks! #51