Re: [PATCH v2 06/25] tcp: authopt: Compute packet signatures

On 11/5/21 4:08 AM, Dmitry Safonov wrote:

On 11/1/21 16:34, Leonard Crestez wrote: [..]

...

+/* Find TCP_AUTHOPT in header.

• • Returns pointer to TCP_AUTHOPT or NULL if not found.
• */

+static u8 *tcp_authopt_find_option(struct tcphdr *th) +{

• int length = (th->doff << 2) - sizeof(*th);
• u8 *ptr = (u8 *)(th + 1);
• while (length >= 2) {

• int opcode = *ptr++;
  

• int opsize;
  

• switch (opcode) {
  

• case TCPOPT_EOL:
  

• 	return NULL;
  

• case TCPOPT_NOP:
  

• 	length--;
  

• 	continue;
  

• default:
  

• 	if (length < 2)
  

• 		return NULL;
  

^ never true, as checked by the loop condition

...

• 	opsize = *ptr++;
  

• 	if (opsize < 2)
  

• 		return NULL;
  

• 	if (opsize > length)
  

• 		return NULL;
  

• 	if (opcode == TCPOPT_AUTHOPT)
  

• 		return ptr - 2;
  

• }
  

• ptr += opsize - 2;
  

• length -= opsize;
  

• }
• return NULL;

+}

Why copy'n'pasting tcp_parse_md5sig_option(), rather than adding a new argument to the function?

No good reason.

There is a requirement in RFC5925 that packets with both AO and MD5 signatures be dropped. This currently works but the implementation is convoluted: after an AO signature is found an error is returned if MD5 is also present.

A better solution would be to do a single scan for both options up front, for example in tcp_{v4,v6}_auth_inbound_check

-- Regards, Leonard