[dpdk-stable] [PATCH] crypto/openssl: fix out-of-place encryption

[dpdk-stable] [PATCH] crypto/openssl: fix out-of-place encryption

[Pablo de Lara]

When authenticating after encrypting, if the operation
is out-of-place, the destination buffer is the one
that will get authenticated.
If the cipher offset is higher than the authentication
offset, it means that part of the text to authenticate
will be plaintext, so this needs to get copied to the
destination buffer, or the result will be incorrect.

Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
Cc: stable@dpdk.org

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/openssl/rte_openssl_pmd.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 199097b..c295dd2 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -2037,6 +2037,26 @@ process_asym_op(struct openssl_qp *qp, struct rte_crypto_op *op,
 	return retval;
 }
 
+static void
+copy_plaintext(struct rte_mbuf *m_src, struct rte_mbuf *m_dst,
+		struct rte_crypto_op *op)
+{
+	uint8_t *p_src, *p_dst;
+
+	p_src = rte_pktmbuf_mtod(m_src, uint8_t *);
+	p_dst = rte_pktmbuf_mtod(m_dst, uint8_t *);
+
+	/**
+	 * Copy the content between cipher offset and auth offset
+	 * for generating correct digest.
+	 */
+	if (op->sym->cipher.data.offset > op->sym->auth.data.offset)
+		memcpy(p_dst + op->sym->auth.data.offset,
+				p_src + op->sym->auth.data.offset,
+				op->sym->cipher.data.offset -
+				op->sym->auth.data.offset);
+}
+
 /** Process crypto operation for mbuf */
 static int
 process_op(struct openssl_qp *qp, struct rte_crypto_op *op,
@@ -2059,6 +2079,9 @@ process_op(struct openssl_qp *qp, struct rte_crypto_op *op,
 		break;
 	case OPENSSL_CHAIN_CIPHER_AUTH:
 		process_openssl_cipher_op(op, sess, msrc, mdst);
+		/* OOP */
+		if (msrc != mdst)
+			copy_plaintext(msrc, mdst, op);
 		process_openssl_auth_op(qp, op, sess, mdst, mdst);
 		break;
 	case OPENSSL_CHAIN_AUTH_CIPHER:
-- 
2.7.5

Re: [dpdk-stable] [dpdk-dev] [PATCH] crypto/openssl: fix out-of-place encryption

[Akhil Goyal]

Hi Pablo,

> When authenticating after encrypting, if the operation
> is out-of-place, the destination buffer is the one
> that will get authenticated.
> If the cipher offset is higher than the authentication
> offset, it means that part of the text to authenticate
> will be plaintext, so this needs to get copied to the
> destination buffer, or the result will be incorrect.
> 
> Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL library")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Do we have a test case in app/test for this case? If not, we should probably add it.

Re: [dpdk-stable] [dpdk-dev] [PATCH] crypto/openssl: fix out-of-place encryption

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Akhil Goyal
> Sent: Wednesday, April 15, 2020 8:23 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org;
> akhil.goyal@intel.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] crypto/openssl: fix out-of-place encryption
> 
> Hi Pablo,
> 
> > When authenticating after encrypting, if the operation is
> > out-of-place, the destination buffer is the one that will get
> > authenticated.
> > If the cipher offset is higher than the authentication offset, it
> > means that part of the text to authenticate will be plaintext, so this
> > needs to get copied to the destination buffer, or the result will be
> > incorrect.
> >
> > Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL
> > library")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Do we have a test case in app/test for this case? If not, we should probably add
> it.

Hi Akhil,

Yes, we do. In test_cryptodev_aes_test_vectors.h, the "AES-128-CBC HMAC-SHA1 Encryption Digest OOP Offset"
is testing this case, in aes_chain_test_cases[].

Thanks,
Pablo

Re: [dpdk-stable] [dpdk-dev] [PATCH] crypto/openssl: fix out-of-place encryption

[Akhil Goyal]

> 
> 
> > Hi Pablo,
> >
> > > When authenticating after encrypting, if the operation is
> > > out-of-place, the destination buffer is the one that will get
> > > authenticated.
> > > If the cipher offset is higher than the authentication offset, it
> > > means that part of the text to authenticate will be plaintext, so this
> > > needs to get copied to the destination buffer, or the result will be
> > > incorrect.
> > >
> > > Fixes: d61f70b4c918 ("crypto/libcrypto: add driver for OpenSSL
> > > library")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >
> > Do we have a test case in app/test for this case? If not, we should probably
> add
> > it.
> 
> Hi Akhil,
> 
> Yes, we do. In test_cryptodev_aes_test_vectors.h, the "AES-128-CBC HMAC-
> SHA1 Encryption Digest OOP Offset"
> is testing this case, in aes_chain_test_cases[].
> 

Acked-by: Akhil Goyal <akhil.goyal@nxp.com>

Applied to dpdk-next-crypto

Thanks.