On Fri, Aug 15, 2025 at 05:19:27PM -0400, Xin Long wrote:
> On Fri, Aug 15, 2025 at 3:09 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> >
> > On Tue, 12 Aug 2025 21:01:21 -0700 Eric Biggers wrote:
> > > + 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?
>
> >
> > 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
