* [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg
2018-03-14 11:05 ` [PATCH net-next 1/5] sctp: add refcnt support for sh_key Xin Long
@ 2018-03-14 11:05 ` Xin Long
2018-03-14 11:05 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Xin Long
2018-03-14 13:53 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Marcelo Ricardo Leitner
2018-03-14 13:53 ` [PATCH net-next 1/5] sctp: add refcnt support for sh_key Marcelo Ricardo Leitner
2018-03-14 13:59 ` Neil Horman
2 siblings, 2 replies; 16+ messages in thread
From: Xin Long @ 2018-03-14 11:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patch is to add support for SCTP AUTH Information for sendmsg,
as described in section 5.3.8 of RFC6458.
With this option, you can provide shared key identifier used for
sending the user message.
It's also a necessary send info for sctp_sendv.
Note that it reuses sinfo->sinfo_tsn to indicate if this option is
set and sinfo->sinfo_ssn to save the shkey ID which can be 0.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 1 +
include/uapi/linux/sctp.h | 14 +++++++++++++-
net/sctp/chunk.c | 11 ++++++++++-
net/sctp/socket.c | 23 +++++++++++++++++++++++
4 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 49ad67b..012fb3e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2118,6 +2118,7 @@ struct sctp_cmsgs {
struct sctp_sndrcvinfo *srinfo;
struct sctp_sndinfo *sinfo;
struct sctp_prinfo *prinfo;
+ struct sctp_authinfo *authinfo;
struct msghdr *addrs_msg;
};
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index e94b6d2..47e781e 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -273,6 +273,18 @@ struct sctp_prinfo {
__u32 pr_value;
};
+/* 5.3.8 SCTP AUTH Information Structure (SCTP_AUTHINFO)
+ *
+ * This cmsghdr structure specifies SCTP options for sendmsg().
+ *
+ * cmsg_level cmsg_type cmsg_data[]
+ * ------------ ------------ -------------------
+ * IPPROTO_SCTP SCTP_AUTHINFO struct sctp_authinfo
+ */
+struct sctp_authinfo {
+ __u16 auth_keynumber;
+};
+
/*
* sinfo_flags: 16 bits (unsigned integer)
*
@@ -310,7 +322,7 @@ typedef enum sctp_cmsg_type {
#define SCTP_NXTINFO SCTP_NXTINFO
SCTP_PRINFO, /* 5.3.7 SCTP PR-SCTP Information Structure */
#define SCTP_PRINFO SCTP_PRINFO
- SCTP_AUTHINFO, /* 5.3.8 SCTP AUTH Information Structure (RESERVED) */
+ SCTP_AUTHINFO, /* 5.3.8 SCTP AUTH Information Structure */
#define SCTP_AUTHINFO SCTP_AUTHINFO
SCTP_DSTADDRV4, /* 5.3.9 SCTP Destination IPv4 Address Structure */
#define SCTP_DSTADDRV4 SCTP_DSTADDRV4
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 9f28a9a..f889a84 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -206,7 +206,16 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
hmac_desc->hmac_len);
- shkey = asoc->shkey;
+ if (sinfo->sinfo_tsn &&
+ sinfo->sinfo_ssn != asoc->active_key_id) {
+ shkey = sctp_auth_get_shkey(asoc, sinfo->sinfo_ssn);
+ if (!shkey) {
+ err = -EINVAL;
+ goto errout;
+ }
+ } else {
+ shkey = asoc->shkey;
+ }
}
/* Check what's our max considering the above */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 003a4ad..9ffdecb 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1987,6 +1987,14 @@ static void sctp_sendmsg_update_sinfo(struct sctp_association *asoc,
if (!cmsgs->srinfo && !cmsgs->prinfo)
sinfo->sinfo_timetolive = asoc->default_timetolive;
+
+ if (cmsgs->authinfo) {
+ /* Reuse sinfo_tsn to indicate that authinfo was set and
+ * sinfo_ssn to save the keyid on tx path.
+ */
+ sinfo->sinfo_tsn = 1;
+ sinfo->sinfo_ssn = cmsgs->authinfo->auth_keynumber;
+ }
}
static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
@@ -7874,6 +7882,21 @@ static int sctp_msghdr_parse(const struct msghdr *msg, struct sctp_cmsgs *cmsgs)
if (cmsgs->prinfo->pr_policy == SCTP_PR_SCTP_NONE)
cmsgs->prinfo->pr_value = 0;
break;
+ case SCTP_AUTHINFO:
+ /* SCTP Socket API Extension
+ * 5.3.8 SCTP AUTH Information Structure (SCTP_AUTHINFO)
+ *
+ * This cmsghdr structure specifies SCTP options for sendmsg().
+ *
+ * cmsg_level cmsg_type cmsg_data[]
+ * ------------ ------------ ---------------------
+ * IPPROTO_SCTP SCTP_AUTHINFO struct sctp_authinfo
+ */
+ if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct sctp_authinfo)))
+ return -EINVAL;
+
+ cmsgs->authinfo = CMSG_DATA(cmsg);
+ break;
case SCTP_DSTADDRV4:
case SCTP_DSTADDRV6:
/* SCTP Socket API Extension
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY
2018-03-14 11:05 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Xin Long
@ 2018-03-14 11:05 ` Xin Long
2018-03-14 11:05 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT Xin Long
2018-03-14 13:53 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Marcelo Ricardo Leitner
2018-03-14 13:53 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Marcelo Ricardo Leitner
1 sibling, 2 replies; 16+ messages in thread
From: Xin Long @ 2018-03-14 11:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patch is to add sockopt SCTP_AUTH_DEACTIVATE_KEY, as described in
section 8.3.4 of RFC6458.
This set option indicates that the application will no longer send user
messages using the indicated key identifier.
Note that RFC requires that only deactivated keys that are no longer used
by an association can be deleted, but for the backward compatibility, it
is not to check deactivated when deleting or replacing one sh_key.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/auth.h | 12 ++++++------
include/uapi/linux/sctp.h | 1 +
net/sctp/auth.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
net/sctp/socket.c | 31 +++++++++++++++++++++++++++++++
4 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
index 017c1aa..687e7f8 100644
--- a/include/net/sctp/auth.h
+++ b/include/net/sctp/auth.h
@@ -65,6 +65,7 @@ struct sctp_shared_key {
struct sctp_auth_bytes *key;
refcount_t refcnt;
__u16 key_id;
+ __u8 deactivated;
};
#define key_for_each(__key, __list_head) \
@@ -113,14 +114,13 @@ void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
struct sctp_hmacalgo *hmacs);
-int sctp_auth_set_key(struct sctp_endpoint *ep,
- struct sctp_association *asoc,
+int sctp_auth_set_key(struct sctp_endpoint *ep, struct sctp_association *asoc,
struct sctp_authkey *auth_key);
int sctp_auth_set_active_key(struct sctp_endpoint *ep,
- struct sctp_association *asoc,
- __u16 key_id);
+ struct sctp_association *asoc, __u16 key_id);
int sctp_auth_del_key_id(struct sctp_endpoint *ep,
- struct sctp_association *asoc,
- __u16 key_id);
+ struct sctp_association *asoc, __u16 key_id);
+int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
+ struct sctp_association *asoc, __u16 key_id);
#endif
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 47e781e..08fc313 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -99,6 +99,7 @@ typedef __s32 sctp_assoc_t;
#define SCTP_RECVRCVINFO 32
#define SCTP_RECVNXTINFO 33
#define SCTP_DEFAULT_SNDINFO 34
+#define SCTP_AUTH_DEACTIVATE_KEY 35
/* Internal Socket Options. Some of the sctp library functions are
* implemented using these socket options.
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index e5214fd..a073123 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -449,8 +449,11 @@ struct sctp_shared_key *sctp_auth_get_shkey(
/* First search associations set of endpoint pair shared keys */
key_for_each(key, &asoc->endpoint_shared_keys) {
- if (key->key_id == key_id)
- return key;
+ if (key->key_id == key_id) {
+ if (!key->deactivated)
+ return key;
+ break;
+ }
}
return NULL;
@@ -905,7 +908,7 @@ int sctp_auth_set_active_key(struct sctp_endpoint *ep,
}
}
- if (!found)
+ if (!found || key->deactivated)
return -EINVAL;
if (asoc) {
@@ -956,3 +959,40 @@ int sctp_auth_del_key_id(struct sctp_endpoint *ep,
return 0;
}
+
+int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
+ struct sctp_association *asoc, __u16 key_id)
+{
+ struct sctp_shared_key *key;
+ struct list_head *sh_keys;
+ int found = 0;
+
+ /* The key identifier MUST NOT be the current active key
+ * The key identifier MUST correst to an existing key
+ */
+ if (asoc) {
+ if (asoc->active_key_id == key_id)
+ return -EINVAL;
+
+ sh_keys = &asoc->endpoint_shared_keys;
+ } else {
+ if (ep->active_key_id == key_id)
+ return -EINVAL;
+
+ sh_keys = &ep->endpoint_shared_keys;
+ }
+
+ key_for_each(key, sh_keys) {
+ if (key->key_id == key_id) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found)
+ return -EINVAL;
+
+ key->deactivated = 1;
+
+ return 0;
+}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9ffdecb..65cc354 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3647,6 +3647,33 @@ static int sctp_setsockopt_del_key(struct sock *sk,
}
/*
+ * 8.3.4 Deactivate a Shared Key (SCTP_AUTH_DEACTIVATE_KEY)
+ *
+ * This set option will deactivate a shared secret key.
+ */
+static int sctp_setsockopt_deactivate_key(struct sock *sk, char __user *optval,
+ unsigned int optlen)
+{
+ struct sctp_endpoint *ep = sctp_sk(sk)->ep;
+ struct sctp_authkeyid val;
+ struct sctp_association *asoc;
+
+ if (!ep->auth_enable)
+ return -EACCES;
+
+ if (optlen != sizeof(struct sctp_authkeyid))
+ return -EINVAL;
+ if (copy_from_user(&val, optval, optlen))
+ return -EFAULT;
+
+ asoc = sctp_id2assoc(sk, val.scact_assoc_id);
+ if (!asoc && val.scact_assoc_id && sctp_style(sk, UDP))
+ return -EINVAL;
+
+ return sctp_auth_deact_key_id(ep, asoc, val.scact_keynumber);
+}
+
+/*
* 8.1.23 SCTP_AUTO_ASCONF
*
* This option will enable or disable the use of the automatic generation of
@@ -4238,6 +4265,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
case SCTP_AUTH_DELETE_KEY:
retval = sctp_setsockopt_del_key(sk, optval, optlen);
break;
+ case SCTP_AUTH_DEACTIVATE_KEY:
+ retval = sctp_setsockopt_deactivate_key(sk, optval, optlen);
+ break;
case SCTP_AUTO_ASCONF:
retval = sctp_setsockopt_auto_asconf(sk, optval, optlen);
break;
@@ -7212,6 +7242,7 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
case SCTP_AUTH_KEY:
case SCTP_AUTH_CHUNK:
case SCTP_AUTH_DELETE_KEY:
+ case SCTP_AUTH_DEACTIVATE_KEY:
retval = -EOPNOTSUPP;
break;
case SCTP_HMAC_IDENT:
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT
2018-03-14 11:05 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Xin Long
@ 2018-03-14 11:05 ` Xin Long
2018-03-14 11:05 ` [PATCH net-next 5/5] sctp: add SCTP_AUTH_NO_AUTH " Xin Long
2018-03-14 13:53 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY " Marcelo Ricardo Leitner
2018-03-14 13:53 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Marcelo Ricardo Leitner
1 sibling, 2 replies; 16+ messages in thread
From: Xin Long @ 2018-03-14 11:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patch is to add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT,
as described in section 6.1.8 of RFC6458.
SCTP_AUTH_FREE_KEY: This report indicates that the SCTP
implementation will no longer use the key identifier specified
in auth_keynumber.
After deactivating a key, it would never be used again, which means
it's refcnt can't be held/increased by new chunks. But there may be
some chunks in out queue still using it. So only when refcnt is 1,
which means no chunk in outqueue is using/holding this key either,
this EVENT would be sent.
When users receive this notification, they could do DEL_KEY sockopt to
remove this shkey, and also tell the peer that this key won't be used
in any chunk thoroughly from now on, then the peer can remove it as
well safely.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/uapi/linux/sctp.h | 6 +++++-
net/sctp/auth.c | 14 ++++++++++++++
net/sctp/sm_make_chunk.c | 20 +++++++++++++++++++-
net/sctp/sm_statefuns.c | 2 +-
net/sctp/socket.c | 19 ++++++++++++++++++-
5 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 08fc313..18ebbfe 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -518,7 +518,11 @@ struct sctp_authkey_event {
sctp_assoc_t auth_assoc_id;
};
-enum { SCTP_AUTH_NEWKEY = 0, };
+enum {
+ SCTP_AUTH_NEW_KEY,
+#define SCTP_AUTH_NEWKEY SCTP_AUTH_NEW_KEY /* compatible with before */
+ SCTP_AUTH_FREE_KEY,
+};
/*
* 6.1.9. SCTP_SENDER_DRY_EVENT
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index a073123..e64630c 100644
--- a/net/sctp/auth.c
+++ b/net/sctp/auth.c
@@ -992,6 +992,20 @@ int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
if (!found)
return -EINVAL;
+ /* refcnt == 1 and !list_empty mean it's not being used anywhere
+ * and deactivated will be set, so it's time to notify userland
+ * that this shkey can be freed.
+ */
+ if (asoc && !list_empty(&key->key_list) &&
+ refcount_read(&key->refcnt) == 1) {
+ struct sctp_ulpevent *ev;
+
+ ev = sctp_ulpevent_make_authkey(asoc, key->key_id,
+ SCTP_AUTH_FREE_KEY, GFP_KERNEL);
+ if (ev)
+ asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
+ }
+
key->deactivated = 1;
return 0;
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 10f071c..cc20bc3 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -89,8 +89,26 @@ static void sctp_control_release_owner(struct sk_buff *skb)
{
struct sctp_chunk *chunk = skb_shinfo(skb)->destructor_arg;
- if (chunk->shkey)
+ if (chunk->shkey) {
+ struct sctp_shared_key *shkey = chunk->shkey;
+ struct sctp_association *asoc = chunk->asoc;
+
+ /* refcnt == 2 and !list_empty mean after this release, it's
+ * not being used anywhere, and it's time to notify userland
+ * that this shkey can be freed if it's been deactivated.
+ */
+ if (shkey->deactivated && !list_empty(&shkey->key_list) &&
+ refcount_read(&shkey->refcnt) == 2) {
+ struct sctp_ulpevent *ev;
+
+ ev = sctp_ulpevent_make_authkey(asoc, shkey->key_id,
+ SCTP_AUTH_FREE_KEY,
+ GFP_KERNEL);
+ if (ev)
+ asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
+ }
sctp_auth_shkey_release(chunk->shkey);
+ }
}
static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 792e0e2..1e41dee 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4246,7 +4246,7 @@ enum sctp_disposition sctp_sf_eat_auth(struct net *net,
struct sctp_ulpevent *ev;
ev = sctp_ulpevent_make_authkey(asoc, ntohs(auth_hdr->shkey_id),
- SCTP_AUTH_NEWKEY, GFP_ATOMIC);
+ SCTP_AUTH_NEW_KEY, GFP_ATOMIC);
if (!ev)
return -ENOMEM;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 65cc354..aeecdd6 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -8166,8 +8166,25 @@ static void sctp_wfree(struct sk_buff *skb)
sk->sk_wmem_queued -= skb->truesize;
sk_mem_uncharge(sk, skb->truesize);
- if (chunk->shkey)
+ if (chunk->shkey) {
+ struct sctp_shared_key *shkey = chunk->shkey;
+
+ /* refcnt == 2 and !list_empty mean after this release, it's
+ * not being used anywhere, and it's time to notify userland
+ * that this shkey can be freed if it's been deactivated.
+ */
+ if (shkey->deactivated && !list_empty(&shkey->key_list) &&
+ refcount_read(&shkey->refcnt) == 2) {
+ struct sctp_ulpevent *ev;
+
+ ev = sctp_ulpevent_make_authkey(asoc, shkey->key_id,
+ SCTP_AUTH_FREE_KEY,
+ GFP_KERNEL);
+ if (ev)
+ asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
+ }
sctp_auth_shkey_release(chunk->shkey);
+ }
sock_wfree(skb);
sctp_wake_up_waiters(sk, asoc);
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH net-next 5/5] sctp: add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT
2018-03-14 11:05 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT Xin Long
@ 2018-03-14 11:05 ` Xin Long
2018-03-14 13:53 ` Marcelo Ricardo Leitner
2018-03-14 13:53 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY " Marcelo Ricardo Leitner
1 sibling, 1 reply; 16+ messages in thread
From: Xin Long @ 2018-03-14 11:05 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patch is to add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT,
as described in section 6.1.8 of RFC6458.
SCTP_AUTH_NO_AUTH: This report indicates that the peer does not
support SCTP authentication as defined in [RFC4895].
Note that the implementation is quite similar as that of
SCTP_ADAPTATION_INDICATION.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/command.h | 1 +
include/uapi/linux/sctp.h | 1 +
net/sctp/sm_sideeffect.c | 13 +++++++++++++
net/sctp/sm_statefuns.c | 43 +++++++++++++++++++++++++++++++++++++++++--
4 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index b55c6a4..6640f84 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -100,6 +100,7 @@ enum sctp_verb {
SCTP_CMD_SET_SK_ERR, /* Set sk_err */
SCTP_CMD_ASSOC_CHANGE, /* generate and send assoc_change event */
SCTP_CMD_ADAPTATION_IND, /* generate and send adaptation event */
+ SCTP_CMD_PEER_NO_AUTH, /* generate and send authentication event */
SCTP_CMD_ASSOC_SHKEY, /* generate the association shared keys */
SCTP_CMD_T1_RETRAN, /* Mark for retransmission after T1 timeout */
SCTP_CMD_UPDATE_INITTAG, /* Update peer inittag */
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 18ebbfe..afd4346 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -522,6 +522,7 @@ enum {
SCTP_AUTH_NEW_KEY,
#define SCTP_AUTH_NEWKEY SCTP_AUTH_NEW_KEY /* compatible with before */
SCTP_AUTH_FREE_KEY,
+ SCTP_AUTH_NO_AUTH,
};
/*
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index b71e7fb..298112c 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1049,6 +1049,16 @@ static void sctp_cmd_assoc_change(struct sctp_cmd_seq *commands,
asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
}
+static void sctp_cmd_peer_no_auth(struct sctp_cmd_seq *commands,
+ struct sctp_association *asoc)
+{
+ struct sctp_ulpevent *ev;
+
+ ev = sctp_ulpevent_make_authkey(asoc, 0, SCTP_AUTH_NO_AUTH, GFP_ATOMIC);
+ if (ev)
+ asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
+}
+
/* Helper function to generate an adaptation indication event */
static void sctp_cmd_adaptation_ind(struct sctp_cmd_seq *commands,
struct sctp_association *asoc)
@@ -1755,6 +1765,9 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
case SCTP_CMD_ADAPTATION_IND:
sctp_cmd_adaptation_ind(commands, asoc);
break;
+ case SCTP_CMD_PEER_NO_AUTH:
+ sctp_cmd_peer_no_auth(commands, asoc);
+ break;
case SCTP_CMD_ASSOC_SHKEY:
error = sctp_auth_asoc_init_active_key(asoc,
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 1e41dee..cc56a67 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -659,7 +659,7 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
void *arg,
struct sctp_cmd_seq *commands)
{
- struct sctp_ulpevent *ev, *ai_ev = NULL;
+ struct sctp_ulpevent *ev, *ai_ev = NULL, *auth_ev = NULL;
struct sctp_association *new_asoc;
struct sctp_init_chunk *peer_init;
struct sctp_chunk *chunk = arg;
@@ -820,6 +820,14 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
goto nomem_aiev;
}
+ if (!new_asoc->peer.auth_capable) {
+ auth_ev = sctp_ulpevent_make_authkey(new_asoc, 0,
+ SCTP_AUTH_NO_AUTH,
+ GFP_ATOMIC);
+ if (!auth_ev)
+ goto nomem_authev;
+ }
+
/* Add all the state machine commands now since we've created
* everything. This way we don't introduce memory corruptions
* during side-effect processing and correclty count established
@@ -847,8 +855,14 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
SCTP_ULPEVENT(ai_ev));
+ if (auth_ev)
+ sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
+ SCTP_ULPEVENT(auth_ev));
+
return SCTP_DISPOSITION_CONSUME;
+nomem_authev:
+ sctp_ulpevent_free(ai_ev);
nomem_aiev:
sctp_ulpevent_free(ev);
nomem_ev:
@@ -953,6 +967,15 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net,
SCTP_ULPEVENT(ev));
}
+ if (!asoc->peer.auth_capable) {
+ ev = sctp_ulpevent_make_authkey(asoc, 0, SCTP_AUTH_NO_AUTH,
+ GFP_ATOMIC);
+ if (!ev)
+ goto nomem;
+ sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
+ SCTP_ULPEVENT(ev));
+ }
+
return SCTP_DISPOSITION_CONSUME;
nomem:
return SCTP_DISPOSITION_NOMEM;
@@ -1908,6 +1931,9 @@ static enum sctp_disposition sctp_sf_do_dupcook_b(
if (asoc->peer.adaptation_ind)
sctp_add_cmd_sf(commands, SCTP_CMD_ADAPTATION_IND, SCTP_NULL());
+ if (!asoc->peer.auth_capable)
+ sctp_add_cmd_sf(commands, SCTP_CMD_PEER_NO_AUTH, SCTP_NULL());
+
return SCTP_DISPOSITION_CONSUME;
nomem:
@@ -1954,7 +1980,7 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
struct sctp_cmd_seq *commands,
struct sctp_association *new_asoc)
{
- struct sctp_ulpevent *ev = NULL, *ai_ev = NULL;
+ struct sctp_ulpevent *ev = NULL, *ai_ev = NULL, *auth_ev = NULL;
struct sctp_chunk *repl;
/* Clarification from Implementor's Guide:
@@ -2001,6 +2027,14 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
goto nomem;
}
+
+ if (!asoc->peer.auth_capable) {
+ auth_ev = sctp_ulpevent_make_authkey(asoc, 0,
+ SCTP_AUTH_NO_AUTH,
+ GFP_ATOMIC);
+ if (!auth_ev)
+ goto nomem;
+ }
}
repl = sctp_make_cookie_ack(new_asoc, chunk);
@@ -2015,10 +2049,15 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
if (ai_ev)
sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
SCTP_ULPEVENT(ai_ev));
+ if (auth_ev)
+ sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
+ SCTP_ULPEVENT(auth_ev));
return SCTP_DISPOSITION_CONSUME;
nomem:
+ if (auth_ev)
+ sctp_ulpevent_free(auth_ev);
if (ai_ev)
sctp_ulpevent_free(ai_ev);
if (ev)
--
2.1.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH net-next 5/5] sctp: add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT
2018-03-14 11:05 ` [PATCH net-next 5/5] sctp: add SCTP_AUTH_NO_AUTH " Xin Long
@ 2018-03-14 13:53 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 13:53 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Wed, Mar 14, 2018 at 07:05:34PM +0800, Xin Long wrote:
> This patch is to add SCTP_AUTH_NO_AUTH type for AUTHENTICATION_EVENT,
> as described in section 6.1.8 of RFC6458.
>
> SCTP_AUTH_NO_AUTH: This report indicates that the peer does not
> support SCTP authentication as defined in [RFC4895].
>
> Note that the implementation is quite similar as that of
> SCTP_ADAPTATION_INDICATION.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/net/sctp/command.h | 1 +
> include/uapi/linux/sctp.h | 1 +
> net/sctp/sm_sideeffect.c | 13 +++++++++++++
> net/sctp/sm_statefuns.c | 43 +++++++++++++++++++++++++++++++++++++++++--
> 4 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index b55c6a4..6640f84 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -100,6 +100,7 @@ enum sctp_verb {
> SCTP_CMD_SET_SK_ERR, /* Set sk_err */
> SCTP_CMD_ASSOC_CHANGE, /* generate and send assoc_change event */
> SCTP_CMD_ADAPTATION_IND, /* generate and send adaptation event */
> + SCTP_CMD_PEER_NO_AUTH, /* generate and send authentication event */
> SCTP_CMD_ASSOC_SHKEY, /* generate the association shared keys */
> SCTP_CMD_T1_RETRAN, /* Mark for retransmission after T1 timeout */
> SCTP_CMD_UPDATE_INITTAG, /* Update peer inittag */
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index 18ebbfe..afd4346 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -522,6 +522,7 @@ enum {
> SCTP_AUTH_NEW_KEY,
> #define SCTP_AUTH_NEWKEY SCTP_AUTH_NEW_KEY /* compatible with before */
> SCTP_AUTH_FREE_KEY,
> + SCTP_AUTH_NO_AUTH,
> };
>
> /*
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index b71e7fb..298112c 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1049,6 +1049,16 @@ static void sctp_cmd_assoc_change(struct sctp_cmd_seq *commands,
> asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
> }
>
> +static void sctp_cmd_peer_no_auth(struct sctp_cmd_seq *commands,
> + struct sctp_association *asoc)
> +{
> + struct sctp_ulpevent *ev;
> +
> + ev = sctp_ulpevent_make_authkey(asoc, 0, SCTP_AUTH_NO_AUTH, GFP_ATOMIC);
> + if (ev)
> + asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
> +}
> +
> /* Helper function to generate an adaptation indication event */
> static void sctp_cmd_adaptation_ind(struct sctp_cmd_seq *commands,
> struct sctp_association *asoc)
> @@ -1755,6 +1765,9 @@ static int sctp_cmd_interpreter(enum sctp_event event_type,
> case SCTP_CMD_ADAPTATION_IND:
> sctp_cmd_adaptation_ind(commands, asoc);
> break;
> + case SCTP_CMD_PEER_NO_AUTH:
> + sctp_cmd_peer_no_auth(commands, asoc);
> + break;
>
> case SCTP_CMD_ASSOC_SHKEY:
> error = sctp_auth_asoc_init_active_key(asoc,
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 1e41dee..cc56a67 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -659,7 +659,7 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
> void *arg,
> struct sctp_cmd_seq *commands)
> {
> - struct sctp_ulpevent *ev, *ai_ev = NULL;
> + struct sctp_ulpevent *ev, *ai_ev = NULL, *auth_ev = NULL;
> struct sctp_association *new_asoc;
> struct sctp_init_chunk *peer_init;
> struct sctp_chunk *chunk = arg;
> @@ -820,6 +820,14 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
> goto nomem_aiev;
> }
>
> + if (!new_asoc->peer.auth_capable) {
> + auth_ev = sctp_ulpevent_make_authkey(new_asoc, 0,
> + SCTP_AUTH_NO_AUTH,
> + GFP_ATOMIC);
> + if (!auth_ev)
> + goto nomem_authev;
> + }
> +
> /* Add all the state machine commands now since we've created
> * everything. This way we don't introduce memory corruptions
> * during side-effect processing and correclty count established
> @@ -847,8 +855,14 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
> sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
> SCTP_ULPEVENT(ai_ev));
>
> + if (auth_ev)
> + sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
> + SCTP_ULPEVENT(auth_ev));
> +
> return SCTP_DISPOSITION_CONSUME;
>
> +nomem_authev:
> + sctp_ulpevent_free(ai_ev);
> nomem_aiev:
> sctp_ulpevent_free(ev);
> nomem_ev:
> @@ -953,6 +967,15 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net,
> SCTP_ULPEVENT(ev));
> }
>
> + if (!asoc->peer.auth_capable) {
> + ev = sctp_ulpevent_make_authkey(asoc, 0, SCTP_AUTH_NO_AUTH,
> + GFP_ATOMIC);
> + if (!ev)
> + goto nomem;
> + sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
> + SCTP_ULPEVENT(ev));
> + }
> +
> return SCTP_DISPOSITION_CONSUME;
> nomem:
> return SCTP_DISPOSITION_NOMEM;
> @@ -1908,6 +1931,9 @@ static enum sctp_disposition sctp_sf_do_dupcook_b(
> if (asoc->peer.adaptation_ind)
> sctp_add_cmd_sf(commands, SCTP_CMD_ADAPTATION_IND, SCTP_NULL());
>
> + if (!asoc->peer.auth_capable)
> + sctp_add_cmd_sf(commands, SCTP_CMD_PEER_NO_AUTH, SCTP_NULL());
> +
> return SCTP_DISPOSITION_CONSUME;
>
> nomem:
> @@ -1954,7 +1980,7 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
> struct sctp_cmd_seq *commands,
> struct sctp_association *new_asoc)
> {
> - struct sctp_ulpevent *ev = NULL, *ai_ev = NULL;
> + struct sctp_ulpevent *ev = NULL, *ai_ev = NULL, *auth_ev = NULL;
> struct sctp_chunk *repl;
>
> /* Clarification from Implementor's Guide:
> @@ -2001,6 +2027,14 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
> goto nomem;
>
> }
> +
> + if (!asoc->peer.auth_capable) {
> + auth_ev = sctp_ulpevent_make_authkey(asoc, 0,
> + SCTP_AUTH_NO_AUTH,
> + GFP_ATOMIC);
> + if (!auth_ev)
> + goto nomem;
> + }
> }
>
> repl = sctp_make_cookie_ack(new_asoc, chunk);
> @@ -2015,10 +2049,15 @@ static enum sctp_disposition sctp_sf_do_dupcook_d(
> if (ai_ev)
> sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
> SCTP_ULPEVENT(ai_ev));
> + if (auth_ev)
> + sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP,
> + SCTP_ULPEVENT(auth_ev));
>
> return SCTP_DISPOSITION_CONSUME;
>
> nomem:
> + if (auth_ev)
> + sctp_ulpevent_free(auth_ev);
> if (ai_ev)
> sctp_ulpevent_free(ai_ev);
> if (ev)
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT
2018-03-14 11:05 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT Xin Long
2018-03-14 11:05 ` [PATCH net-next 5/5] sctp: add SCTP_AUTH_NO_AUTH " Xin Long
@ 2018-03-14 13:53 ` Marcelo Ricardo Leitner
1 sibling, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 13:53 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Wed, Mar 14, 2018 at 07:05:33PM +0800, Xin Long wrote:
> This patch is to add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT,
> as described in section 6.1.8 of RFC6458.
>
> SCTP_AUTH_FREE_KEY: This report indicates that the SCTP
> implementation will no longer use the key identifier specified
> in auth_keynumber.
>
> After deactivating a key, it would never be used again, which means
> it's refcnt can't be held/increased by new chunks. But there may be
> some chunks in out queue still using it. So only when refcnt is 1,
> which means no chunk in outqueue is using/holding this key either,
> this EVENT would be sent.
>
> When users receive this notification, they could do DEL_KEY sockopt to
> remove this shkey, and also tell the peer that this key won't be used
> in any chunk thoroughly from now on, then the peer can remove it as
> well safely.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/uapi/linux/sctp.h | 6 +++++-
> net/sctp/auth.c | 14 ++++++++++++++
> net/sctp/sm_make_chunk.c | 20 +++++++++++++++++++-
> net/sctp/sm_statefuns.c | 2 +-
> net/sctp/socket.c | 19 ++++++++++++++++++-
> 5 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index 08fc313..18ebbfe 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -518,7 +518,11 @@ struct sctp_authkey_event {
> sctp_assoc_t auth_assoc_id;
> };
>
> -enum { SCTP_AUTH_NEWKEY = 0, };
> +enum {
> + SCTP_AUTH_NEW_KEY,
> +#define SCTP_AUTH_NEWKEY SCTP_AUTH_NEW_KEY /* compatible with before */
> + SCTP_AUTH_FREE_KEY,
> +};
>
> /*
> * 6.1.9. SCTP_SENDER_DRY_EVENT
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index a073123..e64630c 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -992,6 +992,20 @@ int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
> if (!found)
> return -EINVAL;
>
> + /* refcnt == 1 and !list_empty mean it's not being used anywhere
> + * and deactivated will be set, so it's time to notify userland
> + * that this shkey can be freed.
> + */
> + if (asoc && !list_empty(&key->key_list) &&
> + refcount_read(&key->refcnt) == 1) {
> + struct sctp_ulpevent *ev;
> +
> + ev = sctp_ulpevent_make_authkey(asoc, key->key_id,
> + SCTP_AUTH_FREE_KEY, GFP_KERNEL);
> + if (ev)
> + asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
> + }
> +
> key->deactivated = 1;
>
> return 0;
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 10f071c..cc20bc3 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -89,8 +89,26 @@ static void sctp_control_release_owner(struct sk_buff *skb)
> {
> struct sctp_chunk *chunk = skb_shinfo(skb)->destructor_arg;
>
> - if (chunk->shkey)
> + if (chunk->shkey) {
> + struct sctp_shared_key *shkey = chunk->shkey;
> + struct sctp_association *asoc = chunk->asoc;
> +
> + /* refcnt == 2 and !list_empty mean after this release, it's
> + * not being used anywhere, and it's time to notify userland
> + * that this shkey can be freed if it's been deactivated.
> + */
> + if (shkey->deactivated && !list_empty(&shkey->key_list) &&
> + refcount_read(&shkey->refcnt) == 2) {
> + struct sctp_ulpevent *ev;
> +
> + ev = sctp_ulpevent_make_authkey(asoc, shkey->key_id,
> + SCTP_AUTH_FREE_KEY,
> + GFP_KERNEL);
> + if (ev)
> + asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
> + }
> sctp_auth_shkey_release(chunk->shkey);
> + }
> }
>
> static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 792e0e2..1e41dee 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4246,7 +4246,7 @@ enum sctp_disposition sctp_sf_eat_auth(struct net *net,
> struct sctp_ulpevent *ev;
>
> ev = sctp_ulpevent_make_authkey(asoc, ntohs(auth_hdr->shkey_id),
> - SCTP_AUTH_NEWKEY, GFP_ATOMIC);
> + SCTP_AUTH_NEW_KEY, GFP_ATOMIC);
>
> if (!ev)
> return -ENOMEM;
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 65cc354..aeecdd6 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -8166,8 +8166,25 @@ static void sctp_wfree(struct sk_buff *skb)
> sk->sk_wmem_queued -= skb->truesize;
> sk_mem_uncharge(sk, skb->truesize);
>
> - if (chunk->shkey)
> + if (chunk->shkey) {
> + struct sctp_shared_key *shkey = chunk->shkey;
> +
> + /* refcnt == 2 and !list_empty mean after this release, it's
> + * not being used anywhere, and it's time to notify userland
> + * that this shkey can be freed if it's been deactivated.
> + */
> + if (shkey->deactivated && !list_empty(&shkey->key_list) &&
> + refcount_read(&shkey->refcnt) == 2) {
> + struct sctp_ulpevent *ev;
> +
> + ev = sctp_ulpevent_make_authkey(asoc, shkey->key_id,
> + SCTP_AUTH_FREE_KEY,
> + GFP_KERNEL);
> + if (ev)
> + asoc->stream.si->enqueue_event(&asoc->ulpq, ev);
> + }
> sctp_auth_shkey_release(chunk->shkey);
> + }
>
> sock_wfree(skb);
> sctp_wake_up_waiters(sk, asoc);
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY
2018-03-14 11:05 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Xin Long
2018-03-14 11:05 ` [PATCH net-next 4/5] sctp: add SCTP_AUTH_FREE_KEY type for AUTHENTICATION_EVENT Xin Long
@ 2018-03-14 13:53 ` Marcelo Ricardo Leitner
1 sibling, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 13:53 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Wed, Mar 14, 2018 at 07:05:32PM +0800, Xin Long wrote:
> This patch is to add sockopt SCTP_AUTH_DEACTIVATE_KEY, as described in
> section 8.3.4 of RFC6458.
>
> This set option indicates that the application will no longer send user
> messages using the indicated key identifier.
>
> Note that RFC requires that only deactivated keys that are no longer used
> by an association can be deleted, but for the backward compatibility, it
> is not to check deactivated when deleting or replacing one sh_key.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/net/sctp/auth.h | 12 ++++++------
> include/uapi/linux/sctp.h | 1 +
> net/sctp/auth.c | 46 +++++++++++++++++++++++++++++++++++++++++++---
> net/sctp/socket.c | 31 +++++++++++++++++++++++++++++++
> 4 files changed, 81 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
> index 017c1aa..687e7f8 100644
> --- a/include/net/sctp/auth.h
> +++ b/include/net/sctp/auth.h
> @@ -65,6 +65,7 @@ struct sctp_shared_key {
> struct sctp_auth_bytes *key;
> refcount_t refcnt;
> __u16 key_id;
> + __u8 deactivated;
> };
>
> #define key_for_each(__key, __list_head) \
> @@ -113,14 +114,13 @@ void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
> int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
> int sctp_auth_ep_set_hmacs(struct sctp_endpoint *ep,
> struct sctp_hmacalgo *hmacs);
> -int sctp_auth_set_key(struct sctp_endpoint *ep,
> - struct sctp_association *asoc,
> +int sctp_auth_set_key(struct sctp_endpoint *ep, struct sctp_association *asoc,
> struct sctp_authkey *auth_key);
> int sctp_auth_set_active_key(struct sctp_endpoint *ep,
> - struct sctp_association *asoc,
> - __u16 key_id);
> + struct sctp_association *asoc, __u16 key_id);
> int sctp_auth_del_key_id(struct sctp_endpoint *ep,
> - struct sctp_association *asoc,
> - __u16 key_id);
> + struct sctp_association *asoc, __u16 key_id);
> +int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
> + struct sctp_association *asoc, __u16 key_id);
>
> #endif
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index 47e781e..08fc313 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -99,6 +99,7 @@ typedef __s32 sctp_assoc_t;
> #define SCTP_RECVRCVINFO 32
> #define SCTP_RECVNXTINFO 33
> #define SCTP_DEFAULT_SNDINFO 34
> +#define SCTP_AUTH_DEACTIVATE_KEY 35
>
> /* Internal Socket Options. Some of the sctp library functions are
> * implemented using these socket options.
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index e5214fd..a073123 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -449,8 +449,11 @@ struct sctp_shared_key *sctp_auth_get_shkey(
>
> /* First search associations set of endpoint pair shared keys */
> key_for_each(key, &asoc->endpoint_shared_keys) {
> - if (key->key_id == key_id)
> - return key;
> + if (key->key_id == key_id) {
> + if (!key->deactivated)
> + return key;
> + break;
> + }
> }
>
> return NULL;
> @@ -905,7 +908,7 @@ int sctp_auth_set_active_key(struct sctp_endpoint *ep,
> }
> }
>
> - if (!found)
> + if (!found || key->deactivated)
> return -EINVAL;
>
> if (asoc) {
> @@ -956,3 +959,40 @@ int sctp_auth_del_key_id(struct sctp_endpoint *ep,
>
> return 0;
> }
> +
> +int sctp_auth_deact_key_id(struct sctp_endpoint *ep,
> + struct sctp_association *asoc, __u16 key_id)
> +{
> + struct sctp_shared_key *key;
> + struct list_head *sh_keys;
> + int found = 0;
> +
> + /* The key identifier MUST NOT be the current active key
> + * The key identifier MUST correst to an existing key
> + */
> + if (asoc) {
> + if (asoc->active_key_id == key_id)
> + return -EINVAL;
> +
> + sh_keys = &asoc->endpoint_shared_keys;
> + } else {
> + if (ep->active_key_id == key_id)
> + return -EINVAL;
> +
> + sh_keys = &ep->endpoint_shared_keys;
> + }
> +
> + key_for_each(key, sh_keys) {
> + if (key->key_id == key_id) {
> + found = 1;
> + break;
> + }
> + }
> +
> + if (!found)
> + return -EINVAL;
> +
> + key->deactivated = 1;
> +
> + return 0;
> +}
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 9ffdecb..65cc354 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -3647,6 +3647,33 @@ static int sctp_setsockopt_del_key(struct sock *sk,
> }
>
> /*
> + * 8.3.4 Deactivate a Shared Key (SCTP_AUTH_DEACTIVATE_KEY)
> + *
> + * This set option will deactivate a shared secret key.
> + */
> +static int sctp_setsockopt_deactivate_key(struct sock *sk, char __user *optval,
> + unsigned int optlen)
> +{
> + struct sctp_endpoint *ep = sctp_sk(sk)->ep;
> + struct sctp_authkeyid val;
> + struct sctp_association *asoc;
> +
> + if (!ep->auth_enable)
> + return -EACCES;
> +
> + if (optlen != sizeof(struct sctp_authkeyid))
> + return -EINVAL;
> + if (copy_from_user(&val, optval, optlen))
> + return -EFAULT;
> +
> + asoc = sctp_id2assoc(sk, val.scact_assoc_id);
> + if (!asoc && val.scact_assoc_id && sctp_style(sk, UDP))
> + return -EINVAL;
> +
> + return sctp_auth_deact_key_id(ep, asoc, val.scact_keynumber);
> +}
> +
> +/*
> * 8.1.23 SCTP_AUTO_ASCONF
> *
> * This option will enable or disable the use of the automatic generation of
> @@ -4238,6 +4265,9 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
> case SCTP_AUTH_DELETE_KEY:
> retval = sctp_setsockopt_del_key(sk, optval, optlen);
> break;
> + case SCTP_AUTH_DEACTIVATE_KEY:
> + retval = sctp_setsockopt_deactivate_key(sk, optval, optlen);
> + break;
> case SCTP_AUTO_ASCONF:
> retval = sctp_setsockopt_auto_asconf(sk, optval, optlen);
> break;
> @@ -7212,6 +7242,7 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
> case SCTP_AUTH_KEY:
> case SCTP_AUTH_CHUNK:
> case SCTP_AUTH_DELETE_KEY:
> + case SCTP_AUTH_DEACTIVATE_KEY:
> retval = -EOPNOTSUPP;
> break;
> case SCTP_HMAC_IDENT:
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg
2018-03-14 11:05 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Xin Long
2018-03-14 11:05 ` [PATCH net-next 3/5] sctp: add sockopt SCTP_AUTH_DEACTIVATE_KEY Xin Long
@ 2018-03-14 13:53 ` Marcelo Ricardo Leitner
1 sibling, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 13:53 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Wed, Mar 14, 2018 at 07:05:31PM +0800, Xin Long wrote:
> This patch is to add support for SCTP AUTH Information for sendmsg,
> as described in section 5.3.8 of RFC6458.
>
> With this option, you can provide shared key identifier used for
> sending the user message.
>
> It's also a necessary send info for sctp_sendv.
>
> Note that it reuses sinfo->sinfo_tsn to indicate if this option is
> set and sinfo->sinfo_ssn to save the shkey ID which can be 0.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/net/sctp/structs.h | 1 +
> include/uapi/linux/sctp.h | 14 +++++++++++++-
> net/sctp/chunk.c | 11 ++++++++++-
> net/sctp/socket.c | 23 +++++++++++++++++++++++
> 4 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 49ad67b..012fb3e 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -2118,6 +2118,7 @@ struct sctp_cmsgs {
> struct sctp_sndrcvinfo *srinfo;
> struct sctp_sndinfo *sinfo;
> struct sctp_prinfo *prinfo;
> + struct sctp_authinfo *authinfo;
> struct msghdr *addrs_msg;
> };
>
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index e94b6d2..47e781e 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -273,6 +273,18 @@ struct sctp_prinfo {
> __u32 pr_value;
> };
>
> +/* 5.3.8 SCTP AUTH Information Structure (SCTP_AUTHINFO)
> + *
> + * This cmsghdr structure specifies SCTP options for sendmsg().
> + *
> + * cmsg_level cmsg_type cmsg_data[]
> + * ------------ ------------ -------------------
> + * IPPROTO_SCTP SCTP_AUTHINFO struct sctp_authinfo
> + */
> +struct sctp_authinfo {
> + __u16 auth_keynumber;
> +};
> +
> /*
> * sinfo_flags: 16 bits (unsigned integer)
> *
> @@ -310,7 +322,7 @@ typedef enum sctp_cmsg_type {
> #define SCTP_NXTINFO SCTP_NXTINFO
> SCTP_PRINFO, /* 5.3.7 SCTP PR-SCTP Information Structure */
> #define SCTP_PRINFO SCTP_PRINFO
> - SCTP_AUTHINFO, /* 5.3.8 SCTP AUTH Information Structure (RESERVED) */
> + SCTP_AUTHINFO, /* 5.3.8 SCTP AUTH Information Structure */
> #define SCTP_AUTHINFO SCTP_AUTHINFO
> SCTP_DSTADDRV4, /* 5.3.9 SCTP Destination IPv4 Address Structure */
> #define SCTP_DSTADDRV4 SCTP_DSTADDRV4
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 9f28a9a..f889a84 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -206,7 +206,16 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
> hmac_desc->hmac_len);
>
> - shkey = asoc->shkey;
> + if (sinfo->sinfo_tsn &&
> + sinfo->sinfo_ssn != asoc->active_key_id) {
> + shkey = sctp_auth_get_shkey(asoc, sinfo->sinfo_ssn);
> + if (!shkey) {
> + err = -EINVAL;
> + goto errout;
> + }
> + } else {
> + shkey = asoc->shkey;
> + }
> }
>
> /* Check what's our max considering the above */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 003a4ad..9ffdecb 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -1987,6 +1987,14 @@ static void sctp_sendmsg_update_sinfo(struct sctp_association *asoc,
>
> if (!cmsgs->srinfo && !cmsgs->prinfo)
> sinfo->sinfo_timetolive = asoc->default_timetolive;
> +
> + if (cmsgs->authinfo) {
> + /* Reuse sinfo_tsn to indicate that authinfo was set and
> + * sinfo_ssn to save the keyid on tx path.
> + */
> + sinfo->sinfo_tsn = 1;
> + sinfo->sinfo_ssn = cmsgs->authinfo->auth_keynumber;
> + }
> }
>
> static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
> @@ -7874,6 +7882,21 @@ static int sctp_msghdr_parse(const struct msghdr *msg, struct sctp_cmsgs *cmsgs)
> if (cmsgs->prinfo->pr_policy == SCTP_PR_SCTP_NONE)
> cmsgs->prinfo->pr_value = 0;
> break;
> + case SCTP_AUTHINFO:
> + /* SCTP Socket API Extension
> + * 5.3.8 SCTP AUTH Information Structure (SCTP_AUTHINFO)
> + *
> + * This cmsghdr structure specifies SCTP options for sendmsg().
> + *
> + * cmsg_level cmsg_type cmsg_data[]
> + * ------------ ------------ ---------------------
> + * IPPROTO_SCTP SCTP_AUTHINFO struct sctp_authinfo
> + */
> + if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct sctp_authinfo)))
> + return -EINVAL;
> +
> + cmsgs->authinfo = CMSG_DATA(cmsg);
> + break;
> case SCTP_DSTADDRV4:
> case SCTP_DSTADDRV6:
> /* SCTP Socket API Extension
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next 1/5] sctp: add refcnt support for sh_key
2018-03-14 11:05 ` [PATCH net-next 1/5] sctp: add refcnt support for sh_key Xin Long
2018-03-14 11:05 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Xin Long
@ 2018-03-14 13:53 ` Marcelo Ricardo Leitner
2018-03-14 13:59 ` Neil Horman
2 siblings, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 13:53 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Neil Horman, davem
On Wed, Mar 14, 2018 at 07:05:30PM +0800, Xin Long wrote:
> With refcnt support for sh_key, chunks auth sh_keys can be decided
> before enqueuing it. Changing the active key later will not affect
> the chunks already enqueued.
>
> Furthermore, this is necessary when adding the support for authinfo
> for sendmsg in next patch.
>
> Note that struct sctp_chunk can't be grown due to that performance
> drop issue on slow cpu, so it just reuses head_skb memory for shkey
> in sctp_chunk.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> include/net/sctp/auth.h | 9 +++--
> include/net/sctp/sm.h | 3 +-
> include/net/sctp/structs.h | 9 +++--
> net/sctp/auth.c | 86 +++++++++++++++++++++++-----------------------
> net/sctp/chunk.c | 5 +++
> net/sctp/output.c | 18 ++++++++--
> net/sctp/sm_make_chunk.c | 15 ++++++--
> net/sctp/sm_statefuns.c | 11 +++---
> net/sctp/socket.c | 6 ++++
> 9 files changed, 104 insertions(+), 58 deletions(-)
>
> diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
> index e5c57d0..017c1aa 100644
> --- a/include/net/sctp/auth.h
> +++ b/include/net/sctp/auth.h
> @@ -62,8 +62,9 @@ struct sctp_auth_bytes {
> /* Definition for a shared key, weather endpoint or association */
> struct sctp_shared_key {
> struct list_head key_list;
> - __u16 key_id;
> struct sctp_auth_bytes *key;
> + refcount_t refcnt;
> + __u16 key_id;
> };
>
> #define key_for_each(__key, __list_head) \
> @@ -103,8 +104,10 @@ int sctp_auth_send_cid(enum sctp_cid chunk,
> 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, gfp_t gfp);
> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
> + struct sctp_shared_key *ep_key, gfp_t gfp);
> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key);
> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
>
> /* API Helpers */
> int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
> index 2883c43..2d0e782 100644
> --- a/include/net/sctp/sm.h
> +++ b/include/net/sctp/sm.h
> @@ -263,7 +263,8 @@ int sctp_process_asconf_ack(struct sctp_association *asoc,
> struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,
> __u32 new_cum_tsn, size_t nstreams,
> struct sctp_fwdtsn_skip *skiplist);
> -struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc);
> +struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc,
> + __u16 key_id);
> struct sctp_chunk *sctp_make_strreset_req(const struct sctp_association *asoc,
> __u16 stream_num, __be16 *stream_list,
> bool out, bool in);
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index ec6e46b..49ad67b 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -577,8 +577,12 @@ struct sctp_chunk {
> /* This points to the sk_buff containing the actual data. */
> struct sk_buff *skb;
>
> - /* In case of GSO packets, this will store the head one */
> - struct sk_buff *head_skb;
> + union {
> + /* In case of GSO packets, this will store the head one */
> + struct sk_buff *head_skb;
> + /* In case of auth enabled, this will point to the shkey */
> + struct sctp_shared_key *shkey;
> + };
>
> /* These are the SCTP headers by reverse order in a packet.
> * Note that some of these may happen more than once. In that
> @@ -1995,6 +1999,7 @@ struct sctp_association {
> * The current generated assocaition shared key (secret)
> */
> struct sctp_auth_bytes *asoc_shared_key;
> + struct sctp_shared_key *shkey;
>
> /* SCTP AUTH: hmac id of the first peer requested algorithm
> * that we support.
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 00667c5..e5214fd 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -101,13 +101,14 @@ struct sctp_shared_key *sctp_auth_shkey_create(__u16 key_id, gfp_t gfp)
> return NULL;
>
> INIT_LIST_HEAD(&new->key_list);
> + refcount_set(&new->refcnt, 1);
> new->key_id = key_id;
>
> return new;
> }
>
> /* Free the shared key structure */
> -static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
> +static void sctp_auth_shkey_destroy(struct sctp_shared_key *sh_key)
> {
> BUG_ON(!list_empty(&sh_key->key_list));
> sctp_auth_key_put(sh_key->key);
> @@ -115,6 +116,17 @@ static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
> kfree(sh_key);
> }
>
> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key)
> +{
> + if (refcount_dec_and_test(&sh_key->refcnt))
> + sctp_auth_shkey_destroy(sh_key);
> +}
> +
> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key)
> +{
> + refcount_inc(&sh_key->refcnt);
> +}
> +
> /* Destroy the entire key list. This is done during the
> * associon and endpoint free process.
> */
> @@ -128,7 +140,7 @@ void sctp_auth_destroy_keys(struct list_head *keys)
>
> key_for_each_safe(ep_key, tmp, keys) {
> list_del_init(&ep_key->key_list);
> - sctp_auth_shkey_free(ep_key);
> + sctp_auth_shkey_release(ep_key);
> }
> }
>
> @@ -409,13 +421,19 @@ int sctp_auth_asoc_init_active_key(struct sctp_association *asoc, gfp_t gfp)
>
> sctp_auth_key_put(asoc->asoc_shared_key);
> asoc->asoc_shared_key = secret;
> + asoc->shkey = ep_key;
>
> /* Update send queue in case any chunk already in there now
> * needs authenticating
> */
> list_for_each_entry(chunk, &asoc->outqueue.out_chunk_list, list) {
> - if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc))
> + if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc)) {
> chunk->auth = 1;
> + if (!chunk->shkey) {
> + chunk->shkey = asoc->shkey;
> + sctp_auth_shkey_hold(chunk->shkey);
> + }
> + }
> }
>
> return 0;
> @@ -703,16 +721,15 @@ int sctp_auth_recv_cid(enum sctp_cid chunk, const struct sctp_association *asoc)
> * after the AUTH chunk in the SCTP packet.
> */
> void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
> - struct sk_buff *skb,
> - struct sctp_auth_chunk *auth,
> - gfp_t gfp)
> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
> + struct sctp_shared_key *ep_key, gfp_t gfp)
> {
> - struct crypto_shash *tfm;
> struct sctp_auth_bytes *asoc_key;
> + struct crypto_shash *tfm;
> __u16 key_id, hmac_id;
> - __u8 *digest;
> unsigned char *end;
> int free_key = 0;
> + __u8 *digest;
>
> /* Extract the info we need:
> * - hmac id
> @@ -724,12 +741,7 @@ void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
> if (key_id == asoc->active_key_id)
> asoc_key = asoc->asoc_shared_key;
> else {
> - struct sctp_shared_key *ep_key;
> -
> - ep_key = sctp_auth_get_shkey(asoc, key_id);
> - if (!ep_key)
> - return;
> -
> + /* ep_key can't be NULL here */
> asoc_key = sctp_auth_asoc_create_secret(asoc, ep_key, gfp);
> if (!asoc_key)
> return;
> @@ -829,7 +841,7 @@ int sctp_auth_set_key(struct sctp_endpoint *ep,
> struct sctp_association *asoc,
> struct sctp_authkey *auth_key)
> {
> - struct sctp_shared_key *cur_key = NULL;
> + struct sctp_shared_key *cur_key, *shkey;
> struct sctp_auth_bytes *key;
> struct list_head *sh_keys;
> int replace = 0;
> @@ -842,46 +854,34 @@ int sctp_auth_set_key(struct sctp_endpoint *ep,
> else
> sh_keys = &ep->endpoint_shared_keys;
>
> - key_for_each(cur_key, sh_keys) {
> - if (cur_key->key_id == auth_key->sca_keynumber) {
> + key_for_each(shkey, sh_keys) {
> + if (shkey->key_id == auth_key->sca_keynumber) {
> replace = 1;
> break;
> }
> }
>
> - /* If we are not replacing a key id, we need to allocate
> - * a shared key.
> - */
> - if (!replace) {
> - cur_key = sctp_auth_shkey_create(auth_key->sca_keynumber,
> - GFP_KERNEL);
> - if (!cur_key)
> - return -ENOMEM;
> - }
> + cur_key = sctp_auth_shkey_create(auth_key->sca_keynumber, GFP_KERNEL);
> + if (!cur_key)
> + return -ENOMEM;
>
> /* Create a new key data based on the info passed in */
> key = sctp_auth_create_key(auth_key->sca_keylength, GFP_KERNEL);
> - if (!key)
> - goto nomem;
> + if (!key) {
> + kfree(cur_key);
> + return -ENOMEM;
> + }
>
> memcpy(key->data, &auth_key->sca_key[0], auth_key->sca_keylength);
> + cur_key->key = key;
>
> - /* If we are replacing, remove the old keys data from the
> - * key id. If we are adding new key id, add it to the
> - * list.
> - */
> - if (replace)
> - sctp_auth_key_put(cur_key->key);
> - else
> - list_add(&cur_key->key_list, sh_keys);
> + if (replace) {
> + list_del_init(&shkey->key_list);
> + sctp_auth_shkey_release(shkey);
> + }
> + list_add(&cur_key->key_list, sh_keys);
>
> - cur_key->key = key;
> return 0;
> -nomem:
> - if (!replace)
> - sctp_auth_shkey_free(cur_key);
> -
> - return -ENOMEM;
> }
>
> int sctp_auth_set_active_key(struct sctp_endpoint *ep,
> @@ -952,7 +952,7 @@ int sctp_auth_del_key_id(struct sctp_endpoint *ep,
>
> /* Delete the shared key */
> list_del_init(&key->key_list);
> - sctp_auth_shkey_free(key);
> + sctp_auth_shkey_release(key);
>
> return 0;
> }
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 991a530..9f28a9a 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -168,6 +168,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> {
> size_t len, first_len, max_data, remaining;
> size_t msg_len = iov_iter_count(from);
> + struct sctp_shared_key *shkey = NULL;
> struct list_head *pos, *temp;
> struct sctp_chunk *chunk;
> struct sctp_datamsg *msg;
> @@ -204,6 +205,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> if (hmac_desc)
> max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
> hmac_desc->hmac_len);
> +
> + shkey = asoc->shkey;
> }
>
> /* Check what's our max considering the above */
> @@ -275,6 +278,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> if (err < 0)
> goto errout_chunk_free;
>
> + chunk->shkey = shkey;
> +
> /* Put the chunk->skb back into the form expected by send. */
> __skb_pull(chunk->skb, (__u8 *)chunk->chunk_hdr -
> chunk->skb->data);
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 01a26ee0..d6e1c90 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -241,10 +241,13 @@ static enum sctp_xmit sctp_packet_bundle_auth(struct sctp_packet *pkt,
> if (!chunk->auth)
> return retval;
>
> - auth = sctp_make_auth(asoc);
> + auth = sctp_make_auth(asoc, chunk->shkey->key_id);
> if (!auth)
> return retval;
>
> + auth->shkey = chunk->shkey;
> + sctp_auth_shkey_hold(auth->shkey);
> +
> retval = __sctp_packet_append_chunk(pkt, auth);
>
> if (retval != SCTP_XMIT_OK)
> @@ -490,7 +493,8 @@ static int sctp_packet_pack(struct sctp_packet *packet,
> }
>
> if (auth) {
> - sctp_auth_calculate_hmac(tp->asoc, nskb, auth, gfp);
> + sctp_auth_calculate_hmac(tp->asoc, nskb, auth,
> + packet->auth->shkey, gfp);
> /* free auth if no more chunks, or add it back */
> if (list_empty(&packet->chunk_list))
> sctp_chunk_free(packet->auth);
> @@ -770,6 +774,16 @@ static enum sctp_xmit sctp_packet_will_fit(struct sctp_packet *packet,
> enum sctp_xmit retval = SCTP_XMIT_OK;
> size_t psize, pmtu, maxsize;
>
> + /* Don't bundle in this packet if this chunk's auth key doesn't
> + * match other chunks already enqueued on this packet. Also,
> + * don't bundle the chunk with auth key if other chunks in this
> + * packet don't have auth key.
> + */
> + if ((packet->auth && chunk->shkey != packet->auth->shkey) ||
> + (!packet->auth && chunk->shkey &&
> + chunk->chunk_hdr->type != SCTP_CID_AUTH))
> + return SCTP_XMIT_PMTU_FULL;
> +
> psize = packet->size;
> if (packet->transport->asoc)
> pmtu = packet->transport->asoc->pathmtu;
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index d01475f..10f071c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -87,7 +87,10 @@ static void *sctp_addto_chunk_fixed(struct sctp_chunk *, int len,
> /* Control chunk destructor */
> static void sctp_control_release_owner(struct sk_buff *skb)
> {
> - /*TODO: do memory release */
> + struct sctp_chunk *chunk = skb_shinfo(skb)->destructor_arg;
> +
> + if (chunk->shkey)
> + sctp_auth_shkey_release(chunk->shkey);
> }
>
> static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
> @@ -102,7 +105,12 @@ static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
> *
> * For now don't do anything for now.
> */
> + if (chunk->auth) {
> + chunk->shkey = asoc->shkey;
> + sctp_auth_shkey_hold(chunk->shkey);
> + }
> skb->sk = asoc ? asoc->base.sk : NULL;
> + skb_shinfo(skb)->destructor_arg = chunk;
> skb->destructor = sctp_control_release_owner;
> }
>
> @@ -1271,7 +1279,8 @@ struct sctp_chunk *sctp_make_op_error(const struct sctp_association *asoc,
> return retval;
> }
>
> -struct sctp_chunk *sctp_make_auth(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;
> @@ -1289,7 +1298,7 @@ struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc)
> return NULL;
>
> auth_hdr.hmac_id = htons(hmac_desc->hmac_id);
> - auth_hdr.shkey_id = htons(asoc->active_key_id);
> + auth_hdr.shkey_id = htons(key_id);
>
> retval->subh.auth_hdr = sctp_addto_chunk(retval, sizeof(auth_hdr),
> &auth_hdr);
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index eb7905f..792e0e2 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4114,6 +4114,7 @@ static enum sctp_ierror sctp_sf_authenticate(
> const union sctp_subtype type,
> struct sctp_chunk *chunk)
> {
> + struct sctp_shared_key *sh_key = NULL;
> struct sctp_authhdr *auth_hdr;
> __u8 *save_digest, *digest;
> struct sctp_hmac *hmac;
> @@ -4135,9 +4136,11 @@ static enum sctp_ierror sctp_sf_authenticate(
> * configured
> */
> key_id = ntohs(auth_hdr->shkey_id);
> - if (key_id != asoc->active_key_id && !sctp_auth_get_shkey(asoc, key_id))
> - return SCTP_IERROR_AUTH_BAD_KEYID;
> -
> + if (key_id != asoc->active_key_id) {
> + sh_key = sctp_auth_get_shkey(asoc, key_id);
> + if (!sh_key)
> + return SCTP_IERROR_AUTH_BAD_KEYID;
> + }
>
> /* Make sure that the length of the signature matches what
> * we expect.
> @@ -4166,7 +4169,7 @@ static enum sctp_ierror sctp_sf_authenticate(
>
> sctp_auth_calculate_hmac(asoc, chunk->skb,
> (struct sctp_auth_chunk *)chunk->chunk_hdr,
> - GFP_ATOMIC);
> + sh_key, GFP_ATOMIC);
>
> /* Discard the packet if the digests do not match */
> if (memcmp(save_digest, digest, sig_len)) {
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index af5cf29..003a4ad 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -156,6 +156,9 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
> /* The sndbuf space is tracked per association. */
> sctp_association_hold(asoc);
>
> + if (chunk->shkey)
> + sctp_auth_shkey_hold(chunk->shkey);
> +
> skb_set_owner_w(chunk->skb, sk);
>
> chunk->skb->destructor = sctp_wfree;
> @@ -8109,6 +8112,9 @@ static void sctp_wfree(struct sk_buff *skb)
> sk->sk_wmem_queued -= skb->truesize;
> sk_mem_uncharge(sk, skb->truesize);
>
> + if (chunk->shkey)
> + sctp_auth_shkey_release(chunk->shkey);
> +
> sock_wfree(skb);
> sctp_wake_up_waiters(sk, asoc);
>
> --
> 2.1.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next 1/5] sctp: add refcnt support for sh_key
2018-03-14 11:05 ` [PATCH net-next 1/5] sctp: add refcnt support for sh_key Xin Long
2018-03-14 11:05 ` [PATCH net-next 2/5] sctp: add support for SCTP AUTH Information for sendmsg Xin Long
2018-03-14 13:53 ` [PATCH net-next 1/5] sctp: add refcnt support for sh_key Marcelo Ricardo Leitner
@ 2018-03-14 13:59 ` Neil Horman
2018-03-14 16:12 ` Xin Long
2 siblings, 1 reply; 16+ messages in thread
From: Neil Horman @ 2018-03-14 13:59 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem
On Wed, Mar 14, 2018 at 07:05:30PM +0800, Xin Long wrote:
> With refcnt support for sh_key, chunks auth sh_keys can be decided
> before enqueuing it. Changing the active key later will not affect
> the chunks already enqueued.
>
> Furthermore, this is necessary when adding the support for authinfo
> for sendmsg in next patch.
>
> Note that struct sctp_chunk can't be grown due to that performance
> drop issue on slow cpu, so it just reuses head_skb memory for shkey
> in sctp_chunk.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> include/net/sctp/auth.h | 9 +++--
> include/net/sctp/sm.h | 3 +-
> include/net/sctp/structs.h | 9 +++--
> net/sctp/auth.c | 86 +++++++++++++++++++++++-----------------------
> net/sctp/chunk.c | 5 +++
> net/sctp/output.c | 18 ++++++++--
> net/sctp/sm_make_chunk.c | 15 ++++++--
> net/sctp/sm_statefuns.c | 11 +++---
> net/sctp/socket.c | 6 ++++
> 9 files changed, 104 insertions(+), 58 deletions(-)
>
> diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
> index e5c57d0..017c1aa 100644
> --- a/include/net/sctp/auth.h
> +++ b/include/net/sctp/auth.h
> @@ -62,8 +62,9 @@ struct sctp_auth_bytes {
> /* Definition for a shared key, weather endpoint or association */
> struct sctp_shared_key {
> struct list_head key_list;
> - __u16 key_id;
> struct sctp_auth_bytes *key;
> + refcount_t refcnt;
> + __u16 key_id;
> };
>
> #define key_for_each(__key, __list_head) \
> @@ -103,8 +104,10 @@ int sctp_auth_send_cid(enum sctp_cid chunk,
> 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, gfp_t gfp);
> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
> + struct sctp_shared_key *ep_key, gfp_t gfp);
> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key);
> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
>
> /* API Helpers */
> int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
> index 2883c43..2d0e782 100644
> --- a/include/net/sctp/sm.h
> +++ b/include/net/sctp/sm.h
> @@ -263,7 +263,8 @@ int sctp_process_asconf_ack(struct sctp_association *asoc,
> struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,
> __u32 new_cum_tsn, size_t nstreams,
> struct sctp_fwdtsn_skip *skiplist);
> -struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc);
> +struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc,
> + __u16 key_id);
> struct sctp_chunk *sctp_make_strreset_req(const struct sctp_association *asoc,
> __u16 stream_num, __be16 *stream_list,
> bool out, bool in);
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index ec6e46b..49ad67b 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -577,8 +577,12 @@ struct sctp_chunk {
> /* This points to the sk_buff containing the actual data. */
> struct sk_buff *skb;
>
> - /* In case of GSO packets, this will store the head one */
> - struct sk_buff *head_skb;
> + union {
> + /* In case of GSO packets, this will store the head one */
> + struct sk_buff *head_skb;
> + /* In case of auth enabled, this will point to the shkey */
> + struct sctp_shared_key *shkey;
> + };
Why do you need to add this at all? You add the shared key pointer to the
association in this patch, and sctp_chunk already has a pointer to the
association, so you should already be able to find the shared key via
chunk->asoc->shkey, no?
>
> /* These are the SCTP headers by reverse order in a packet.
> * Note that some of these may happen more than once. In that
> @@ -1995,6 +1999,7 @@ struct sctp_association {
> * The current generated assocaition shared key (secret)
> */
> struct sctp_auth_bytes *asoc_shared_key;
> + struct sctp_shared_key *shkey;
>
> /* SCTP AUTH: hmac id of the first peer requested algorithm
> * that we support.
> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
> index 00667c5..e5214fd 100644
> --- a/net/sctp/auth.c
> +++ b/net/sctp/auth.c
> @@ -101,13 +101,14 @@ struct sctp_shared_key *sctp_auth_shkey_create(__u16 key_id, gfp_t gfp)
> return NULL;
>
> INIT_LIST_HEAD(&new->key_list);
> + refcount_set(&new->refcnt, 1);
> new->key_id = key_id;
>
> return new;
> }
>
> /* Free the shared key structure */
> -static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
> +static void sctp_auth_shkey_destroy(struct sctp_shared_key *sh_key)
> {
> BUG_ON(!list_empty(&sh_key->key_list));
> sctp_auth_key_put(sh_key->key);
> @@ -115,6 +116,17 @@ static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
> kfree(sh_key);
> }
>
> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key)
> +{
> + if (refcount_dec_and_test(&sh_key->refcnt))
> + sctp_auth_shkey_destroy(sh_key);
> +}
> +
> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key)
> +{
> + refcount_inc(&sh_key->refcnt);
> +}
> +
> /* Destroy the entire key list. This is done during the
> * associon and endpoint free process.
> */
> @@ -128,7 +140,7 @@ void sctp_auth_destroy_keys(struct list_head *keys)
>
> key_for_each_safe(ep_key, tmp, keys) {
> list_del_init(&ep_key->key_list);
> - sctp_auth_shkey_free(ep_key);
> + sctp_auth_shkey_release(ep_key);
> }
> }
>
> @@ -409,13 +421,19 @@ int sctp_auth_asoc_init_active_key(struct sctp_association *asoc, gfp_t gfp)
>
> sctp_auth_key_put(asoc->asoc_shared_key);
> asoc->asoc_shared_key = secret;
> + asoc->shkey = ep_key;
>
> /* Update send queue in case any chunk already in there now
> * needs authenticating
> */
> list_for_each_entry(chunk, &asoc->outqueue.out_chunk_list, list) {
> - if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc))
> + if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc)) {
> chunk->auth = 1;
> + if (!chunk->shkey) {
> + chunk->shkey = asoc->shkey;
> + sctp_auth_shkey_hold(chunk->shkey);
> + }
> + }
Do we really need to take a reference for every chunk we send? Wouldn't it be
sufficient to just hold the shared key until the association is released?
> }
>
> return 0;
> @@ -703,16 +721,15 @@ int sctp_auth_recv_cid(enum sctp_cid chunk, const struct sctp_association *asoc)
> * after the AUTH chunk in the SCTP packet.
> */
> void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
> - struct sk_buff *skb,
> - struct sctp_auth_chunk *auth,
> - gfp_t gfp)
> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
> + struct sctp_shared_key *ep_key, gfp_t gfp)
> {
> - struct crypto_shash *tfm;
> struct sctp_auth_bytes *asoc_key;
> + struct crypto_shash *tfm;
> __u16 key_id, hmac_id;
> - __u8 *digest;
> unsigned char *end;
> int free_key = 0;
> + __u8 *digest;
>
> /* Extract the info we need:
> * - hmac id
> @@ -724,12 +741,7 @@ void sctp_auth_calculate_hmac(const struct sctp_association *asoc,
> if (key_id == asoc->active_key_id)
> asoc_key = asoc->asoc_shared_key;
> else {
> - struct sctp_shared_key *ep_key;
> -
> - ep_key = sctp_auth_get_shkey(asoc, key_id);
> - if (!ep_key)
> - return;
> -
> + /* ep_key can't be NULL here */
> asoc_key = sctp_auth_asoc_create_secret(asoc, ep_key, gfp);
> if (!asoc_key)
> return;
> @@ -829,7 +841,7 @@ int sctp_auth_set_key(struct sctp_endpoint *ep,
> struct sctp_association *asoc,
> struct sctp_authkey *auth_key)
> {
> - struct sctp_shared_key *cur_key = NULL;
> + struct sctp_shared_key *cur_key, *shkey;
> struct sctp_auth_bytes *key;
> struct list_head *sh_keys;
> int replace = 0;
> @@ -842,46 +854,34 @@ int sctp_auth_set_key(struct sctp_endpoint *ep,
> else
> sh_keys = &ep->endpoint_shared_keys;
>
> - key_for_each(cur_key, sh_keys) {
> - if (cur_key->key_id == auth_key->sca_keynumber) {
> + key_for_each(shkey, sh_keys) {
> + if (shkey->key_id == auth_key->sca_keynumber) {
> replace = 1;
> break;
> }
> }
>
> - /* If we are not replacing a key id, we need to allocate
> - * a shared key.
> - */
> - if (!replace) {
> - cur_key = sctp_auth_shkey_create(auth_key->sca_keynumber,
> - GFP_KERNEL);
> - if (!cur_key)
> - return -ENOMEM;
> - }
> + cur_key = sctp_auth_shkey_create(auth_key->sca_keynumber, GFP_KERNEL);
> + if (!cur_key)
> + return -ENOMEM;
>
> /* Create a new key data based on the info passed in */
> key = sctp_auth_create_key(auth_key->sca_keylength, GFP_KERNEL);
> - if (!key)
> - goto nomem;
> + if (!key) {
> + kfree(cur_key);
> + return -ENOMEM;
> + }
>
> memcpy(key->data, &auth_key->sca_key[0], auth_key->sca_keylength);
> + cur_key->key = key;
>
> - /* If we are replacing, remove the old keys data from the
> - * key id. If we are adding new key id, add it to the
> - * list.
> - */
> - if (replace)
> - sctp_auth_key_put(cur_key->key);
> - else
> - list_add(&cur_key->key_list, sh_keys);
> + if (replace) {
> + list_del_init(&shkey->key_list);
> + sctp_auth_shkey_release(shkey);
> + }
> + list_add(&cur_key->key_list, sh_keys);
>
> - cur_key->key = key;
> return 0;
> -nomem:
> - if (!replace)
> - sctp_auth_shkey_free(cur_key);
> -
> - return -ENOMEM;
> }
>
> int sctp_auth_set_active_key(struct sctp_endpoint *ep,
> @@ -952,7 +952,7 @@ int sctp_auth_del_key_id(struct sctp_endpoint *ep,
>
> /* Delete the shared key */
> list_del_init(&key->key_list);
> - sctp_auth_shkey_free(key);
> + sctp_auth_shkey_release(key);
>
> return 0;
> }
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 991a530..9f28a9a 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -168,6 +168,7 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> {
> size_t len, first_len, max_data, remaining;
> size_t msg_len = iov_iter_count(from);
> + struct sctp_shared_key *shkey = NULL;
> struct list_head *pos, *temp;
> struct sctp_chunk *chunk;
> struct sctp_datamsg *msg;
> @@ -204,6 +205,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> if (hmac_desc)
> max_data -= SCTP_PAD4(sizeof(struct sctp_auth_chunk) +
> hmac_desc->hmac_len);
> +
> + shkey = asoc->shkey;
> }
>
> /* Check what's our max considering the above */
> @@ -275,6 +278,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
> if (err < 0)
> goto errout_chunk_free;
>
> + chunk->shkey = shkey;
> +
> /* Put the chunk->skb back into the form expected by send. */
> __skb_pull(chunk->skb, (__u8 *)chunk->chunk_hdr -
> chunk->skb->data);
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 01a26ee0..d6e1c90 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -241,10 +241,13 @@ static enum sctp_xmit sctp_packet_bundle_auth(struct sctp_packet *pkt,
> if (!chunk->auth)
> return retval;
>
> - auth = sctp_make_auth(asoc);
> + auth = sctp_make_auth(asoc, chunk->shkey->key_id);
> if (!auth)
> return retval;
>
> + auth->shkey = chunk->shkey;
> + sctp_auth_shkey_hold(auth->shkey);
> +
> retval = __sctp_packet_append_chunk(pkt, auth);
>
> if (retval != SCTP_XMIT_OK)
> @@ -490,7 +493,8 @@ static int sctp_packet_pack(struct sctp_packet *packet,
> }
>
> if (auth) {
> - sctp_auth_calculate_hmac(tp->asoc, nskb, auth, gfp);
> + sctp_auth_calculate_hmac(tp->asoc, nskb, auth,
> + packet->auth->shkey, gfp);
> /* free auth if no more chunks, or add it back */
> if (list_empty(&packet->chunk_list))
> sctp_chunk_free(packet->auth);
> @@ -770,6 +774,16 @@ static enum sctp_xmit sctp_packet_will_fit(struct sctp_packet *packet,
> enum sctp_xmit retval = SCTP_XMIT_OK;
> size_t psize, pmtu, maxsize;
>
> + /* Don't bundle in this packet if this chunk's auth key doesn't
> + * match other chunks already enqueued on this packet. Also,
> + * don't bundle the chunk with auth key if other chunks in this
> + * packet don't have auth key.
> + */
> + if ((packet->auth && chunk->shkey != packet->auth->shkey) ||
> + (!packet->auth && chunk->shkey &&
> + chunk->chunk_hdr->type != SCTP_CID_AUTH))
> + return SCTP_XMIT_PMTU_FULL;
> +
> psize = packet->size;
> if (packet->transport->asoc)
> pmtu = packet->transport->asoc->pathmtu;
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index d01475f..10f071c 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -87,7 +87,10 @@ static void *sctp_addto_chunk_fixed(struct sctp_chunk *, int len,
> /* Control chunk destructor */
> static void sctp_control_release_owner(struct sk_buff *skb)
> {
> - /*TODO: do memory release */
> + struct sctp_chunk *chunk = skb_shinfo(skb)->destructor_arg;
> +
> + if (chunk->shkey)
> + sctp_auth_shkey_release(chunk->shkey);
> }
>
> static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
> @@ -102,7 +105,12 @@ static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
> *
> * For now don't do anything for now.
> */
> + if (chunk->auth) {
> + chunk->shkey = asoc->shkey;
> + sctp_auth_shkey_hold(chunk->shkey);
> + }
> skb->sk = asoc ? asoc->base.sk : NULL;
> + skb_shinfo(skb)->destructor_arg = chunk;
> skb->destructor = sctp_control_release_owner;
> }
>
> @@ -1271,7 +1279,8 @@ struct sctp_chunk *sctp_make_op_error(const struct sctp_association *asoc,
> return retval;
> }
>
> -struct sctp_chunk *sctp_make_auth(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;
> @@ -1289,7 +1298,7 @@ struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc)
> return NULL;
>
> auth_hdr.hmac_id = htons(hmac_desc->hmac_id);
> - auth_hdr.shkey_id = htons(asoc->active_key_id);
> + auth_hdr.shkey_id = htons(key_id);
>
> retval->subh.auth_hdr = sctp_addto_chunk(retval, sizeof(auth_hdr),
> &auth_hdr);
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index eb7905f..792e0e2 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -4114,6 +4114,7 @@ static enum sctp_ierror sctp_sf_authenticate(
> const union sctp_subtype type,
> struct sctp_chunk *chunk)
> {
> + struct sctp_shared_key *sh_key = NULL;
> struct sctp_authhdr *auth_hdr;
> __u8 *save_digest, *digest;
> struct sctp_hmac *hmac;
> @@ -4135,9 +4136,11 @@ static enum sctp_ierror sctp_sf_authenticate(
> * configured
> */
> key_id = ntohs(auth_hdr->shkey_id);
> - if (key_id != asoc->active_key_id && !sctp_auth_get_shkey(asoc, key_id))
> - return SCTP_IERROR_AUTH_BAD_KEYID;
> -
> + if (key_id != asoc->active_key_id) {
> + sh_key = sctp_auth_get_shkey(asoc, key_id);
> + if (!sh_key)
> + return SCTP_IERROR_AUTH_BAD_KEYID;
> + }
>
> /* Make sure that the length of the signature matches what
> * we expect.
> @@ -4166,7 +4169,7 @@ static enum sctp_ierror sctp_sf_authenticate(
>
> sctp_auth_calculate_hmac(asoc, chunk->skb,
> (struct sctp_auth_chunk *)chunk->chunk_hdr,
> - GFP_ATOMIC);
> + sh_key, GFP_ATOMIC);
>
> /* Discard the packet if the digests do not match */
> if (memcmp(save_digest, digest, sig_len)) {
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index af5cf29..003a4ad 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -156,6 +156,9 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
> /* The sndbuf space is tracked per association. */
> sctp_association_hold(asoc);
>
> + if (chunk->shkey)
> + sctp_auth_shkey_hold(chunk->shkey);
> +
> skb_set_owner_w(chunk->skb, sk);
>
> chunk->skb->destructor = sctp_wfree;
> @@ -8109,6 +8112,9 @@ static void sctp_wfree(struct sk_buff *skb)
> sk->sk_wmem_queued -= skb->truesize;
> sk_mem_uncharge(sk, skb->truesize);
>
> + if (chunk->shkey)
> + sctp_auth_shkey_release(chunk->shkey);
> +
> sock_wfree(skb);
> sctp_wake_up_waiters(sk, asoc);
>
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next 1/5] sctp: add refcnt support for sh_key
2018-03-14 13:59 ` Neil Horman
@ 2018-03-14 16:12 ` Xin Long
2018-03-14 19:41 ` Marcelo Ricardo Leitner
0 siblings, 1 reply; 16+ messages in thread
From: Xin Long @ 2018-03-14 16:12 UTC (permalink / raw)
To: Neil Horman; +Cc: network dev, linux-sctp, Marcelo Ricardo Leitner, davem
On Wed, Mar 14, 2018 at 9:59 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Wed, Mar 14, 2018 at 07:05:30PM +0800, Xin Long wrote:
>> With refcnt support for sh_key, chunks auth sh_keys can be decided
>> before enqueuing it. Changing the active key later will not affect
>> the chunks already enqueued.
>>
>> Furthermore, this is necessary when adding the support for authinfo
>> for sendmsg in next patch.
>>
>> Note that struct sctp_chunk can't be grown due to that performance
>> drop issue on slow cpu, so it just reuses head_skb memory for shkey
>> in sctp_chunk.
>>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> include/net/sctp/auth.h | 9 +++--
>> include/net/sctp/sm.h | 3 +-
>> include/net/sctp/structs.h | 9 +++--
>> net/sctp/auth.c | 86 +++++++++++++++++++++++-----------------------
>> net/sctp/chunk.c | 5 +++
>> net/sctp/output.c | 18 ++++++++--
>> net/sctp/sm_make_chunk.c | 15 ++++++--
>> net/sctp/sm_statefuns.c | 11 +++---
>> net/sctp/socket.c | 6 ++++
>> 9 files changed, 104 insertions(+), 58 deletions(-)
>>
>> diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
>> index e5c57d0..017c1aa 100644
>> --- a/include/net/sctp/auth.h
>> +++ b/include/net/sctp/auth.h
>> @@ -62,8 +62,9 @@ struct sctp_auth_bytes {
>> /* Definition for a shared key, weather endpoint or association */
>> struct sctp_shared_key {
>> struct list_head key_list;
>> - __u16 key_id;
>> struct sctp_auth_bytes *key;
>> + refcount_t refcnt;
>> + __u16 key_id;
>> };
>>
>> #define key_for_each(__key, __list_head) \
>> @@ -103,8 +104,10 @@ int sctp_auth_send_cid(enum sctp_cid chunk,
>> 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, gfp_t gfp);
>> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
>> + struct sctp_shared_key *ep_key, gfp_t gfp);
>> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key);
>> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
>>
>> /* API Helpers */
>> int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
>> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
>> index 2883c43..2d0e782 100644
>> --- a/include/net/sctp/sm.h
>> +++ b/include/net/sctp/sm.h
>> @@ -263,7 +263,8 @@ int sctp_process_asconf_ack(struct sctp_association *asoc,
>> struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,
>> __u32 new_cum_tsn, size_t nstreams,
>> struct sctp_fwdtsn_skip *skiplist);
>> -struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc);
>> +struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc,
>> + __u16 key_id);
>> struct sctp_chunk *sctp_make_strreset_req(const struct sctp_association *asoc,
>> __u16 stream_num, __be16 *stream_list,
>> bool out, bool in);
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index ec6e46b..49ad67b 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -577,8 +577,12 @@ struct sctp_chunk {
>> /* This points to the sk_buff containing the actual data. */
>> struct sk_buff *skb;
>>
>> - /* In case of GSO packets, this will store the head one */
>> - struct sk_buff *head_skb;
>> + union {
>> + /* In case of GSO packets, this will store the head one */
>> + struct sk_buff *head_skb;
>> + /* In case of auth enabled, this will point to the shkey */
>> + struct sctp_shared_key *shkey;
>> + };
> Why do you need to add this at all? You add the shared key pointer to the
> association in this patch, and sctp_chunk already has a pointer to the
> association, so you should already be able to find the shared key via
> chunk->asoc->shkey, no?
We need this, because one asoc can have a list of shared keys.
When sending a msg, we can even choose one of these keys with
cmsg info (cmsgs->authinfo) for this msg, which is that
5.3.8. SCTP AUTH Information Structure (SCTP_AUTHINFO)
is supposed to do. (Patch 2/5)
After this patchset, asoc->shkey (or we can say active shkey) is
more like default shkey. It will be used only when SCTP_AUTHINFO
is not set in cmsg info.
>
>
>>
>> /* These are the SCTP headers by reverse order in a packet.
>> * Note that some of these may happen more than once. In that
>> @@ -1995,6 +1999,7 @@ struct sctp_association {
>> * The current generated assocaition shared key (secret)
>> */
>> struct sctp_auth_bytes *asoc_shared_key;
>> + struct sctp_shared_key *shkey;
>>
>> /* SCTP AUTH: hmac id of the first peer requested algorithm
>> * that we support.
>> diff --git a/net/sctp/auth.c b/net/sctp/auth.c
>> index 00667c5..e5214fd 100644
>> --- a/net/sctp/auth.c
>> +++ b/net/sctp/auth.c
>> @@ -101,13 +101,14 @@ struct sctp_shared_key *sctp_auth_shkey_create(__u16 key_id, gfp_t gfp)
>> return NULL;
>>
>> INIT_LIST_HEAD(&new->key_list);
>> + refcount_set(&new->refcnt, 1);
>> new->key_id = key_id;
>>
>> return new;
>> }
>>
>> /* Free the shared key structure */
>> -static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
>> +static void sctp_auth_shkey_destroy(struct sctp_shared_key *sh_key)
>> {
>> BUG_ON(!list_empty(&sh_key->key_list));
>> sctp_auth_key_put(sh_key->key);
>> @@ -115,6 +116,17 @@ static void sctp_auth_shkey_free(struct sctp_shared_key *sh_key)
>> kfree(sh_key);
>> }
>>
>> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key)
>> +{
>> + if (refcount_dec_and_test(&sh_key->refcnt))
>> + sctp_auth_shkey_destroy(sh_key);
>> +}
>> +
>> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key)
>> +{
>> + refcount_inc(&sh_key->refcnt);
>> +}
>> +
>> /* Destroy the entire key list. This is done during the
>> * associon and endpoint free process.
>> */
>> @@ -128,7 +140,7 @@ void sctp_auth_destroy_keys(struct list_head *keys)
>>
>> key_for_each_safe(ep_key, tmp, keys) {
>> list_del_init(&ep_key->key_list);
>> - sctp_auth_shkey_free(ep_key);
>> + sctp_auth_shkey_release(ep_key);
>> }
>> }
>>
>> @@ -409,13 +421,19 @@ int sctp_auth_asoc_init_active_key(struct sctp_association *asoc, gfp_t gfp)
>>
>> sctp_auth_key_put(asoc->asoc_shared_key);
>> asoc->asoc_shared_key = secret;
>> + asoc->shkey = ep_key;
>>
>> /* Update send queue in case any chunk already in there now
>> * needs authenticating
>> */
>> list_for_each_entry(chunk, &asoc->outqueue.out_chunk_list, list) {
>> - if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc))
>> + if (sctp_auth_send_cid(chunk->chunk_hdr->type, asoc)) {
>> chunk->auth = 1;
>> + if (!chunk->shkey) {
>> + chunk->shkey = asoc->shkey;
>> + sctp_auth_shkey_hold(chunk->shkey);
>> + }
>> + }
> Do we really need to take a reference for every chunk we send? Wouldn't it be
> sufficient to just hold the shared key until the association is released?
As we can use other shkey(not only asoc->shkey) for one chunk, and even
different shkeys for different chunks, we have to hold it in case that this
shkey may be removed/freed when this chunk is still queuing up in
outqueue, until this chunk has been transmitted and freed.
make sense?
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next 1/5] sctp: add refcnt support for sh_key
2018-03-14 16:12 ` Xin Long
@ 2018-03-14 19:41 ` Marcelo Ricardo Leitner
0 siblings, 0 replies; 16+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-14 19:41 UTC (permalink / raw)
To: Xin Long; +Cc: Neil Horman, network dev, linux-sctp, davem
On Thu, Mar 15, 2018 at 12:12:32AM +0800, Xin Long wrote:
> On Wed, Mar 14, 2018 at 9:59 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Wed, Mar 14, 2018 at 07:05:30PM +0800, Xin Long wrote:
> >> With refcnt support for sh_key, chunks auth sh_keys can be decided
> >> before enqueuing it. Changing the active key later will not affect
> >> the chunks already enqueued.
> >>
> >> Furthermore, this is necessary when adding the support for authinfo
> >> for sendmsg in next patch.
> >>
> >> Note that struct sctp_chunk can't be grown due to that performance
> >> drop issue on slow cpu, so it just reuses head_skb memory for shkey
> >> in sctp_chunk.
> >>
> >> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >> ---
> >> include/net/sctp/auth.h | 9 +++--
> >> include/net/sctp/sm.h | 3 +-
> >> include/net/sctp/structs.h | 9 +++--
> >> net/sctp/auth.c | 86 +++++++++++++++++++++++-----------------------
> >> net/sctp/chunk.c | 5 +++
> >> net/sctp/output.c | 18 ++++++++--
> >> net/sctp/sm_make_chunk.c | 15 ++++++--
> >> net/sctp/sm_statefuns.c | 11 +++---
> >> net/sctp/socket.c | 6 ++++
> >> 9 files changed, 104 insertions(+), 58 deletions(-)
> >>
> >> diff --git a/include/net/sctp/auth.h b/include/net/sctp/auth.h
> >> index e5c57d0..017c1aa 100644
> >> --- a/include/net/sctp/auth.h
> >> +++ b/include/net/sctp/auth.h
> >> @@ -62,8 +62,9 @@ struct sctp_auth_bytes {
> >> /* Definition for a shared key, weather endpoint or association */
> >> struct sctp_shared_key {
> >> struct list_head key_list;
> >> - __u16 key_id;
> >> struct sctp_auth_bytes *key;
> >> + refcount_t refcnt;
> >> + __u16 key_id;
> >> };
> >>
> >> #define key_for_each(__key, __list_head) \
> >> @@ -103,8 +104,10 @@ int sctp_auth_send_cid(enum sctp_cid chunk,
> >> 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, gfp_t gfp);
> >> + struct sk_buff *skb, struct sctp_auth_chunk *auth,
> >> + struct sctp_shared_key *ep_key, gfp_t gfp);
> >> +void sctp_auth_shkey_release(struct sctp_shared_key *sh_key);
> >> +void sctp_auth_shkey_hold(struct sctp_shared_key *sh_key);
> >>
> >> /* API Helpers */
> >> int sctp_auth_ep_add_chunkid(struct sctp_endpoint *ep, __u8 chunk_id);
> >> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
> >> index 2883c43..2d0e782 100644
> >> --- a/include/net/sctp/sm.h
> >> +++ b/include/net/sctp/sm.h
> >> @@ -263,7 +263,8 @@ int sctp_process_asconf_ack(struct sctp_association *asoc,
> >> struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,
> >> __u32 new_cum_tsn, size_t nstreams,
> >> struct sctp_fwdtsn_skip *skiplist);
> >> -struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc);
> >> +struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc,
> >> + __u16 key_id);
> >> struct sctp_chunk *sctp_make_strreset_req(const struct sctp_association *asoc,
> >> __u16 stream_num, __be16 *stream_list,
> >> bool out, bool in);
> >> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> >> index ec6e46b..49ad67b 100644
> >> --- a/include/net/sctp/structs.h
> >> +++ b/include/net/sctp/structs.h
> >> @@ -577,8 +577,12 @@ struct sctp_chunk {
> >> /* This points to the sk_buff containing the actual data. */
> >> struct sk_buff *skb;
> >>
> >> - /* In case of GSO packets, this will store the head one */
> >> - struct sk_buff *head_skb;
> >> + union {
> >> + /* In case of GSO packets, this will store the head one */
> >> + struct sk_buff *head_skb;
> >> + /* In case of auth enabled, this will point to the shkey */
> >> + struct sctp_shared_key *shkey;
> >> + };
> > Why do you need to add this at all? You add the shared key pointer to the
> > association in this patch, and sctp_chunk already has a pointer to the
> > association, so you should already be able to find the shared key via
> > chunk->asoc->shkey, no?
> We need this, because one asoc can have a list of shared keys.
> When sending a msg, we can even choose one of these keys with
> cmsg info (cmsgs->authinfo) for this msg, which is that
> 5.3.8. SCTP AUTH Information Structure (SCTP_AUTHINFO)
> is supposed to do. (Patch 2/5)
>
> After this patchset, asoc->shkey (or we can say active shkey) is
> more like default shkey. It will be used only when SCTP_AUTHINFO
> is not set in cmsg info.
And even then, now once the chunk is enqueued, asoc->shkey is allowed
to change without affecting it.
This is probably one of the main improvements on this patchset.
M.
^ permalink raw reply [flat|nested] 16+ messages in thread