Skip to content
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

Support for multipeer #79

Closed
wants to merge 28 commits into from
Closed

Support for multipeer #79

wants to merge 28 commits into from

Conversation

lstipakov
Copy link
Member

No description provided.

The current implementation uses "relative" WDF timers
which are "not ticked" at low power states and on resume
they continue to where they were left of. This makes keepalive
timeout detection sub-optimal, since in worst case a client
has to wait for "ping-restart" seconds to reconnect, which could
be several minutes.

Refactor timers in a way that we only have single timer ticking
every second. At that tick we compare "last" and "now" timestamps
and do actions, similar to what openvpn2 is doing.

Fixes OpenVPN#64

Backported from b750b2c ("timer: refactor timers implementation")
and ed455c4 ("Fix bug check in timer management routines")

Signed-off-by: Lev Stipakov <[email protected]>
The current way to get a version information is to open DCO device and
make IOCTL call. This has a few issues:

 - If DCO device is already in use, an another app won't be to get the
version, since the device is exclusive

 - With the multiple DCO devices there is a high chance that \\.\ovpn-dco
device, which we use to get version information, is already in use. To
open another device, we use via device interface enumeration, which
requires a lot of boilerplate code to work.

To make it easier for userspace to get the device version, create
a non-exclusive control device \\.\ovpn-dco-ver which supports single IOCTL
to get the version number. This device is created when the first
network device is created and removed with the last network device.

Bump version to 2.3.0.

OpenVPN#75

Backported from f4adb2 ("More robust way to get a version info")

Signed-off-by: Lev Stipakov <[email protected]>
Add support for features:

 - AEAD auth tag at the end

 - 64bit pktid

For that, introduce a new OVPN_IOCTL_NEW_KEY_V2 ioctl,
which accepts a new data structure, which includes a
field to store bit flags for crypto options for
abovementioned features.

Refactor encrypt/decrypt routine to accomodate possible
wire format changes.

Refactor pktid code to support 64bit pktid. While on it,
remove unused variable.

Bump version to 2.4.0.

OpenVPN#74

Backported from 5acb8f ("Implement DATA_V3 protocol features")

Signed-off-by: Lev Stipakov <[email protected]>
@lstipakov lstipakov changed the title timer: refactor timers implementation Port release/1 bugfixes/features to master Sep 4, 2024
THis is to select between client mode (OVPN_MODE_P2P, default)
and server mode (OVPN_MODE_MP).

At the moment only ioctl is added, no functional changes.

Co-authored-by: Leon Dang <[email protected]>

Signed-off-by: Leon Dang <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
lstipakov and others added 4 commits September 16, 2024 14:29
In preparation for MP support, rename existing OvpnDelPeer()
implementation to OvpnStopVPN() - which deletes all peers and
closes the device socket. This looks more like "stop VPN" behavior
for both P2P and MP modes.

Co-authored-by: Leon Dang <[email protected]>

Signed-off-by: Leon Dang <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
Return an error to a userspace if P2P-only IOCTLs
are used in MP mode.

Signed-off-by: Lev Stipakov <[email protected]>
This starts listening on a provided local address.

OpenVPN#82

Co-authored-by: Leon Dang <[email protected]>
Signed-off-by: Leon Dang <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
This includes following functionality:

 - get driver version
 - set mode
 - start VPN (p2p/mp)
 - new peer (p2p)
 - send/receive CC messages

OpenVPN#83

Signed-off-by: Lev Stipakov <[email protected]>
In multipeer UDP mode, we expect userspace to prepend CC packets
with SOCKADDR to know where to send the control packet. Likewise,
when we receive the control packet, we prepend it with remote SOCKADDR
before pushing to userspace.

OpenVPN#84

Co-authored-by: Leon Dang <[email protected]>

Signed-off-by: Leon Dang <[email protected]>
Signed-off-by: Lev Stipakov <[email protected]>
In multipeer UDP mode, we expect userspace to prepend CC packets
with SOCKADDR to know where to send the control packet. Likewise,
when we receive the control packet, we prepend it with remote SOCKADDR
before pushing to userspace.

Signed-off-by: Lev Stipakov <[email protected]>
Commit

    f7877d ("Implement DATA_V3 protocol features")

has added support for 64bit pkitd. This means that
headroom for TX buffer should be also increased by 4 bytes.

Signed-off-by: Lev Stipakov <[email protected]>
Make RX_BUFFER compatible with buffer-manipulation
templated functions, such as:

 - OvpnBufferTrim (new)
 - OvpnBufferPull (new)
 - OvpnBufferPut (existing, added template)

Those functions are now compatible with both TX and RX buffers.

This refactor allowed to simplify OvpnEvtRxQueueAdvance() and
remove peer access from it, since the plaintext buffers
dequeued there already have crypto overhead stripped.

Signed-off-by: Lev Stipakov <[email protected]>
@lstipakov lstipakov force-pushed the master branch 2 times, most recently from e90d9e9 to c3c6b1b Compare October 9, 2024 11:32
lstipakov and others added 10 commits October 18, 2024 11:15
Implement adding peer in MP mode.

Also add this to gui client.

GitHub: OpenVPN#85

Signed-off-by: Lev Stipakov <[email protected]>
Signed-off-by: Leon Dang <[email protected]>

Co-authored-by: Leon Dang <[email protected]>
Key material is read from data64.key.

Signed-off-by: Lev Stipakov <[email protected]>
Instead of getting the first peer, find peer by
peer-id passed in crypto data.

Signed-off-by: Lev Stipakov <[email protected]>
Those are receiving peer-id so they could
work in MP mode too.

Signed-off-by: Lev Stipakov <[email protected]>
In MP mode OvpnSocketSend requires destination address to where
data should be sent, so we pass peer's remote address.

To make code work for both MP and P2P cases, store peer remote address
for P2P mode.

Signed-off-by: Lev Stipakov <[email protected]>
By default Windows uses splay trees, which require
exclusive lock on lookup. Switch to AVL trees,
since they support concurrent read using shared locks.

Signed-off-by: Lev Stipakov <[email protected]>
Add support for IOCTL_MP_SET_PEER. This is used
to set keepalive internal, keepalive timeout
and MSS for a specific peer.

Note that proper userspace notification for
expired peer is stil missing.

GitHub: OpenVPN#85

Signed-off-by: Lev Stipakov <[email protected]>
@lstipakov lstipakov changed the title Port release/1 bugfixes/features to master Support for multipeer Oct 18, 2024
Existing mechanism of userspace notifications
(by erroring out pending read requests) is not
flexible enough to be used by multipeer.

This adds a new OVPN_IOCTL_NOTIFY_EVENT ioctl.
When request arrives, we check if there are pending
notifications. If yes, then we complete request, writing
notification command, peer-id and peer delete reason (if applicable).
If there are no pending notifications, request is queued.

When notification occurs (such as peer keepalive timeout),
we check if there is a pending requests in the queue (see above).
If yes, we complete request with notification details. If there
are no pending requests, we add notification event to a queue.

Events queue is implemented with a C++ class and a kernel linked
lists. The queue is a member of device context. Since there is no
C++ runtime and constructors are not called for context members,
we have to use a separate method for initialization.

GitHub: OpenVPN#87

Signed-off-by: Lev Stipakov <[email protected]>
Use overlapped I/O with OVPN_IOCTL_NOTIFY_EVENT ioctl
to get notifications from the kernel.

Factor out read/write/notification callbacks into own
functions for better readability.

GitHub: OpenVPN#87

Signed-off-by: Lev Stipakov <[email protected]>
Instead of subscribe for notifications at startup,
do it on button click. This is because current driver
version doesn't yet support notifications.

Signed-off-by: Lev Stipakov <[email protected]>
On keepalive timeout, delete peer context from all tables,
unititialize crypto/timer and and free the peer context
memory.

Github: OpenVPN#88

Signed-off-by: Lev Stipakov <[email protected]>
@cron2 cron2 removed their assignment Oct 23, 2024
@cron2
Copy link

cron2 commented Oct 23, 2024

I'm unassigning me - as discussed in Karlsruhe, I'm not really the deeply knowledgeable Windows person, and while this does interest me a lot, I need to spend the time on moving forward patches to "openpvn main".

The current locking mechanism (global read-write spinlock)
works fine for client (since almost all operations are read-only,
such as encrypt/decrypt) but doesn't scale well for multipeer,
since amount of state-modifying operations (add/remove peer,
modify keys) increases a lot. Instead of a single lock:

 - Use a global spinlock when accessing peers tables
 (by peer-id, by vpn4 and vpn6 addresses). Acquire for
 shared access for lookup and for exclusive access
 when modifying the tables. After lookup, increment
 peer reference counter and return a pointer to a peer context.

 - When doing read-only operation on a peer, acquire
 peer-specific spinlock for shared access. For state-modifying
 operation, acquire peer-specific spinlock for exclusive access.

 - After finishing operations on a peer, decrement peer
 reference counter. If after decrement peer reference counter
 is zero, free peer context.

This approach reduces contention and ensures that peer context
won't be deleted during peer-specific operation.

GitHub: OpenVPN#89

Signed-off-by: Lev Stipakov <[email protected]>
@lstipakov lstipakov closed this Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants