[dpdk-dev,v2] net/ixgbe: removed ipsec keys from private data
Checks
Commit Message
All ipsec related setting are being held in the driver
private data to allow easy add and remove of SAs. There
is no need to keep a record of the keys, and also
storing the keys can be a security issue.
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Acked-by: Declan Doherty <declan.doherty@intel.com>
---
drivers/net/ixgbe/ixgbe_ipsec.c | 78 ++++++++++++++++++-----------------------
drivers/net/ixgbe/ixgbe_ipsec.h | 4 ---
2 files changed, 35 insertions(+), 47 deletions(-)
Comments
On Wed, 20 Dec 2017 11:32:51 +0000
Radu Nicolau <radu.nicolau@intel.com> wrote:
> All ipsec related setting are being held in the driver
> private data to allow easy add and remove of SAs. There
> is no need to keep a record of the keys, and also
> storing the keys can be a security issue.
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Acked-by: Declan Doherty <declan.doherty@intel.com>
> ---
> drivers/net/ixgbe/ixgbe_ipsec.c | 78 ++++++++++++++++++-----------------------
> drivers/net/ixgbe/ixgbe_ipsec.h | 4 ---
> 2 files changed, 35 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
> index 105da11..a7ba358 100644
> --- a/drivers/net/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ixgbe/ixgbe_ipsec.c
> @@ -70,6 +70,8 @@ static void
> ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
> {
> struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
> + dev->data->dev_private);
> int i = 0;
>
> /* clear Rx IP table*/
> @@ -106,6 +108,10 @@ ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
> IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
> IXGBE_WAIT_TWRITE;
> }
> +
> + memset(priv->rx_ip_tbl, 0, sizeof(priv->rx_ip_tbl));
> + memset(priv->rx_sa_tbl, 0, sizeof(priv->rx_sa_tbl));
> + memset(priv->tx_sa_tbl, 0, sizeof(priv->tx_sa_tbl));
GCC has been known to optimize out this kind of memset.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=8537
On 12/20/2017 3:46 PM, Stephen Hemminger wrote:
> On Wed, 20 Dec 2017 11:32:51 +0000
> Radu Nicolau <radu.nicolau@intel.com> wrote:
>
>> All ipsec related setting are being held in the driver
>> private data to allow easy add and remove of SAs. There
>> is no need to keep a record of the keys, and also
>> storing the keys can be a security issue.
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> Acked-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>> drivers/net/ixgbe/ixgbe_ipsec.c | 78 ++++++++++++++++++-----------------------
>> drivers/net/ixgbe/ixgbe_ipsec.h | 4 ---
>> 2 files changed, 35 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
>> index 105da11..a7ba358 100644
>> --- a/drivers/net/ixgbe/ixgbe_ipsec.c
>> +++ b/drivers/net/ixgbe/ixgbe_ipsec.c
>> @@ -70,6 +70,8 @@ static void
>> ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
>> {
>> struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>> + struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
>> + dev->data->dev_private);
>> int i = 0;
>>
>> /* clear Rx IP table*/
>> @@ -106,6 +108,10 @@ ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
>> IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
>> IXGBE_WAIT_TWRITE;
>> }
>> +
>> + memset(priv->rx_ip_tbl, 0, sizeof(priv->rx_ip_tbl));
>> + memset(priv->rx_sa_tbl, 0, sizeof(priv->rx_sa_tbl));
>> + memset(priv->tx_sa_tbl, 0, sizeof(priv->tx_sa_tbl));
> GCC has been known to optimize out this kind of memset.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=8537
>
Thanks for pointing it out, I will send an update.
@@ -70,6 +70,8 @@ static void
ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
{
struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct ixgbe_ipsec *priv = IXGBE_DEV_PRIVATE_TO_IPSEC(
+ dev->data->dev_private);
int i = 0;
/* clear Rx IP table*/
@@ -106,6 +108,10 @@ ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
IXGBE_WAIT_TWRITE;
}
+
+ memset(priv->rx_ip_tbl, 0, sizeof(priv->rx_ip_tbl));
+ memset(priv->rx_sa_tbl, 0, sizeof(priv->rx_sa_tbl));
+ memset(priv->tx_sa_tbl, 0, sizeof(priv->tx_sa_tbl));
}
static int
@@ -117,6 +123,8 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
dev->data->dev_private);
uint32_t reg_val;
int sa_index = -1;
+ uint32_t key[4];
+ uint32_t salt;
if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
int i, ip_index = -1;
@@ -173,16 +181,11 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
priv->rx_sa_tbl[sa_index].spi =
rte_cpu_to_be_32(ic_session->spi);
priv->rx_sa_tbl[sa_index].ip_index = ip_index;
- priv->rx_sa_tbl[sa_index].key[3] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
- priv->rx_sa_tbl[sa_index].key[2] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
- priv->rx_sa_tbl[sa_index].key[1] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
- priv->rx_sa_tbl[sa_index].key[0] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
- priv->rx_sa_tbl[sa_index].salt =
- rte_cpu_to_be_32(ic_session->salt);
+ key[3] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
+ key[2] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
+ key[1] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
+ key[0] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
+ salt = rte_cpu_to_be_32(ic_session->salt);
priv->rx_sa_tbl[sa_index].mode = IPSRXMOD_VALID;
if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION)
priv->rx_sa_tbl[sa_index].mode |=
@@ -224,19 +227,16 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
/* write Key table entry*/
reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
IPSRXIDX_TABLE_KEY | (sa_index << 3);
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0),
- priv->rx_sa_tbl[sa_index].key[0]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1),
- priv->rx_sa_tbl[sa_index].key[1]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2),
- priv->rx_sa_tbl[sa_index].key[2]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3),
- priv->rx_sa_tbl[sa_index].key[3]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT,
- priv->rx_sa_tbl[sa_index].salt);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0), key[0]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1), key[1]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2), key[2]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3), key[3]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, salt);
IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD,
priv->rx_sa_tbl[sa_index].mode);
IXGBE_WAIT_RWRITE;
+ memset(key, 0, sizeof(key));
+ salt = 0;
} else { /* sess->dir == RTE_CRYPTO_OUTBOUND */
int i;
@@ -257,32 +257,24 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
priv->tx_sa_tbl[sa_index].spi =
rte_cpu_to_be_32(ic_session->spi);
- priv->tx_sa_tbl[sa_index].key[3] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
- priv->tx_sa_tbl[sa_index].key[2] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
- priv->tx_sa_tbl[sa_index].key[1] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
- priv->tx_sa_tbl[sa_index].key[0] =
- rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
- priv->tx_sa_tbl[sa_index].salt =
- rte_cpu_to_be_32(ic_session->salt);
+ key[3] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
+ key[2] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
+ key[1] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
+ key[0] = rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
+ salt = rte_cpu_to_be_32(ic_session->salt);
+ priv->tx_sa_tbl[i].used = 1;
+ ic_session->sa_index = sa_index;
+ /* write Key table entry*/
reg_val = IPSRXIDX_RX_EN | IPSRXIDX_WRITE | (sa_index << 3);
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0),
- priv->tx_sa_tbl[sa_index].key[0]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1),
- priv->tx_sa_tbl[sa_index].key[1]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2),
- priv->tx_sa_tbl[sa_index].key[2]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3),
- priv->tx_sa_tbl[sa_index].key[3]);
- IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT,
- priv->tx_sa_tbl[sa_index].salt);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), key[0]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1), key[1]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2), key[2]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3), key[3]);
+ IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, salt);
IXGBE_WAIT_TWRITE;
-
- priv->tx_sa_tbl[i].used = 1;
- ic_session->sa_index = sa_index;
+ memset(key, 0, sizeof(key));
+ salt = 0;
}
return 0;
@@ -107,16 +107,12 @@ struct ixgbe_crypto_rx_ip_table {
struct ixgbe_crypto_rx_sa_table {
uint32_t spi;
uint32_t ip_index;
- uint32_t key[4];
- uint32_t salt;
uint8_t mode;
uint8_t used;
};
struct ixgbe_crypto_tx_sa_table {
uint32_t spi;
- uint32_t key[4];
- uint32_t salt;
uint8_t used;
};