netdev - Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250818173158.GA12939@google.com>
Date: Mon, 18 Aug 2025 17:31:58 +0000
From: Eric Biggers <ebiggers@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Xin Long <lucien.xin@...il.com>, Paolo Abeni <pabeni@...hat.com>,
	linux-sctp@...r.kernel.org, netdev@...r.kernel.org,
	Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
	linux-crypto@...r.kernel.org
Subject: Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to
 use HMAC-SHA256

On Mon, Aug 18, 2025 at 08:43:45AM -0700, Jakub Kicinski wrote:
> On Sat, 16 Aug 2025 13:15:12 -0400 Xin Long wrote:
> > > > Ideally we'd just fail the write and remove the last mentions of md5 and
> > > > sha1 from the code.  But I'm concerned there could be a case where
> > > > userspace is enabling cookie authentication by setting
> > > > cookie_hmac_alg=md5 or cookie_hmac_alg=sha1, and by just failing the
> > > > write the system would end up with cookie authentication not enabled.
> > > >
> > > > It would have been nice if this sysctl had just been a boolean toggle.
> > > >
> > > > A deprecation warning might be a good idea.  How about the following on
> > > > top of this patch:  
> > >
> > > No strong opinion but I find the deprecation warnings futile.
> > > Chances are we'll be printing this until the end of time.
> > > Either someone hard-cares and we'll need to revert, or nobody
> > > does and we can deprecate today.  
> > Reviewing past network sysctl changes, several commits have simply
> > removed or renamed parameters:
> > 
> > 4a7f60094411 ("tcp: remove thin_dupack feature")
> > 4396e46187ca ("tcp: remove tcp_tw_recycle")
> > d8b81175e412 ("tcp: remove sk_{tr}x_skb_cache")
> > 3e0b8f529c10 ("net/ipv6: Expand and rename accept_unsolicited_na to
> > accept_untracked_na")
> > 5027d54a9c30 ("net: change accept_ra_min_rtr_lft to affect all RA lifetimes")
> > 
> > It seems to me that if we deprecate something, it's okay to change the
> > sysctls, so I would prefer rejecting writes with md5 or sha1, or even
> > better following Eric’s suggestion and turn this into a simple boolean
> > toggle.
> 
> Slight preference towards reject. bool is worse in case we need to
> revert (if it takes a few releases for the regression report to appear
> we may have to maintain backward compat with both string and bool
> formats going forward).

To be clear, by "It would have been nice if this sysctl had just been a
boolean toggle", I meant it would have been nice if it had been that way
*originally*.  I wasn't suggesting making that change now.

It would be safest to continue to honor existing attempts to enable
cookie authentication (by writing md5 or sha1), as this patch does.

If you'd prefer that those attempts be rejected instead, I can do that,
but how about I do it as a separate patch on top of this one?  That way
if there's a problem with it, we can just revert that patch, instead of
the entire upgrade to the cookie auth.

- Eric