Add basic IPSec support to wire
#821
Conversation
Codecov Report
@@ Coverage Diff @@
## main #821 +/- ##
==========================================
+ Coverage 79.39% 79.52% +0.13%
==========================================
Files 76 78 +2
Lines 27504 27762 +258
==========================================
+ Hits 21836 22078 +242
- Misses 5668 5684 +16
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This is completely normal for smoltcp; the wire module is intended to be usable without the rest anyways. |
|
@Dirbaio What is your opinion? |
src/wire/ipsec_ah.rs
Outdated
| #[test] | ||
| fn test_construct() { | ||
| let mut bytes = vec![0xa5; 24]; | ||
| let mut packet: Packet<&mut Vec<u8>> = Packet::new_unchecked(&mut bytes); |
There was a problem hiding this comment.
Why do you need to specify the type of packet?
src/wire/ipsec_ah.rs
Outdated
| 0xaf, 0xd2, 0xe7, 0xa1, 0x73, 0xd3, 0x29, 0x0b, 0xfe, 0x6b, 0x63, 0x73, | ||
| ]; | ||
| packet.integrity_check_value_mut().copy_from_slice(&ICV); | ||
| assert_eq!(&*packet.into_inner(), &PACKET_BYTES2[..]); |
There was a problem hiding this comment.
I think you could just compare with bytes here.
src/wire/ipsec_ah.rs
Outdated
| fn test_emit() { | ||
| let mut bytes = vec![0x17; 24]; | ||
| let mut packet = Packet::new_unchecked(&mut bytes); | ||
| packet_repr().emit(&mut packet); | ||
| assert_eq!(&*packet.into_inner(), &PACKET_BYTES2[..]); | ||
| } |
There was a problem hiding this comment.
Could you add an assert for the buffer_len function?
src/wire/ipsec_esp.rs
Outdated
| #[test] | ||
| fn test_construct() { | ||
| let mut bytes = vec![0xa5; 8]; | ||
| let mut packet: Packet<&mut Vec<u8>> = Packet::new_unchecked(&mut bytes); |
There was a problem hiding this comment.
Same as previous remark about the type.
src/wire/ipsec_esp.rs
Outdated
| let mut packet: Packet<&mut Vec<u8>> = Packet::new_unchecked(&mut bytes); | ||
| packet.set_security_parameters_index(0xfb5128a6); | ||
| packet.set_sequence_number(2); | ||
| assert_eq!(&*packet.into_inner(), &PACKET_BYTES[..8]); |
There was a problem hiding this comment.
I think you can compare with bytes here.
src/wire/ipsec_esp.rs
Outdated
| #[test] | ||
| fn test_emit() { | ||
| let mut bytes = vec![0x17; 8]; | ||
| let mut packet = Packet::new_unchecked(&mut bytes); | ||
| packet_repr().emit(&mut packet); | ||
| assert_eq!(&*packet.into_inner(), &PACKET_BYTES[..8]); | ||
| } |
There was a problem hiding this comment.
Can you add an assert for the buffer_len function?
src/wire/mod.rs
Outdated
| #[cfg(feature = "proto-ipsec-ah")] | ||
| pub use self::ipsec_ah::{Packet as IPSecAuthHeaderPacket, Repr as IPSecAuthHeaderRepr}; | ||
|
|
||
| #[cfg(feature = "proto-ipsec-esp")] | ||
| pub use self::ipsec_esp::{Packet as IPSecEspPacket, Repr as IPSecEspRepr}; |
There was a problem hiding this comment.
Maybe the letter P should be lowercase, then it matches the other types that have IP in their name.
Cargo.toml
Outdated
There was a problem hiding this comment.
Maybe you could add a proto-ipsec feature flag that enables proto-ipsec-esp and proto-ipsec-ah. Then use that one in the default feature set.
There was a problem hiding this comment.
Or maybe don't add the feature to the default feature set since it is not used anywhere other than the wire module.
There was a problem hiding this comment.
I added the proto-ipsec feature. It is added in the default features to make CI run its tests, otherwise there is no need for it to be in the defaults as it is not actually implemented yet.
There was a problem hiding this comment.
You can add a line in the ci.sh file with the proto-ipsec feature.
There was a problem hiding this comment.
proto-ipsec has been added to FEATURES_TEST and FEATURES_CHECK
There was a problem hiding this comment.
I think all of your comments are fixed now
|
Thank you very much! |
As mentioned #816 here, supporting the
IPSecprotocol kernel aspects would be nice.The first step for supporting
IPSecwill be implementingAHandESPrelatedPacketandReprin thewire.I don't know if it is OK to merge the wire representations and then add complete support to other layers or not.