ipsec: implement DecodingLayer #117
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this patch, the IPSecAH and IPSecESP layer would not fully implement
DecodingLayer. This would prevent them to be added as a DecodingLayer
to gopacket.NewDecodingLayerParser():
cannot use &IPSecAH{} (value of type *IPSecAH) as gopacket.DecodingLayer value in argument to gopacket.NewDecodingLayerParser: *IPSecAH does not implement gopacket.DecodingLayer (missing method CanDecode)
This patch implements (*IPSecAH).CanDecode() and (*IPSecESP).CanDecode()
and a test to ensure they can be used as a DecodingLayer.
Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
smagnani96
added a commit
to cilium/cilium
that referenced
this pull request
May 7, 2025
This patch adds some extra info to our monitor output making use of the IsIPSec()/IsWireguard() flags added in the previous patch. While doing so, cleanup some unused and unreachable code in decoding layers (IPSecAH/IPSecESP), given we do not provide those layers from our cache. In case we change our mind (ex want to dump additional esp-related info such as SPI), we'd need: 1. wait for gopacket/gopacket#117 to be merged 2. update our vendor 3. add esp (ah we can ignore) in our monitor cache and decoding layers list Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
smagnani96
added a commit
to cilium/cilium
that referenced
this pull request
May 7, 2025
This patch adds some extra info to our monitor output making use of the IsIPSec()/IsWireguard() flags added in the previous patch. While doing so, cleanup some unused and unreachable code in decoding layers (IPSecAH/IPSecESP), given we do not provide those layers from our cache. In case we change our mind (ex want to dump additional esp-related info such as SPI), we'd need: 1. wait for gopacket/gopacket#117 to be merged 2. update our vendor 3. add esp (ah we can ignore) in our monitor cache and decoding layers list Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
smagnani96
added a commit
to cilium/cilium
that referenced
this pull request
May 8, 2025
In this patch, we cleanup some unused and unreachable code in decoding layers (IPSecAH/IPSecESP), given we do not provide those layers from our cache. In case we change our mind (ex want to dump additional esp-related info such as SPI), we'd need: 1. wait for gopacket/gopacket#117 to be merged 2. update our vendor 3. add esp (ah we can ignore) in our monitor cache and decoding layers list Given the previous commit, we can now tell from Monitor whether a packet is IPSec/WireGuard, thanks to the datapath flag. Signed-off-by: Simone Magnani <simone.magnani@isovalent.com>
There was a problem hiding this comment.
Pull Request Overview
The patch fully implements the gopacket.DecodingLayer interface for IPSec AH and ESP by adding CanDecode/NextLayerType methods, updating the decode logic to use the shared helper, and adding minimal tests.
- Added
CanDecodeandNextLayerTypefor both IPSecAH and IPSecESP - Refactored
decodeIPSecAH/decodeIPSecESPto delegate todecodingLayerDecoder - Introduced basic tests to ensure
IPSecAHandIPSecESPsatisfy theDecodingLayerinterface
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| layers/ipsec.go | Implemented CanDecode/NextLayerType, updated DecodeFromBytes & decode functions |
| layers/ipsec_test.go | Added TestIPSecAHAsDecodingLayer and TestIPSecESPAsDecodingLayer |
Comments suppressed due to low confidence (2)
layers/ipsec_test.go:120
- The test only creates the parser but doesn't verify decoding behavior. Consider adding a minimal valid AH packet parse and asserting on decoded fields.
func TestIPSecAHAsDecodingLayer(t *testing.T) {
layers/ipsec_test.go:164
- Similarly, this test should exercise
DecodeFromBytesfor ESP and validate the parsed SPI, Seq, and payload.
func TestIPSecESPAsDecodingLayer(t *testing.T) {
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
mosajjal
approved these changes
Jul 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before this patch, the IPSecAH and IPSecESP layer would not fully implement DecodingLayer. This would prevent them to be added as a DecodingLayer to gopacket.NewDecodingLayerParser():
This patch implements (*IPSecAH).CanDecode() and (*IPSecESP).CanDecode() and a test to ensure they can be used as a DecodingLayer.