2024-11-09, 03:01:21 +0200, Sergey Ryazanov wrote:
On 29.10.2024 12:47, Antonio Quartulli wrote:
+/* When the OpenVPN protocol is ran in AEAD mode, use
- the OpenVPN packet ID as the AEAD nonce:
- 00000005 521c3b01 4308c041
- [seq # ] [ nonce_tail ]
- [ 12-byte full IV ] -> NONCE_SIZE
- [4-bytes -> NONCE_WIRE_SIZE
- on wire]
- */
Nice diagram! Can we go futher and define the OpenVPN packet header as a stucture? Referencing the structure instead of using magic sizes and offsets can greatly improve the code readability. Especially when it comes to header construction/parsing in the encryption/decryption code.
E.g. define a structures like this:
struct ovpn_pkt_hdr { __be32 op; __be32 pktid; u8 auth[]; } __attribute__((packed));
struct ovpn_aead_iv { __be32 pktid; u8 nonce[OVPN_NONCE_TAIL_SIZE]; } __attribute__((packed));
__attribute__((packed)) should not be needed here as the fields in both structs look properly aligned, and IIRC using packed can cause the compiler to generate worse code.
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 8516c1ccd57a7c7634a538fe3ac16c858f647420..84d294aab20b79b8e9cb9b736a074105c99338f3 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -1975,4 +1975,19 @@ enum { #define IFLA_DSA_MAX (__IFLA_DSA_MAX - 1) +/* OVPN section */
+enum ovpn_mode {
- OVPN_MODE_P2P,
- OVPN_MODE_MP,
+};
Mode min/max values can be defined here and the netlink policy can reference these values:
enum ovpn_mode { OVPN_MODE_P2P, OVPN_MODE_MP, __OVPN_MODE_MAX };
#define OVPN_MODE_MIN OVPN_MODE_P2P #define OVPN_MODE_MAX (__OVPN_MODE_MAX - 1)
... = NLA_POLICY_RANGE(NLA_U8, OVPN_MODE_MIN, OVPN_MODE_MAX)
I don't think there's much benefit to that, other than making the diff smaller on a (very unlikely) patch that would add a new mode in the future. It even looks more inconvenient to me when reading the code ("ok what are _MIN and _MAX? the code is using _P2P and _MP, do they match?").