* [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth
@ 2025-08-13 4:01 Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 1/3] selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec Eric Biggers
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Eric Biggers @ 2025-08-13 4:01 UTC (permalink / raw)
To: linux-sctp, netdev, Xin Long, Marcelo Ricardo Leitner
Cc: linux-crypto, Eric Biggers
This series converts SCTP chunk and cookie authentication to use the
crypto library API instead of crypto_shash. This is much simpler (the
diffstat should speak for itself), and also faster too. In addition,
this series upgrades the cookie authentication to use HMAC-SHA256.
I've tested that kernels with this series applied can continue to
communicate using SCTP with older ones, in either direction, using any
choice of None, HMAC-SHA1, or HMAC-SHA256 chunk authentication.
Changed in v2:
- Added patch which adds CONFIG_CRYPTO_SHA1 to some selftests configs
Eric Biggers (3):
selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec
sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication
sctp: Convert cookie authentication to use HMAC-SHA256
Documentation/networking/ip-sysctl.rst | 11 +-
include/net/netns/sctp.h | 4 +-
include/net/sctp/auth.h | 17 +-
include/net/sctp/constants.h | 9 +-
include/net/sctp/structs.h | 35 +---
net/sctp/Kconfig | 47 ++----
net/sctp/auth.c | 166 ++++---------------
net/sctp/chunk.c | 3 +-
net/sctp/endpointola.c | 23 +--
net/sctp/protocol.c | 11 +-
net/sctp/sm_make_chunk.c | 60 +++----
net/sctp/sm_statefuns.c | 2 +-
net/sctp/socket.c | 41 +----
net/sctp/sysctl.c | 51 +++---
tools/testing/selftests/net/config | 1 +
tools/testing/selftests/net/netfilter/config | 1 +
16 files changed, 124 insertions(+), 358 deletions(-)
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
--
2.50.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH net-next v2 1/3] selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec
2025-08-13 4:01 [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Eric Biggers
@ 2025-08-13 4:01 ` Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 2/3] sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication Eric Biggers
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2025-08-13 4:01 UTC (permalink / raw)
To: linux-sctp, netdev, Xin Long, Marcelo Ricardo Leitner
Cc: linux-crypto, Eric Biggers, Paolo Abeni, Florian Westphal
xfrm_policy.sh, nft_flowtable.sh, and vrf-xfrm-tests.sh use 'ip xfrm'
with SHA-1, either 'auth sha1' or 'auth-trunc hmac(sha1)'. That
requires CONFIG_CRYPTO_SHA1, which CONFIG_INET_ESP intentionally doesn't
select (as per its help text). Previously, the config for these tests
relied on CONFIG_CRYPTO_SHA1 being selected by the unrelated option
CONFIG_IP_SCTP. Since CONFIG_IP_SCTP is being changed to no longer do
that, instead add CONFIG_CRYPTO_SHA1 to the configs explicitly.
Reported-by: Paolo Abeni <pabeni@redhat.com>
Closes: https://lore.kernel.org/r/766e4508-aaba-4cdc-92b4-e116e52ae13b@redhat.com
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
tools/testing/selftests/net/config | 1 +
tools/testing/selftests/net/netfilter/config | 1 +
2 files changed, 2 insertions(+)
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index c24417d0047bb..d548611e2698e 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -24,10 +24,11 @@ CONFIG_VLAN_8021Q=y
CONFIG_GENEVE=m
CONFIG_IFB=y
CONFIG_INET_DIAG=y
CONFIG_INET_ESP=y
CONFIG_INET_ESP_OFFLOAD=y
+CONFIG_CRYPTO_SHA1=y
CONFIG_NET_FOU=y
CONFIG_NET_FOU_IP_TUNNELS=y
CONFIG_NETFILTER=y
CONFIG_NETFILTER_ADVANCED=y
CONFIG_NETFILTER_XTABLES_LEGACY=y
diff --git a/tools/testing/selftests/net/netfilter/config b/tools/testing/selftests/net/netfilter/config
index 79d5b33966ba1..305e46b819cbe 100644
--- a/tools/testing/selftests/net/netfilter/config
+++ b/tools/testing/selftests/net/netfilter/config
@@ -11,10 +11,11 @@ CONFIG_BRIDGE_NETFILTER=m
CONFIG_BRIDGE_NF_EBTABLES=m
CONFIG_BRIDGE_VLAN_FILTERING=y
CONFIG_CGROUP_BPF=y
CONFIG_DUMMY=m
CONFIG_INET_ESP=m
+CONFIG_CRYPTO_SHA1=m
CONFIG_IP_NF_MATCH_RPFILTER=m
CONFIG_IP6_NF_MATCH_RPFILTER=m
CONFIG_IP_NF_IPTABLES=m
CONFIG_IP_NF_IPTABLES_LEGACY=m
CONFIG_IP6_NF_IPTABLES=m
--
2.50.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v2 2/3] sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication
2025-08-13 4:01 [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 1/3] selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec Eric Biggers
@ 2025-08-13 4:01 ` Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256 Eric Biggers
2025-08-18 17:42 ` [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Xin Long
3 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2025-08-13 4:01 UTC (permalink / raw)
To: linux-sctp, netdev, Xin Long, Marcelo Ricardo Leitner
Cc: linux-crypto, Eric Biggers
For SCTP chunk authentication, use the HMAC-SHA1 and HMAC-SHA256 library
functions instead of crypto_shash. This is simpler and faster. There's
no longer any need to pre-allocate 'crypto_shash' objects; the SCTP code
now simply calls into the HMAC code directly.
As part of this, make SCTP always support both HMAC-SHA1 and
HMAC-SHA256. Previously, it only guaranteed support for HMAC-SHA1.
However, HMAC-SHA256 tended to be supported too anyway, as it was
supported if CONFIG_CRYPTO_SHA256 was enabled elsewhere in the kconfig.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
include/net/sctp/auth.h | 17 ++--
include/net/sctp/constants.h | 4 -
include/net/sctp/structs.h | 5 --
net/sctp/Kconfig | 15 ++--
net/sctp/auth.c | 166 ++++++-----------------------------
net/sctp/chunk.c | 3 +-
net/sctp/sm_make_chunk.c | 2 +-
net/sctp/sm_statefuns.c | 2 +-
net/sctp/socket.c | 10 ---
9 files changed, 47 insertions(+), 177 deletions(-)
diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
index d4b3b2dcd15b7..3d5879e08e78a 100644
--- a/include/net/sctp/auth.h
+++ b/include/net/sctp/auth.h
@@ -20,20 +20,15 @@
struct sctp_endpoint;
struct sctp_association;
struct sctp_authkey;
struct sctp_hmacalgo;
-struct crypto_shash;
-/*
- * Define a generic struct that will hold all the info
- * necessary for an HMAC transform
- */
+/* Defines an HMAC algorithm supported by SCTP chunk authentication */
struct sctp_hmac {
- __u16 hmac_id; /* one of the above ids */
- char *hmac_name; /* name for loading */
- __u16 hmac_len; /* length of the signature */
+ __u16 hmac_id; /* one of SCTP_AUTH_HMAC_ID_* */
+ __u16 hmac_len; /* length of the HMAC value in bytes */
};
/* This is generic structure that containst authentication bytes used
* as keying material. It's a what is referred to as byte-vector all
* over SCTP-AUTH
@@ -76,13 +71,13 @@ struct sctp_shared_key *sctp_auth_get_shkey(
__u16 key_id);
int sctp_auth_asoc_copy_shkeys(const struct sctp_endpoint *ep,
struct sctp_association *asoc,
gfp_t gfp);
int sctp_auth_init_hmacs(struct sctp_endpoint *ep, gfp_t gfp);
-void sctp_auth_destroy_hmacs(struct crypto_shash *auth_hmacs[]);
-struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id);
-struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc);
+const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id);
+const struct sctp_hmac *
+sctp_auth_asoc_get_hmac(const struct sctp_association *asoc);
void sctp_auth_asoc_set_default_hmac(struct sctp_association *asoc,
struct sctp_hmac_algo_param *hmacs);
int sctp_auth_asoc_verify_hmac_id(const struct sctp_association *asoc,
__be16 hmac_id);
int sctp_auth_send_cid(enum sctp_cid chunk,
diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index 5859e0a16a584..8e0f4c4f77506 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -415,20 +415,16 @@ enum sctp_lower_cwnd {
*/
enum {
SCTP_AUTH_HMAC_ID_RESERVED_0,
SCTP_AUTH_HMAC_ID_SHA1,
SCTP_AUTH_HMAC_ID_RESERVED_2,
-#if defined (CONFIG_CRYPTO_SHA256) || defined (CONFIG_CRYPTO_SHA256_MODULE)
SCTP_AUTH_HMAC_ID_SHA256,
-#endif
__SCTP_AUTH_HMAC_MAX
};
#define SCTP_AUTH_HMAC_ID_MAX __SCTP_AUTH_HMAC_MAX - 1
#define SCTP_AUTH_NUM_HMACS __SCTP_AUTH_HMAC_MAX
-#define SCTP_SHA1_SIG_SIZE 20
-#define SCTP_SHA256_SIG_SIZE 32
/* SCTP-AUTH, Section 3.2
* The chunk types for INIT, INIT-ACK, SHUTDOWN-COMPLETE and AUTH chunks
* MUST NOT be listed in the CHUNKS parameter
*/
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 8a540ad9b5090..6be6aec25731e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1327,15 +1327,10 @@ struct sctp_endpoint {
__u32 sndbuf_policy;
/* rcvbuf acct. policy. */
__u32 rcvbuf_policy;
- /* SCTP AUTH: array of the HMACs that will be allocated
- * we need this per association so that we don't serialize
- */
- struct crypto_shash **auth_hmacs;
-
/* SCTP-AUTH: hmacs for the endpoint encoded into parameter */
struct sctp_hmac_algo_param *auth_hmacs_list;
/* SCTP-AUTH: chunks to authenticate encoded into parameter */
struct sctp_chunks_param *auth_chunk_list;
diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
index 24d5a35ce894a..192027555b4d8 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -5,13 +5,12 @@
menuconfig IP_SCTP
tristate "The SCTP Protocol"
depends on INET
depends on IPV6 || IPV6=n
- select CRYPTO
- select CRYPTO_HMAC
- select CRYPTO_SHA1
+ select CRYPTO_LIB_SHA1
+ select CRYPTO_LIB_SHA256
select NET_CRC32C
select NET_UDP_TUNNEL
help
Stream Control Transmission Protocol
@@ -77,19 +76,21 @@ endchoice
config SCTP_COOKIE_HMAC_MD5
bool "Enable optional MD5 hmac cookie generation"
help
Enable optional MD5 hmac based SCTP cookie generation
- select CRYPTO_HMAC if SCTP_COOKIE_HMAC_MD5
- select CRYPTO_MD5 if SCTP_COOKIE_HMAC_MD5
+ select CRYPTO
+ select CRYPTO_HMAC
+ select CRYPTO_MD5
config SCTP_COOKIE_HMAC_SHA1
bool "Enable optional SHA1 hmac cookie generation"
help
Enable optional SHA1 hmac based SCTP cookie generation
- select CRYPTO_HMAC if SCTP_COOKIE_HMAC_SHA1
- select CRYPTO_SHA1 if SCTP_COOKIE_HMAC_SHA1
+ select CRYPTO
+ select CRYPTO_HMAC
+ select CRYPTO_SHA1
config INET_SCTP_DIAG
depends on INET_DIAG
def_tristate INET_DIAG
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index c58fffc86a0c2..82aad477590e2 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -10,40 +10,41 @@
*
* Written or modified by:
* Vlad Yasevich <vladislav.yasevich@hp.com>
*/
-#include <crypto/hash.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
#include <linux/slab.h>
#include <linux/types.h>
-#include <linux/scatterlist.h>
#include <net/sctp/sctp.h>
#include <net/sctp/auth.h>
-static struct sctp_hmac sctp_hmac_list[SCTP_AUTH_NUM_HMACS] = {
+static const struct sctp_hmac sctp_hmac_list[SCTP_AUTH_NUM_HMACS] = {
{
/* id 0 is reserved. as all 0 */
.hmac_id = SCTP_AUTH_HMAC_ID_RESERVED_0,
},
{
.hmac_id = SCTP_AUTH_HMAC_ID_SHA1,
- .hmac_name = "hmac(sha1)",
- .hmac_len = SCTP_SHA1_SIG_SIZE,
+ .hmac_len = SHA1_DIGEST_SIZE,
},
{
/* id 2 is reserved as well */
.hmac_id = SCTP_AUTH_HMAC_ID_RESERVED_2,
},
-#if IS_ENABLED(CONFIG_CRYPTO_SHA256)
{
.hmac_id = SCTP_AUTH_HMAC_ID_SHA256,
- .hmac_name = "hmac(sha256)",
- .hmac_len = SCTP_SHA256_SIG_SIZE,
+ .hmac_len = SHA256_DIGEST_SIZE,
}
-#endif
};
+static bool sctp_hmac_supported(__u16 hmac_id)
+{
+ return hmac_id < ARRAY_SIZE(sctp_hmac_list) &&
+ sctp_hmac_list[hmac_id].hmac_len != 0;
+}
void sctp_auth_key_put(struct sctp_auth_bytes *key)
{
if (!key)
return;
@@ -442,88 +443,20 @@ struct sctp_shared_key *sctp_auth_get_shkey(
}
return NULL;
}
-/*
- * Initialize all the possible digest transforms that we can use. Right
- * now, the supported digests are SHA1 and SHA256. We do this here once
- * because of the restrictiong that transforms may only be allocated in
- * user context. This forces us to pre-allocated all possible transforms
- * at the endpoint init time.
- */
-int sctp_auth_init_hmacs(struct sctp_endpoint *ep, gfp_t gfp)
-{
- struct crypto_shash *tfm = NULL;
- __u16 id;
-
- /* If the transforms are already allocated, we are done */
- if (ep->auth_hmacs)
- return 0;
-
- /* Allocated the array of pointers to transorms */
- ep->auth_hmacs = kcalloc(SCTP_AUTH_NUM_HMACS,
- sizeof(struct crypto_shash *),
- gfp);
- if (!ep->auth_hmacs)
- return -ENOMEM;
-
- for (id = 0; id < SCTP_AUTH_NUM_HMACS; id++) {
-
- /* See is we support the id. Supported IDs have name and
- * length fields set, so that we can allocated and use
- * them. We can safely just check for name, for without the
- * name, we can't allocate the TFM.
- */
- if (!sctp_hmac_list[id].hmac_name)
- continue;
-
- /* If this TFM has been allocated, we are all set */
- if (ep->auth_hmacs[id])
- continue;
-
- /* Allocate the ID */
- tfm = crypto_alloc_shash(sctp_hmac_list[id].hmac_name, 0, 0);
- if (IS_ERR(tfm))
- goto out_err;
-
- ep->auth_hmacs[id] = tfm;
- }
-
- return 0;
-
-out_err:
- /* Clean up any successful allocations */
- sctp_auth_destroy_hmacs(ep->auth_hmacs);
- ep->auth_hmacs = NULL;
- return -ENOMEM;
-}
-
-/* Destroy the hmac tfm array */
-void sctp_auth_destroy_hmacs(struct crypto_shash *auth_hmacs[])
-{
- int i;
-
- if (!auth_hmacs)
- return;
-
- for (i = 0; i < SCTP_AUTH_NUM_HMACS; i++) {
- crypto_free_shash(auth_hmacs[i]);
- }
- kfree(auth_hmacs);
-}
-
-
-struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id)
+const struct sctp_hmac *sctp_auth_get_hmac(__u16 hmac_id)
{
return &sctp_hmac_list[hmac_id];
}
/* Get an hmac description information that we can use to build
* the AUTH chunk
*/
-struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
+const struct sctp_hmac *
+sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
{
struct sctp_hmac_algo_param *hmacs;
__u16 n_elt;
__u16 id = 0;
int i;
@@ -541,30 +474,14 @@ struct sctp_hmac *sctp_auth_asoc_get_hmac(const struct sctp_association *asoc)
n_elt = (ntohs(hmacs->param_hdr.length) -
sizeof(struct sctp_paramhdr)) >> 1;
for (i = 0; i < n_elt; i++) {
id = ntohs(hmacs->hmac_ids[i]);
-
- /* Check the id is in the supported range. And
- * see if we support the id. Supported IDs have name and
- * length fields set, so that we can allocate and use
- * them. We can safely just check for name, for without the
- * name, we can't allocate the TFM.
- */
- if (id > SCTP_AUTH_HMAC_ID_MAX ||
- !sctp_hmac_list[id].hmac_name) {
- id = 0;
- continue;
- }
-
- break;
+ if (sctp_hmac_supported(id))
+ return &sctp_hmac_list[id];
}
-
- if (id == 0)
- return NULL;
-
- return &sctp_hmac_list[id];
+ return NULL;
}
static int __sctp_auth_find_hmacid(__be16 *hmacs, int n_elts, __be16 hmac_id)
{
int found = 0;
@@ -604,31 +521,23 @@ int sctp_auth_asoc_verify_hmac_id(const struct sctp_association *asoc,
* algorithm it supports.
*/
void sctp_auth_asoc_set_default_hmac(struct sctp_association *asoc,
struct sctp_hmac_algo_param *hmacs)
{
- struct sctp_endpoint *ep;
__u16 id;
int i;
int n_params;
/* if the default id is already set, use it */
if (asoc->default_hmac_id)
return;
n_params = (ntohs(hmacs->param_hdr.length) -
sizeof(struct sctp_paramhdr)) >> 1;
- ep = asoc->ep;
for (i = 0; i < n_params; i++) {
id = ntohs(hmacs->hmac_ids[i]);
-
- /* Check the id is in the supported range */
- if (id > SCTP_AUTH_HMAC_ID_MAX)
- continue;
-
- /* If this TFM has been allocated, use this id */
- if (ep->auth_hmacs[id]) {
+ if (sctp_hmac_supported(id)) {
asoc->default_hmac_id = id;
break;
}
}
}
@@ -707,14 +616,13 @@ int sctp_auth_recv_cid(enum sctp_cid chunk, const struct sctp_association *asoc)
void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
struct sk_buff *skb, struct sctp_auth_chunk *auth,
struct sctp_shared_key *ep_key, gfp_t gfp)
{
struct sctp_auth_bytes *asoc_key;
- struct crypto_shash *tfm;
__u16 key_id, hmac_id;
- unsigned char *end;
int free_key = 0;
+ size_t data_len;
__u8 *digest;
/* Extract the info we need:
* - hmac id
* - key id
@@ -731,23 +639,21 @@ void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
return;
free_key = 1;
}
- /* set up scatter list */
- end = skb_tail_pointer(skb);
-
- tfm = asoc->ep->auth_hmacs[hmac_id];
-
+ data_len = skb_tail_pointer(skb) - (unsigned char *)auth;
digest = (u8 *)(&auth->auth_hdr + 1);
- if (crypto_shash_setkey(tfm, &asoc_key->data[0], asoc_key->len))
- goto free;
-
- crypto_shash_tfm_digest(tfm, (u8 *)auth, end - (unsigned char *)auth,
- digest);
+ if (hmac_id == SCTP_AUTH_HMAC_ID_SHA1) {
+ hmac_sha1_usingrawkey(asoc_key->data, asoc_key->len,
+ (const u8 *)auth, data_len, digest);
+ } else {
+ WARN_ON_ONCE(hmac_id != SCTP_AUTH_HMAC_ID_SHA256);
+ hmac_sha256_usingrawkey(asoc_key->data, asoc_key->len,
+ (const u8 *)auth, data_len, digest);
+ }
-free:
if (free_key)
sctp_auth_key_put(asoc_key);
}
/* API Helpers */
@@ -786,18 +692,15 @@ int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
* SHA1 is specified.
*/
for (i = 0; i < hmacs->shmac_num_idents; i++) {
id = hmacs->shmac_idents[i];
- if (id > SCTP_AUTH_HMAC_ID_MAX)
+ if (!sctp_hmac_supported(id))
return -EOPNOTSUPP;
if (SCTP_AUTH_HMAC_ID_SHA1 == id)
has_sha1 = 1;
-
- if (!sctp_hmac_list[id].hmac_name)
- return -EOPNOTSUPP;
}
if (!has_sha1)
return -EINVAL;
@@ -1019,12 +922,10 @@ int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
return 0;
}
int sctp_auth_init(struct sctp_endpoint *ep, gfp_t gfp)
{
- int err = -ENOMEM;
-
/* Allocate space for HMACS and CHUNKS authentication
* variables. There are arrays that we encode directly
* into parameters to make the rest of the operations easier.
*/
if (!ep->auth_hmacs_list) {
@@ -1058,32 +959,23 @@ int sctp_auth_init(struct sctp_endpoint *ep, gfp_t gfp)
auth_chunks->param_hdr.length =
htons(sizeof(struct sctp_paramhdr));
ep->auth_chunk_list = auth_chunks;
}
- /* Allocate and initialize transorms arrays for supported
- * HMACs.
- */
- err = sctp_auth_init_hmacs(ep, gfp);
- if (err)
- goto nomem;
-
return 0;
nomem:
/* Free all allocations */
kfree(ep->auth_hmacs_list);
kfree(ep->auth_chunk_list);
ep->auth_hmacs_list = NULL;
ep->auth_chunk_list = NULL;
- return err;
+ return -ENOMEM;
}
void sctp_auth_free(struct sctp_endpoint *ep)
{
kfree(ep->auth_hmacs_list);
kfree(ep->auth_chunk_list);
ep->auth_hmacs_list = NULL;
ep->auth_chunk_list = NULL;
- sctp_auth_destroy_hmacs(ep->auth_hmacs);
- ep->auth_hmacs = NULL;
}
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index fd4f8243cc35f..c655b571ca01b 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -182,11 +182,12 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
/* If the peer requested that we authenticate DATA chunks
* we need to account for bundling of the AUTH chunks along with
* DATA.
*/
if (sctp_auth_send_cid(SCTP_CID_DATA, asoc)) {
- struct sctp_hmac *hmac_desc = sctp_auth_asoc_get_hmac(asoc);
+ const struct sctp_hmac *hmac_desc =
+ sctp_auth_asoc_get_hmac(asoc);
if (hmac_desc)
max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
hmac_desc->hmac_len);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 3ead591c72fd3..6961607379191 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1317,11 +1317,11 @@ struct sctp_chunk *sctp_make_op_error(const struct sctp_association *asoc,
struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc,
__u16 key_id)
{
struct sctp_authhdr auth_hdr;
- struct sctp_hmac *hmac_desc;
+ const struct sctp_hmac *hmac_desc;
struct sctp_chunk *retval;
/* Get the first hmac that the peer told us to use */
hmac_desc = sctp_auth_asoc_get_hmac(asoc);
if (unlikely(!hmac_desc))
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index a0524ba8d7878..f4001f592911f 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4359,11 +4359,11 @@ static enum sctp_ierror sctp_sf_authenticate(
struct sctp_chunk *chunk)
{
struct sctp_shared_key *sh_key = NULL;
struct sctp_authhdr *auth_hdr;
__u8 *save_digest, *digest;
- struct sctp_hmac *hmac;
+ const struct sctp_hmac *hmac;
unsigned int sig_len;
__u16 key_id;
/* Pull in the auth header, so we can do some more verification */
auth_hdr = (struct sctp_authhdr *)chunk->skb->data;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 4921416434f9a..0292881a847ca 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -9579,20 +9579,10 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
err = sctp_bind_addr_dup(&newsp->ep->base.bind_addr,
&oldsp->ep->base.bind_addr, GFP_KERNEL);
if (err)
return err;
- /* New ep's auth_hmacs should be set if old ep's is set, in case
- * that net->sctp.auth_enable has been changed to 0 by users and
- * new ep's auth_hmacs couldn't be set in sctp_endpoint_init().
- */
- if (oldsp->ep->auth_hmacs) {
- err = sctp_auth_init_hmacs(newsp->ep, GFP_KERNEL);
- if (err)
- return err;
- }
-
sctp_auto_asconf_init(newsp);
/* Move any messages in the old socket's receive queue that are for the
* peeled off association to the new socket's receive queue.
*/
--
2.50.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-13 4:01 [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 1/3] selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 2/3] sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication Eric Biggers
@ 2025-08-13 4:01 ` Eric Biggers
2025-08-15 19:09 ` Jakub Kicinski
2025-08-18 17:42 ` [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Xin Long
3 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2025-08-13 4:01 UTC (permalink / raw)
To: linux-sctp, netdev, Xin Long, Marcelo Ricardo Leitner
Cc: linux-crypto, Eric Biggers
Convert SCTP cookies to use HMAC-SHA256, instead of the previous choice
of the legacy algorithms HMAC-MD5 and HMAC-SHA1. Simplify and optimize
the code by using the HMAC-SHA256 library instead of crypto_shash, and
by preparing the HMAC key when it is generated instead of per-operation.
This doesn't break compatibility, since the cookie format is an
implementation detail, not part of the SCTP protocol itself.
Note that the cookie size doesn't change either. The HMAC field was
already 32 bytes, even though previously at most 20 bytes were actually
compared. 32 bytes exactly fits an untruncated HMAC-SHA256 value. So,
although we could safely truncate the MAC to something slightly shorter,
for now just keep the cookie size the same.
I also considered SipHash, but that would generate only 8-byte MACs. An
8-byte MAC *might* suffice here. However, there's quite a lot of
information in the SCTP cookies: more than in TCP SYN cookies. So
absent an analysis that occasional forgeries of all that information is
okay in SCTP, I errored on the side of caution.
Remove HMAC-MD5 and HMAC-SHA1 as options, since the new HMAC-SHA256
option is just better. It's faster as well as more secure. For
example, benchmarking on x86_64, cookie authentication is now nearly 3x
as fast as the previous default choice and implementation of HMAC-MD5.
Also just make the kernel always support cookie authentication if SCTP
is supported at all, rather than making it optional in the build. (It
was sort of optional before, but it didn't really work properly. E.g.,
a kernel with CONFIG_SCTP_COOKIE_HMAC_MD5=n still supported HMAC-MD5
cookie authentication if CONFIG_CRYPTO_HMAC and CONFIG_CRYPTO_MD5
happened to be enabled in the kconfig for other reasons.)
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
Documentation/networking/ip-sysctl.rst | 11 ++---
include/net/netns/sctp.h | 4 +-
include/net/sctp/constants.h | 5 +--
include/net/sctp/structs.h | 30 ++++---------
net/sctp/Kconfig | 44 +++++--------------
net/sctp/endpointola.c | 23 +++++-----
net/sctp/protocol.c | 11 ++---
net/sctp/sm_make_chunk.c | 58 ++++++++------------------
net/sctp/socket.c | 31 +-------------
net/sctp/sysctl.c | 51 ++++++++++------------
10 files changed, 81 insertions(+), 187 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index bb620f554598b..0f939ccc2c0dd 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -3506,20 +3506,17 @@ cookie_preserve_enable - BOOLEAN
cookie_hmac_alg - STRING
Select the hmac algorithm used when generating the cookie value sent by
a listening sctp socket to a connecting client in the INIT-ACK chunk.
Valid values are:
- * md5
- * sha1
+ * sha256
* none
- Ability to assign md5 or sha1 as the selected alg is predicated on the
- configuration of those algorithms at build time (CONFIG_CRYPTO_MD5 and
- CONFIG_CRYPTO_SHA1).
+ md5 and sha1 are also accepted for backwards compatibility, but cause
+ sha256 to be selected.
- Default: Dependent on configuration. MD5 if available, else SHA1 if
- available, else none.
+ Default: sha256
rcvbuf_policy - INTEGER
Determines if the receive buffer is attributed to the socket or to
association. SCTP supports the capability to create multiple
associations on a single socket. When using this capability, it is
diff --git a/include/net/netns/sctp.h b/include/net/netns/sctp.h
index d25cd7a9c5ff8..c0f97f36389e6 100644
--- a/include/net/netns/sctp.h
+++ b/include/net/netns/sctp.h
@@ -73,12 +73,12 @@ struct netns_sctp {
int max_burst;
/* Whether Cookie Preservative is enabled(1) or not(0) */
int cookie_preserve_enable;
- /* The namespace default hmac alg */
- char *sctp_hmac_alg;
+ /* Whether cookie authentication is enabled(1) or not(0) */
+ int cookie_auth_enable;
/* Valid.Cookie.Life - 60 seconds */
unsigned int valid_cookie_life;
/* Delayed SACK timeout 200ms default*/
diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index 8e0f4c4f77506..ae3376ba0b991 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -294,13 +294,12 @@ enum { SCTP_MAX_GABS = 16 };
#define SCTP_DEFAULT_MAXSEGMENT 1500 /* MTU size, this is the limit
* to which we will raise the P-MTU.
*/
#define SCTP_DEFAULT_MINSEGMENT 512 /* MTU size ... if no mtu disc */
-#define SCTP_SECRET_SIZE 32 /* Number of octets in a 256 bits. */
-
-#define SCTP_SIGNATURE_SIZE 20 /* size of a SLA-1 signature */
+#define SCTP_COOKIE_KEY_SIZE 32 /* size of cookie HMAC key */
+#define SCTP_COOKIE_MAC_SIZE 32 /* size of HMAC field in cookies */
#define SCTP_COOKIE_MULTIPLE 32 /* Pad out our cookie to make our hash
* functions simpler to write.
*/
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 6be6aec25731e..2ae390219efdd 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -30,10 +30,11 @@
*/
#ifndef __sctp_structs_h__
#define __sctp_structs_h__
+#include <crypto/sha2.h>
#include <linux/ktime.h>
#include <linux/generic-radix-tree.h>
#include <linux/rhashtable-types.h>
#include <linux/socket.h> /* linux/in.h needs this!! */
#include <linux/in.h> /* We get struct sockaddr_in. */
@@ -66,11 +67,10 @@ struct sctp_chunk;
struct sctp_inq;
struct sctp_outq;
struct sctp_bind_addr;
struct sctp_ulpq;
struct sctp_ep_common;
-struct crypto_shash;
struct sctp_stream;
#include <net/sctp/tsnmap.h>
#include <net/sctp/ulpevent.h>
@@ -153,14 +153,10 @@ struct sctp_sock {
enum sctp_socket_type type;
/* PF_ family specific functions. */
struct sctp_pf *pf;
- /* Access to HMAC transform. */
- struct crypto_shash *hmac;
- char *sctp_hmac_alg;
-
/* What is our base endpointer? */
struct sctp_endpoint *ep;
struct sctp_bind_bucket *bind_hash;
/* Various Socket Options. */
@@ -225,11 +221,12 @@ struct sctp_sock {
disable_fragments:1,
v4mapped:1,
frag_interleave:1,
recvrcvinfo:1,
recvnxtinfo:1,
- data_ready_signalled:1;
+ data_ready_signalled:1,
+ cookie_auth_enable:1;
atomic_t pd_mode;
/* Fields after this point will be skipped on copies, like on accept
* and peeloff operations
@@ -333,11 +330,11 @@ struct sctp_cookie {
};
/* The format of our cookie that we send to our peer. */
struct sctp_signed_cookie {
- __u8 signature[SCTP_SECRET_SIZE];
+ __u8 mac[SCTP_COOKIE_MAC_SIZE];
__u32 __pad; /* force sctp_cookie alignment to 64 bits */
struct sctp_cookie c;
} __packed;
/* This is another convenience type to allocate memory for address
@@ -1305,26 +1302,13 @@ struct sctp_endpoint {
* is implemented.
*/
/* This is really a list of struct sctp_association entries. */
struct list_head asocs;
- /* Secret Key: A secret key used by this endpoint to compute
- * the MAC. This SHOULD be a cryptographic quality
- * random number with a sufficient length.
- * Discussion in [RFC1750] can be helpful in
- * selection of the key.
- */
- __u8 secret_key[SCTP_SECRET_SIZE];
-
- /* digest: This is a digest of the sctp cookie. This field is
- * only used on the receive path when we try to validate
- * that the cookie has not been tampered with. We put
- * this here so we pre-allocate this once and can re-use
- * on every receive.
- */
- __u8 *digest;
-
+ /* Cookie authentication key used by this endpoint */
+ struct hmac_sha256_key cookie_auth_key;
+
/* sendbuf acct. policy. */
__u32 sndbuf_policy;
/* rcvbuf acct. policy. */
__u32 rcvbuf_policy;
diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
index 192027555b4d8..e947646a380cd 100644
--- a/net/sctp/Kconfig
+++ b/net/sctp/Kconfig
@@ -7,10 +7,11 @@ menuconfig IP_SCTP
tristate "The SCTP Protocol"
depends on INET
depends on IPV6 || IPV6=n
select CRYPTO_LIB_SHA1
select CRYPTO_LIB_SHA256
+ select CRYPTO_LIB_UTILS
select NET_CRC32C
select NET_UDP_TUNNEL
help
Stream Control Transmission Protocol
@@ -46,52 +47,29 @@ config SCTP_DBG_OBJCNT
type of objects that are currently allocated. This is useful for
identifying memory leaks. This debug information can be viewed by
'cat /proc/net/sctp/sctp_dbg_objcnt'
If unsure, say N
+
choice
- prompt "Default SCTP cookie HMAC encoding"
- default SCTP_DEFAULT_COOKIE_HMAC_MD5
+ prompt "Default SCTP cookie authentication method"
+ default SCTP_DEFAULT_COOKIE_HMAC_SHA256
help
- This option sets the default sctp cookie hmac algorithm
- when in doubt select 'md5'
+ This option sets the default SCTP cookie authentication method, for
+ when a method hasn't been explicitly selected via the
+ net.sctp.cookie_hmac_alg sysctl.
-config SCTP_DEFAULT_COOKIE_HMAC_MD5
- bool "Enable optional MD5 hmac cookie generation"
- help
- Enable optional MD5 hmac based SCTP cookie generation
- select SCTP_COOKIE_HMAC_MD5
+ If unsure, choose the default (HMAC-SHA256).
-config SCTP_DEFAULT_COOKIE_HMAC_SHA1
- bool "Enable optional SHA1 hmac cookie generation"
- help
- Enable optional SHA1 hmac based SCTP cookie generation
- select SCTP_COOKIE_HMAC_SHA1
+config SCTP_DEFAULT_COOKIE_HMAC_SHA256
+ bool "HMAC-SHA256"
config SCTP_DEFAULT_COOKIE_HMAC_NONE
- bool "Use no hmac alg in SCTP cookie generation"
- help
- Use no hmac algorithm in SCTP cookie generation
+ bool "None"
endchoice
-config SCTP_COOKIE_HMAC_MD5
- bool "Enable optional MD5 hmac cookie generation"
- help
- Enable optional MD5 hmac based SCTP cookie generation
- select CRYPTO
- select CRYPTO_HMAC
- select CRYPTO_MD5
-
-config SCTP_COOKIE_HMAC_SHA1
- bool "Enable optional SHA1 hmac cookie generation"
- help
- Enable optional SHA1 hmac based SCTP cookie generation
- select CRYPTO
- select CRYPTO_HMAC
- select CRYPTO_SHA1
-
config INET_SCTP_DIAG
depends on INET_DIAG
def_tristate INET_DIAG
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 7e77b450697c0..31e989dfe8466 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -33,24 +33,29 @@
#include <net/sctp/sm.h>
/* Forward declarations for internal helpers. */
static void sctp_endpoint_bh_rcv(struct work_struct *work);
+static void gen_cookie_auth_key(struct hmac_sha256_key *key)
+{
+ u8 raw_key[SCTP_COOKIE_KEY_SIZE];
+
+ get_random_bytes(raw_key, sizeof(raw_key));
+ hmac_sha256_preparekey(key, raw_key, sizeof(raw_key));
+ memzero_explicit(raw_key, sizeof(raw_key));
+}
+
/*
* Initialize the base fields of the endpoint structure.
*/
static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
struct sock *sk,
gfp_t gfp)
{
struct net *net = sock_net(sk);
struct sctp_shared_key *null_key;
- ep->digest = kzalloc(SCTP_SIGNATURE_SIZE, gfp);
- if (!ep->digest)
- return NULL;
-
ep->asconf_enable = net->sctp.addip_enable;
ep->auth_enable = net->sctp.auth_enable;
if (ep->auth_enable) {
if (sctp_auth_init(ep, gfp))
goto nomem;
@@ -88,12 +93,12 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
sock_set_flag(sk, SOCK_USE_WRITE_QUEUE);
/* Get the receive buffer policy for this endpoint */
ep->rcvbuf_policy = net->sctp.rcvbuf_policy;
- /* Initialize the secret key used with cookie. */
- get_random_bytes(ep->secret_key, sizeof(ep->secret_key));
+ /* Generate the cookie authentication key. */
+ gen_cookie_auth_key(&ep->cookie_auth_key);
/* SCTP-AUTH extensions*/
INIT_LIST_HEAD(&ep->endpoint_shared_keys);
null_key = sctp_auth_shkey_create(0, gfp);
if (!null_key)
@@ -116,11 +121,10 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
return ep;
nomem_shkey:
sctp_auth_free(ep);
nomem:
- kfree(ep->digest);
return NULL;
}
/* Create a sctp_endpoint with all that boring stuff initialized.
@@ -203,24 +207,21 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
if (unlikely(!ep->base.dead)) {
WARN(1, "Attempt to destroy undead endpoint %p!\n", ep);
return;
}
- /* Free the digest buffer */
- kfree(ep->digest);
-
/* SCTP-AUTH: Free up AUTH releated data such as shared keys
* chunks and hmacs arrays that were allocated
*/
sctp_auth_destroy_keys(&ep->endpoint_shared_keys);
sctp_auth_free(ep);
/* Cleanup. */
sctp_inq_free(&ep->base.inqueue);
sctp_bind_addr_free(&ep->base.bind_addr);
- memset(ep->secret_key, 0, sizeof(ep->secret_key));
+ memzero_explicit(&ep->cookie_auth_key, sizeof(ep->cookie_auth_key));
sk = ep->base.sk;
/* Remove and free the port */
if (sctp_sk(sk)->bind_hash)
sctp_put_port(sk);
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index a5ccada55f2b2..3b2373b3bd5df 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1332,18 +1332,13 @@ static int __net_init sctp_defaults_init(struct net *net)
net->sctp.valid_cookie_life = SCTP_DEFAULT_COOKIE_LIFE;
/* Whether Cookie Preservative is enabled(1) or not(0) */
net->sctp.cookie_preserve_enable = 1;
- /* Default sctp sockets to use md5 as their hmac alg */
-#if defined (CONFIG_SCTP_DEFAULT_COOKIE_HMAC_MD5)
- net->sctp.sctp_hmac_alg = "md5";
-#elif defined (CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1)
- net->sctp.sctp_hmac_alg = "sha1";
-#else
- net->sctp.sctp_hmac_alg = NULL;
-#endif
+ /* Whether cookie authentication is enabled(1) or not(0) */
+ net->sctp.cookie_auth_enable =
+ !IS_ENABLED(CONFIG_SCTP_DEFAULT_COOKIE_HMAC_NONE);
/* Max.Burst - 4 */
net->sctp.max_burst = SCTP_DEFAULT_MAX_BURST;
/* Disable of Primary Path Switchover by default */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 6961607379191..2c0017d058d40 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -28,11 +28,11 @@
* Kevin Gao <kevin.gao@intel.com>
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <crypto/hash.h>
+#include <crypto/utils.h>
#include <linux/types.h>
#include <linux/kernel.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
#include <linux/net.h>
@@ -1672,12 +1672,14 @@ static struct sctp_cookie_param *sctp_pack_cookie(
/* Clear this memory since we are sending this data structure
* out on the network.
*/
retval = kzalloc(*cookie_len, GFP_ATOMIC);
- if (!retval)
- goto nodata;
+ if (!retval) {
+ *cookie_len = 0;
+ return NULL;
+ }
cookie = (struct sctp_signed_cookie *) retval->body;
/* Set up the parameter header. */
retval->p.type = SCTP_PARAM_STATE_COOKIE;
@@ -1704,30 +1706,18 @@ static struct sctp_cookie_param *sctp_pack_cookie(
/* Copy the raw local address list of the association. */
memcpy((__u8 *)(cookie + 1) +
ntohs(init_chunk->chunk_hdr->length), raw_addrs, addrs_len);
- if (sctp_sk(ep->base.sk)->hmac) {
- struct crypto_shash *tfm = sctp_sk(ep->base.sk)->hmac;
- int err;
-
- /* Sign the message. */
- err = crypto_shash_setkey(tfm, ep->secret_key,
- sizeof(ep->secret_key)) ?:
- crypto_shash_tfm_digest(tfm, (u8 *)&cookie->c, bodysize,
- cookie->signature);
- if (err)
- goto free_cookie;
+ /* Sign the cookie, if cookie authentication is enabled. */
+ if (sctp_sk(ep->base.sk)->cookie_auth_enable) {
+ static_assert(sizeof(cookie->mac) == SHA256_DIGEST_SIZE);
+ hmac_sha256(&ep->cookie_auth_key, (const u8 *)&cookie->c,
+ bodysize, cookie->mac);
}
return retval;
-
-free_cookie:
- kfree(retval);
-nodata:
- *cookie_len = 0;
- return NULL;
}
/* Unpack the cookie from COOKIE ECHO chunk, recreating the association. */
struct sctp_association *sctp_unpack_cookie(
const struct sctp_endpoint *ep,
@@ -1738,11 +1728,10 @@ struct sctp_association *sctp_unpack_cookie(
struct sctp_association *retval = NULL;
int headersize, bodysize, fixed_size;
struct sctp_signed_cookie *cookie;
struct sk_buff *skb = chunk->skb;
struct sctp_cookie *bear_cookie;
- __u8 *digest = ep->digest;
enum sctp_scope scope;
unsigned int len;
ktime_t kt;
/* Header size is static data prior to the actual cookie, including
@@ -1768,34 +1757,23 @@ struct sctp_association *sctp_unpack_cookie(
/* Process the cookie. */
cookie = chunk->subh.cookie_hdr;
bear_cookie = &cookie->c;
- if (!sctp_sk(ep->base.sk)->hmac)
- goto no_hmac;
+ /* Verify the cookie's MAC, if cookie authentication is enabled. */
+ if (sctp_sk(ep->base.sk)->cookie_auth_enable) {
+ u8 mac[SHA256_DIGEST_SIZE];
- /* Check the signature. */
- {
- struct crypto_shash *tfm = sctp_sk(ep->base.sk)->hmac;
- int err;
-
- err = crypto_shash_setkey(tfm, ep->secret_key,
- sizeof(ep->secret_key)) ?:
- crypto_shash_tfm_digest(tfm, (u8 *)bear_cookie, bodysize,
- digest);
- if (err) {
- *error = -SCTP_IERROR_NOMEM;
+ hmac_sha256(&ep->cookie_auth_key, (const u8 *)bear_cookie,
+ bodysize, mac);
+ static_assert(sizeof(cookie->mac) == sizeof(mac));
+ if (crypto_memneq(mac, cookie->mac, sizeof(mac))) {
+ *error = -SCTP_IERROR_BAD_SIG;
goto fail;
}
}
- if (memcmp(digest, cookie->signature, SCTP_SIGNATURE_SIZE)) {
- *error = -SCTP_IERROR_BAD_SIG;
- goto fail;
- }
-
-no_hmac:
/* IG Section 2.35.2:
* 3) Compare the port numbers and the verification tag contained
* within the COOKIE ECHO chunk to the actual port numbers and the
* verification tag within the SCTP common header of the received
* packet. If these values do not match the packet MUST be silently
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 0292881a847ca..ed8293a342402 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -35,11 +35,10 @@
* Kevin Gao <kevin.gao@intel.com>
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <crypto/hash.h>
#include <linux/types.h>
#include <linux/kernel.h>
#include <linux/wait.h>
#include <linux/time.h>
#include <linux/sched/signal.h>
@@ -4985,11 +4984,11 @@ static int sctp_init_sock(struct sock *sk)
sp->default_timetolive = 0;
sp->default_rcv_context = 0;
sp->max_burst = net->sctp.max_burst;
- sp->sctp_hmac_alg = net->sctp.sctp_hmac_alg;
+ sp->cookie_auth_enable = net->sctp.cookie_auth_enable;
/* Initialize default setup parameters. These parameters
* can be modified with the SCTP_INITMSG socket option or
* overridden by the SCTP_INIT CMSG.
*/
@@ -5077,12 +5076,10 @@ static int sctp_init_sock(struct sock *sk)
*/
sp->ep = sctp_endpoint_new(sk, GFP_KERNEL);
if (!sp->ep)
return -ENOMEM;
- sp->hmac = NULL;
-
sk->sk_destruct = sctp_destruct_sock;
SCTP_DBG_OBJCNT_INC(sock);
sk_sockets_allocated_inc(sk);
@@ -5115,22 +5112,12 @@ static void sctp_destroy_sock(struct sock *sk)
sctp_endpoint_free(sp->ep);
sk_sockets_allocated_dec(sk);
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
}
-/* Triggered when there are no references on the socket anymore */
-static void sctp_destruct_common(struct sock *sk)
-{
- struct sctp_sock *sp = sctp_sk(sk);
-
- /* Free up the HMAC transform. */
- crypto_free_shash(sp->hmac);
-}
-
static void sctp_destruct_sock(struct sock *sk)
{
- sctp_destruct_common(sk);
inet_sock_destruct(sk);
}
/* API 4.1.7 shutdown() - TCP Style Syntax
* int shutdown(int socket, int how);
@@ -8528,26 +8515,12 @@ static int sctp_get_port(struct sock *sk, unsigned short snum)
*/
static int sctp_listen_start(struct sock *sk, int backlog)
{
struct sctp_sock *sp = sctp_sk(sk);
struct sctp_endpoint *ep = sp->ep;
- struct crypto_shash *tfm = NULL;
- char alg[32];
int err;
- /* Allocate HMAC for generating cookie. */
- if (!sp->hmac && sp->sctp_hmac_alg) {
- sprintf(alg, "hmac(%s)", sp->sctp_hmac_alg);
- tfm = crypto_alloc_shash(alg, 0, 0);
- if (IS_ERR(tfm)) {
- net_info_ratelimited("failed to load transform for %s: %ld\n",
- sp->sctp_hmac_alg, PTR_ERR(tfm));
- return -ENOSYS;
- }
- sctp_sk(sk)->hmac = tfm;
- }
-
/*
* If a bind() or sctp_bindx() is not called prior to a listen()
* call that allows new associations to be accepted, the system
* picks an ephemeral port and will choose an address set equivalent
* to binding with a wildcard address.
@@ -9559,11 +9532,10 @@ static int sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
/* Restore the ep value that was overwritten with the above structure
* copy.
*/
newsp->ep = newep;
- newsp->hmac = NULL;
/* Hook this new socket in to the bind_hash list. */
head = &sctp_port_hashtable[sctp_phashfn(sock_net(oldsk),
inet_sk(oldsk)->inet_num)];
spin_lock_bh(&head->lock);
@@ -9711,11 +9683,10 @@ struct proto sctp_prot = {
#if IS_ENABLED(CONFIG_IPV6)
static void sctp_v6_destruct_sock(struct sock *sk)
{
- sctp_destruct_common(sk);
inet6_sock_destruct(sk);
}
static int sctp_v6_init_sock(struct sock *sk)
{
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index ee3eac338a9de..19acc57c3ed97 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -172,11 +172,11 @@ static struct ctl_table sctp_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec,
},
{
.procname = "cookie_hmac_alg",
- .data = &init_net.sctp.sctp_hmac_alg,
+ .data = &init_net.sctp.cookie_auth_enable,
.maxlen = 8,
.mode = 0644,
.proc_handler = proc_sctp_do_hmac_alg,
},
{
@@ -386,50 +386,41 @@ static struct ctl_table sctp_net_table[] = {
static int proc_sctp_do_hmac_alg(const struct ctl_table *ctl, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
struct net *net = container_of(ctl->data, struct net,
- sctp.sctp_hmac_alg);
+ sctp.cookie_auth_enable);
struct ctl_table tbl;
- bool changed = false;
- char *none = "none";
char tmp[8] = {0};
int ret;
memset(&tbl, 0, sizeof(struct ctl_table));
if (write) {
tbl.data = tmp;
- tbl.maxlen = sizeof(tmp);
- } else {
- tbl.data = net->sctp.sctp_hmac_alg ? : none;
- tbl.maxlen = strlen(tbl.data);
- }
-
- ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
- if (write && ret == 0) {
-#ifdef CONFIG_CRYPTO_MD5
- if (!strncmp(tmp, "md5", 3)) {
- net->sctp.sctp_hmac_alg = "md5";
- changed = true;
+ 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")) {
+ net->sctp.cookie_auth_enable = 1;
+ return 0;
}
-#endif
-#ifdef CONFIG_CRYPTO_SHA1
- if (!strncmp(tmp, "sha1", 4)) {
- net->sctp.sctp_hmac_alg = "sha1";
- changed = true;
+ if (!strcmp(tmp, "none")) {
+ net->sctp.cookie_auth_enable = 0;
+ return 0;
}
-#endif
- if (!strncmp(tmp, "none", 4)) {
- net->sctp.sctp_hmac_alg = NULL;
- changed = true;
- }
- if (!changed)
- ret = -EINVAL;
+ return -EINVAL;
}
-
- return ret;
+ 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);
}
static int proc_sctp_do_rto_min(const struct ctl_table *ctl, int write,
void *buffer, size_t *lenp, loff_t *ppos)
{
--
2.50.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-13 4:01 ` [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256 Eric Biggers
@ 2025-08-15 19:09 ` Jakub Kicinski
2025-08-15 21:19 ` Xin Long
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2025-08-15 19:09 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-sctp, netdev, Xin Long, Marcelo Ricardo Leitner,
linux-crypto
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.
It'd be great to get an ack / review from SCTP maintainers, otherwise
we'll apply by Monday..
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-15 19:09 ` Jakub Kicinski
@ 2025-08-15 21:19 ` Xin Long
2025-08-15 21:50 ` Eric Biggers
0 siblings, 1 reply; 13+ messages in thread
From: Xin Long @ 2025-08-15 21:19 UTC (permalink / raw)
To: Jakub Kicinski, Paolo Abeni
Cc: Eric Biggers, linux-sctp, netdev, Marcelo Ricardo Leitner,
linux-crypto
On Fri, Aug 15, 2025 at 3:09 PM Jakub Kicinski <kuba@kernel.org> 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-15 21:19 ` Xin Long
@ 2025-08-15 21:50 ` Eric Biggers
2025-08-16 1:06 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2025-08-15 21:50 UTC (permalink / raw)
To: Xin Long
Cc: Jakub Kicinski, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
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@kernel.org> 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-15 21:50 ` Eric Biggers
@ 2025-08-16 1:06 ` Jakub Kicinski
2025-08-16 17:15 ` Xin Long
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2025-08-16 1:06 UTC (permalink / raw)
To: Eric Biggers
Cc: Xin Long, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
On Fri, 15 Aug 2025 14:50:09 -0700 Eric Biggers wrote:
> > > 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:
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-16 1:06 ` Jakub Kicinski
@ 2025-08-16 17:15 ` Xin Long
2025-08-18 15:43 ` Jakub Kicinski
0 siblings, 1 reply; 13+ messages in thread
From: Xin Long @ 2025-08-16 17:15 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eric Biggers, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
On Fri, Aug 15, 2025 at 9:06 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 15 Aug 2025 14:50:09 -0700 Eric Biggers wrote:
> > > > 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:
>
> 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.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-16 17:15 ` Xin Long
@ 2025-08-18 15:43 ` Jakub Kicinski
2025-08-18 17:31 ` Eric Biggers
0 siblings, 1 reply; 13+ messages in thread
From: Jakub Kicinski @ 2025-08-18 15:43 UTC (permalink / raw)
To: Xin Long
Cc: Eric Biggers, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
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).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-18 15:43 ` Jakub Kicinski
@ 2025-08-18 17:31 ` Eric Biggers
2025-08-18 17:41 ` Xin Long
0 siblings, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2025-08-18 17:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Xin Long, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256
2025-08-18 17:31 ` Eric Biggers
@ 2025-08-18 17:41 ` Xin Long
0 siblings, 0 replies; 13+ messages in thread
From: Xin Long @ 2025-08-18 17:41 UTC (permalink / raw)
To: Eric Biggers
Cc: Jakub Kicinski, Paolo Abeni, linux-sctp, netdev,
Marcelo Ricardo Leitner, linux-crypto
On Mon, Aug 18, 2025 at 1:32 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> 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.
>
Sounds good to me.
Thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth
2025-08-13 4:01 [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Eric Biggers
` (2 preceding siblings ...)
2025-08-13 4:01 ` [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256 Eric Biggers
@ 2025-08-18 17:42 ` Xin Long
3 siblings, 0 replies; 13+ messages in thread
From: Xin Long @ 2025-08-18 17:42 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-sctp, netdev, Marcelo Ricardo Leitner, linux-crypto
On Wed, Aug 13, 2025 at 12:03 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> This series converts SCTP chunk and cookie authentication to use the
> crypto library API instead of crypto_shash. This is much simpler (the
> diffstat should speak for itself), and also faster too. In addition,
> this series upgrades the cookie authentication to use HMAC-SHA256.
>
> I've tested that kernels with this series applied can continue to
> communicate using SCTP with older ones, in either direction, using any
> choice of None, HMAC-SHA1, or HMAC-SHA256 chunk authentication.
>
> Changed in v2:
> - Added patch which adds CONFIG_CRYPTO_SHA1 to some selftests configs
>
> Eric Biggers (3):
> selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec
> sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication
> sctp: Convert cookie authentication to use HMAC-SHA256
>
> Documentation/networking/ip-sysctl.rst | 11 +-
> include/net/netns/sctp.h | 4 +-
> include/net/sctp/auth.h | 17 +-
> include/net/sctp/constants.h | 9 +-
> include/net/sctp/structs.h | 35 +---
> net/sctp/Kconfig | 47 ++----
> net/sctp/auth.c | 166 ++++---------------
> net/sctp/chunk.c | 3 +-
> net/sctp/endpointola.c | 23 +--
> net/sctp/protocol.c | 11 +-
> net/sctp/sm_make_chunk.c | 60 +++----
> net/sctp/sm_statefuns.c | 2 +-
> net/sctp/socket.c | 41 +----
> net/sctp/sysctl.c | 51 +++---
> tools/testing/selftests/net/config | 1 +
> tools/testing/selftests/net/netfilter/config | 1 +
> 16 files changed, 124 insertions(+), 358 deletions(-)
>
>
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> --
> 2.50.1
>
Acked-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-08-18 17:42 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-13 4:01 [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 1/3] selftests: net: Explicitly enable CONFIG_CRYPTO_SHA1 for IPsec Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 2/3] sctp: Use HMAC-SHA1 and HMAC-SHA256 library for chunk authentication Eric Biggers
2025-08-13 4:01 ` [PATCH net-next v2 3/3] sctp: Convert cookie authentication to use HMAC-SHA256 Eric Biggers
2025-08-15 19:09 ` Jakub Kicinski
2025-08-15 21:19 ` Xin Long
2025-08-15 21:50 ` Eric Biggers
2025-08-16 1:06 ` Jakub Kicinski
2025-08-16 17:15 ` Xin Long
2025-08-18 15:43 ` Jakub Kicinski
2025-08-18 17:31 ` Eric Biggers
2025-08-18 17:41 ` Xin Long
2025-08-18 17:42 ` [PATCH net-next v2 0/3] sctp: Convert to use crypto lib, and upgrade cookie auth Xin Long
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).