On 8/31/22 21:48, Dmitry Safonov wrote:
On 8/23/22 15:45, Leonard Crestez wrote:
On 8/18/22 19:59, Dmitry Safonov wrote:
[..]
+#define TCP_AO 38 /* (Add/Set MKT) */
+#define TCP_AO_DEL 39 /* (Delete MKT) */
+#define TCP_AO_MOD 40 /* (Modify MKT) */
The TCP_AO_MOD sockopt doesn't actually modify and MKT, it only controls
per-socket properties. It is equivalent to my TCP_AUTHOPT sockopt while
TCP_AO is equivalent to TCP_AUTHOPT_KEY. My equivalent of TCP_AO_DEL
sockopt is a flag inside tcp_authopt_key.
Fair point, the comment could be "Modify AO", rather than "Modify MKT".
On the other side, this can later support more per-key changes than in
the initial proposal: i.e., zero per-key counters. Password and rcv/snd
ids can't change to follow RFC text, but non-essentials may.
So, the comment to the command here is not really incorrect.
I think it makes sense to at least separate per-key and per-socket
options. This way a sockopt for per-socket info doesn't contain fields
used to identify keys which is much clearer.
I also have two fields called "recv_keyid" and "recv_rnextkeyid" which
inform userspace about what the remote is sending, I'm not seeing an
equivalent on your side.
Sounds like a good candidate for getsockopt() for logs/debugging.
The specification around send_keyid in the RFC is conflicting:
* User must be able to control it
I don't see where you read it, care to point it out?
I see choosing the current_key by marking the preferred key during
an establishment of a connection, but I don't see any "MUST control
current_key". We allow changing current_key, but that's actually
not something required by RFC, the only thing required is to respect
rnext_key that's asked by peer.
* Implementation must respect rnextkeyid in incoming packet
I solved this apparent conflict by adding a
"TCP_AUTHOPT_FLAG_LOCK_KEYID" flag so that user can choose if it wants
to control the sending key or let it be controlled from the other side.
That's exactly violating the above "Implementation must respect
rnextkeyid in incoming packet". See RFC5925 (7.5.2.e).
This is based on paragraphs towards the end of Section 7.1:
>> TCP SEND, or a sequence of commands resulting in a SEND, MUST be
augmented so that the preferred outgoing MKT (current_key) and/or the
preferred incoming MKT (rnext_key) of a connection can be indicated.
This is for TCP SEND, not just open/connect. I'm reading this as a
requirement that userspace *MUST* be able to control the current key.
Yes, it does seem contradict 7.5.2.e which is why I implemented this as
a "key lock flag".
>> TCP RECEIVE, or the sequence of commands resulting in a RECEIVE,
MUST be augmented so that the KeyID and RNextKeyID of a recently
received segment is available to the user out of band (e.g., as an
additional parameter to RECEIVE or via a STATUS call).
It seems to me that it *MUST* be possible for userspace to read the
incoming rnextkeyid and handle it by itself. It could choose to follow
7.5.2.e or it could do something entirely different. When it can't
respect rnextkeyid because the key is not yet valid then userspace has
more information to make an alternative current_key decision.
[..]
Only two algorithms are defined in RFC5926 and you have to treat one of
them as a special case. I remain convinced that generic support for
arbitrary algorithms is undesirable; it's better for the algorithm to be
specified as an enum.
So, why limit a new TCP sign feature to already insecure algorithms?
One can already use any crypto algorithms for example, in tunnels.
And I don't see any benefit in defining new magic macros, only downside.
Adding support for arbitrary algorithms increases complexity for no
real-world gain. There are also lots of corner cases that must be
treated correctly like odd traffic_keylen and maclen, having an enum
means that userspace can't attempt to trick us. The ABI is also smaller.
There's also a special case in one of the two concrete KDFs defined by
RFC5925. What if there are more, will the ABI be expanded to support all
the cases?
Disagreements over whether a particular form of extensibility is
"useful" are unlikely to result in any sort of useful conclusion. I'm
lazy so I only care about interop with existing implementations from
Juniper and Cisco.
--
Regards,
Leonard
