Add security module BPSec #3
Conversation
PatrickGarvey
commented
Mar 29, 2024
PatrickGarvey
commented
- Added optional feature BPsec
- add security module. For now only Integrity Block
- add security test for Integrity Block
…fined in RFC 9172 and a test case defined in RFC 9173
| .security_context_parameters(sec_ctx_para) // 2 Parameters: HMAC 512/512 and No Additional Scope | ||
| .build() | ||
| .unwrap(); | ||
| let key = hex!("1a2b1a2b1a2b1a2b1a2b1a2b1a2b1a2b"); |
There was a problem hiding this comment.
why use hex! instead of the unhexify helper from bp7? less dependencies means faster compile times
| # bpsec dependencies | ||
| sha2 = { version ="0.10.6", optional = true } | ||
| hmac = { version ="0.12.1", optional = true } | ||
| hex-literal = { version ="0.3.4", optional = true } |
There was a problem hiding this comment.
seems like an unnecessary dependency as a similar function is provided by bp7 helpers. not at compile time but unless used in a hot code path this should not be a problem
| @@ -0,0 +1,692 @@ | |||
| //use std::convert::TryInto; | |||
There was a problem hiding this comment.
unused imports should be removed from source to keep it clean or need a comment
There was a problem hiding this comment.
this file should be split up into a subfolder and multple files to make things more clear:
src/sec with a general mod.rs and an integrity.rs and maybe later on a confidentiality.rs
| use bitflags::bitflags; | ||
| use thiserror::Error; | ||
|
|
||
| // use aes_gcm::aead::{ |
There was a problem hiding this comment.
why is this commented out code block here?
|
|
||
| let security_context_parameters = seq.next_element()?.unwrap(); | ||
|
|
||
| // TODO: deal with multiple targets |
There was a problem hiding this comment.
so only a single target block is supported?
| use bp7::bundle::Block; | ||
|
|
||
| #[test] | ||
| fn security_data_tests() { |
There was a problem hiding this comment.
there is nothing security related in here, why is this test needed?
| // Two Parts: First create IPPT then ASB | ||
|
|
||
| // First Create Integrity-Protected Plaintext | ||
| let sec_block_header: (CanonicalBlockType, u64, bp7::flags::BlockControlFlagsType) = (bp7::security::INTEGRITY_BLOCK, 2, BlockControlFlags::empty().bits()); |
There was a problem hiding this comment.
why is the block header a tuple and not a struct which would help in self describing the parameters? and I can only target a single canonical block here?
|
|
||
|
|
||
| // Second Create Abstract Security Block | ||
| let sec_ctx_para = bp7::security::BibSecurityContextParameter::new(Some((1,7)),None,Some((3,0x0000))); |
There was a problem hiding this comment.
maybe the magic numbers from the test should also be explained directly in a comment like 1,7,3 etc
| let key = hex!("1a2b1a2b1a2b1a2b1a2b1a2b1a2b1a2b"); | ||
| sec_block_payload.compute_hmac(key, ippt_list); | ||
|
|
||
| // TODO: key mgmt |
| @@ -34,6 +34,7 @@ harness = false | |||
| default = ["binary-build"] | |||
| binary-build = ["instant"] | |||
| benchmark-helpers = ["instant"] | |||
| bpsec = ["dep:sha2", "dep:hmac", "dep:hex-literal"] | |||
There was a problem hiding this comment.
maybe also add bpsec to the default feature list
