Attention is currently required from: cron2, flichtenheld, its_Giaan.
plaisthos has posted comments on this change by its_Giaan. (
http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email )
Change subject: multipeer: introduce asymmetric peer-id
......................................................................
Patch Set 4: Code-Review-2
(8 comments)
Patchset:
PS4:
The part that picks the "peer-id" pushed and parsed options.c that sets peer-id
on receiving peer-id
if (found & OPT_P_PEER_ID)
{
msg(D_PUSH_DEBUG, "OPTIONS IMPORT: peer-id set");
c->c2.tls_multi->use_peer_id = true;
c->c2.tls_multi->tx_peer_id = c->options.peer_id;
}
should probably also be adjusted to set both rx and tx as peer-id as pushed
option should set both.
File src/openvpn/ssl.c:
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/7b89f526_1248648b?usp=email :
PS4, Line 1179: ret->rx_peer_id = MAX_PEER_ID;
Add comment here that we also use the rx peer id to identify DCO clients as
this has become now a important distinction.
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/18e30fd6_0e66350f?usp=email :
PS4, Line 1982: }
This is still not guarded by DCO capability. With the current version we still
always indicate to the peer that we are always asymmetric peer ID capable even
if the underlying DCO module is not able to use a different peer ID for TX.
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/3fb67b50_b42dfeaf?usp=email :
PS4, Line 2165: if (multi->rx_peer_id == MAX_PEER_ID && session->opt->mode
!= MODE_SERVER)
This feel be a very hacky place to set the multi rx peer id. I think there is a
better place to do that.
File src/openvpn/ssl_ncp.c:
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/c5df3a60_53fab78f?usp=email :
PS4, Line 425: if (tx_peer_id)
This also need to take DCO capability into account.
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/1a14b321_0b6cf1e1?usp=email :
PS4, Line 450: if (multi->use_peer_id)
I think this parts needs to be skipped if we are using/negotiated asymmetric
peer-id as it would overwrite both rx and tx ids with the EKM generated ones.
Probably move the if (tx_peer_id) above and have this as else path with a
comment that asymmetric peer id trumps EKM
File src/openvpn/ssl_util.h:
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/17f8e5bb_aeb5d80c?usp=email :
PS4, Line 56: uint32_t extract_asymmetric_peer_id(const char *peer_info);
Add doxygen please
File src/openvpn/ssl_util.c:
http://gerrit.openvpn.net/c/openvpn/+/1089/comment/aa5446c4_405e7c5e?usp=email :
PS4, Line 90: return 0;
0 is a valid peer id. So I would rather have -1 (and int32_t as return type) or
MAX_PEER_ID, MAX_UINT value or similar as not defined.
In fact the first client that typically connects to a p2mp server is assigned
value 0.
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 4
Gerrit-Owner: its_Giaan <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: its_Giaan <[email protected]>
Gerrit-Attention: cron2 <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Comment-Date: Tue, 07 Oct 2025 15:50:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
