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?").
--
Sabrina
