Conversation
|
Take a look at https://docs.rs/block-padding/latest/block_padding/ |
There was a problem hiding this comment.
Pull request overview
This PR implements padding for encrypted messages to comply with a specification requirement. The changes introduce ISO/IEC 7816-4 padding with a minimum message size of 1KB and bucket-based sizing for messages between 1-8KB.
Key Changes:
- Added new padding module with pad/unpad functions using ISO/IEC 7816-4 standard
- Modified WebRTC send API to accept byte slices instead of strings
- Integrated padding into message send/receive flow
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libturms/src/padding.rs | New padding module implementing bucket-based message padding with ISO/IEC 7816-4 |
| libturms/src/channel.rs | Integrated padding: unpads received messages and pads outgoing DHKey events |
| p2p/src/webrtc.rs | Changed send method signature from String to generic byte slice parameter |
| p2p/src/lib.rs | Moved lint attributes to proper location at file start |
| p2p/src/models.rs | Added documentation comments for X3DH and URGENT flag |
| error/src/lib.rs | Added RandOs error variant for OS random number generation errors |
| error/Cargo.toml | Added rand and rand_chacha dependencies |
| Cargo.lock | Updated with rand dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| } | ||
|
|
||
| /// Fill an entry with bunch of paddings using ISO/IEC 7816-4. |
There was a problem hiding this comment.
The documentation says "Fill an entry with bunch of paddings" which should be "Fill an entry with padding" or "Add padding to an entry" - the phrase "bunch of paddings" is grammatically incorrect.
| /// Fill an entry with bunch of paddings using ISO/IEC 7816-4. | |
| /// Add padding to an entry using ISO/IEC 7816-4. |
| out | ||
| } | ||
|
|
||
| /// Remove zero padding at the end of data using ISO/IEC 7816-4. |
There was a problem hiding this comment.
The documentation says "Remove zero padding at the end of data" which should be "Remove padding from the end of data" since ISO/IEC 7816-4 padding includes both the 0x80 marker byte and zero bytes.
| /// Remove zero padding at the end of data using ISO/IEC 7816-4. | |
| /// Remove padding from the end of data using ISO/IEC 7816-4. |
| //! Deterministic ISO/IEC 7816-4 padding. | ||
|
|
||
| // Minimum required by specification. | ||
| const MIN_LENGTH: usize = 1000; // ~1kB. | ||
|
|
||
| /// Padding structure. | ||
| #[derive(Debug, Clone)] | ||
| pub(crate) struct Padding; | ||
|
|
||
| impl Padding { | ||
| fn bucket_size(len: usize) -> usize { | ||
| match len { | ||
| 0..=MIN_LENGTH => MIN_LENGTH, | ||
| 1001..=8192 => len.div_ceil(1024) * 1024, | ||
| _ => len, | ||
| } | ||
| } | ||
|
|
||
| /// Fill an entry with bunch of paddings using ISO/IEC 7816-4. | ||
| pub fn pad(data: impl AsRef<[u8]>) -> Vec<u8> { | ||
| let data = data.as_ref(); | ||
|
|
||
| let target_len = Self::bucket_size(data.len()); | ||
| let mut out = Vec::with_capacity(target_len); | ||
|
|
||
| out.extend_from_slice(data); | ||
| out.push(0x80); | ||
|
|
||
| if out.len() < target_len { | ||
| out.resize(MIN_LENGTH, 0x00); | ||
| } | ||
|
|
||
| out | ||
| } | ||
|
|
||
| /// Remove zero padding at the end of data using ISO/IEC 7816-4. | ||
| pub fn unpad(mut data: Vec<u8>) -> Vec<u8> { | ||
| // Scan from the end. | ||
| while let Some(&last) = data.last() { | ||
| match last { | ||
| 0x00 => { | ||
| data.pop(); | ||
| }, | ||
| 0x80 => { | ||
| data.pop(); | ||
| return data; | ||
| }, | ||
| _ => break, | ||
| } | ||
| } | ||
|
|
||
| data | ||
| } | ||
| } |
There was a problem hiding this comment.
The padding functionality is critical for message privacy but lacks test coverage. Consider adding tests to verify: 1) correct padding for messages under 1000 bytes, 2) correct padding for messages between 1001-8192 bytes (bucket rounding to 1024), 3) correct padding for messages over 8192 bytes, 4) round-trip testing (pad then unpad returns original data), and 5) edge cases like empty messages or messages exactly at bucket boundaries.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 participant
Follow specification.