Skip to content

Conversation

@somniumism
Copy link
Contributor

@somniumism somniumism commented Jan 22, 2020

The network module is using the block cipher mode aes-256-cbc.
However, it does not contain message authentication.
Then, I added aes-gcm mode, which is an authenticated message encryption method.

I used the aes-gcm-siv crate to implement the aes-gcm mode. AES-GCM-SIV has been designed
to preserve both privacy and integrity, even if nonces are repeated.

And I added directory /target to .gitignore.

Fixed #5

Copy link

@sgkim126 sgkim126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter if you add a new mode but replacing the current mode to another does.

@somniumism somniumism changed the title Replace to the aes-cbc mode to aes-gcm mode Add aes-gcm mode to contain the message authentication in network module Jan 23, 2020
@somniumism
Copy link
Contributor Author

@sgkim126 AES-GCM-SIV has been designed to preserve both privacy and integrity, even if nonces are repeated. You can confirm this content here: aes-gcm-siv@0.3.0 and AES-GCM-SIV:Informational. And I 'added' the aes-gcm mode, not replace. : ) With @HoOngEe, Could you review this PR?

The network module is using the block cipher mode `aes-256-cbc`.
However, it does not contain message authentication.
Then, I added `aes-gcm-siv` mode, which is an authenticated message encryption method.

I used the aes-gcm-siv crate to implement the aes-gcm-siv mode. AES-GCM-SIV has been designed
to preserve both privacy and integrity, even if nonces are repeated.
let aead = Aes256GcmSiv::new(generic_key);

let temp_nonce = &nonce.to_be_bytes();
let nonce = GenericArray::from_slice(&temp_nonce[0..12]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you explain why do you take only 12 bytes from the nonce?

Copy link
Contributor Author

@somniumism somniumism Jan 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgkim126 When using AES-GCM, a 96-bit IV is generally recommended. I referred to the following a answer. I think there is no problem with using only 12 bytes out of 16 bytes because we are using a fixed nonce.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you will use only 12 bytes, IMHO, you should change the type of a nonce parameter as [u8: 12]. Discarding information inside a function may make the user misuse.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, nonce reuse is not recommended in AES-GCM-SIV so we should not use a fixed nonce anymore.

@somniumism
Copy link
Contributor Author

CodeChain is using AES-CBC mode in the Network module. AES-CBC doesn’t contain message authentication, so we need to apply AEAD to avoid the adaptive chosen ciphertext attack. However, in our p2p protocol, we use the nonce shared in the handshake step. So if we use AES-GCM, it leaks the key when the same iv is used twice.

I proposed to apply AES-GCM-SIV because using it can prevent the worst-case in AES-GCM. However, AES-GCM-SIV also recommends that we don’t use iv more than once. Therefore, it is necessary to change our p2p protocol to use AES-GCM-SIV for Foundry.

I investigated examples related to the network or p2p protocol using AES-GCM-SIV to refer to examples in creating the new protocol. However, I don’t find such protocols or examples in real-world applications. It’s a relatively new cipher, so it seems difficult to find a protocol that is using it yet.

Therefore, since there is no immediate way to add new modes or change protocols, I close this PR. Getting rid of the attack possibility and using AEAD is meaningful, so if a new way to apply AEAD is devised, then it will continue from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace the block cipher mode used in the Network module

3 participants