On Tue, Aug 12, 2025 at 07:27:48AM +0200, Claudio Jeker wrote:
> On Sat, Aug 09, 2025 at 01:40:12PM +0200, Alexander Bluhm wrote:
> > Hi,
> > 
> > I would like to convert these nested macros into am inline function
> > to calculate the has for the TCP SYN cache.
> > 
> > ok?
> 
> Why is this not using siphash? I do not trust these hash function.
> This hash seems to be trivially reversible and so it is possible to attack
> it.

Because SIP hash is slow and nobody made a performance analysis for
TCP SYN.

We have SIP hash in inet PCB, where it can take significant amount
of CPU time.  When sending short UDP packets from Linux to OpenBSD
the hash function takes 11.3% CPU time.

http://bluhm.genua.de/perform/results/2025-07-30T08:32:36Z/2025-07-30T00%3A00%3A00Z/btrace/netbench.pl_-v_-B1000000000_-b1000000_-d1_-f0_-i3_-N10_-cperform%40lt13_-a10.3.46.40_-t10_udpbench-btrace-kstack.0.svg?s=in_pcbhash

The trivial SYN hash can be attacked, but the SYN cache will regularly
reshuffle its random.  I have implemented this to prevent timing
attacks on the length of the hash buckets.

Maybe xxhash could be an alternative between slow SIP hash and trvial
hashes.  https://github.com/Cyan4973/xxHash

bluhm

> > Index: netinet/tcp_input.c
> > ===================================================================
> > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
> > diff -u -p -r1.457 tcp_input.c
> > --- netinet/tcp_input.c	24 Jul 2025 21:34:07 -0000	1.457
> > +++ netinet/tcp_input.c	8 Aug 2025 15:17:55 -0000
> > @@ -3216,42 +3216,38 @@ struct syn_cache_set tcp_syn_cache[2];		
> >  int tcp_syn_cache_active;			/* [S] */
> >  struct mutex syn_cache_mtx = MUTEX_INITIALIZER(IPL_SOFTNET);
> >  
> > -#define SYN_HASH(sa, sp, dp, rand) \
> > -	(((sa)->s_addr ^ (rand)[0]) *				\
> > -	(((((u_int32_t)(dp))<<16) + ((u_int32_t)(sp))) ^ (rand)[4]))
> > -#ifndef INET6
> > -#define	SYN_HASHALL(hash, src, dst, rand) \
> > -do {									\
> > -	hash = SYN_HASH(&satosin_const(src)->sin_addr,			\
> > -		satosin_const(src)->sin_port,				\
> > -		satosin_const(dst)->sin_port, (rand));			\
> > -} while (/*CONSTCOND*/ 0)
> > -#else
> > -#define SYN_HASH6(sa, sp, dp, rand) \
> > -	(((sa)->s6_addr32[0] ^ (rand)[0]) *			\
> > -	((sa)->s6_addr32[1] ^ (rand)[1]) *			\
> > -	((sa)->s6_addr32[2] ^ (rand)[2]) *			\
> > -	((sa)->s6_addr32[3] ^ (rand)[3]) *			\
> > -	(((((u_int32_t)(dp))<<16) + ((u_int32_t)(sp))) ^ (rand)[4]))
> > -
> > -#define SYN_HASHALL(hash, src, dst, rand) \
> > -do {									\
> > -	switch ((src)->sa_family) {					\
> > -	case AF_INET:							\
> > -		hash = SYN_HASH(&satosin_const(src)->sin_addr,		\
> > -			satosin_const(src)->sin_port,			\
> > -			satosin_const(dst)->sin_port, (rand));		\
> > -		break;							\
> > -	case AF_INET6:							\
> > -		hash = SYN_HASH6(&satosin6_const(src)->sin6_addr,	\
> > -			satosin6_const(src)->sin6_port,			\
> > -			satosin6_const(dst)->sin6_port, (rand));	\
> > -		break;							\
> > -	default:							\
> > -		hash = 0;						\
> > -	}								\
> > -} while (/*CONSTCOND*/0)
> > -#endif /* INET6 */
> > +static inline uint32_t
> > +syn_cache_hash(const struct sockaddr *src, const struct sockaddr *dst,
> > +    uint32_t rand[])
> > +{
> > +	switch (src->sa_family) {
> > +	case AF_INET: {
> > +		uint32_t src_port = satosin_const(src)->sin_port;
> > +		uint32_t dst_port = satosin_const(dst)->sin_port;
> > +		const in_addr_t *src_addr =
> > +		    &satosin_const(src)->sin_addr.s_addr;
> > +
> > +		return ((((dst_port << 16) + src_port) ^ rand[4]) *
> > +		    (*src_addr ^ rand[0]));
> > +	    }
> > +#ifdef INET6
> > +	case AF_INET6: {
> > +		uint32_t src_port = satosin6_const(src)->sin6_port;
> > +		uint32_t dst_port = satosin6_const(dst)->sin6_port;
> > +		const uint32_t *src_addr6 =
> > +		    satosin6_const(src)->sin6_addr.s6_addr32;
> > +
> > +		return ((((dst_port << 16) + src_port) ^ rand[4]) *
> > +		    (src_addr6[0] ^ rand[0]) *
> > +		    (src_addr6[1] ^ rand[1]) *
> > +		    (src_addr6[2] ^ rand[2]) *
> > +		    (src_addr6[3] ^ rand[3]));
> > +	    }
> > +#endif
> > +	default:
> > +		unhandled_af(src->sa_family);
> > +	}
> > +}
> >  
> >  void
> >  syn_cache_rm(struct syn_cache *sc)
> > @@ -3345,7 +3341,7 @@ syn_cache_insert(struct syn_cache *sc, s
> >  		tcpstat_inc(tcps_sc_seedrandom);
> >  	}
> >  
> > -	SYN_HASHALL(sc->sc_hash, &sc->sc_src.sa, &sc->sc_dst.sa,
> > +	sc->sc_hash = syn_cache_hash(&sc->sc_src.sa, &sc->sc_dst.sa,
> >  	    set->scs_random);
> >  	scp = &set->scs_buckethead[sc->sc_hash % set->scs_size];
> >  	sc->sc_buckethead = scp;
> > @@ -3564,7 +3560,7 @@ syn_cache_lookup(const struct sockaddr *
> >  	for (i = 0; i < 2; i++) {
> >  		if (sets[i]->scs_count == 0)
> >  			continue;
> > -		SYN_HASHALL(hash, src, dst, sets[i]->scs_random);
> > +		hash = syn_cache_hash(src, dst, sets[i]->scs_random);
> >  		scp = &sets[i]->scs_buckethead[hash % sets[i]->scs_size];
> >  		*headp = scp;
> >  		TAILQ_FOREACH(sc, &scp->sch_bucket, sc_bucketq) {
> > 
> 
> -- 
> :wq Claudio