From 2ccbe2cb79f2f74ab739252299b6f9ff27586f2c Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Fri, 22 Jul 2016 15:07:56 +0200 Subject: [PATCH 1/3] macsec: limit ICV length to 16 octets IEEE 802.1AE-2006 standard recommends that the ICV element in a MACsec frame should not exceed 16 octets: add MACSEC_STD_ICV_LEN in uapi definitions accordingly, and avoid accepting configurations where the ICV length exceeds the standard value. Leave definition of MACSEC_MAX_ICV_LEN unchanged for backwards compatibility with userspace programs. Fixes: dece8d2b78d1 ("uapi: add MACsec bits") Signed-off-by: Davide Caratti Signed-off-by: David S. Miller --- drivers/net/macsec.c | 4 ++-- include/uapi/linux/if_macsec.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 0cbb935078da..18cfb46c5911 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -510,7 +510,7 @@ static bool macsec_validate_skb(struct sk_buff *skb, u16 icv_len) } #define MACSEC_NEEDED_HEADROOM (macsec_extra_len(true)) -#define MACSEC_NEEDED_TAILROOM MACSEC_MAX_ICV_LEN +#define MACSEC_NEEDED_TAILROOM MACSEC_STD_ICV_LEN static void macsec_fill_iv(unsigned char *iv, sci_t sci, u32 pn) { @@ -3217,7 +3217,7 @@ static int macsec_validate_attr(struct nlattr *tb[], struct nlattr *data[]) case MACSEC_DEFAULT_CIPHER_ID: case MACSEC_DEFAULT_CIPHER_ALT: if (icv_len < MACSEC_MIN_ICV_LEN || - icv_len > MACSEC_MAX_ICV_LEN) + icv_len > MACSEC_STD_ICV_LEN) return -EINVAL; break; default: diff --git a/include/uapi/linux/if_macsec.h b/include/uapi/linux/if_macsec.h index f7d4831a2cc7..02fc49cb72d8 100644 --- a/include/uapi/linux/if_macsec.h +++ b/include/uapi/linux/if_macsec.h @@ -26,6 +26,8 @@ #define MACSEC_MIN_ICV_LEN 8 #define MACSEC_MAX_ICV_LEN 32 +/* upper limit for ICV length as recommended by IEEE802.1AE-2006 */ +#define MACSEC_STD_ICV_LEN 16 enum macsec_attrs { MACSEC_ATTR_UNSPEC, From 34aedfee22967236adc3d9147c8b47b7f5bad26c Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Fri, 22 Jul 2016 15:07:57 +0200 Subject: [PATCH 2/3] macsec: fix error codes when a SA is created preserve the return value of AEAD functions that are called when a SA is created, to avoid inappropriate display of "RTNETLINK answers: Cannot allocate memory" message. Signed-off-by: Davide Caratti Signed-off-by: David S. Miller --- drivers/net/macsec.c | 58 +++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 18cfb46c5911..0045108d7159 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -1270,22 +1270,22 @@ static struct crypto_aead *macsec_alloc_tfm(char *key, int key_len, int icv_len) int ret; tfm = crypto_alloc_aead("gcm(aes)", 0, 0); - if (!tfm || IS_ERR(tfm)) - return NULL; + + if (IS_ERR(tfm)) + return tfm; ret = crypto_aead_setkey(tfm, key, key_len); - if (ret < 0) { - crypto_free_aead(tfm); - return NULL; - } + if (ret < 0) + goto fail; ret = crypto_aead_setauthsize(tfm, icv_len); - if (ret < 0) { - crypto_free_aead(tfm); - return NULL; - } + if (ret < 0) + goto fail; return tfm; +fail: + crypto_free_aead(tfm); + return ERR_PTR(ret); } static int init_rx_sa(struct macsec_rx_sa *rx_sa, char *sak, int key_len, @@ -1293,12 +1293,12 @@ static int init_rx_sa(struct macsec_rx_sa *rx_sa, char *sak, int key_len, { rx_sa->stats = alloc_percpu(struct macsec_rx_sa_stats); if (!rx_sa->stats) - return -1; + return -ENOMEM; rx_sa->key.tfm = macsec_alloc_tfm(sak, key_len, icv_len); - if (!rx_sa->key.tfm) { + if (IS_ERR(rx_sa->key.tfm)) { free_percpu(rx_sa->stats); - return -1; + return PTR_ERR(rx_sa->key.tfm); } rx_sa->active = false; @@ -1391,12 +1391,12 @@ static int init_tx_sa(struct macsec_tx_sa *tx_sa, char *sak, int key_len, { tx_sa->stats = alloc_percpu(struct macsec_tx_sa_stats); if (!tx_sa->stats) - return -1; + return -ENOMEM; tx_sa->key.tfm = macsec_alloc_tfm(sak, key_len, icv_len); - if (!tx_sa->key.tfm) { + if (IS_ERR(tx_sa->key.tfm)) { free_percpu(tx_sa->stats); - return -1; + return PTR_ERR(tx_sa->key.tfm); } tx_sa->active = false; @@ -1629,6 +1629,7 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) unsigned char assoc_num; struct nlattr *tb_rxsc[MACSEC_RXSC_ATTR_MAX + 1]; struct nlattr *tb_sa[MACSEC_SA_ATTR_MAX + 1]; + int err; if (!attrs[MACSEC_ATTR_IFINDEX]) return -EINVAL; @@ -1665,13 +1666,19 @@ static int macsec_add_rxsa(struct sk_buff *skb, struct genl_info *info) } rx_sa = kmalloc(sizeof(*rx_sa), GFP_KERNEL); - if (!rx_sa || init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), - secy->key_len, secy->icv_len)) { - kfree(rx_sa); + if (!rx_sa) { rtnl_unlock(); return -ENOMEM; } + err = init_rx_sa(rx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), + secy->key_len, secy->icv_len); + if (err < 0) { + kfree(rx_sa); + rtnl_unlock(); + return err; + } + if (tb_sa[MACSEC_SA_ATTR_PN]) { spin_lock_bh(&rx_sa->lock); rx_sa->next_pn = nla_get_u32(tb_sa[MACSEC_SA_ATTR_PN]); @@ -1777,6 +1784,7 @@ static int macsec_add_txsa(struct sk_buff *skb, struct genl_info *info) struct macsec_tx_sa *tx_sa; unsigned char assoc_num; struct nlattr *tb_sa[MACSEC_SA_ATTR_MAX + 1]; + int err; if (!attrs[MACSEC_ATTR_IFINDEX]) return -EINVAL; @@ -1813,13 +1821,19 @@ static int macsec_add_txsa(struct sk_buff *skb, struct genl_info *info) } tx_sa = kmalloc(sizeof(*tx_sa), GFP_KERNEL); - if (!tx_sa || init_tx_sa(tx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), - secy->key_len, secy->icv_len)) { - kfree(tx_sa); + if (!tx_sa) { rtnl_unlock(); return -ENOMEM; } + err = init_tx_sa(tx_sa, nla_data(tb_sa[MACSEC_SA_ATTR_KEY]), + secy->key_len, secy->icv_len); + if (err < 0) { + kfree(tx_sa); + rtnl_unlock(); + return err; + } + nla_memcpy(tx_sa->key.id, tb_sa[MACSEC_SA_ATTR_KEYID], MACSEC_KEYID_LEN); spin_lock_bh(&tx_sa->lock); From f04c392d2dd97a985878f4380a1b054791301acf Mon Sep 17 00:00:00 2001 From: Davide Caratti Date: Fri, 22 Jul 2016 15:07:58 +0200 Subject: [PATCH 3/3] macsec: validate ICV length on link creation Test the cipher suite initialization in case ICV length has a value different than its default. If this test fails, creation of a new macsec link will also fail. This avoids situations where further security associations can't be added due to failures of crypto_aead_setauthsize(), caused by unsupported user-provided values of the ICV length. Signed-off-by: Davide Caratti Signed-off-by: David S. Miller --- drivers/net/macsec.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 0045108d7159..d8b2b49d6d5f 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -3224,8 +3224,20 @@ static int macsec_validate_attr(struct nlattr *tb[], struct nlattr *data[]) if (data[IFLA_MACSEC_CIPHER_SUITE]) csid = nla_get_u64(data[IFLA_MACSEC_CIPHER_SUITE]); - if (data[IFLA_MACSEC_ICV_LEN]) + if (data[IFLA_MACSEC_ICV_LEN]) { icv_len = nla_get_u8(data[IFLA_MACSEC_ICV_LEN]); + if (icv_len != DEFAULT_ICV_LEN) { + char dummy_key[DEFAULT_SAK_LEN] = { 0 }; + struct crypto_aead *dummy_tfm; + + dummy_tfm = macsec_alloc_tfm(dummy_key, + DEFAULT_SAK_LEN, + icv_len); + if (IS_ERR(dummy_tfm)) + return PTR_ERR(dummy_tfm); + crypto_free_aead(dummy_tfm); + } + } switch (csid) { case MACSEC_DEFAULT_CIPHER_ID: