Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: inline syn cache hash function
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
tech@openbsd.org
Date:
Tue, 12 Aug 2025 14:16:03 +0200

Download raw body.

Thread
On Tue, Aug 12, 2025 at 01:56:35PM +0200, Alexander Bluhm wrote:
> 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.

Nobody did a security analysis for the TCP SYN hash either or even looking
if it is a good hash function.
 
> 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
 
I do not trust flamegraphs. They misreport a lot of things because of the
way dt(4) works. Also a lot of the time can be attributed to slow string
functions.
 
> 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
 
Maybe.
 
> > > 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
> 

-- 
:wq Claudio