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

[Leonard Crestez]

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