On Fri, Aug 15, 2025 at 05:19:27PM -0400, Xin Long wrote:

On Fri, Aug 15, 2025 at 3:09 PM Jakub Kicinski [off-list ref] wrote:

quoted

On Tue, 12 Aug 2025 21:01:21 -0700 Eric Biggers wrote:

quoted

+     if (net->sctp.cookie_auth_enable)
+             tbl.data = (char *)"sha256";
+     else
+             tbl.data = (char *)"none";
+     tbl.maxlen = strlen(tbl.data);
+     return proc_dostring(&tbl, 0, buffer, lenp, ppos);

I wonder if someone out there expects to read back what they wrote,
but let us find out.

I feel it's a bit weird to have:

# sysctl net.sctp.cookie_hmac_alg="md5"
net.sctp.cookie_hmac_alg = md5
# sysctl net.sctp.cookie_hmac_alg
net.sctp.cookie_hmac_alg = sha256

This patch deprecates md5 and sha1 use there.
So generally, for situations like this, should we also issue a
warning, or just fail it?

Paolo, what do you think?

quoted

It'd be great to get an ack / review from SCTP maintainers, otherwise
we'll apply by Monday..

Other than that, LGTM.
Sorry for the late reply, I was running some SCTP-auth related tests
against the patchset.

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:

diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 19acc57c3ed97..72af4a843ae52 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c

@@ -399,20 +399,28 @@ static int proc_sctp_do_hmac_alg(const struct ctl_table *ctl, int write,
 		tbl.data = tmp;
 		tbl.maxlen = sizeof(tmp) - 1;
 		ret = proc_dostring(&tbl, 1, buffer, lenp, ppos);
 		if (ret)
 			return ret;
-		if (!strcmp(tmp, "sha256") ||
-		    /* for backwards compatibility */
-		    !strcmp(tmp, "md5") || !strcmp(tmp, "sha1")) {
+		if (!strcmp(tmp, "sha256")) {
 			net->sctp.cookie_auth_enable = 1;
 			return 0;
 		}
 		if (!strcmp(tmp, "none")) {
 			net->sctp.cookie_auth_enable = 0;
 			return 0;
 		}
+		/*
+		 * Accept md5 and sha1 for backwards compatibility, but treat
+		 * them simply as requests to enable cookie authentication.
+		 */
+		if (!strcmp(tmp, "md5") || !strcmp(tmp, "sha1")) {
+			pr_warn_once("net.sctp.cookie_hmac_alg=%s is deprecated. Use net.sctp.cookie_hmac_alg=sha256\n",
+				     tmp);
+			net->sctp.cookie_auth_enable = 1;
+			return 0;
+		}
 		return -EINVAL;
 	}
 	if (net->sctp.cookie_auth_enable)
 		tbl.data = (char *)"sha256";
 	else