Adding traits for symmetric ciphers#44
Conversation
# Conflicts: # QUALITY_AND_STYLE.md # crypto/core/src/traits.rs
| SecurityStrength::_192bit, | ||
| SecurityStrength::_256bit, | ||
| ]; | ||
| for ss in security_strengths.iter() { |
There was a problem hiding this comment.
set_security_strength calls drop_hazardous_operations() on every success and rejects raising strength without the flag (key_material.rs: lines 406–455).
Since allow_hazardous_operations() is called only once before the loop, the 2nd iteration (None → _112bit) returns HazardousOperationNotPermitted and this .unwrap() panics. It also rejects strengths above the key length, so KEY_LEN < 32 panics regardless.
Recommend that you re-enable hazardous ops each iteration and bound tested strengths by KEY_LEN*8. (Same pattern in the BlockCipher and AEAD tests.)
There was a problem hiding this comment.
Well spotted.
Actually, I've already solved this problem on release/0.1.2alpha via #40 .
Once this PR gets rebased on top of that (or merged into it), this problem will go away.
There was a problem hiding this comment.
@ounsworth noted: will pull #40 so I can use their version of hazardous operations
- Use Added a closure-based do_hazardous_operations() #40 hazardous operations
| let mac_key = | ||
| KeyMaterial::<KEY_LEN>::from_bytes_as_type(&DUMMY_SEED_512[..KEY_LEN], KeyType::MACKey) | ||
| .unwrap(); | ||
| match C::encrypt_out(&mac_key, msg, &mut ct) { |
There was a problem hiding this comment.
If we want to test the wrong key check, wouldn't we want to use aead_encrypt_out?
There was a problem hiding this comment.
Note: test both encrypt_out and aead_encrypt_out
| /// In order for these one-shot APIs to be usable securely in all contexts, the init data will be generated | ||
| /// securely by the block cipher implementation and returned along with the ciphertext, and there is no API for the | ||
| /// user to provide the init data. If you require this functionality, see the documentation for the underlying implementation. | ||
| pub trait BlockCipher<const KEY_LEN: usize, const INIT_DATA_LEN: usize, const BLOCK_LEN: usize>: |
There was a problem hiding this comment.
Design Question: Encryption has do_encrypt_final/_out but decryption has no do_decrypt_final.
Padding modes need a decrypt-side finalization to strip/validate padding. Was this intentional, or should we add it?
There was a problem hiding this comment.
I thought I had added one. Maybe I just thought about adding one. Yes. We will certainly need one.
There was a problem hiding this comment.
- TODO: @officialfrancismendoza add do_decrypt_final
2 participants
This is prep work for @officialfrancismendoza 's ASCON branch.
These are a first-pass at these traits, and will almost certainly be refined as Francis works through ASCON, and subsequently AES.
Francis, feel free to merge to
release/0.1.2alphaand rebase your ASCON branch on top.