-
Notifications
You must be signed in to change notification settings - Fork 675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add WireGuard protocol support #1557
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1557 +/- ##
==========================================
+ Coverage 82.89% 83.03% +0.14%
==========================================
Files 273 276 +3
Lines 46220 46689 +469
Branches 9422 9389 -33
==========================================
+ Hits 38314 38769 +455
- Misses 7055 7119 +64
+ Partials 851 801 -50
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Looks good. Can you please add tests for the layer in |
@Dimi1010 Sure, I'm working on adding the tests for the layer in Test/Packet++ and will push the commits shortly. |
8382544
to
34c6705
Compare
Tests/Packet++Test/PacketExamples/WireGuardHandshakeInitiation.dat
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nadongjun I have some comments about the current implementation, so I didn't review the tests yet. Once we finalize the implementation I'll review the tests
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
…tructures Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
…ge to handle nextLayer Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
…tor and message creation Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
…ta handling Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
@nadongjun I resolved the comments that were fixed and kept open those that weren't. Can you please go over the open comments and resolve them? |
@seladb I’ll take a look at the open comments and address them soon. Thank you for your detailed feedback. |
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
* | ||
* @return The sender index as a 32-bit integer. | ||
*/ | ||
uint32_t getSenderIndex() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have getter methods for all fields (in this class and the other classes), but we don't have setter methods. Do we want to add setter methods also?
For example:
void setSenderIndex(uint32_t senderIndex);
void setInitiatorEphemeral(std::array<uint8_t, 32> initEphemeral);
...
void setReceiverIndex(uint32_t receiverIndex);
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... Would it be alright if I create a separate PR to work on adding setter methods and WireGuard packet edit tests later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sure 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include tests for all methods and properties in the wireguard layer classes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I've made the changes.
… WireGuardCookieReply Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
* | ||
* @return The sender index as a 32-bit integer. | ||
*/ | ||
uint32_t getSenderIndex() const; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sure 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nadongjun overall looks good!
Please find a few more comments, we're getting close to getting this PR ready to merge 🙂
@seladb Thank you! I’ll address the remaining feedback quickly. |
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Signed-off-by: Dongjun Na <kmu5544616@gmail.com>
Thank you so much @nadongjun for working on it, much appreciated! 🙏 🥇 |
@nadongjun nice job :) |
Reference
Summary:
parseNextLayer
function within the UDP layer. The following features are included in this enhancement:What's Changed:
WireGuard Message Structures
wg_common_header
): Base structure for all WireGuard messages containing the message type and reserved fields.wg_handshake_initiation
): Structure representing the Handshake Initiation message with fields such as sender index, ephemeral keys, and MACs.wg_handshake_response
): Structure representing the Handshake Response message with fields such as sender and receiver indexes, ephemeral keys, and MACs.wg_cookie_reply
): Structure representing the Cookie Reply message with fields such as receiver index, nonce, and encrypted cookie.wg_transport_data
): Structure representing the Transport Data message with fields such as receiver index, counter, and encrypted data.WireGuardLayer Class Methods
Handshake Initiation, Handshake Response, Cookie Reply, Transport Data
).Static Methods
isWireguardPorts(uint16_t portSrc, uint16_t portDst)
: Checks if the given ports match the WireGuard port (51820).isWireGuard(uint8_t* data, size_t dataLen)
: Validates if the provided data represents a WireGuard message based on its type.Layer Methods
parseNextLayer()
: No operation as WireGuard does not have subsequent layers.getHeaderLen()
: Returns the length of datacomputeCalculateFields()
: No fields to compute; method is left empty.toString()
: Converts the WireGuard layer to a string representation for easier debugging and logging.getOsiModelLayer()
: Returns the OSI model layer corresponding to the Network layer.UDP Layer function
UDPLayer::parseNextLayer()
: Integrates the ability to recognize and parse WireGuard messagesExample Usage:
Example Usage::Result
INPUT
: WireGuard Pcap fileOUTPUT
: result.log