[PATCH 0/5] crypto: use timing-safe digest comparison

[PATCH 0/5] crypto: use timing-safe digest comparison

[Stephen Hemminger]

Timing attacks in DPDK crypto were fixed earlier but
several drivers did not use the new timing safe comparison
operation.

First patch drops the experimental flag off rte_memeq_timingsafe().
The function is a static inline with no exported symbol, no ABI change.
This avoids having to turn on experimental flag in other drivers.

The rest convert the digest verify comparisons in the uadk, ccp,
armv8 and cnxk PMDs.

This problem was reported for several drivers and for those
the Reported-by was added.

Stephen Hemminger (5):
  eal: take experimental flag off of rte_memeq_timingsafe
  crypto/uadk: use timing-safe digest comparison
  crypto/ccp: use timing-safe digest comparison
  crypto/armv8: use timing-safe digest comparison
  crypto/cnxk: use timing-safe digest comparison

 doc/guides/rel_notes/release_26_07.rst | 4 ++++
 drivers/crypto/armv8/rte_armv8_pmd.c   | 4 ++--
 drivers/crypto/ccp/ccp_crypto.c        | 8 ++++----
 drivers/crypto/cnxk/cnxk_se.h          | 2 +-
 drivers/crypto/uadk/uadk_crypto_pmd.c  | 4 ++--
 lib/eal/include/rte_memory.h           | 4 ----
 6 files changed, 13 insertions(+), 13 deletions(-)

-- 
2.53.0

[PATCH 1/5] eal: take experimental flag off of rte_memeq_timingsafe

[Stephen Hemminger]

This function is needed in other places, and don't want to
have to propagate allow_experimental_api into those drivers.
It is stable enough and inline so no ABI exposure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 doc/guides/rel_notes/release_26_07.rst | 4 ++++
 lib/eal/include/rte_memory.h           | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 0b1cac3e0d..a9ca81905c 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -218,6 +218,10 @@ API Changes
   - ``rte_pmd_mlx5_enable_steering``
   - ``rte_pmd_mlx5_disable_steering``
 
+* **eal: promoted timing-safe memory comparison from experimental to stable.**
+
+  The inline function ``rte_memeq_timingsafe()`` is no longer marked experimental.
+
 
 ABI Changes
 -----------
diff --git a/lib/eal/include/rte_memory.h b/lib/eal/include/rte_memory.h
index b6e97ad695..940770f1eb 100644
--- a/lib/eal/include/rte_memory.h
+++ b/lib/eal/include/rte_memory.h
@@ -747,9 +747,6 @@ void
 rte_memzero_explicit(void *dst, size_t sz);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice.
- *
  * Timing-safe memory equality comparison.
  *
  * This function compares two memory regions in constant time,
@@ -770,7 +767,6 @@ rte_memzero_explicit(void *dst, size_t sz);
  * @return
  *   true if the memory regions are identical, false if they differ.
  */
-__rte_experimental
 static inline bool
 rte_memeq_timingsafe(const void *a, const void *b, size_t n)
 {
-- 
2.53.0

[PATCH 2/5] crypto/uadk: use timing-safe digest comparison

[Stephen Hemminger]

Digest verification used memcmp() to compare the computed and
expected MAC. memcmp() returns as soon as the first differing byte
is found, so its run time depends on how many leading bytes match.
An attacker submitting forged digests can use that timing signal to
recover the correct value one byte at a time.

Use rte_memeq_timingsafe(), whose run time depends only on the
length, for the verify comparison.

Bugzilla ID: 1773
Fixes: aba5b230ca04 ("crypto/uadk: use async mode")
Cc: stable@dpdk.org

Reported-by: Siraj Luthfi Ananda <sirajluthfi@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/uadk/uadk_crypto_pmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/uadk/uadk_crypto_pmd.c b/drivers/crypto/uadk/uadk_crypto_pmd.c
index 3c4e83e56f..221ad546da 100644
--- a/drivers/crypto/uadk/uadk_crypto_pmd.c
+++ b/drivers/crypto/uadk/uadk_crypto_pmd.c
@@ -1111,8 +1111,8 @@ uadk_crypto_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 		if (sess->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY) {
 			uint8_t *dst = qp->temp_digest[i % BURST_MAX];
 
-			if (memcmp(dst, op->sym->auth.digest.data,
-				   sess->auth.digest_length) != 0)
+			if (!rte_memeq_timingsafe(dst, op->sym->auth.digest.data,
+						  sess->auth.digest_length))
 				op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
 
-- 
2.53.0

[PATCH 3/5] crypto/ccp: use timing-safe digest comparison

[Stephen Hemminger]

Both the CPU HMAC verify path and the offload digest verify path
compared the computed and expected MAC with memcmp(), which short
circuits on the first mismatching byte and leaks the number of
matching leading bytes through timing.

Use rte_memeq_timingsafe() for both verify comparisons.

Bugzilla ID: 1773
Fixes: 6c561b03b54c ("crypto/ccp: support CPU based MD5 and SHA2 family")
Fixes: 70f0f8a8d78c ("crypto/ccp: support burst enqueue/dequeue")
Cc: stable@dpdk.org

Reported-by: Siraj Luthfi Ananda <sirajluthfi@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/ccp/ccp_crypto.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c
index 5899d83bae..b07a786d8e 100644
--- a/drivers/crypto/ccp/ccp_crypto.c
+++ b/drivers/crypto/ccp/ccp_crypto.c
@@ -1490,8 +1490,8 @@ static int cpu_crypto_auth(struct ccp_qp *qp,
 	}
 
 	if (sess->auth.op == CCP_AUTH_OP_VERIFY) {
-		if (memcmp(dst, op->sym->auth.digest.data,
-			   sess->auth.digest_length) != 0) {
+		if (!rte_memeq_timingsafe(dst, op->sym->auth.digest.data,
+					  sess->auth.digest_length)) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		} else {
 			op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
@@ -2801,8 +2801,8 @@ static inline void ccp_auth_dq_prepare(struct rte_crypto_op *op)
 
 	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 	if (session->auth.op == CCP_AUTH_OP_VERIFY) {
-		if (memcmp(addr + offset, digest_data,
-			   session->auth.digest_length) != 0)
+		if (!rte_memeq_timingsafe(addr + offset, digest_data,
+					  session->auth.digest_length))
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 
 	} else {
-- 
2.53.0

[PATCH 4/5] crypto/armv8: use timing-safe digest comparison

[Stephen Hemminger]

The chained-op verify path compared the computed and expected MAC
with memcmp(), whose run time depends on the number of matching
leading bytes and can leak the digest to an attacker submitting
forged values.

Use rte_memeq_timingsafe() for the verify comparison.

Bugzilla ID: 1773
Fixes: 169ca3db550c ("crypto/armv8: add PMD optimized for ARMv8 processors")
Cc: stable@dpdk.org

Reported-by: Siraj Luthfi Ananda <sirajluthfi@gmail.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/armv8/rte_armv8_pmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/armv8/rte_armv8_pmd.c b/drivers/crypto/armv8/rte_armv8_pmd.c
index 320e2d4b3b..a7caac186d 100644
--- a/drivers/crypto/armv8/rte_armv8_pmd.c
+++ b/drivers/crypto/armv8/rte_armv8_pmd.c
@@ -631,8 +631,8 @@ process_armv8_chained_op(struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
 
 	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 	if (sess->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY) {
-		if (memcmp(adst, op->sym->auth.digest.data,
-				sess->auth.digest_length) != 0) {
+		if (!rte_memeq_timingsafe(adst, op->sym->auth.digest.data,
+					  sess->auth.digest_length)) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
 	}
-- 
2.53.0

[PATCH 5/5] crypto/cnxk: use timing-safe digest comparison

[Stephen Hemminger]

compl_auth_verify() compared the generated and received MAC with
memcmp(), which returns early on the first differing byte and leaks
the number of matching leading bytes through timing.

Use rte_memeq_timingsafe() for the verify comparison.

Bugzilla ID: 1773
Fixes: 786963fdcf3e ("crypto/cnxk: add digest support")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/crypto/cnxk/cnxk_se.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/cnxk/cnxk_se.h b/drivers/crypto/cnxk/cnxk_se.h
index 8dbf3e73c7..d2306a9daf 100644
--- a/drivers/crypto/cnxk/cnxk_se.h
+++ b/drivers/crypto/cnxk/cnxk_se.h
@@ -3282,7 +3282,7 @@ compl_auth_verify(struct rte_crypto_op *op, uint8_t *gen_mac, uint64_t mac_len)
 		return;
 	}
 
-	if (memcmp(mac, gen_mac, mac_len))
+	if (!rte_memeq_timingsafe(mac, gen_mac, mac_len))
 		op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 	else
 		op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
-- 
2.53.0