Re: [PATCH 08/31] net/tcp: Introduce TCP_AO setsockopt()s

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/18/22 19:59, Dmitry Safonov wrote:
Add 3 setsockopt()s:
1. to add a new Master Key Tuple (MKT) on a socket
2. to delete present MKT from a socket
3. to change flags of an MKT

Userspace has to introduce keys on every socket it wants to use TCP-AO
option on, similarly to TCP_MD5SIG/TCP_MD5SIG_EXT.
RFC5925 prohibits definition of MKTs that would match the same peer,
so do sanity checks on the data provided by userspace. Be as
conservative as possible, including refusal of defining MKT on
an established connection with no AO, removing the key in-use and etc.

(1) and (2) are to be used by userspace key manager to add/remove keys.
(3) main purpose is to set rnext_key, which (as prescribed by RFC5925)
is the key id that will be requested in TCP-AO header from the peer to
sign their segments with.

At this moment the life of ao_info ends in tcp_v4_destroy_sock().


+#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.

+struct tcp_ao { /* setsockopt(TCP_AO) */
+	struct __kernel_sockaddr_storage tcpa_addr;
+	char	tcpa_alg_name[64];
+	__u16	tcpa_flags;

This field accept TCP_AO_CMDF_CURR and TCP_AO_CMDF_NEXT which means that you are combining key addition with key selection. Not clear it shouldn't just always be a separate sockopt?

+	__u8	tcpa_prefix;
+	__u8	tcpa_sndid;
+	__u8	tcpa_rcvid;
+	__u8	tcpa_maclen;
+	__u8	tcpa_keyflags;
+	__u8	tcpa_keylen;
+	__u8	tcpa_key[TCP_AO_MAXKEYLEN];
+} __attribute__((aligned(8)));
+
+struct tcp_ao_del { /* setsockopt(TCP_AO_DEL) */
+	struct __kernel_sockaddr_storage tcpa_addr;
+	__u16	tcpa_flags;
+	__u8	tcpa_prefix;
+	__u8	tcpa_sndid;
+	__u8	tcpa_rcvid;
+	__u8	tcpa_current;
+	__u8	tcpa_rnext;
+} __attribute__((aligned(8)));
+
+struct tcp_ao_mod { /* setsockopt(TCP_AO_MOD) */
+	__u16	tcpa_flags;
+	__u8	tcpa_current;
+	__u8	tcpa_rnext;
+} __attribute__((aligned(8)));

This is quite similar to my "struct tcp_authopt" in the fact that it is intented to support controlling the "current keys".

* tcpa_current is equivalent to send_keyid
* tcpa_rnext is equivalent to send_rnextkeyid

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.

The specification around send_keyid in the RFC is conflicting:
* User must be able to control it
* 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.

The "send_rnextkeyid" is also optional in my patch, if TCP_AUTHOPT_FLAG_LOCK_RNEXTKEYID is not passed then the recv_id of the sending key is sent.

Here's a link to my implementation of key selection controls:

https://lore.kernel.org/netdev/2956d99e7fbf9ff2a8cc720c67baaef35bc32343.1660852705.git.cdleonard@xxxxxxxxx/

+static int tcp_ao_parse_crypto(struct tcp_ao *cmd, struct tcp_ao_key *key)
+{
+	unsigned int syn_tcp_option_space;
+	struct crypto_pool_ahash hp;
+	bool is_kdf_aes_128_cmac = false;
+	struct crypto_ahash *tfm;
+	int err, pool_id;
+
+	/* Force null-termination of tcpa_alg_name */
+	cmd->tcpa_alg_name[ARRAY_SIZE(cmd->tcpa_alg_name) - 1] = '\0';
+
+	/* RFC5926, 3.1.1.2. KDF_AES_128_CMAC */
+	if (!strcmp("cmac(aes128)", cmd->tcpa_alg_name)) {
+		strcpy(cmd->tcpa_alg_name, "cmac(aes)");
+		is_kdf_aes_128_cmac = (cmd->tcpa_keylen != 16);
+	}

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.

--
Regards,
Leonard



[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux