* [PATCH net 0/3] macsec: fix configurable ICV length
@ 2016-07-22 13:07 Davide Caratti
2016-07-22 13:07 ` [PATCH net 1/3] macsec: limit ICV length to 16 octets Davide Caratti
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Davide Caratti @ 2016-07-22 13:07 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Sabrina Dubroca, Hannes Frederic Sowa
This series provides a fix for macsec configurable ICV length. The
maximum length of ICV element has been made compliant to IEEE 802.1AE,
and error reporting in case of cipher suite configuration failure has been
improved. Finally, a test has been added to netlink verify() callback in
order to avoid creation of macsec interfaces having user-provided ICV length
values that are not supported by the cipher suite.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Davide Caratti (3):
macsec: limit ICV length to 16 octets
macsec: fix error codes when a SA is created
macsec: validate ICV length on link creation
drivers/net/macsec.c | 76 ++++++++++++++++++++++++++++--------------
include/uapi/linux/if_macsec.h | 2 ++
2 files changed, 53 insertions(+), 25 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net 1/3] macsec: limit ICV length to 16 octets 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti @ 2016-07-22 13:07 ` Davide Caratti 2016-07-22 13:07 ` [PATCH net 2/3] macsec: fix error codes when a SA is created Davide Caratti ` (3 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: Davide Caratti @ 2016-07-22 13:07 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Sabrina Dubroca, Hannes Frederic Sowa 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 <dcaratti@redhat.com> --- 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 8bcd78f..7c0e732 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -508,7 +508,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) { @@ -3199,7 +3199,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 f7d4831..02fc49c 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, -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/3] macsec: fix error codes when a SA is created 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti 2016-07-22 13:07 ` [PATCH net 1/3] macsec: limit ICV length to 16 octets Davide Caratti @ 2016-07-22 13:07 ` Davide Caratti 2016-07-22 13:07 ` [PATCH net 3/3] macsec: validate ICV length on link creation Davide Caratti ` (2 subsequent siblings) 4 siblings, 0 replies; 6+ messages in thread From: Davide Caratti @ 2016-07-22 13:07 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Sabrina Dubroca, Hannes Frederic Sowa 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 <dcaratti@redhat.com> --- 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 7c0e732..6d45ba6 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -1263,22 +1263,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, @@ -1286,12 +1286,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; @@ -1384,12 +1384,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; @@ -1622,6 +1622,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; @@ -1658,13 +1659,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]); @@ -1770,6 +1777,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; @@ -1806,13 +1814,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); -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 3/3] macsec: validate ICV length on link creation 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti 2016-07-22 13:07 ` [PATCH net 1/3] macsec: limit ICV length to 16 octets Davide Caratti 2016-07-22 13:07 ` [PATCH net 2/3] macsec: fix error codes when a SA is created Davide Caratti @ 2016-07-22 13:07 ` Davide Caratti 2016-07-25 10:09 ` [PATCH net 0/3] macsec: fix configurable ICV length Sabrina Dubroca 2016-07-25 17:55 ` David Miller 4 siblings, 0 replies; 6+ messages in thread From: Davide Caratti @ 2016-07-22 13:07 UTC (permalink / raw) To: netdev; +Cc: David S. Miller, Sabrina Dubroca, Hannes Frederic Sowa 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 <dcaratti@redhat.com> --- 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 6d45ba6..5441517 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -3206,8 +3206,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: -- 2.5.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] macsec: fix configurable ICV length 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti ` (2 preceding siblings ...) 2016-07-22 13:07 ` [PATCH net 3/3] macsec: validate ICV length on link creation Davide Caratti @ 2016-07-25 10:09 ` Sabrina Dubroca 2016-07-25 17:55 ` David Miller 4 siblings, 0 replies; 6+ messages in thread From: Sabrina Dubroca @ 2016-07-25 10:09 UTC (permalink / raw) To: Davide Caratti; +Cc: netdev, David S. Miller, Hannes Frederic Sowa 2016-07-22, 15:07:55 +0200, Davide Caratti wrote: > This series provides a fix for macsec configurable ICV length. The > maximum length of ICV element has been made compliant to IEEE 802.1AE, > and error reporting in case of cipher suite configuration failure has been > improved. Finally, a test has been added to netlink verify() callback in > order to avoid creation of macsec interfaces having user-provided ICV length > values that are not supported by the cipher suite. > > Signed-off-by: Davide Caratti <dcaratti@redhat.com> > > Davide Caratti (3): > macsec: limit ICV length to 16 octets > macsec: fix error codes when a SA is created > macsec: validate ICV length on link creation > > drivers/net/macsec.c | 76 ++++++++++++++++++++++++++++-------------- > include/uapi/linux/if_macsec.h | 2 ++ > 2 files changed, 53 insertions(+), 25 deletions(-) Acked-by: Sabrina Dubroca <sd@queasysnail.net> -- Sabrina ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/3] macsec: fix configurable ICV length 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti ` (3 preceding siblings ...) 2016-07-25 10:09 ` [PATCH net 0/3] macsec: fix configurable ICV length Sabrina Dubroca @ 2016-07-25 17:55 ` David Miller 4 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2016-07-25 17:55 UTC (permalink / raw) To: dcaratti; +Cc: netdev, sd, hannes From: Davide Caratti <dcaratti@redhat.com> Date: Fri, 22 Jul 2016 15:07:55 +0200 > This series provides a fix for macsec configurable ICV length. The > maximum length of ICV element has been made compliant to IEEE 802.1AE, > and error reporting in case of cipher suite configuration failure has been > improved. Finally, a test has been added to netlink verify() callback in > order to avoid creation of macsec interfaces having user-provided ICV length > values that are not supported by the cipher suite. > > Signed-off-by: Davide Caratti <dcaratti@redhat.com> Series applied and queued up for -stable, thanks. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-25 17:55 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-22 13:07 [PATCH net 0/3] macsec: fix configurable ICV length Davide Caratti 2016-07-22 13:07 ` [PATCH net 1/3] macsec: limit ICV length to 16 octets Davide Caratti 2016-07-22 13:07 ` [PATCH net 2/3] macsec: fix error codes when a SA is created Davide Caratti 2016-07-22 13:07 ` [PATCH net 3/3] macsec: validate ICV length on link creation Davide Caratti 2016-07-25 10:09 ` [PATCH net 0/3] macsec: fix configurable ICV length Sabrina Dubroca 2016-07-25 17:55 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).