[RFC PATCH 0/3] add TLS record processing security offload

[RFC PATCH 0/3] add TLS record processing security offload

[Anoob Joseph]

Add Transport Layer Security (TLS) and Datagram Transport Layer Security
(DTLS). The protocols provide communications privacy for L4 protocols
such as TCP & UDP.

TLS (and DTLS) protocol is composed of two layers,
1. TLS Record Protocol
2. TLS Handshake Protocol

While TLS Handshake Protocol helps in establishing security parameters
by which client and server can communicate, TLS Record Protocol provides
the connection security. TLS Record Protocol leverages symmetric
cryptographic operations such as data encryption and authentication for
providing security to the communications.

Cryptodevs that are capable of offloading TLS Record Protocol may
perform other operations like IV generation, header insertion, atomic
sequence number updates and anti-replay window check in addition to
cryptographic transformations.

In record write operations, message content type is a per packet field
which is used in constructing the TLS header. One session is expected
to handle all types of content types and so, 'rte_crypto_op.aux_flags'
is used for passing the same.

The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.

Akhil Goyal (1):
  net: add headers for TLS/DTLS packets

Anoob Joseph (2):
  security: add TLS record processing
  cryptodev: add details of datapath handling of TLS records

 doc/api/doxy-api-index.md              |   2 +
 doc/guides/prog_guide/rte_security.rst |  68 +++++++++++++++
 lib/cryptodev/rte_crypto.h             |   6 ++
 lib/net/meson.build                    |   2 +
 lib/net/rte_dtls.h                     |  61 ++++++++++++++
 lib/net/rte_tls.h                      |  48 +++++++++++
 lib/security/rte_security.c            |   4 +
 lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
 8 files changed, 301 insertions(+)
 create mode 100644 lib/net/rte_dtls.h
 create mode 100644 lib/net/rte_tls.h

-- 
2.25.1

[RFC PATCH 1/3] net: add headers for TLS/DTLS packets

[Anoob Joseph]

From: Akhil Goyal <gakhil@marvell.com>

Added TLS and DTLS packet headers for L4 security applications.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/api/doxy-api-index.md |  2 ++
 lib/net/meson.build       |  2 ++
 lib/net/rte_dtls.h        | 61 +++++++++++++++++++++++++++++++++++++++
 lib/net/rte_tls.h         | 48 ++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 lib/net/rte_dtls.h
 create mode 100644 lib/net/rte_tls.h

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index fdeda13932..03e2445bb1 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -128,6 +128,8 @@ The public API headers are grouped by topics:
   [eCPRI](@ref rte_ecpri.h),
   [PDCP hdr](@ref rte_pdcp_hdr.h),
   [PDCP](@ref rte_pdcp.h),
+  [TLS](@ref rte_tls.h),
+  [DTLS](@ref rte_dtls.h),
   [L2TPv2](@ref rte_l2tpv2.h),
   [PPP](@ref rte_ppp.h),
   [IB](@ref rte_ib.h)
diff --git a/lib/net/meson.build b/lib/net/meson.build
index b1bc27bad5..0b69138949 100644
--- a/lib/net/meson.build
+++ b/lib/net/meson.build
@@ -5,6 +5,8 @@ headers = files(
         'rte_ip.h',
         'rte_tcp.h',
         'rte_udp.h',
+        'rte_tls.h',
+        'rte_dtls.h',
         'rte_esp.h',
         'rte_sctp.h',
         'rte_icmp.h',
diff --git a/lib/net/rte_dtls.h b/lib/net/rte_dtls.h
new file mode 100644
index 0000000000..1455c07a92
--- /dev/null
+++ b/lib/net/rte_dtls.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Marvell.
+ */
+
+#ifndef RTE_DTLS_H
+#define RTE_DTLS_H
+
+/**
+ * @file
+ *
+ * Datagram transport layer security(DTLS) related defines.
+ */
+
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define RTE_DTLS_TYPE_INVALID	0 /**< Invalid DTLS message type. */
+#define RTE_DTLS_TYPE_CCS	20 /**< Change cipher message. */
+#define RTE_DTLS_TYPE_ALERT	21 /**< Alert message. */
+#define RTE_DTLS_TYPE_HANDSHAKE	22 /**< Handshake message for DTLS. */
+#define RTE_DTLS_TYPE_APPDATA	23 /**< DTLS application data message. */
+#define RTE_DTLS_TYPE_HEARTBEAT	24 /**< DTLS 1.3 heartbeat message. */
+#define RTE_DTLS_TYPE_CIPHERTEXT_WITH_CID	25 /**< DTLS 1.3 ciphertext with CID message. */
+#define RTE_DTLS_TYPE_ACK	26 /**< DTLS 1.3 ACK message. */
+#define RTE_DTLS_TYPE_MAX	255 /**< Maximum value as DTLS content type. */
+
+#define RTE_DTLS_VERSION_1_2	0xFEFD /**< DTLS 1.2 version. 1's complement of 1.2. */
+#define RTE_DTLS_VERSION_1_3	0xFEFC /**< DTLS 1.3 version. 1's complement of 1.3. */
+
+/**
+ * DTLS Header
+ */
+__extension__
+struct rte_dtls_hdr {
+	/** Content type of DTLS packet. Defined as RTE_DTLS_TYPE_*. */
+	uint8_t type;
+	/** DTLS Version defined as RTE_DTLS_VERSION*. */
+	rte_be16_t version;
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	/** The sequence number for the DTLS record. */
+	uint64_t sequence_number : 48;
+	/** A counter value that is incremented on every cipher state change. */
+	uint64_t epoch : 16;
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	/** A counter value that is incremented on every cipher state change. */
+	uint64_t epoch : 16;
+	/** The sequence number for the DTLS record. */
+	uint64_t sequence_number : 48;
+#endif
+	/** The length (in bytes) of the following DTLS packet. */
+	rte_be16_t length;
+} __rte_packed;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_DTLS_H */
diff --git a/lib/net/rte_tls.h b/lib/net/rte_tls.h
new file mode 100644
index 0000000000..d708d06014
--- /dev/null
+++ b/lib/net/rte_tls.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Marvell.
+ */
+
+#ifndef RTE_TLS_H
+#define RTE_TLS_H
+
+/**
+ * @file
+ *
+ * Transport layer security(TLS) related defines.
+ */
+
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define RTE_TLS_TYPE_INVALID	0 /**< Invalid TLS message type. */
+#define RTE_TLS_TYPE_CCS	20 /**< Change cipher message. */
+#define RTE_TLS_TYPE_ALERT	21 /**< Alert message. */
+#define RTE_TLS_TYPE_HANDSHAKE	22 /**< Handshake message for TLS. */
+#define RTE_TLS_TYPE_APPDATA	23 /**< TLS application data message. */
+#define RTE_TLS_TYPE_HEARTBEAT	24 /**< TLS 1.3 heartbeat message. */
+#define RTE_TLS_TYPE_MAX	255 /**< Maximum value as TLS content type. */
+
+#define RTE_TLS_VERSION_1_2	0x0303 /**< TLS 1.2 version. */
+#define RTE_TLS_VERSION_1_3	0x0304 /**< TLS 1.3 version. */
+
+/**
+ * TLS Header
+ */
+__extension__
+struct rte_tls_hdr {
+	/** Content type of TLS packet. Defined as RTE_TLS_TYPE_*. */
+	uint8_t type;
+	/** TLS Version defined as RTE_TLS_VERSION*. */
+	rte_be16_t version;
+	/** The length (in bytes) of the following TLS packet. */
+	rte_be16_t length;
+} __rte_packed;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_TLS_H */
-- 
2.25.1

[RFC PATCH 2/3] security: add TLS record processing

[Anoob Joseph]

Add Transport Layer Security (TLS) and Datagram Transport Layer Security
(DTLS). The protocols provide communications privacy for L4 protocols
such as TCP & UDP.

TLS (and DTLS) protocol is composed of two layers,
1. TLS Record Protocol
2. TLS Handshake Protocol

While TLS Handshake Protocol helps in establishing security parameters
by which client and server can communicate, TLS Record Protocol provides
the connection security. TLS Record Protocol leverages symmetric
cryptographic operations such as data encryption and authentication for
providing security to the communications.

Cryptodevs that are capable of offloading TLS Record Protocol may
perform other operations like IV generation, header insertion, atomic
sequence number updates and anti-replay window check in addition to
cryptographic transformations.

The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
 lib/security/rte_security.c            |   4 +
 lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
 3 files changed, 172 insertions(+)

diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
index 7418e35c1b..7716d7239f 100644
--- a/doc/guides/prog_guide/rte_security.rst
+++ b/doc/guides/prog_guide/rte_security.rst
@@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create`` returns a handle for SC,
 and this handle is set in ``rte_security_macsec_xform``
 to create a MACsec session using ``rte_security_session_create``.
 
+TLS-Record Protocol
+~~~~~~~~~~~~~~~~~~~
+
+The Transport Layer Protocol provides communications security over the Internet. The protocol
+allows client/server applications to communicate in a way that is designed to prevent eavesdropping,
+tampering, or message forgery.
+
+TLS protocol is composed of two layers: the TLS Record Protocol and the TLS Handshake Protocol. At
+the lowest level, layered on top of some reliable transport protocol (e.g., TCP), is the TLS Record
+Protocol. The TLS Record Protocol provides connection security that has two basic properties:
+
+   -  The connection is private.  Symmetric cryptography is used for data
+      encryption (e.g., AES, DES, etc.).  The keys for this symmetric encryption
+      are generated uniquely for each connection and are based on a secret
+      negotiated by another protocol (such as the TLS Handshake Protocol). The
+      Record Protocol can also be used without encryption.
+
+   -  The connection is reliable.  Message transport includes a message
+      integrity check using a keyed MAC.  Secure hash functions (e.g.,
+      SHA-1, etc.) are used for MAC computations.  The Record Protocol
+      can operate without a MAC, but is generally only used in this mode
+      while another protocol is using the Record Protocol as a transport
+      for negotiating security parameters.
+
+.. code-block:: c
+
+             Record Write                   Record Read
+             ------------                   -----------
+
+             TLSPlaintext                  TLSCiphertext
+                  |                              |
+                  ~                              ~
+                  |                              |
+                  V                              V
+        +---------|----------+        +----------|---------+
+        | Seq. no generation |        | Seq. no generation |
+        +---------|----------+        +----------|---------+
+                  |                              |
+        +---------|----------+        +----------|---------+
+        |  Header insertion  |        |    Decryption &    |
+        +---------|----------+        |  MAC verification  |
+                  |                   +----------|---------+
+        +---------|----------+                   |
+        |  MAC generation &  |        +----------|---------+
+        |     Encryption     |        | TLS Header removal |
+        +---------|----------+        +----------|---------+
+                  |                              |
+                  ~                              ~
+                  |                              |
+                  V                              V
+            TLSCiphertext                  TLSPlaintext
+
+Supported Versions
+^^^^^^^^^^^^^^^^^^
+
+* TLS 1.2
+* TLS 1.3
+* DTLS 1.2
 
 Device Features and Capabilities
 ---------------------------------
diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index c4d64bb8e9..bd7b026547 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -282,6 +282,10 @@ rte_security_capability_get(struct rte_security_ctx *instance,
 				if (capability->docsis.direction ==
 							idx->docsis.direction)
 					return capability;
+			} else if (idx->protocol == RTE_SECURITY_PROTOCOL_TLS_RECORD) {
+				if (capability->tls_record.ver == idx->tls_record.ver &&
+				    capability->tls_record.type == idx->tls_record.type)
+					return capability;
 			}
 		}
 	}
diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 3b2df526ba..b9d064ed84 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
 	/**< DOCSIS direction */
 };
 
+/** Salt len to be used with AEAD algos in TLS 1.2 */
+#define RTE_SECURITY_TLS_1_2_SALT_LEN 4
+/** Salt len to be used with AEAD algos in TLS 1.3 */
+#define RTE_SECURITY_TLS_1_3_SALT_LEN 12
+/** Salt len to be used with AEAD algos in DTLS 1.2 */
+#define RTE_SECURITY_DTLS_1_2_SALT_LEN 4
+
+/** TLS version */
+enum rte_security_tls_version {
+	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
+	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
+	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
+};
+
+/** TLS session type */
+enum rte_security_tls_sess_type {
+	/** Record read session
+	 * - Decrypt & digest verification.
+	 */
+	RTE_SECURITY_TLS_SESS_TYPE_READ,
+	/** Record write session
+	 * - Encrypt & digest generation.
+	 */
+	RTE_SECURITY_TLS_SESS_TYPE_WRITE,
+};
+
+/**
+ * Configure soft and hard lifetime of a TLS record session
+ *
+ * Lifetime of a TLS record session would specify the maximum number of packets that can be
+ * processed. TLS record processing operations would start failing once hard limit is reached.
+ *
+ * Soft limits can be specified to generate notification when the TLS record session is approaching
+ * hard limits for lifetime. This would result in a warning returned in ``rte_crypto_op.aux_flags``.
+ */
+struct rte_security_tls_record_lifetime {
+	/** Soft expiry limit in number of packets */
+	uint64_t packets_soft_limit;
+	/** Hard expiry limit in number of packets */
+	uint64_t packets_hard_limit;
+};
+
+/**
+ * TLS record protocol session configuration.
+ *
+ * This structure contains data required to create a TLS record security session.
+ */
+struct rte_security_tls_record_xform {
+	/** TLS record version. */
+	enum rte_security_tls_version ver;
+	/** TLS record session type. */
+	enum rte_security_tls_sess_type type;
+	/** TLS record session lifetime. */
+	struct rte_security_tls_record_lifetime life;
+	union {
+		/** TLS 1.2 parameters. */
+		struct {
+			/** Starting sequence number. */
+			uint64_t seq_no;
+			/** Salt to be used for AEAD algos. */
+			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
+		} tls_1_2;
+
+		/** TLS 1.3 parameters. */
+		struct {
+			/** Starting sequence number. */
+			uint64_t seq_no;
+			/** Salt to be used for AEAD algos. */
+			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
+			/**
+			 * Minimum payload length (in case of write sessions). For shorter inputs,
+			 * the payload would be padded appropriately before performing crypto
+			 * transformations.
+			 */
+			uint32_t min_payload_len;
+		} tls_1_3;
+
+		/** DTLS 1.2 parameters */
+		struct {
+			/** Epoch value to be used. */
+			uint16_t epoch;
+			/** 6B starting sequence number to be used. */
+			uint64_t seq_no;
+			/** Salt to be used for AEAD algos. */
+			uint8_t salt[RTE_SECURITY_DTLS_1_2_SALT_LEN];
+			/** Anti replay window size to enable sequence replay attack handling.
+			 * Anti replay check is disabled if the window size is 0.
+			 */
+			uint32_t ar_win_sz;
+		} dtls_1_2;
+	};
+};
+
 /**
  * Security session action type.
  */
@@ -654,6 +747,8 @@ enum rte_security_session_protocol {
 	/**< PDCP Protocol */
 	RTE_SECURITY_PROTOCOL_DOCSIS,
 	/**< DOCSIS Protocol */
+	RTE_SECURITY_PROTOCOL_TLS_RECORD,
+	/**< TLS Record Protocol */
 };
 
 /**
@@ -670,6 +765,7 @@ struct rte_security_session_conf {
 		struct rte_security_macsec_xform macsec;
 		struct rte_security_pdcp_xform pdcp;
 		struct rte_security_docsis_xform docsis;
+		struct rte_security_tls_record_xform tls;
 	};
 	/**< Configuration parameters for security session */
 	struct rte_crypto_sym_xform *crypto_xform;
@@ -1190,6 +1286,16 @@ struct rte_security_capability {
 			/**< DOCSIS direction */
 		} docsis;
 		/**< DOCSIS capability */
+		struct {
+			enum rte_security_tls_version ver;
+			/**< TLS record version. */
+			enum rte_security_tls_sess_type type;
+			/**< TLS record session type. */
+			uint32_t ar_win_size;
+			/**< Maximum anti replay window size supported for DTLS 1.2 record read
+			 * operation. Value of 0 means anti replay check is not supported.
+			 */
+		} tls_record;
 	};
 
 	const struct rte_cryptodev_capabilities *crypto_capabilities;
@@ -1251,6 +1357,10 @@ struct rte_security_capability_idx {
 		struct {
 			enum rte_security_docsis_direction direction;
 		} docsis;
+		struct {
+			enum rte_security_tls_version ver;
+			enum rte_security_tls_sess_type type;
+		} tls_record;
 	};
 };
 
-- 
2.25.1

[RFC PATCH 3/3] cryptodev: add details of datapath handling of TLS records

[Anoob Joseph]

TLS/DTLS record processing requires content type to be provided per
packet (for record write operation). Extend usage of
rte_crypto_op.aux_flags for the same purpose.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/guides/prog_guide/rte_security.rst | 10 ++++++++++
 lib/cryptodev/rte_crypto.h             |  6 ++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
index 7716d7239f..6cb69bc949 100644
--- a/doc/guides/prog_guide/rte_security.rst
+++ b/doc/guides/prog_guide/rte_security.rst
@@ -451,6 +451,16 @@ Protocol. The TLS Record Protocol provides connection security that has two basi
                   V                              V
             TLSCiphertext                  TLSPlaintext
 
+TLS and DTLS header formation (in record write operation) would depend on the
+type of content. It is a per packet variable and would need to be handled by
+the same session. Application may pass this info to a cryptodev performing
+lookaside protocol offload by passing the same in ``rte_crypto_op.aux_flags``.
+
+In record read operation, application is required to preserve any info it may
+need from the TLS/DTLS header (such as content type and sequence number) as the
+cryptodev would remove the header and padding as part of the lookaside protocol
+processing.
+
 Supported Versions
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index 9b8d0331a4..7c12a2b705 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -101,6 +101,12 @@ struct rte_crypto_op {
 			/**< Operation specific auxiliary/additional flags.
 			 * These flags carry additional information from the
 			 * operation. Processing of the same is optional.
+			 *
+			 * With TLS record offload (RTE_SECURITY_PROTOCOL_TLS_RECORD),
+			 * application would be required to provide the message
+			 * type of the input provided. The 'aux_flags' field
+			 * can be used for passing the same. Message types are
+			 * listed as RTE_TLS_TYPE_* and RTE_DTLS_TYPE_*.
 			 */
 			uint8_t reserved[2];
 			/**< Reserved bytes to fill 64 bits for
-- 
2.25.1

RE: [RFC PATCH 0/3] add TLS record processing security offload

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 8:17 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [RFC PATCH 0/3] add TLS record processing security offload
> 
> Add Transport Layer Security (TLS) and Datagram Transport Layer Security
> (DTLS). The protocols provide communications privacy for L4 protocols
> such as TCP & UDP.
> 
> TLS (and DTLS) protocol is composed of two layers,
> 1. TLS Record Protocol
> 2. TLS Handshake Protocol
> 
> While TLS Handshake Protocol helps in establishing security parameters
> by which client and server can communicate, TLS Record Protocol provides
> the connection security. TLS Record Protocol leverages symmetric
> cryptographic operations such as data encryption and authentication for
> providing security to the communications.
> 
> Cryptodevs that are capable of offloading TLS Record Protocol may
> perform other operations like IV generation, header insertion, atomic
> sequence number updates and anti-replay window check in addition to
> cryptographic transformations.
> 
> In record write operations, message content type is a per packet field
> which is used in constructing the TLS header. One session is expected
> to handle all types of content types and so, 'rte_crypto_op.aux_flags'
> is used for passing the same.
>
> The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
> 
> Akhil Goyal (1):
>   net: add headers for TLS/DTLS packets
> 
> Anoob Joseph (2):
>   security: add TLS record processing
>   cryptodev: add details of datapath handling of TLS records

Hi Folks,

I've reviewed these 3 patches, generally fine, with two main opens;

1) The part that I do not fully understand how it is defined is the
'rte_crypto_op.aux_flags' field usage, and what values to read/write there.

2) Error handling (again with aux_flags) is not well defined, and is critical
to correct (high-bw/high-packet-count) usage. I do not understand how to
do correct error handling today with aux_flags, so more docs/examples required.

Some detail-level comments inline in the patch files.

Regards -Harry

<snip>

RE: [RFC PATCH 1/3] net: add headers for TLS/DTLS packets

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 8:17 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [RFC PATCH 1/3] net: add headers for TLS/DTLS packets
> 
> From: Akhil Goyal <gakhil@marvell.com>
> 
> Added TLS and DTLS packet headers for L4 security applications.
> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> ---
>  doc/api/doxy-api-index.md |  2 ++
>  lib/net/meson.build       |  2 ++
>  lib/net/rte_dtls.h        | 61 +++++++++++++++++++++++++++++++++++++++
>  lib/net/rte_tls.h         | 48 ++++++++++++++++++++++++++++++
>  4 files changed, 113 insertions(+)
>  create mode 100644 lib/net/rte_dtls.h
>  create mode 100644 lib/net/rte_tls.h
> 
> diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
> index fdeda13932..03e2445bb1 100644
> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -128,6 +128,8 @@ The public API headers are grouped by topics:
>    [eCPRI](@ref rte_ecpri.h),
>    [PDCP hdr](@ref rte_pdcp_hdr.h),
>    [PDCP](@ref rte_pdcp.h),
> +  [TLS](@ref rte_tls.h),
> +  [DTLS](@ref rte_dtls.h),
>    [L2TPv2](@ref rte_l2tpv2.h),
>    [PPP](@ref rte_ppp.h),
>    [IB](@ref rte_ib.h)
> diff --git a/lib/net/meson.build b/lib/net/meson.build
> index b1bc27bad5..0b69138949 100644
> --- a/lib/net/meson.build
> +++ b/lib/net/meson.build
> @@ -5,6 +5,8 @@ headers = files(
>          'rte_ip.h',
>          'rte_tcp.h',
>          'rte_udp.h',
> +        'rte_tls.h',
> +        'rte_dtls.h',
>          'rte_esp.h',
>          'rte_sctp.h',
>          'rte_icmp.h',
> diff --git a/lib/net/rte_dtls.h b/lib/net/rte_dtls.h
> new file mode 100644
> index 0000000000..1455c07a92
> --- /dev/null
> +++ b/lib/net/rte_dtls.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Marvell.
> + */
> +
> +#ifndef RTE_DTLS_H
> +#define RTE_DTLS_H
> +
> +/**
> + * @file
> + *
> + * Datagram transport layer security(DTLS) related defines.
> + */
> +
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define RTE_DTLS_TYPE_INVALID	0 /**< Invalid DTLS message type. */
> +#define RTE_DTLS_TYPE_CCS	20 /**< Change cipher message. */

I don't see the "CCS" acronym in the RFC, suggest to make more accurate/verbose;
- Rename to RTE_DTLS_TYPE_CHANGE_CIPHER (or RTE_DTLS_TYPE_CHANGE_CIPHER_SPEC if preferred)
- Reword description to "change cipher" to "change cipher spec message."


> +#define RTE_DTLS_TYPE_ALERT	21 /**< Alert message. */
> +#define RTE_DTLS_TYPE_HANDSHAKE	22 /**< Handshake message for
> DTLS. */
> +#define RTE_DTLS_TYPE_APPDATA	23 /**< DTLS application data message.
> */
> +#define RTE_DTLS_TYPE_HEARTBEAT	24 /**< DTLS 1.3 heartbeat message. */
> +#define RTE_DTLS_TYPE_CIPHERTEXT_WITH_CID	25 /**< DTLS 1.3
> ciphertext with CID message. */
> +#define RTE_DTLS_TYPE_ACK	26 /**< DTLS 1.3 ACK message. */
> +#define RTE_DTLS_TYPE_MAX	255 /**< Maximum value as DTLS
> content type. */
> +
> +#define RTE_DTLS_VERSION_1_2	0xFEFD /**< DTLS 1.2 version. 1's
> complement of 1.2. */
> +#define RTE_DTLS_VERSION_1_3	0xFEFC /**< DTLS 1.3 version. 1's
> complement of 1.3. */
>
> +
> +/**
> + * DTLS Header
> + */
> +__extension__
> +struct rte_dtls_hdr {
> +	/** Content type of DTLS packet. Defined as RTE_DTLS_TYPE_*. */
> +	uint8_t type;
> +	/** DTLS Version defined as RTE_DTLS_VERSION*. */
> +	rte_be16_t version;

(same comment on be16_t vs struct as in TLS version below, no rework needed)

> +#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
> +	/** The sequence number for the DTLS record. */
> +	uint64_t sequence_number : 48;
> +	/** A counter value that is incremented on every cipher state change.
> */
> +	uint64_t epoch : 16;
> +#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
> +	/** A counter value that is incremented on every cipher state change.
> */
> +	uint64_t epoch : 16;
> +	/** The sequence number for the DTLS record. */
> +	uint64_t sequence_number : 48;
> +#endif
> +	/** The length (in bytes) of the following DTLS packet. */
> +	rte_be16_t length;
> +} __rte_packed;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_DTLS_H */
> diff --git a/lib/net/rte_tls.h b/lib/net/rte_tls.h
> new file mode 100644
> index 0000000000..d708d06014
> --- /dev/null
> +++ b/lib/net/rte_tls.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2023 Marvell.
> + */
> +
> +#ifndef RTE_TLS_H
> +#define RTE_TLS_H
> +
> +/**
> + * @file
> + *
> + * Transport layer security(TLS) related defines.
> + */
> +
> +#include <rte_byteorder.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#define RTE_TLS_TYPE_INVALID	0 /**< Invalid TLS message type. */
> +#define RTE_TLS_TYPE_CCS	20 /**< Change cipher message. */
> +#define RTE_TLS_TYPE_ALERT	21 /**< Alert message. */
> +#define RTE_TLS_TYPE_HANDSHAKE	22 /**< Handshake message for TLS. */
> +#define RTE_TLS_TYPE_APPDATA	23 /**< TLS application data message. */
> +#define RTE_TLS_TYPE_HEARTBEAT	24 /**< TLS 1.3 heartbeat message. */
> +#define RTE_TLS_TYPE_MAX	255 /**< Maximum value as TLS content type.
> */
> +
> +#define RTE_TLS_VERSION_1_2	0x0303 /**< TLS 1.2 version. */
> +#define RTE_TLS_VERSION_1_3	0x0304 /**< TLS 1.3 version. */
> +
> +/**
> + * TLS Header
> + */
> +__extension__
> +struct rte_tls_hdr {
> +	/** Content type of TLS packet. Defined as RTE_TLS_TYPE_*. */
> +	uint8_t type;
> +	/** TLS Version defined as RTE_TLS_VERSION*. */
> +	rte_be16_t version;

In the RFC, version is defined as "struct ProtocolVersion" with two uint8's?
https://www.rfc-editor.org/rfc/rfc5246.html#appendix-A.1
With correct endianness handling this is the same, but the struct with is simpler  to RFC,
while rte_be16_t is easier to ensure single load/store at code level.
No need to change, just pointing out the different impl (but same effect) as RFC.

> +	/** The length (in bytes) of the following TLS packet. */
> +	rte_be16_t length;
> +} __rte_packed;
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* RTE_TLS_H */
> --
> 2.25.1

RE: [RFC PATCH 2/3] security: add TLS record processing

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 8:17 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [RFC PATCH 2/3] security: add TLS record processing
> 
> Add Transport Layer Security (TLS) and Datagram Transport Layer Security
> (DTLS). The protocols provide communications privacy for L4 protocols
> such as TCP & UDP.
> 
> TLS (and DTLS) protocol is composed of two layers,
> 1. TLS Record Protocol
> 2. TLS Handshake Protocol
> 
> While TLS Handshake Protocol helps in establishing security parameters
> by which client and server can communicate, TLS Record Protocol provides
> the connection security. TLS Record Protocol leverages symmetric
> cryptographic operations such as data encryption and authentication for
> providing security to the communications.
> 
> Cryptodevs that are capable of offloading TLS Record Protocol may
> perform other operations like IV generation, header insertion, atomic
> sequence number updates and anti-replay window check in addition to
> cryptographic transformations.
> 
> The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.

From the code below, my understanding is that *ONLY* the record layer is being
added/supported? The difference is described well above, but the intended
support added is not clearly defined.

Suggest reword the last line to clarify:
"Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and DTLS 1.2."

 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> ---
>  doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
>  lib/security/rte_security.c            |   4 +
>  lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
>  3 files changed, 172 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_security.rst
> b/doc/guides/prog_guide/rte_security.rst
> index 7418e35c1b..7716d7239f 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create`` returns a
> handle for SC,
>  and this handle is set in ``rte_security_macsec_xform``
>  to create a MACsec session using ``rte_security_session_create``.
> 
> +TLS-Record Protocol
> +~~~~~~~~~~~~~~~~~~~
> +
> +The Transport Layer Protocol provides communications security over the
> Internet. The protocol
> +allows client/server applications to communicate in a way that is designed to
> prevent eavesdropping,
> +tampering, or message forgery.
> +
> +TLS protocol is composed of two layers: the TLS Record Protocol and the TLS
> Handshake Protocol. At
> +the lowest level, layered on top of some reliable transport protocol (e.g., TCP),
> is the TLS Record
> +Protocol. The TLS Record Protocol provides connection security that has two
> basic properties:
> +
> +   -  The connection is private.  Symmetric cryptography is used for data
> +      encryption (e.g., AES, DES, etc.).  The keys for this symmetric encryption
> +      are generated uniquely for each connection and are based on a secret
> +      negotiated by another protocol (such as the TLS Handshake Protocol). The
> +      Record Protocol can also be used without encryption.
> +
> +   -  The connection is reliable.  Message transport includes a message
> +      integrity check using a keyed MAC.  Secure hash functions (e.g.,
> +      SHA-1, etc.) are used for MAC computations.  The Record Protocol
> +      can operate without a MAC, but is generally only used in this mode
> +      while another protocol is using the Record Protocol as a transport
> +      for negotiating security parameters.
> +
> +.. code-block:: c

The code block below isn't C? Is there a better code block type for a text diagram?

> +             Record Write                   Record Read
> +             ------------                   -----------
> +
> +             TLSPlaintext                  TLSCiphertext
> +                  |                              |
> +                  ~                              ~
> +                  |                              |
> +                  V                              V
> +        +---------|----------+        +----------|---------+
> +        | Seq. no generation |        | Seq. no generation |
> +        +---------|----------+        +----------|---------+
> +                  |                              |
> +        +---------|----------+        +----------|---------+
> +        |  Header insertion  |        |    Decryption &    |
> +        +---------|----------+        |  MAC verification  |
> +                  |                   +----------|---------+
> +        +---------|----------+                   |
> +        |  MAC generation &  |        +----------|---------+
> +        |     Encryption     |        | TLS Header removal |
> +        +---------|----------+        +----------|---------+
> +                  |                              |
> +                  ~                              ~
> +                  |                              |
> +                  V                              V
> +            TLSCiphertext                  TLSPlaintext
> +
> +Supported Versions
> +^^^^^^^^^^^^^^^^^^
> +
> +* TLS 1.2
> +* TLS 1.3
> +* DTLS 1.2
> 
>  Device Features and Capabilities
>  ---------------------------------
> diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
> index c4d64bb8e9..bd7b026547 100644
> --- a/lib/security/rte_security.c
> +++ b/lib/security/rte_security.c
> @@ -282,6 +282,10 @@ rte_security_capability_get(struct rte_security_ctx
> *instance,
>  				if (capability->docsis.direction ==
>  							idx->docsis.direction)
>  					return capability;
> +			} else if (idx->protocol ==
> RTE_SECURITY_PROTOCOL_TLS_RECORD) {
> +				if (capability->tls_record.ver == idx-
> >tls_record.ver &&
> +				    capability->tls_record.type == idx-
> >tls_record.type)
> +					return capability;
>  			}
>  		}
>  	}
> diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> index 3b2df526ba..b9d064ed84 100644
> --- a/lib/security/rte_security.h
> +++ b/lib/security/rte_security.h
> @@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
>  	/**< DOCSIS direction */
>  };
> 
> +/** Salt len to be used with AEAD algos in TLS 1.2 */
> +#define RTE_SECURITY_TLS_1_2_SALT_LEN 4
> +/** Salt len to be used with AEAD algos in TLS 1.3 */
> +#define RTE_SECURITY_TLS_1_3_SALT_LEN 12
> +/** Salt len to be used with AEAD algos in DTLS 1.2 */
> +#define RTE_SECURITY_DTLS_1_2_SALT_LEN 4
> +
> +/** TLS version */
> +enum rte_security_tls_version {
> +	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
> +	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
> +	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
> +};
> +
> +/** TLS session type */
> +enum rte_security_tls_sess_type {
> +	/** Record read session
> +	 * - Decrypt & digest verification.
> +	 */
> +	RTE_SECURITY_TLS_SESS_TYPE_READ,
> +	/** Record write session
> +	 * - Encrypt & digest generation.
> +	 */
> +	RTE_SECURITY_TLS_SESS_TYPE_WRITE,
> +};
> +
> +/**
> + * Configure soft and hard lifetime of a TLS record session
> + *
> + * Lifetime of a TLS record session would specify the maximum number of
> packets that can be
> + * processed. TLS record processing operations would start failing once hard
> limit is reached.
> + *
> + * Soft limits can be specified to generate notification when the TLS record
> session is approaching
> + * hard limits for lifetime. This would result in a warning returned in
> ``rte_crypto_op.aux_flags``.

Can we define "a warning" better? Perhaps an example of a soft-limit and
hard-limit, what the user can check aux_flags for, to identify? Or link to the
appropriate part of the crypto_op.aux_flags documentation to help the user.

> + */
> +struct rte_security_tls_record_lifetime {
> +	/** Soft expiry limit in number of packets */
> +	uint64_t packets_soft_limit;
> +	/** Hard expiry limit in number of packets */
> +	uint64_t packets_hard_limit;
> +};
> +
> +/**
> + * TLS record protocol session configuration.
> + *
> + * This structure contains data required to create a TLS record security session.
> + */
> +struct rte_security_tls_record_xform {
> +	/** TLS record version. */
> +	enum rte_security_tls_version ver;
> +	/** TLS record session type. */
> +	enum rte_security_tls_sess_type type;
> +	/** TLS record session lifetime. */
> +	struct rte_security_tls_record_lifetime life;
> +	union {
> +		/** TLS 1.2 parameters. */
> +		struct {
> +			/** Starting sequence number. */
> +			uint64_t seq_no;
> +			/** Salt to be used for AEAD algos. */
> +			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
> +		} tls_1_2;
> +
> +		/** TLS 1.3 parameters. */
> +		struct {
> +			/** Starting sequence number. */
> +			uint64_t seq_no;
> +			/** Salt to be used for AEAD algos. */
> +			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
> +			/**
> +			 * Minimum payload length (in case of write sessions).
> For shorter inputs,
> +			 * the payload would be padded appropriately before
> performing crypto

Replace "would be"  with "must be"? And who must do this step, is it the application?

> +			 * transformations.
> +			 */
> +			uint32_t min_payload_len;
> +		} tls_1_3;
> +
> +		/** DTLS 1.2 parameters */
> +		struct {
> +			/** Epoch value to be used. */
> +			uint16_t epoch;
> +			/** 6B starting sequence number to be used. */
> +			uint64_t seq_no;
> +			/** Salt to be used for AEAD algos. */
> +			uint8_t salt[RTE_SECURITY_DTLS_1_2_SALT_LEN];
> +			/** Anti replay window size to enable sequence replay
> attack handling.
> +			 * Anti replay check is disabled if the window size is 0.
> +			 */
> +			uint32_t ar_win_sz;
> +		} dtls_1_2;
> +	};
> +};
> +
>  /**
>   * Security session action type.
>   */
> @@ -654,6 +747,8 @@ enum rte_security_session_protocol {
>  	/**< PDCP Protocol */
>  	RTE_SECURITY_PROTOCOL_DOCSIS,
>  	/**< DOCSIS Protocol */
> +	RTE_SECURITY_PROTOCOL_TLS_RECORD,
> +	/**< TLS Record Protocol */
>  };
> 
>  /**
> @@ -670,6 +765,7 @@ struct rte_security_session_conf {
>  		struct rte_security_macsec_xform macsec;
>  		struct rte_security_pdcp_xform pdcp;
>  		struct rte_security_docsis_xform docsis;
> +		struct rte_security_tls_record_xform tls;

Do we see TLS handshake xform being added in future? If so, is 'tls' a good name, perhaps 'tls_record'?
That would allow 'tls_handshake' in future, with consistent naming scheme without API/ABI break.


>  	};
>  	/**< Configuration parameters for security session */
>  	struct rte_crypto_sym_xform *crypto_xform;
> @@ -1190,6 +1286,16 @@ struct rte_security_capability {
>  			/**< DOCSIS direction */
>  		} docsis;
>  		/**< DOCSIS capability */
> +		struct {
> +			enum rte_security_tls_version ver;
> +			/**< TLS record version. */
> +			enum rte_security_tls_sess_type type;
> +			/**< TLS record session type. */
> +			uint32_t ar_win_size;
> +			/**< Maximum anti replay window size supported for
> DTLS 1.2 record read
> +			 * operation. Value of 0 means anti replay check is not
> supported.
> +			 */
> +		} tls_record;

Missing      /**< TLS Record Capability */     docstring here.

>  	};
> 
>  	const struct rte_cryptodev_capabilities *crypto_capabilities;
> @@ -1251,6 +1357,10 @@ struct rte_security_capability_idx {
>  		struct {
>  			enum rte_security_docsis_direction direction;
>  		} docsis;
> +		struct {
> +			enum rte_security_tls_version ver;
> +			enum rte_security_tls_sess_type type;
> +		} tls_record;
>  	};
>  };
> 
> --
> 2.25.1

RE: [RFC PATCH 3/3] cryptodev: add details of datapath handling of TLS records

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Friday, August 11, 2023 8:17 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [RFC PATCH 3/3] cryptodev: add details of datapath handling of TLS
> records
> 
> TLS/DTLS record processing requires content type to be provided per
> packet (for record write operation). Extend usage of
> rte_crypto_op.aux_flags for the same purpose.

I understand the goal to extend the usage of the aux_flags, but I do not understand
what data/structs/values I should use to set or error-check the aux-flags here.



> 
> Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> ---
>  doc/guides/prog_guide/rte_security.rst | 10 ++++++++++
>  lib/cryptodev/rte_crypto.h             |  6 ++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_security.rst
> b/doc/guides/prog_guide/rte_security.rst
> index 7716d7239f..6cb69bc949 100644
> --- a/doc/guides/prog_guide/rte_security.rst
> +++ b/doc/guides/prog_guide/rte_security.rst
> @@ -451,6 +451,16 @@ Protocol. The TLS Record Protocol provides
> connection security that has two basi
>                    V                              V
>              TLSCiphertext                  TLSPlaintext
> 
> +TLS and DTLS header formation (in record write operation) would depend on
> the
> +type of content. It is a per packet variable and would need to be handled by
> +the same session. Application may pass this info to a cryptodev performing
> +lookaside protocol offload by passing the same in ``rte_crypto_op.aux_flags``.
> +
> +In record read operation, application is required to preserve any info it may
> +need from the TLS/DTLS header (such as content type and sequence number)
> as the
> +cryptodev would remove the header and padding as part of the lookaside
> protocol
> +processing.
> +
>  Supported Versions
>  ^^^^^^^^^^^^^^^^^^
> 
> diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
> index 9b8d0331a4..7c12a2b705 100644
> --- a/lib/cryptodev/rte_crypto.h
> +++ b/lib/cryptodev/rte_crypto.h
> @@ -101,6 +101,12 @@ struct rte_crypto_op {
>  			/**< Operation specific auxiliary/additional flags.
>  			 * These flags carry additional information from the
>  			 * operation. Processing of the same is optional.

It says "processing is optional" here, but in TLS/DTLS, it is proposed that the
soft-error and hard-errors are returned to the user through this struct?
That is not optional, and failing to check that is a failure mode which can result
in IV-reuse, and hence decryption of payload by a malicious actor?

I see this part of the API as being critical to correct usage, and it does not seem
well defined or clear to me at this point. If I am mis-understanding, please clarify,
as likely other developers will likely mis-understand too. Example code snippets of
good hardened error-handling for soft-error and hard-error would help.


> +			 * With TLS record offload
> (RTE_SECURITY_PROTOCOL_TLS_RECORD),
> +			 * application would be required to provide the message
> +			 * type of the input provided. The 'aux_flags' field
> +			 * can be used for passing the same. Message types are
> +			 * listed as RTE_TLS_TYPE_* and RTE_DTLS_TYPE_*.
>  			 */

Same comment as above the "aux_fields can be used" string does not explain to the user
*how* to use the field correctly. Examples (in rte_security.rst?) would help.

>  			uint8_t reserved[2];
>  			/**< Reserved bytes to fill 64 bits for
> --
> 2.25.1

RE: [RFC PATCH 2/3] security: add TLS record processing

[Anoob Joseph]

Hi Harry,

Thanks for the review. Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Wednesday, September 20, 2023 2:53 PM
> To: Anoob Joseph <anoobj@marvell.com>; Thomas Monjalon
> <thomas@monjalon.net>; Akhil Goyal <gakhil@marvell.com>; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>
> Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record processing
> 
> External Email
> 
> ----------------------------------------------------------------------
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Friday, August 11, 2023 8:17 AM
> > To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> > <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin
> > Ananyev <konstantin.v.ananyev@yandex.ru>
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > <vvelumuri@marvell.com>
> > Subject: [RFC PATCH 2/3] security: add TLS record processing
> >
> > Add Transport Layer Security (TLS) and Datagram Transport Layer
> > Security (DTLS). The protocols provide communications privacy for L4
> > protocols such as TCP & UDP.
> >
> > TLS (and DTLS) protocol is composed of two layers, 1. TLS Record
> > Protocol 2. TLS Handshake Protocol
> >
> > While TLS Handshake Protocol helps in establishing security parameters
> > by which client and server can communicate, TLS Record Protocol
> > provides the connection security. TLS Record Protocol leverages
> > symmetric cryptographic operations such as data encryption and
> > authentication for providing security to the communications.
> >
> > Cryptodevs that are capable of offloading TLS Record Protocol may
> > perform other operations like IV generation, header insertion, atomic
> > sequence number updates and anti-replay window check in addition to
> > cryptographic transformations.
> >
> > The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
> 
> From the code below, my understanding is that *ONLY* the record layer is
> being added/supported? The difference is described well above, but the
> intended support added is not clearly defined.
> 
> Suggest reword the last line to clarify:
> "Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and DTLS 1.2."

[Anoob] Indeed. Will reword as suggested.

> 
> 
> > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> > ---
> >  doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
> >  lib/security/rte_security.c            |   4 +
> >  lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
> >  3 files changed, 172 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_security.rst
> > b/doc/guides/prog_guide/rte_security.rst
> > index 7418e35c1b..7716d7239f 100644
> > --- a/doc/guides/prog_guide/rte_security.rst
> > +++ b/doc/guides/prog_guide/rte_security.rst
> > @@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create`` returns
> > a handle for SC,  and this handle is set in
> > ``rte_security_macsec_xform``  to create a MACsec session using
> > ``rte_security_session_create``.
> >
> > +TLS-Record Protocol
> > +~~~~~~~~~~~~~~~~~~~
> > +
> > +The Transport Layer Protocol provides communications security over
> > +the
> > Internet. The protocol
> > +allows client/server applications to communicate in a way that is
> > +designed to
> > prevent eavesdropping,
> > +tampering, or message forgery.
> > +
> > +TLS protocol is composed of two layers: the TLS Record Protocol and
> > +the TLS
> > Handshake Protocol. At
> > +the lowest level, layered on top of some reliable transport protocol
> > +(e.g., TCP),
> > is the TLS Record
> > +Protocol. The TLS Record Protocol provides connection security that
> > +has two
> > basic properties:
> > +
> > +   -  The connection is private.  Symmetric cryptography is used for data
> > +      encryption (e.g., AES, DES, etc.).  The keys for this symmetric
> encryption
> > +      are generated uniquely for each connection and are based on a secret
> > +      negotiated by another protocol (such as the TLS Handshake Protocol).
> The
> > +      Record Protocol can also be used without encryption.
> > +
> > +   -  The connection is reliable.  Message transport includes a message
> > +      integrity check using a keyed MAC.  Secure hash functions (e.g.,
> > +      SHA-1, etc.) are used for MAC computations.  The Record Protocol
> > +      can operate without a MAC, but is generally only used in this mode
> > +      while another protocol is using the Record Protocol as a transport
> > +      for negotiating security parameters.
> > +
> > +.. code-block:: c
> 
> The code block below isn't C? Is there a better code block type for a text
> diagram?

[Anoob] Valid point. I was just following the general scheme followed in this file. May be, I'll introduce a .svg image for newly added code.

> 
> > +             Record Write                   Record Read
> > +             ------------                   -----------
> > +
> > +             TLSPlaintext                  TLSCiphertext
> > +                  |                              |
> > +                  ~                              ~
> > +                  |                              |
> > +                  V                              V
> > +        +---------|----------+        +----------|---------+
> > +        | Seq. no generation |        | Seq. no generation |
> > +        +---------|----------+        +----------|---------+
> > +                  |                              |
> > +        +---------|----------+        +----------|---------+
> > +        |  Header insertion  |        |    Decryption &    |
> > +        +---------|----------+        |  MAC verification  |
> > +                  |                   +----------|---------+
> > +        +---------|----------+                   |
> > +        |  MAC generation &  |        +----------|---------+
> > +        |     Encryption     |        | TLS Header removal |
> > +        +---------|----------+        +----------|---------+
> > +                  |                              |
> > +                  ~                              ~
> > +                  |                              |
> > +                  V                              V
> > +            TLSCiphertext                  TLSPlaintext
> > +
> > +Supported Versions
> > +^^^^^^^^^^^^^^^^^^
> > +
> > +* TLS 1.2
> > +* TLS 1.3
> > +* DTLS 1.2
> >
> >  Device Features and Capabilities
> >  ---------------------------------
> > diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
> > index c4d64bb8e9..bd7b026547 100644
> > --- a/lib/security/rte_security.c
> > +++ b/lib/security/rte_security.c
> > @@ -282,6 +282,10 @@ rte_security_capability_get(struct
> > rte_security_ctx *instance,
> >  				if (capability->docsis.direction ==
> >  							idx->docsis.direction)
> >  					return capability;
> > +			} else if (idx->protocol ==
> > RTE_SECURITY_PROTOCOL_TLS_RECORD) {
> > +				if (capability->tls_record.ver == idx-
> > >tls_record.ver &&
> > +				    capability->tls_record.type == idx-
> > >tls_record.type)
> > +					return capability;
> >  			}
> >  		}
> >  	}
> > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> > index 3b2df526ba..b9d064ed84 100644
> > --- a/lib/security/rte_security.h
> > +++ b/lib/security/rte_security.h
> > @@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
> >  	/**< DOCSIS direction */
> >  };
> >
> > +/** Salt len to be used with AEAD algos in TLS 1.2 */ #define
> > +RTE_SECURITY_TLS_1_2_SALT_LEN 4
> > +/** Salt len to be used with AEAD algos in TLS 1.3 */ #define
> > +RTE_SECURITY_TLS_1_3_SALT_LEN 12
> > +/** Salt len to be used with AEAD algos in DTLS 1.2 */ #define
> > +RTE_SECURITY_DTLS_1_2_SALT_LEN 4
> > +
> > +/** TLS version */
> > +enum rte_security_tls_version {
> > +	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
> > +	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
> > +	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
> > +};
> > +
> > +/** TLS session type */
> > +enum rte_security_tls_sess_type {
> > +	/** Record read session
> > +	 * - Decrypt & digest verification.
> > +	 */
> > +	RTE_SECURITY_TLS_SESS_TYPE_READ,
> > +	/** Record write session
> > +	 * - Encrypt & digest generation.
> > +	 */
> > +	RTE_SECURITY_TLS_SESS_TYPE_WRITE,
> > +};
> > +
> > +/**
> > + * Configure soft and hard lifetime of a TLS record session
> > + *
> > + * Lifetime of a TLS record session would specify the maximum number
> > +of
> > packets that can be
> > + * processed. TLS record processing operations would start failing
> > + once hard
> > limit is reached.
> > + *
> > + * Soft limits can be specified to generate notification when the TLS
> > + record
> > session is approaching
> > + * hard limits for lifetime. This would result in a warning returned
> > + in
> > ``rte_crypto_op.aux_flags``.
> 
> Can we define "a warning" better? Perhaps an example of a soft-limit and
> hard-limit, what the user can check aux_flags for, to identify? Or link to the
> appropriate part of the crypto_op.aux_flags documentation to help the user.
> 

[Anoob] The concept of lifetime is present in most protocols. Idea is to limit the max number of operations performed with a session. Soft expiry notification is to help application prepare for an expiry and setup a new session before the current one expires. The idea was borrowed from IPsec which has the  'RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY' flag defined. But I realize, it should be better defined. I can rename the flag to 'RTE_CRYPTO_OP_AUX_FLAGS_SEC_SOFT_EXPIRY' to avoid redefining same flag for each security offload. Do you agree to this suggestion?

https://elixir.bootlin.com/dpdk/latest/source/lib/cryptodev/rte_crypto.h#L67

Do note that once hard expiry is hit, the operation would fail. Expectation is, cryptodev would return 'RTE_CRYPTO_OP_STATUS_ERROR' in case of errors.

> > + */
> > +struct rte_security_tls_record_lifetime {
> > +	/** Soft expiry limit in number of packets */
> > +	uint64_t packets_soft_limit;
> > +	/** Hard expiry limit in number of packets */
> > +	uint64_t packets_hard_limit;
> > +};
> > +
> > +/**
> > + * TLS record protocol session configuration.
> > + *
> > + * This structure contains data required to create a TLS record security
> session.
> > + */
> > +struct rte_security_tls_record_xform {
> > +	/** TLS record version. */
> > +	enum rte_security_tls_version ver;
> > +	/** TLS record session type. */
> > +	enum rte_security_tls_sess_type type;
> > +	/** TLS record session lifetime. */
> > +	struct rte_security_tls_record_lifetime life;
> > +	union {
> > +		/** TLS 1.2 parameters. */
> > +		struct {
> > +			/** Starting sequence number. */
> > +			uint64_t seq_no;
> > +			/** Salt to be used for AEAD algos. */
> > +			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
> > +		} tls_1_2;
> > +
> > +		/** TLS 1.3 parameters. */
> > +		struct {
> > +			/** Starting sequence number. */
> > +			uint64_t seq_no;
> > +			/** Salt to be used for AEAD algos. */
> > +			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
> > +			/**
> > +			 * Minimum payload length (in case of write
> sessions).
> > For shorter inputs,
> > +			 * the payload would be padded appropriately before
> > performing crypto
> 
> Replace "would be"  with "must be"? And who must do this step, is it the
> application?

[Anoob] Padding is performed by the PMD/cryptodev device. I'll change "would be" to "will be". Would that address your concern?

> 
> > +			 * transformations.
> > +			 */
> > +			uint32_t min_payload_len;
> > +		} tls_1_3;
> > +
> > +		/** DTLS 1.2 parameters */
> > +		struct {
> > +			/** Epoch value to be used. */
> > +			uint16_t epoch;
> > +			/** 6B starting sequence number to be used. */
> > +			uint64_t seq_no;
> > +			/** Salt to be used for AEAD algos. */
> > +			uint8_t salt[RTE_SECURITY_DTLS_1_2_SALT_LEN];
> > +			/** Anti replay window size to enable sequence
> replay
> > attack handling.
> > +			 * Anti replay check is disabled if the window size is 0.
> > +			 */
> > +			uint32_t ar_win_sz;
> > +		} dtls_1_2;
> > +	};
> > +};
> > +
> >  /**
> >   * Security session action type.
> >   */
> > @@ -654,6 +747,8 @@ enum rte_security_session_protocol {
> >  	/**< PDCP Protocol */
> >  	RTE_SECURITY_PROTOCOL_DOCSIS,
> >  	/**< DOCSIS Protocol */
> > +	RTE_SECURITY_PROTOCOL_TLS_RECORD,
> > +	/**< TLS Record Protocol */
> >  };
> >
> >  /**
> > @@ -670,6 +765,7 @@ struct rte_security_session_conf {
> >  		struct rte_security_macsec_xform macsec;
> >  		struct rte_security_pdcp_xform pdcp;
> >  		struct rte_security_docsis_xform docsis;
> > +		struct rte_security_tls_record_xform tls;
> 
> Do we see TLS handshake xform being added in future? If so, is 'tls' a good
> name, perhaps 'tls_record'?
> That would allow 'tls_handshake' in future, with consistent naming scheme
> without API/ABI break.

[Anoob] In the future, I would like to see TLS handshake also offloaded in a similar manner. But that would need some fundamental changes in security library. Like, current security library is pretty much tied to symmetric operations but a handshake would involve both symmetric & asymmetric operations.

Said that, I agree with your suggestion to rename the field. Will change it to 'tls_record' in next version.

> 
> 
> >  	};
> >  	/**< Configuration parameters for security session */
> >  	struct rte_crypto_sym_xform *crypto_xform; @@ -1190,6 +1286,16
> @@
> > struct rte_security_capability {
> >  			/**< DOCSIS direction */
> >  		} docsis;
> >  		/**< DOCSIS capability */
> > +		struct {
> > +			enum rte_security_tls_version ver;
> > +			/**< TLS record version. */
> > +			enum rte_security_tls_sess_type type;
> > +			/**< TLS record session type. */
> > +			uint32_t ar_win_size;
> > +			/**< Maximum anti replay window size supported
> for
> > DTLS 1.2 record read
> > +			 * operation. Value of 0 means anti replay check is
> not
> > supported.
> > +			 */
> > +		} tls_record;
> 
> Missing      /**< TLS Record Capability */     docstring here.

[Anoob] Agreed. Will add in next version.

> 
> >  	};
> >
> >  	const struct rte_cryptodev_capabilities *crypto_capabilities; @@
> > -1251,6 +1357,10 @@ struct rte_security_capability_idx {
> >  		struct {
> >  			enum rte_security_docsis_direction direction;
> >  		} docsis;
> > +		struct {
> > +			enum rte_security_tls_version ver;
> > +			enum rte_security_tls_sess_type type;
> > +		} tls_record;
> >  	};
> >  };
> >
> > --
> > 2.25.1

RE: [RFC PATCH 2/3] security: add TLS record processing

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Wednesday, September 20, 2023 12:52 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>; Thomas Monjalon <thomas@monjalon.net>; Akhil
> Goyal <gakhil@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Subject: RE: [RFC PATCH 2/3] security: add TLS record processing
> 
> Hi Harry,
> 
> Thanks for the review. Please see inline.
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Sent: Wednesday, September 20, 2023 2:53 PM
> > To: Anoob Joseph <anoobj@marvell.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Akhil Goyal <gakhil@marvell.com>; Jerin Jacob
> > Kollanukkaran <jerinj@marvell.com>; Konstantin Ananyev
> > <konstantin.v.ananyev@yandex.ru>
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > <vvelumuri@marvell.com>
> > Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record processing
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > > -----Original Message-----
> > > From: Anoob Joseph <anoobj@marvell.com>
> > > Sent: Friday, August 11, 2023 8:17 AM
> > > To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> > > <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin
> > > Ananyev <konstantin.v.ananyev@yandex.ru>
> > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > <vvelumuri@marvell.com>
> > > Subject: [RFC PATCH 2/3] security: add TLS record processing
> > >
> > > Add Transport Layer Security (TLS) and Datagram Transport Layer
> > > Security (DTLS). The protocols provide communications privacy for L4
> > > protocols such as TCP & UDP.
> > >
> > > TLS (and DTLS) protocol is composed of two layers, 1. TLS Record
> > > Protocol 2. TLS Handshake Protocol
> > >
> > > While TLS Handshake Protocol helps in establishing security parameters
> > > by which client and server can communicate, TLS Record Protocol
> > > provides the connection security. TLS Record Protocol leverages
> > > symmetric cryptographic operations such as data encryption and
> > > authentication for providing security to the communications.
> > >
> > > Cryptodevs that are capable of offloading TLS Record Protocol may
> > > perform other operations like IV generation, header insertion, atomic
> > > sequence number updates and anti-replay window check in addition to
> > > cryptographic transformations.
> > >
> > > The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
> >
> > From the code below, my understanding is that *ONLY* the record layer is
> > being added/supported? The difference is described well above, but the
> > intended support added is not clearly defined.
> >
> > Suggest reword the last line to clarify:
> > "Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and DTLS 1.2."
> 
> [Anoob] Indeed. Will reword as suggested.

Thanks.

> > > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> > > ---
> > >  doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
> > >  lib/security/rte_security.c            |   4 +
> > >  lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
> > >  3 files changed, 172 insertions(+)
> > >
> > > diff --git a/doc/guides/prog_guide/rte_security.rst
> > > b/doc/guides/prog_guide/rte_security.rst
> > > index 7418e35c1b..7716d7239f 100644
> > > --- a/doc/guides/prog_guide/rte_security.rst
> > > +++ b/doc/guides/prog_guide/rte_security.rst
> > > @@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create`` returns
> > > a handle for SC,  and this handle is set in
> > > ``rte_security_macsec_xform``  to create a MACsec session using
> > > ``rte_security_session_create``.
> > >
> > > +TLS-Record Protocol
> > > +~~~~~~~~~~~~~~~~~~~
> > > +
> > > +The Transport Layer Protocol provides communications security over
> > > +the
> > > Internet. The protocol
> > > +allows client/server applications to communicate in a way that is
> > > +designed to
> > > prevent eavesdropping,
> > > +tampering, or message forgery.
> > > +
> > > +TLS protocol is composed of two layers: the TLS Record Protocol and
> > > +the TLS
> > > Handshake Protocol. At
> > > +the lowest level, layered on top of some reliable transport protocol
> > > +(e.g., TCP),
> > > is the TLS Record
> > > +Protocol. The TLS Record Protocol provides connection security that
> > > +has two
> > > basic properties:
> > > +
> > > +   -  The connection is private.  Symmetric cryptography is used for data
> > > +      encryption (e.g., AES, DES, etc.).  The keys for this symmetric
> > encryption
> > > +      are generated uniquely for each connection and are based on a secret
> > > +      negotiated by another protocol (such as the TLS Handshake Protocol).
> > The
> > > +      Record Protocol can also be used without encryption.
> > > +
> > > +   -  The connection is reliable.  Message transport includes a message
> > > +      integrity check using a keyed MAC.  Secure hash functions (e.g.,
> > > +      SHA-1, etc.) are used for MAC computations.  The Record Protocol
> > > +      can operate without a MAC, but is generally only used in this mode
> > > +      while another protocol is using the Record Protocol as a transport
> > > +      for negotiating security parameters.
> > > +
> > > +.. code-block:: c
> >
> > The code block below isn't C? Is there a better code block type for a text
> > diagram?
> 
> [Anoob] Valid point. I was just following the general scheme followed in this file.
> May be, I'll introduce a .svg image for newly added code.

This was a nit-pick that perhaps "code-block:: text-diagram" or so exists.
No need to make a .svg in my opinion, the text-diagrams are clear.


> > > +             Record Write                   Record Read
> > > +             ------------                   -----------
> > > +
> > > +             TLSPlaintext                  TLSCiphertext
> > > +                  |                              |
> > > +                  ~                              ~
> > > +                  |                              |
> > > +                  V                              V
> > > +        +---------|----------+        +----------|---------+
> > > +        | Seq. no generation |        | Seq. no generation |
> > > +        +---------|----------+        +----------|---------+
> > > +                  |                              |
> > > +        +---------|----------+        +----------|---------+
> > > +        |  Header insertion  |        |    Decryption &    |
> > > +        +---------|----------+        |  MAC verification  |
> > > +                  |                   +----------|---------+
> > > +        +---------|----------+                   |
> > > +        |  MAC generation &  |        +----------|---------+
> > > +        |     Encryption     |        | TLS Header removal |
> > > +        +---------|----------+        +----------|---------+
> > > +                  |                              |
> > > +                  ~                              ~
> > > +                  |                              |
> > > +                  V                              V
> > > +            TLSCiphertext                  TLSPlaintext
> > > +
> > > +Supported Versions
> > > +^^^^^^^^^^^^^^^^^^
> > > +
> > > +* TLS 1.2
> > > +* TLS 1.3
> > > +* DTLS 1.2
> > >
> > >  Device Features and Capabilities
> > >  ---------------------------------
> > > diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
> > > index c4d64bb8e9..bd7b026547 100644
> > > --- a/lib/security/rte_security.c
> > > +++ b/lib/security/rte_security.c
> > > @@ -282,6 +282,10 @@ rte_security_capability_get(struct
> > > rte_security_ctx *instance,
> > >  				if (capability->docsis.direction ==
> > >  							idx->docsis.direction)
> > >  					return capability;
> > > +			} else if (idx->protocol ==
> > > RTE_SECURITY_PROTOCOL_TLS_RECORD) {
> > > +				if (capability->tls_record.ver == idx-
> > > >tls_record.ver &&
> > > +				    capability->tls_record.type == idx-
> > > >tls_record.type)
> > > +					return capability;
> > >  			}
> > >  		}
> > >  	}
> > > diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
> > > index 3b2df526ba..b9d064ed84 100644
> > > --- a/lib/security/rte_security.h
> > > +++ b/lib/security/rte_security.h
> > > @@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
> > >  	/**< DOCSIS direction */
> > >  };
> > >
> > > +/** Salt len to be used with AEAD algos in TLS 1.2 */ #define
> > > +RTE_SECURITY_TLS_1_2_SALT_LEN 4
> > > +/** Salt len to be used with AEAD algos in TLS 1.3 */ #define
> > > +RTE_SECURITY_TLS_1_3_SALT_LEN 12
> > > +/** Salt len to be used with AEAD algos in DTLS 1.2 */ #define
> > > +RTE_SECURITY_DTLS_1_2_SALT_LEN 4
> > > +
> > > +/** TLS version */
> > > +enum rte_security_tls_version {
> > > +	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
> > > +	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
> > > +	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
> > > +};
> > > +
> > > +/** TLS session type */
> > > +enum rte_security_tls_sess_type {
> > > +	/** Record read session
> > > +	 * - Decrypt & digest verification.
> > > +	 */
> > > +	RTE_SECURITY_TLS_SESS_TYPE_READ,
> > > +	/** Record write session
> > > +	 * - Encrypt & digest generation.
> > > +	 */
> > > +	RTE_SECURITY_TLS_SESS_TYPE_WRITE,
> > > +};
> > > +
> > > +/**
> > > + * Configure soft and hard lifetime of a TLS record session
> > > + *
> > > + * Lifetime of a TLS record session would specify the maximum number
> > > +of
> > > packets that can be
> > > + * processed. TLS record processing operations would start failing
> > > + once hard
> > > limit is reached.
> > > + *
> > > + * Soft limits can be specified to generate notification when the TLS
> > > + record
> > > session is approaching
> > > + * hard limits for lifetime. This would result in a warning returned
> > > + in
> > > ``rte_crypto_op.aux_flags``.
> >
> > Can we define "a warning" better? Perhaps an example of a soft-limit and
> > hard-limit, what the user can check aux_flags for, to identify? Or link to the
> > appropriate part of the crypto_op.aux_flags documentation to help the user.
> >
> 
> [Anoob] The concept of lifetime is present in most protocols. Idea is to limit the
> max number of operations performed with a session. Soft expiry notification is
> to help application prepare for an expiry and setup a new session before the
> current one expires.

Understood, yes.

> The idea was borrowed from IPsec which has the
> 'RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY' flag defined. But I realize, it
> should be better defined. I can rename the flag to
> 'RTE_CRYPTO_OP_AUX_FLAGS_SEC_SOFT_EXPIRY' to avoid redefining same flag
> for each security offload. Do you agree to this suggestion?
>
> https://elixir.bootlin.com/dpdk/latest/source/lib/cryptodev/rte_crypto.h#L67

So we cannot "just rename" the flag, its an API break. It likely is possible to add an
additional #define with the same value as IPSEC_SOFT_EXPIRY, and call it SEC_SOFT_EXPIRY.
Is that the best/most descriptive name? SYM_ALG_SOFT_EXPIRY? I'm not sure here, input welcomed.

Perhaps we can improve the doc-string there, to explain what it means a bit more verbosely.

> Do note that once hard expiry is hit, the operation would fail. Expectation is,
> cryptodev would return 'RTE_CRYPTO_OP_STATUS_ERROR' in case of errors.

That is good.


> > > + */
> > > +struct rte_security_tls_record_lifetime {
> > > +	/** Soft expiry limit in number of packets */
> > > +	uint64_t packets_soft_limit;
> > > +	/** Hard expiry limit in number of packets */
> > > +	uint64_t packets_hard_limit;
> > > +};
> > > +
> > > +/**
> > > + * TLS record protocol session configuration.
> > > + *
> > > + * This structure contains data required to create a TLS record security
> > session.
> > > + */
> > > +struct rte_security_tls_record_xform {
> > > +	/** TLS record version. */
> > > +	enum rte_security_tls_version ver;
> > > +	/** TLS record session type. */
> > > +	enum rte_security_tls_sess_type type;
> > > +	/** TLS record session lifetime. */
> > > +	struct rte_security_tls_record_lifetime life;
> > > +	union {
> > > +		/** TLS 1.2 parameters. */
> > > +		struct {
> > > +			/** Starting sequence number. */
> > > +			uint64_t seq_no;
> > > +			/** Salt to be used for AEAD algos. */
> > > +			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
> > > +		} tls_1_2;
> > > +
> > > +		/** TLS 1.3 parameters. */
> > > +		struct {
> > > +			/** Starting sequence number. */
> > > +			uint64_t seq_no;
> > > +			/** Salt to be used for AEAD algos. */
> > > +			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
> > > +			/**
> > > +			 * Minimum payload length (in case of write
> > sessions).
> > > For shorter inputs,
> > > +			 * the payload would be padded appropriately before
> > > performing crypto
> >
> > Replace "would be"  with "must be"? And who must do this step, is it the
> > application?
> 
> [Anoob] Padding is performed by the PMD/cryptodev device. I'll change "would
> be" to "will be". Would that address your concern?

I suppose my concern is "is it clear to PMD authors that they must implement X in their PMD",
and to ensure we (DPDK community) do our best to clarify API demands, and to ensure future
contributions are of high quality too.

For example, could we have a security library unit-test that checks the padding case, to
ensure correct & consistent behaviour across different crypto PMDs?

Same thoughts for SOFT and HARD error variants, (although they might take... hours?) to execute.
But it is nice to automate-and-test these "corner cases".

It's not about wording, its about clarity between PMD devs, security library devs, and application facing APIs,
to ensure that DPDK provides the most/best help it can to provide correctness. Does that clarify what I'd like to see?

For this patchset, could we document a list of "caveats" when implementing PMD functionality for TLS-record security offload, and indicate that:
1) Padding must be added by the PMD based on security library flags& algo in use, not application layer (I know this is demanded by the sym algos anyway, but let's make it explicit)
2) It is strongly recommended to build unit-tests for _SOFT and _HARD error cases (potentially by "fast forwarding" the internal counters via private APIs to avoid hours of enc/decryption)

I think that limits scope-impact to this patchset, but is clear for PMD implementations in future what expectations are.
Thoughts, is that a good way forward?

> >
> > > +			 * transformations.
> > > +			 */
> > > +			uint32_t min_payload_len;
> > > +		} tls_1_3;
> > > +
> > > +		/** DTLS 1.2 parameters */
> > > +		struct {
> > > +			/** Epoch value to be used. */
> > > +			uint16_t epoch;
> > > +			/** 6B starting sequence number to be used. */
> > > +			uint64_t seq_no;
> > > +			/** Salt to be used for AEAD algos. */
> > > +			uint8_t salt[RTE_SECURITY_DTLS_1_2_SALT_LEN];
> > > +			/** Anti replay window size to enable sequence
> > replay
> > > attack handling.
> > > +			 * Anti replay check is disabled if the window size is 0.
> > > +			 */
> > > +			uint32_t ar_win_sz;
> > > +		} dtls_1_2;
> > > +	};
> > > +};
> > > +
> > >  /**
> > >   * Security session action type.
> > >   */
> > > @@ -654,6 +747,8 @@ enum rte_security_session_protocol {
> > >  	/**< PDCP Protocol */
> > >  	RTE_SECURITY_PROTOCOL_DOCSIS,
> > >  	/**< DOCSIS Protocol */
> > > +	RTE_SECURITY_PROTOCOL_TLS_RECORD,
> > > +	/**< TLS Record Protocol */
> > >  };
> > >
> > >  /**
> > > @@ -670,6 +765,7 @@ struct rte_security_session_conf {
> > >  		struct rte_security_macsec_xform macsec;
> > >  		struct rte_security_pdcp_xform pdcp;
> > >  		struct rte_security_docsis_xform docsis;
> > > +		struct rte_security_tls_record_xform tls;
> >
> > Do we see TLS handshake xform being added in future? If so, is 'tls' a good
> > name, perhaps 'tls_record'?
> > That would allow 'tls_handshake' in future, with consistent naming scheme
> > without API/ABI break.
> 
> [Anoob] In the future, I would like to see TLS handshake also offloaded in a
> similar manner. But that would need some fundamental changes in security
> library. Like, current security library is pretty much tied to symmetric
> operations but a handshake would involve both symmetric & asymmetric
> operations.
> 
> Said that, I agree with your suggestion to rename the field. Will change it to
> 'tls_record' in next version.

Agreed, that handshake is significantly more complex, thanks for rename to "tls_record".


> 
> >
> >
> > >  	};
> > >  	/**< Configuration parameters for security session */
> > >  	struct rte_crypto_sym_xform *crypto_xform; @@ -1190,6 +1286,16
> > @@
> > > struct rte_security_capability {
> > >  			/**< DOCSIS direction */
> > >  		} docsis;
> > >  		/**< DOCSIS capability */
> > > +		struct {
> > > +			enum rte_security_tls_version ver;
> > > +			/**< TLS record version. */
> > > +			enum rte_security_tls_sess_type type;
> > > +			/**< TLS record session type. */
> > > +			uint32_t ar_win_size;
> > > +			/**< Maximum anti replay window size supported
> > for
> > > DTLS 1.2 record read
> > > +			 * operation. Value of 0 means anti replay check is
> > not
> > > supported.
> > > +			 */
> > > +		} tls_record;
> >
> > Missing      /**< TLS Record Capability */     docstring here.
> 
> [Anoob] Agreed. Will add in next version.

Thanks!

<snip>

RE: [RFC PATCH 2/3] security: add TLS record processing

[Anoob Joseph]

Hi Harry,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Van Haaren, Harry <harry.van.haaren@intel.com>
> Sent: Thursday, September 21, 2023 2:09 PM
> To: Anoob Joseph <anoobj@marvell.com>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> <vvelumuri@marvell.com>; Thomas Monjalon <thomas@monjalon.net>;
> Akhil Goyal <gakhil@marvell.com>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record processing
> 
> External Email
> 
> ----------------------------------------------------------------------
> > -----Original Message-----
> > From: Anoob Joseph <anoobj@marvell.com>
> > Sent: Wednesday, September 20, 2023 12:52 PM
> > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > <vvelumuri@marvell.com>; Thomas Monjalon <thomas@monjalon.net>;
> Akhil
> > Goyal <gakhil@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Konstantin Ananyev
> > <konstantin.v.ananyev@yandex.ru>
> > Subject: RE: [RFC PATCH 2/3] security: add TLS record processing
> >
> > Hi Harry,
> >
> > Thanks for the review. Please see inline.
> >
> > Thanks,
> > Anoob
> >
> > > -----Original Message-----
> > > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Sent: Wednesday, September 20, 2023 2:53 PM
> > > To: Anoob Joseph <anoobj@marvell.com>; Thomas Monjalon
> > > <thomas@monjalon.net>; Akhil Goyal <gakhil@marvell.com>; Jerin Jacob
> > > Kollanukkaran <jerinj@marvell.com>; Konstantin Ananyev
> > > <konstantin.v.ananyev@yandex.ru>
> > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > <vvelumuri@marvell.com>
> > > Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record
> > > processing
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > --
> > > > -----Original Message-----
> > > > From: Anoob Joseph <anoobj@marvell.com>
> > > > Sent: Friday, August 11, 2023 8:17 AM
> > > > To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> > > > <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin
> > > > Ananyev <konstantin.v.ananyev@yandex.ru>
> > > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org;
> Matz,
> > > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > > <vvelumuri@marvell.com>
> > > > Subject: [RFC PATCH 2/3] security: add TLS record processing
> > > >
> > > > Add Transport Layer Security (TLS) and Datagram Transport Layer
> > > > Security (DTLS). The protocols provide communications privacy for
> > > > L4 protocols such as TCP & UDP.
> > > >
> > > > TLS (and DTLS) protocol is composed of two layers, 1. TLS Record
> > > > Protocol 2. TLS Handshake Protocol
> > > >
> > > > While TLS Handshake Protocol helps in establishing security
> > > > parameters by which client and server can communicate, TLS Record
> > > > Protocol provides the connection security. TLS Record Protocol
> > > > leverages symmetric cryptographic operations such as data
> > > > encryption and authentication for providing security to the
> communications.
> > > >
> > > > Cryptodevs that are capable of offloading TLS Record Protocol may
> > > > perform other operations like IV generation, header insertion,
> > > > atomic sequence number updates and anti-replay window check in
> > > > addition to cryptographic transformations.
> > > >
> > > > The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
> > >
> > > From the code below, my understanding is that *ONLY* the record
> > > layer is being added/supported? The difference is described well
> > > above, but the intended support added is not clearly defined.
> > >
> > > Suggest reword the last line to clarify:
> > > "Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and DTLS
> 1.2."
> >
> > [Anoob] Indeed. Will reword as suggested.
> 
> Thanks.
> 
> > > > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> > > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > > Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> > > > ---
> > > >  doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
> > > >  lib/security/rte_security.c            |   4 +
> > > >  lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
> > > >  3 files changed, 172 insertions(+)
> > > >
> > > > diff --git a/doc/guides/prog_guide/rte_security.rst
> > > > b/doc/guides/prog_guide/rte_security.rst
> > > > index 7418e35c1b..7716d7239f 100644
> > > > --- a/doc/guides/prog_guide/rte_security.rst
> > > > +++ b/doc/guides/prog_guide/rte_security.rst
> > > > @@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create``
> > > > returns a handle for SC,  and this handle is set in
> > > > ``rte_security_macsec_xform``  to create a MACsec session using
> > > > ``rte_security_session_create``.
> > > >
> > > > +TLS-Record Protocol
> > > > +~~~~~~~~~~~~~~~~~~~
> > > > +
> > > > +The Transport Layer Protocol provides communications security
> > > > +over the
> > > > Internet. The protocol
> > > > +allows client/server applications to communicate in a way that is
> > > > +designed to
> > > > prevent eavesdropping,
> > > > +tampering, or message forgery.
> > > > +
> > > > +TLS protocol is composed of two layers: the TLS Record Protocol
> > > > +and the TLS
> > > > Handshake Protocol. At
> > > > +the lowest level, layered on top of some reliable transport
> > > > +protocol (e.g., TCP),
> > > > is the TLS Record
> > > > +Protocol. The TLS Record Protocol provides connection security
> > > > +that has two
> > > > basic properties:
> > > > +
> > > > +   -  The connection is private.  Symmetric cryptography is used for data
> > > > +      encryption (e.g., AES, DES, etc.).  The keys for this
> > > > + symmetric
> > > encryption
> > > > +      are generated uniquely for each connection and are based on a
> secret
> > > > +      negotiated by another protocol (such as the TLS Handshake
> Protocol).
> > > The
> > > > +      Record Protocol can also be used without encryption.
> > > > +
> > > > +   -  The connection is reliable.  Message transport includes a message
> > > > +      integrity check using a keyed MAC.  Secure hash functions (e.g.,
> > > > +      SHA-1, etc.) are used for MAC computations.  The Record Protocol
> > > > +      can operate without a MAC, but is generally only used in this mode
> > > > +      while another protocol is using the Record Protocol as a transport
> > > > +      for negotiating security parameters.
> > > > +
> > > > +.. code-block:: c
> > >
> > > The code block below isn't C? Is there a better code block type for
> > > a text diagram?
> >
> > [Anoob] Valid point. I was just following the general scheme followed in
> this file.
> > May be, I'll introduce a .svg image for newly added code.
> 
> This was a nit-pick that perhaps "code-block:: text-diagram" or so exists.
> No need to make a .svg in my opinion, the text-diagrams are clear.

[Anoob] Thanks for the confirmation. I do not think "code-block:: text-diagram" exists. Anyway, I'll improve the diagrams to make the padding etc more clear.

> 
> 
> > > > +             Record Write                   Record Read
> > > > +             ------------                   -----------
> > > > +
> > > > +             TLSPlaintext                  TLSCiphertext
> > > > +                  |                              |
> > > > +                  ~                              ~
> > > > +                  |                              |
> > > > +                  V                              V
> > > > +        +---------|----------+        +----------|---------+
> > > > +        | Seq. no generation |        | Seq. no generation |
> > > > +        +---------|----------+        +----------|---------+
> > > > +                  |                              |
> > > > +        +---------|----------+        +----------|---------+
> > > > +        |  Header insertion  |        |    Decryption &    |
> > > > +        +---------|----------+        |  MAC verification  |
> > > > +                  |                   +----------|---------+
> > > > +        +---------|----------+                   |
> > > > +        |  MAC generation &  |        +----------|---------+
> > > > +        |     Encryption     |        | TLS Header removal |
> > > > +        +---------|----------+        +----------|---------+
> > > > +                  |                              |
> > > > +                  ~                              ~
> > > > +                  |                              |
> > > > +                  V                              V
> > > > +            TLSCiphertext                  TLSPlaintext
> > > > +
> > > > +Supported Versions
> > > > +^^^^^^^^^^^^^^^^^^
> > > > +
> > > > +* TLS 1.2
> > > > +* TLS 1.3
> > > > +* DTLS 1.2
> > > >
> > > >  Device Features and Capabilities
> > > >  --------------------------------- diff --git
> > > > a/lib/security/rte_security.c b/lib/security/rte_security.c index
> > > > c4d64bb8e9..bd7b026547 100644
> > > > --- a/lib/security/rte_security.c
> > > > +++ b/lib/security/rte_security.c
> > > > @@ -282,6 +282,10 @@ rte_security_capability_get(struct
> > > > rte_security_ctx *instance,
> > > >  				if (capability->docsis.direction ==
> > > >  							idx->docsis.direction)
> > > >  					return capability;
> > > > +			} else if (idx->protocol ==
> > > > RTE_SECURITY_PROTOCOL_TLS_RECORD) {
> > > > +				if (capability->tls_record.ver == idx-
> > > > >tls_record.ver &&
> > > > +				    capability->tls_record.type == idx-
> > > > >tls_record.type)
> > > > +					return capability;
> > > >  			}
> > > >  		}
> > > >  	}
> > > > diff --git a/lib/security/rte_security.h
> > > > b/lib/security/rte_security.h index 3b2df526ba..b9d064ed84 100644
> > > > --- a/lib/security/rte_security.h
> > > > +++ b/lib/security/rte_security.h
> > > > @@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
> > > >  	/**< DOCSIS direction */
> > > >  };
> > > >
> > > > +/** Salt len to be used with AEAD algos in TLS 1.2 */ #define
> > > > +RTE_SECURITY_TLS_1_2_SALT_LEN 4
> > > > +/** Salt len to be used with AEAD algos in TLS 1.3 */ #define
> > > > +RTE_SECURITY_TLS_1_3_SALT_LEN 12
> > > > +/** Salt len to be used with AEAD algos in DTLS 1.2 */ #define
> > > > +RTE_SECURITY_DTLS_1_2_SALT_LEN 4
> > > > +
> > > > +/** TLS version */
> > > > +enum rte_security_tls_version {
> > > > +	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
> > > > +	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
> > > > +	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
> > > > +};
> > > > +
> > > > +/** TLS session type */
> > > > +enum rte_security_tls_sess_type {
> > > > +	/** Record read session
> > > > +	 * - Decrypt & digest verification.
> > > > +	 */
> > > > +	RTE_SECURITY_TLS_SESS_TYPE_READ,
> > > > +	/** Record write session
> > > > +	 * - Encrypt & digest generation.
> > > > +	 */
> > > > +	RTE_SECURITY_TLS_SESS_TYPE_WRITE, };
> > > > +
> > > > +/**
> > > > + * Configure soft and hard lifetime of a TLS record session
> > > > + *
> > > > + * Lifetime of a TLS record session would specify the maximum
> > > > +number of
> > > > packets that can be
> > > > + * processed. TLS record processing operations would start
> > > > + failing once hard
> > > > limit is reached.
> > > > + *
> > > > + * Soft limits can be specified to generate notification when the
> > > > + TLS record
> > > > session is approaching
> > > > + * hard limits for lifetime. This would result in a warning
> > > > + returned in
> > > > ``rte_crypto_op.aux_flags``.
> > >
> > > Can we define "a warning" better? Perhaps an example of a soft-limit
> > > and hard-limit, what the user can check aux_flags for, to identify?
> > > Or link to the appropriate part of the crypto_op.aux_flags documentation
> to help the user.
> > >
> >
> > [Anoob] The concept of lifetime is present in most protocols. Idea is
> > to limit the max number of operations performed with a session. Soft
> > expiry notification is to help application prepare for an expiry and
> > setup a new session before the current one expires.
> 
> Understood, yes.
> 
> > The idea was borrowed from IPsec which has the
> > 'RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY' flag defined. But I
> > realize, it should be better defined. I can rename the flag to
> > 'RTE_CRYPTO_OP_AUX_FLAGS_SEC_SOFT_EXPIRY' to avoid redefining
> same
> > flag for each security offload. Do you agree to this suggestion?
> >
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.co
> > m_dpdk_latest_source_lib_cryptodev_rte-5Fcrypto.h-
> 23L67&d=DwIFAg&c=nKj
> > Wec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRSxyLWs2n6B-
> WYLn1v9SyTMrT5EQqh2TU&m=F6
> >
> WuwwuvXG0eJ1IWAMFgcXlMdQwKFDx6C1qsQgSaDa2XGr3PZ6LEDtSw0OeC
> DstG&s=0iO8Y
> > sr0f4cnE8ihg40sZfEEZfPzEXvJBvcNcyOdS98&e=
> 
> So we cannot "just rename" the flag, its an API break. It likely is possible to
> add an additional #define with the same value as IPSEC_SOFT_EXPIRY, and
> call it SEC_SOFT_EXPIRY.
> Is that the best/most descriptive name? SYM_ALG_SOFT_EXPIRY? I'm not
> sure here, input welcomed.
> 
> Perhaps we can improve the doc-string there, to explain what it means a bit
> more verbosely.

[Anoob] Agreed. Let me try to improve these aspects in next version. We can revisit this topic after that and see if we are able to reach a conclusion.

> 
> > Do note that once hard expiry is hit, the operation would fail.
> > Expectation is, cryptodev would return 'RTE_CRYPTO_OP_STATUS_ERROR'
> in case of errors.
> 
> That is good.
> 
> 
> > > > + */
> > > > +struct rte_security_tls_record_lifetime {
> > > > +	/** Soft expiry limit in number of packets */
> > > > +	uint64_t packets_soft_limit;
> > > > +	/** Hard expiry limit in number of packets */
> > > > +	uint64_t packets_hard_limit;
> > > > +};
> > > > +
> > > > +/**
> > > > + * TLS record protocol session configuration.
> > > > + *
> > > > + * This structure contains data required to create a TLS record
> > > > +security
> > > session.
> > > > + */
> > > > +struct rte_security_tls_record_xform {
> > > > +	/** TLS record version. */
> > > > +	enum rte_security_tls_version ver;
> > > > +	/** TLS record session type. */
> > > > +	enum rte_security_tls_sess_type type;
> > > > +	/** TLS record session lifetime. */
> > > > +	struct rte_security_tls_record_lifetime life;
> > > > +	union {
> > > > +		/** TLS 1.2 parameters. */
> > > > +		struct {
> > > > +			/** Starting sequence number. */
> > > > +			uint64_t seq_no;
> > > > +			/** Salt to be used for AEAD algos. */
> > > > +			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
> > > > +		} tls_1_2;
> > > > +
> > > > +		/** TLS 1.3 parameters. */
> > > > +		struct {
> > > > +			/** Starting sequence number. */
> > > > +			uint64_t seq_no;
> > > > +			/** Salt to be used for AEAD algos. */
> > > > +			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
> > > > +			/**
> > > > +			 * Minimum payload length (in case of write
> > > sessions).
> > > > For shorter inputs,
> > > > +			 * the payload would be padded appropriately before
> > > > performing crypto
> > >
> > > Replace "would be"  with "must be"? And who must do this step, is it
> > > the application?
> >
> > [Anoob] Padding is performed by the PMD/cryptodev device. I'll change
> > "would be" to "will be". Would that address your concern?
> 
> I suppose my concern is "is it clear to PMD authors that they must implement
> X in their PMD", and to ensure we (DPDK community) do our best to clarify
> API demands, and to ensure future contributions are of high quality too.
> 
> For example, could we have a security library unit-test that checks the
> padding case, to ensure correct & consistent behaviour across different
> crypto PMDs?

[Anoob] Glad that you brought up that point. For IPsec, we have rather extensive framework which covers all the features that are supported today in rte_security. Features like soft expiry & hard expiry are tested on platforms that support the same. As for TLS, we are working on adding unit test framework. It would be part of the next version.

> 
> Same thoughts for SOFT and HARD error variants, (although they might
> take... hours?) to execute.
> But it is nice to automate-and-test these "corner cases".

[Anoob] It won't. Soft expiry can be very low value as well. It is already covered. Please check.
https://elixir.bootlin.com/dpdk/latest/source/app/test/test_cryptodev.c#L10272

> 
> It's not about wording, its about clarity between PMD devs, security library
> devs, and application facing APIs, to ensure that DPDK provides the
> most/best help it can to provide correctness. Does that clarify what I'd like to
> see?

[Anoob] I understand your concern. My understanding is that for security offloads (rte_security lib), documentation has complete details of application & PMD expectations. If we have missed some features, we can take a re-look at the features and add as required. Other than documentation, do you have any other suggestions in mind?

> 
> For this patchset, could we document a list of "caveats" when implementing
> PMD functionality for TLS-record security offload, and indicate that:
> 1) Padding must be added by the PMD based on security library flags& algo in
> use, not application layer (I know this is demanded by the sym algos anyway,
> but let's make it explicit)

[Anoob] Agreed. Will update documentation as required.

> 2) It is strongly recommended to build unit-tests for _SOFT and _HARD error
> cases (potentially by "fast forwarding" the internal counters via private APIs
> to avoid hours of enc/decryption)

[Anoob] For IPsec, it is already added. For TLS, we have plans to add the same.

> 
> I think that limits scope-impact to this patchset, but is clear for PMD
> implementations in future what expectations are.
> Thoughts, is that a good way forward?

[Anoob] Indeed. Having a very well defined API interface for applications is a very positive change. I'm open to more ideas than just updating the documentation.

Appreciate your feedback.

<snip>

RE: [RFC PATCH 2/3] security: add TLS record processing

[Van Haaren, Harry]

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Thursday, September 21, 2023 11:55 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz, Olivier
> <olivier.matz@6wind.com>; Vidya Sagar Velumuri <vvelumuri@marvell.com>;
> Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal <gakhil@marvell.com>;
> Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>
> Subject: RE: [RFC PATCH 2/3] security: add TLS record processing
> 
> Hi Harry,
> 
> Please see inline.

Read all comments inline below, agree that improving docs is the only action.
I wasn't aware a unit-test suite for the TLS record work was in progress, glad to see that.

All looks good, happy to Ack next version with doc updates.


> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > Sent: Thursday, September 21, 2023 2:09 PM
> > To: Anoob Joseph <anoobj@marvell.com>
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > <vvelumuri@marvell.com>; Thomas Monjalon <thomas@monjalon.net>;
> > Akhil Goyal <gakhil@marvell.com>; Jerin Jacob Kollanukkaran
> > <jerinj@marvell.com>; Konstantin Ananyev
> > <konstantin.v.ananyev@yandex.ru>
> > Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record processing
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > > -----Original Message-----
> > > From: Anoob Joseph <anoobj@marvell.com>
> > > Sent: Wednesday, September 20, 2023 12:52 PM
> > > To: Van Haaren, Harry <harry.van.haaren@intel.com>
> > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > <vvelumuri@marvell.com>; Thomas Monjalon <thomas@monjalon.net>;
> > Akhil
> > > Goyal <gakhil@marvell.com>; Jerin Jacob Kollanukkaran
> > > <jerinj@marvell.com>; Konstantin Ananyev
> > > <konstantin.v.ananyev@yandex.ru>
> > > Subject: RE: [RFC PATCH 2/3] security: add TLS record processing
> > >
> > > Hi Harry,
> > >
> > > Thanks for the review. Please see inline.
> > >
> > > Thanks,
> > > Anoob
> > >
> > > > -----Original Message-----
> > > > From: Van Haaren, Harry <harry.van.haaren@intel.com>
> > > > Sent: Wednesday, September 20, 2023 2:53 PM
> > > > To: Anoob Joseph <anoobj@marvell.com>; Thomas Monjalon
> > > > <thomas@monjalon.net>; Akhil Goyal <gakhil@marvell.com>; Jerin Jacob
> > > > Kollanukkaran <jerinj@marvell.com>; Konstantin Ananyev
> > > > <konstantin.v.ananyev@yandex.ru>
> > > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org; Matz,
> > > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > > <vvelumuri@marvell.com>
> > > > Subject: [EXT] RE: [RFC PATCH 2/3] security: add TLS record
> > > > processing
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > --
> > > > > -----Original Message-----
> > > > > From: Anoob Joseph <anoobj@marvell.com>
> > > > > Sent: Friday, August 11, 2023 8:17 AM
> > > > > To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> > > > > <gakhil@marvell.com>; Jerin Jacob <jerinj@marvell.com>; Konstantin
> > > > > Ananyev <konstantin.v.ananyev@yandex.ru>
> > > > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; dev@dpdk.org;
> > Matz,
> > > > > Olivier <olivier.matz@6wind.com>; Vidya Sagar Velumuri
> > > > > <vvelumuri@marvell.com>
> > > > > Subject: [RFC PATCH 2/3] security: add TLS record processing
> > > > >
> > > > > Add Transport Layer Security (TLS) and Datagram Transport Layer
> > > > > Security (DTLS). The protocols provide communications privacy for
> > > > > L4 protocols such as TCP & UDP.
> > > > >
> > > > > TLS (and DTLS) protocol is composed of two layers, 1. TLS Record
> > > > > Protocol 2. TLS Handshake Protocol
> > > > >
> > > > > While TLS Handshake Protocol helps in establishing security
> > > > > parameters by which client and server can communicate, TLS Record
> > > > > Protocol provides the connection security. TLS Record Protocol
> > > > > leverages symmetric cryptographic operations such as data
> > > > > encryption and authentication for providing security to the
> > communications.
> > > > >
> > > > > Cryptodevs that are capable of offloading TLS Record Protocol may
> > > > > perform other operations like IV generation, header insertion,
> > > > > atomic sequence number updates and anti-replay window check in
> > > > > addition to cryptographic transformations.
> > > > >
> > > > > The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
> > > >
> > > > From the code below, my understanding is that *ONLY* the record
> > > > layer is being added/supported? The difference is described well
> > > > above, but the intended support added is not clearly defined.
> > > >
> > > > Suggest reword the last line to clarify:
> > > > "Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and DTLS
> > 1.2."
> > >
> > > [Anoob] Indeed. Will reword as suggested.
> >
> > Thanks.
> >
> > > > > Signed-off-by: Akhil Goyal <gakhil@marvell.com>
> > > > > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > > > > Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
> > > > > ---
> > > > >  doc/guides/prog_guide/rte_security.rst |  58 +++++++++++++
> > > > >  lib/security/rte_security.c            |   4 +
> > > > >  lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
> > > > >  3 files changed, 172 insertions(+)
> > > > >
> > > > > diff --git a/doc/guides/prog_guide/rte_security.rst
> > > > > b/doc/guides/prog_guide/rte_security.rst
> > > > > index 7418e35c1b..7716d7239f 100644
> > > > > --- a/doc/guides/prog_guide/rte_security.rst
> > > > > +++ b/doc/guides/prog_guide/rte_security.rst
> > > > > @@ -399,6 +399,64 @@ The API ``rte_security_macsec_sc_create``
> > > > > returns a handle for SC,  and this handle is set in
> > > > > ``rte_security_macsec_xform``  to create a MACsec session using
> > > > > ``rte_security_session_create``.
> > > > >
> > > > > +TLS-Record Protocol
> > > > > +~~~~~~~~~~~~~~~~~~~
> > > > > +
> > > > > +The Transport Layer Protocol provides communications security
> > > > > +over the
> > > > > Internet. The protocol
> > > > > +allows client/server applications to communicate in a way that is
> > > > > +designed to
> > > > > prevent eavesdropping,
> > > > > +tampering, or message forgery.
> > > > > +
> > > > > +TLS protocol is composed of two layers: the TLS Record Protocol
> > > > > +and the TLS
> > > > > Handshake Protocol. At
> > > > > +the lowest level, layered on top of some reliable transport
> > > > > +protocol (e.g., TCP),
> > > > > is the TLS Record
> > > > > +Protocol. The TLS Record Protocol provides connection security
> > > > > +that has two
> > > > > basic properties:
> > > > > +
> > > > > +   -  The connection is private.  Symmetric cryptography is used for data
> > > > > +      encryption (e.g., AES, DES, etc.).  The keys for this
> > > > > + symmetric
> > > > encryption
> > > > > +      are generated uniquely for each connection and are based on a
> > secret
> > > > > +      negotiated by another protocol (such as the TLS Handshake
> > Protocol).
> > > > The
> > > > > +      Record Protocol can also be used without encryption.
> > > > > +
> > > > > +   -  The connection is reliable.  Message transport includes a message
> > > > > +      integrity check using a keyed MAC.  Secure hash functions (e.g.,
> > > > > +      SHA-1, etc.) are used for MAC computations.  The Record Protocol
> > > > > +      can operate without a MAC, but is generally only used in this mode
> > > > > +      while another protocol is using the Record Protocol as a transport
> > > > > +      for negotiating security parameters.
> > > > > +
> > > > > +.. code-block:: c
> > > >
> > > > The code block below isn't C? Is there a better code block type for
> > > > a text diagram?
> > >
> > > [Anoob] Valid point. I was just following the general scheme followed in
> > this file.
> > > May be, I'll introduce a .svg image for newly added code.
> >
> > This was a nit-pick that perhaps "code-block:: text-diagram" or so exists.
> > No need to make a .svg in my opinion, the text-diagrams are clear.
> 
> [Anoob] Thanks for the confirmation. I do not think "code-block:: text-diagram"
> exists. Anyway, I'll improve the diagrams to make the padding etc more clear.
> 
> >
> >
> > > > > +             Record Write                   Record Read
> > > > > +             ------------                   -----------
> > > > > +
> > > > > +             TLSPlaintext                  TLSCiphertext
> > > > > +                  |                              |
> > > > > +                  ~                              ~
> > > > > +                  |                              |
> > > > > +                  V                              V
> > > > > +        +---------|----------+        +----------|---------+
> > > > > +        | Seq. no generation |        | Seq. no generation |
> > > > > +        +---------|----------+        +----------|---------+
> > > > > +                  |                              |
> > > > > +        +---------|----------+        +----------|---------+
> > > > > +        |  Header insertion  |        |    Decryption &    |
> > > > > +        +---------|----------+        |  MAC verification  |
> > > > > +                  |                   +----------|---------+
> > > > > +        +---------|----------+                   |
> > > > > +        |  MAC generation &  |        +----------|---------+
> > > > > +        |     Encryption     |        | TLS Header removal |
> > > > > +        +---------|----------+        +----------|---------+
> > > > > +                  |                              |
> > > > > +                  ~                              ~
> > > > > +                  |                              |
> > > > > +                  V                              V
> > > > > +            TLSCiphertext                  TLSPlaintext
> > > > > +
> > > > > +Supported Versions
> > > > > +^^^^^^^^^^^^^^^^^^
> > > > > +
> > > > > +* TLS 1.2
> > > > > +* TLS 1.3
> > > > > +* DTLS 1.2
> > > > >
> > > > >  Device Features and Capabilities
> > > > >  --------------------------------- diff --git
> > > > > a/lib/security/rte_security.c b/lib/security/rte_security.c index
> > > > > c4d64bb8e9..bd7b026547 100644
> > > > > --- a/lib/security/rte_security.c
> > > > > +++ b/lib/security/rte_security.c
> > > > > @@ -282,6 +282,10 @@ rte_security_capability_get(struct
> > > > > rte_security_ctx *instance,
> > > > >  				if (capability->docsis.direction ==
> > > > >  							idx-
> >docsis.direction)
> > > > >  					return capability;
> > > > > +			} else if (idx->protocol ==
> > > > > RTE_SECURITY_PROTOCOL_TLS_RECORD) {
> > > > > +				if (capability->tls_record.ver == idx-
> > > > > >tls_record.ver &&
> > > > > +				    capability->tls_record.type == idx-
> > > > > >tls_record.type)
> > > > > +					return capability;
> > > > >  			}
> > > > >  		}
> > > > >  	}
> > > > > diff --git a/lib/security/rte_security.h
> > > > > b/lib/security/rte_security.h index 3b2df526ba..b9d064ed84 100644
> > > > > --- a/lib/security/rte_security.h
> > > > > +++ b/lib/security/rte_security.h
> > > > > @@ -620,6 +620,99 @@ struct rte_security_docsis_xform {
> > > > >  	/**< DOCSIS direction */
> > > > >  };
> > > > >
> > > > > +/** Salt len to be used with AEAD algos in TLS 1.2 */ #define
> > > > > +RTE_SECURITY_TLS_1_2_SALT_LEN 4
> > > > > +/** Salt len to be used with AEAD algos in TLS 1.3 */ #define
> > > > > +RTE_SECURITY_TLS_1_3_SALT_LEN 12
> > > > > +/** Salt len to be used with AEAD algos in DTLS 1.2 */ #define
> > > > > +RTE_SECURITY_DTLS_1_2_SALT_LEN 4
> > > > > +
> > > > > +/** TLS version */
> > > > > +enum rte_security_tls_version {
> > > > > +	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
> > > > > +	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
> > > > > +	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
> > > > > +};
> > > > > +
> > > > > +/** TLS session type */
> > > > > +enum rte_security_tls_sess_type {
> > > > > +	/** Record read session
> > > > > +	 * - Decrypt & digest verification.
> > > > > +	 */
> > > > > +	RTE_SECURITY_TLS_SESS_TYPE_READ,
> > > > > +	/** Record write session
> > > > > +	 * - Encrypt & digest generation.
> > > > > +	 */
> > > > > +	RTE_SECURITY_TLS_SESS_TYPE_WRITE, };
> > > > > +
> > > > > +/**
> > > > > + * Configure soft and hard lifetime of a TLS record session
> > > > > + *
> > > > > + * Lifetime of a TLS record session would specify the maximum
> > > > > +number of
> > > > > packets that can be
> > > > > + * processed. TLS record processing operations would start
> > > > > + failing once hard
> > > > > limit is reached.
> > > > > + *
> > > > > + * Soft limits can be specified to generate notification when the
> > > > > + TLS record
> > > > > session is approaching
> > > > > + * hard limits for lifetime. This would result in a warning
> > > > > + returned in
> > > > > ``rte_crypto_op.aux_flags``.
> > > >
> > > > Can we define "a warning" better? Perhaps an example of a soft-limit
> > > > and hard-limit, what the user can check aux_flags for, to identify?
> > > > Or link to the appropriate part of the crypto_op.aux_flags documentation
> > to help the user.
> > > >
> > >
> > > [Anoob] The concept of lifetime is present in most protocols. Idea is
> > > to limit the max number of operations performed with a session. Soft
> > > expiry notification is to help application prepare for an expiry and
> > > setup a new session before the current one expires.
> >
> > Understood, yes.
> >
> > > The idea was borrowed from IPsec which has the
> > > 'RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY' flag defined. But I
> > > realize, it should be better defined. I can rename the flag to
> > > 'RTE_CRYPTO_OP_AUX_FLAGS_SEC_SOFT_EXPIRY' to avoid redefining
> > same
> > > flag for each security offload. Do you agree to this suggestion?
> > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.co
> > > m_dpdk_latest_source_lib_cryptodev_rte-5Fcrypto.h-
> > 23L67&d=DwIFAg&c=nKj
> > > Wec2b6R0mOyPaz7xtfQ&r=jPfB8rwwviRSxyLWs2n6B-
> > WYLn1v9SyTMrT5EQqh2TU&m=F6
> > >
> > WuwwuvXG0eJ1IWAMFgcXlMdQwKFDx6C1qsQgSaDa2XGr3PZ6LEDtSw0OeC
> > DstG&s=0iO8Y
> > > sr0f4cnE8ihg40sZfEEZfPzEXvJBvcNcyOdS98&e=
> >
> > So we cannot "just rename" the flag, its an API break. It likely is possible to
> > add an additional #define with the same value as IPSEC_SOFT_EXPIRY, and
> > call it SEC_SOFT_EXPIRY.
> > Is that the best/most descriptive name? SYM_ALG_SOFT_EXPIRY? I'm not
> > sure here, input welcomed.
> >
> > Perhaps we can improve the doc-string there, to explain what it means a bit
> > more verbosely.
> 
> [Anoob] Agreed. Let me try to improve these aspects in next version. We can
> revisit this topic after that and see if we are able to reach a conclusion.
> 
> >
> > > Do note that once hard expiry is hit, the operation would fail.
> > > Expectation is, cryptodev would return 'RTE_CRYPTO_OP_STATUS_ERROR'
> > in case of errors.
> >
> > That is good.
> >
> >
> > > > > + */
> > > > > +struct rte_security_tls_record_lifetime {
> > > > > +	/** Soft expiry limit in number of packets */
> > > > > +	uint64_t packets_soft_limit;
> > > > > +	/** Hard expiry limit in number of packets */
> > > > > +	uint64_t packets_hard_limit;
> > > > > +};
> > > > > +
> > > > > +/**
> > > > > + * TLS record protocol session configuration.
> > > > > + *
> > > > > + * This structure contains data required to create a TLS record
> > > > > +security
> > > > session.
> > > > > + */
> > > > > +struct rte_security_tls_record_xform {
> > > > > +	/** TLS record version. */
> > > > > +	enum rte_security_tls_version ver;
> > > > > +	/** TLS record session type. */
> > > > > +	enum rte_security_tls_sess_type type;
> > > > > +	/** TLS record session lifetime. */
> > > > > +	struct rte_security_tls_record_lifetime life;
> > > > > +	union {
> > > > > +		/** TLS 1.2 parameters. */
> > > > > +		struct {
> > > > > +			/** Starting sequence number. */
> > > > > +			uint64_t seq_no;
> > > > > +			/** Salt to be used for AEAD algos. */
> > > > > +			uint8_t salt[RTE_SECURITY_TLS_1_2_SALT_LEN];
> > > > > +		} tls_1_2;
> > > > > +
> > > > > +		/** TLS 1.3 parameters. */
> > > > > +		struct {
> > > > > +			/** Starting sequence number. */
> > > > > +			uint64_t seq_no;
> > > > > +			/** Salt to be used for AEAD algos. */
> > > > > +			uint8_t salt[RTE_SECURITY_TLS_1_3_SALT_LEN];
> > > > > +			/**
> > > > > +			 * Minimum payload length (in case of write
> > > > sessions).
> > > > > For shorter inputs,
> > > > > +			 * the payload would be padded appropriately
> before
> > > > > performing crypto
> > > >
> > > > Replace "would be"  with "must be"? And who must do this step, is it
> > > > the application?
> > >
> > > [Anoob] Padding is performed by the PMD/cryptodev device. I'll change
> > > "would be" to "will be". Would that address your concern?
> >
> > I suppose my concern is "is it clear to PMD authors that they must implement
> > X in their PMD", and to ensure we (DPDK community) do our best to clarify
> > API demands, and to ensure future contributions are of high quality too.
> >
> > For example, could we have a security library unit-test that checks the
> > padding case, to ensure correct & consistent behaviour across different
> > crypto PMDs?
> 
> [Anoob] Glad that you brought up that point. For IPsec, we have rather extensive
> framework which covers all the features that are supported today in rte_security.
> Features like soft expiry & hard expiry are tested on platforms that support the
> same. As for TLS, we are working on adding unit test framework. It would be part
> of the next version.

Perfect, I wasn't aware of plans to add it - great! 

> > Same thoughts for SOFT and HARD error variants, (although they might
> > take... hours?) to execute.
> > But it is nice to automate-and-test these "corner cases".
> 
> [Anoob] It won't. Soft expiry can be very low value as well. It is already covered.
> Please check.
> https://elixir.bootlin.com/dpdk/latest/source/app/test/test_cryptodev.c#L10272

Ah great, good to hear.

> > It's not about wording, its about clarity between PMD devs, security library
> > devs, and application facing APIs, to ensure that DPDK provides the
> > most/best help it can to provide correctness. Does that clarify what I'd like to
> > see?
> 
> [Anoob] I understand your concern. My understanding is that for security offloads
> (rte_security lib), documentation has complete details of application & PMD
> expectations. If we have missed some features, we can take a re-look at the
> features and add as required. Other than documentation, do you have any other
> suggestions in mind?

Not right now - with additional unit tests (like IPsec has) all seems well in hand.


> > For this patchset, could we document a list of "caveats" when implementing
> > PMD functionality for TLS-record security offload, and indicate that:
> > 1) Padding must be added by the PMD based on security library flags& algo in
> > use, not application layer (I know this is demanded by the sym algos anyway,
> > but let's make it explicit)
> 
> [Anoob] Agreed. Will update documentation as required.
> 
> > 2) It is strongly recommended to build unit-tests for _SOFT and _HARD error
> > cases (potentially by "fast forwarding" the internal counters via private APIs
> > to avoid hours of enc/decryption)
> 
> [Anoob] For IPsec, it is already added. For TLS, we have plans to add the same.
> 
> >
> > I think that limits scope-impact to this patchset, but is clear for PMD
> > implementations in future what expectations are.
> > Thoughts, is that a good way forward?
> 
> [Anoob] Indeed. Having a very well defined API interface for applications is a very
> positive change. I'm open to more ideas than just updating the documentation.
> 
> Appreciate your feedback.
> 
> <snip>

[PATCH v2 0/5] add TLS record processing security offload

[Anoob Joseph]

Add Transport Layer Security (TLS) and Datagram Transport Layer Security
(DTLS). The protocols provide communications privacy for L4 protocols
such as TCP & UDP.

TLS (and DTLS) protocol is composed of two layers,
1. TLS Record Protocol
2. TLS Handshake Protocol

While TLS Handshake Protocol helps in establishing security parameters
by which client and server can communicate, TLS Record Protocol provides
the connection security. TLS Record Protocol leverages symmetric
cryptographic operations such as data encryption and authentication for
providing security to the communications.

Cryptodevs that are capable of offloading TLS Record Protocol may
perform other operations like IV generation, header insertion, atomic
sequence number updates and anti-replay window check in addition to
cryptographic transformations.

In record write operations, message content type is a per packet field
which is used in constructing the TLS header. One session is expected
to handle all types of content types and so, 'rte_crypto_op.aux_flags'
is used for passing the same.

Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and
DTLS 1.2.

Changes in v2:
- Replaced CCS with CHANGE_CIPHER_SPEC (Harry)
- Split patches to clarify lifetime tracking and notification of the
  same in rte_crypto_op (Harry)
- Use 8 bits reserved space in rte_crypto_op to pass content type from
  application to PMD (in case of record write) and from PMD to
  application (in case of of record read). TLS 1.3 has content type as
  part of trailer which would be encrypted and would be removed by PMD.
- Updated documentation (Harry)

Akhil Goyal (1):
  net: add headers for TLS/DTLS packets

Anoob Joseph (2):
  security: add TLS record processing
  security: support extra padding with TLS

Vidya Sagar Velumuri (2):
  security: support TLS record lifetime notification
  cryptodev: add details of datapath handling of TLS records

 doc/api/doxy-api-index.md              |   2 +
 doc/guides/prog_guide/rte_security.rst |  74 +++++++++++++
 doc/guides/rel_notes/release_23_11.rst |   6 ++
 lib/cryptodev/rte_crypto.h             |  50 ++++++++-
 lib/net/meson.build                    |   2 +
 lib/net/rte_dtls.h                     |  61 +++++++++++
 lib/net/rte_tls.h                      |  48 +++++++++
 lib/security/rte_security.c            |   4 +
 lib/security/rte_security.h            | 144 +++++++++++++++++++++++++
 9 files changed, 388 insertions(+), 3 deletions(-)
 create mode 100644 lib/net/rte_dtls.h
 create mode 100644 lib/net/rte_tls.h

-- 
2.25.1

[PATCH v2 1/5] net: add headers for TLS/DTLS packets

[Anoob Joseph]

From: Akhil Goyal <gakhil@marvell.com>

Added TLS and DTLS packet headers for L4 security applications.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/api/doxy-api-index.md |  2 ++
 lib/net/meson.build       |  2 ++
 lib/net/rte_dtls.h        | 61 +++++++++++++++++++++++++++++++++++++++
 lib/net/rte_tls.h         | 48 ++++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 lib/net/rte_dtls.h
 create mode 100644 lib/net/rte_tls.h

diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index fdeda13932..03e2445bb1 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -128,6 +128,8 @@ The public API headers are grouped by topics:
   [eCPRI](@ref rte_ecpri.h),
   [PDCP hdr](@ref rte_pdcp_hdr.h),
   [PDCP](@ref rte_pdcp.h),
+  [TLS](@ref rte_tls.h),
+  [DTLS](@ref rte_dtls.h),
   [L2TPv2](@ref rte_l2tpv2.h),
   [PPP](@ref rte_ppp.h),
   [IB](@ref rte_ib.h)
diff --git a/lib/net/meson.build b/lib/net/meson.build
index b1bc27bad5..0b69138949 100644
--- a/lib/net/meson.build
+++ b/lib/net/meson.build
@@ -5,6 +5,8 @@ headers = files(
         'rte_ip.h',
         'rte_tcp.h',
         'rte_udp.h',
+        'rte_tls.h',
+        'rte_dtls.h',
         'rte_esp.h',
         'rte_sctp.h',
         'rte_icmp.h',
diff --git a/lib/net/rte_dtls.h b/lib/net/rte_dtls.h
new file mode 100644
index 0000000000..49bded1d96
--- /dev/null
+++ b/lib/net/rte_dtls.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Marvell.
+ */
+
+#ifndef RTE_DTLS_H
+#define RTE_DTLS_H
+
+/**
+ * @file
+ *
+ * Datagram transport layer security(DTLS) related defines.
+ */
+
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define RTE_DTLS_TYPE_INVALID			0 /**< Invalid DTLS message type. */
+#define RTE_DTLS_TYPE_CHANGE_CIPHER_SPEC	20 /**< Change cipher spec message. */
+#define RTE_DTLS_TYPE_ALERT			21 /**< Alert message. */
+#define RTE_DTLS_TYPE_HANDSHAKE			22 /**< Handshake message for DTLS. */
+#define RTE_DTLS_TYPE_APPDATA			23 /**< DTLS application data message. */
+#define RTE_DTLS_TYPE_HEARTBEAT			24 /**< DTLS 1.3 heartbeat message. */
+#define RTE_DTLS_TYPE_CIPHERTEXT_WITH_CID	25 /**< DTLS 1.3 ciphertext with CID message. */
+#define RTE_DTLS_TYPE_ACK			26 /**< DTLS 1.3 ACK message. */
+#define RTE_DTLS_TYPE_MAX			255 /**< Maximum value as DTLS content type. */
+
+#define RTE_DTLS_VERSION_1_2	0xFEFD /**< DTLS 1.2 version. 1's complement of 1.2. */
+#define RTE_DTLS_VERSION_1_3	0xFEFC /**< DTLS 1.3 version. 1's complement of 1.3. */
+
+/**
+ * DTLS Header
+ */
+__extension__
+struct rte_dtls_hdr {
+	/** Content type of DTLS packet. Defined as RTE_DTLS_TYPE_*. */
+	uint8_t type;
+	/** DTLS Version defined as RTE_DTLS_VERSION*. */
+	rte_be16_t version;
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+	/** The sequence number for the DTLS record. */
+	uint64_t sequence_number : 48;
+	/** A counter value that is incremented on every cipher state change. */
+	uint64_t epoch : 16;
+#elif RTE_BYTE_ORDER == RTE_BIG_ENDIAN
+	/** A counter value that is incremented on every cipher state change. */
+	uint64_t epoch : 16;
+	/** The sequence number for the DTLS record. */
+	uint64_t sequence_number : 48;
+#endif
+	/** The length (in bytes) of the following DTLS packet. */
+	rte_be16_t length;
+} __rte_packed;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_DTLS_H */
diff --git a/lib/net/rte_tls.h b/lib/net/rte_tls.h
new file mode 100644
index 0000000000..ee1e3aa249
--- /dev/null
+++ b/lib/net/rte_tls.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2023 Marvell.
+ */
+
+#ifndef RTE_TLS_H
+#define RTE_TLS_H
+
+/**
+ * @file
+ *
+ * Transport layer security(TLS) related defines.
+ */
+
+#include <rte_byteorder.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define RTE_TLS_TYPE_INVALID		0 /**< Invalid TLS message type. */
+#define RTE_TLS_TYPE_CHANGE_CIPHER_SPEC	20 /**< Change cipher spec message. */
+#define RTE_TLS_TYPE_ALERT		21 /**< Alert message. */
+#define RTE_TLS_TYPE_HANDSHAKE		22 /**< Handshake message for TLS. */
+#define RTE_TLS_TYPE_APPDATA		23 /**< TLS application data message. */
+#define RTE_TLS_TYPE_HEARTBEAT		24 /**< TLS 1.3 heartbeat message. */
+#define RTE_TLS_TYPE_MAX		255 /**< Maximum value as TLS content type. */
+
+#define RTE_TLS_VERSION_1_2	0x0303 /**< TLS 1.2 version. */
+#define RTE_TLS_VERSION_1_3	0x0304 /**< TLS 1.3 version. */
+
+/**
+ * TLS Header
+ */
+__extension__
+struct rte_tls_hdr {
+	/** Content type of TLS packet. Defined as RTE_TLS_TYPE_*. */
+	uint8_t type;
+	/** TLS Version defined as RTE_TLS_VERSION*. */
+	rte_be16_t version;
+	/** The length (in bytes) of the following TLS packet. */
+	rte_be16_t length;
+} __rte_packed;
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* RTE_TLS_H */
-- 
2.25.1

[PATCH v2 2/5] security: add TLS record processing

[Anoob Joseph]

Add Transport Layer Security (TLS) and Datagram Transport Layer Security
(DTLS). The protocols provide communications privacy for L4 protocols
such as TCP & UDP.

TLS (and DTLS) protocol is composed of two layers,
1. TLS Record Protocol
2. TLS Handshake Protocol

While TLS Handshake Protocol helps in establishing security parameters
by which client and server can communicate, TLS Record Protocol provides
the connection security. TLS Record Protocol leverages symmetric
cryptographic operations such as data encryption and authentication for
providing security to the communications.

Cryptodevs that are capable of offloading TLS Record Protocol may
perform other operations like IV generation, header insertion, atomic
sequence number updates and anti-replay window check in addition to
cryptographic transformations.

Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and
DTLS 1.2.

Signed-off-by: Akhil Goyal <gakhil@marvell.com>
Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/guides/prog_guide/rte_security.rst |  62 ++++++++++++++
 lib/security/rte_security.c            |   4 +
 lib/security/rte_security.h            | 110 +++++++++++++++++++++++++
 3 files changed, 176 insertions(+)

diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
index ad8c6374bd..f90dee5df0 100644
--- a/doc/guides/prog_guide/rte_security.rst
+++ b/doc/guides/prog_guide/rte_security.rst
@@ -399,6 +399,66 @@ The API ``rte_security_macsec_sc_create`` returns a handle for SC,
 and this handle is set in ``rte_security_macsec_xform``
 to create a MACsec session using ``rte_security_session_create``.
 
+TLS-Record Protocol
+~~~~~~~~~~~~~~~~~~~
+
+The Transport Layer Protocol provides communications security over the Internet. The protocol
+allows client/server applications to communicate in a way that is designed to prevent eavesdropping,
+tampering, or message forgery.
+
+TLS protocol is composed of two layers: the TLS Record Protocol and the TLS Handshake Protocol. At
+the lowest level, layered on top of some reliable transport protocol (e.g., TCP), is the TLS Record
+Protocol. The TLS Record Protocol provides connection security that has two basic properties:
+
+   -  The connection is private.  Symmetric cryptography is used for data
+      encryption (e.g., AES, DES, etc.).  The keys for this symmetric encryption
+      are generated uniquely for each connection and are based on a secret
+      negotiated during TLS Handshake Protocol. The Record Protocol can also be
+      used without encryption.
+
+   -  The connection is reliable.  Message transport includes a message
+      integrity check using a keyed MAC.  Secure hash functions (e.g.,
+      SHA-1, etc.) are used for MAC computations. The Record Protocol can
+      operate without a MAC when it is being used as a transport for negotiating
+      security parameters by another protocol.
+
+.. code-block:: c
+
+             Record Write                   Record Read
+             ------------                   -----------
+
+             TLSPlaintext                  TLSCiphertext
+                  |                              |
+                  ~                              ~
+                  |                              |
+                  V                              V
+       +----------|-----------+       +----------|-----------+
+       | Generate sequence no.|       | Generate sequence no.|
+       +----------|-----------+       +----------------------+
+                  |                   |    AR check (DTLS)   |
+       +----------|-----------+       +----------|-----------+
+       |  Insert TLS header   |                  |
+       |     & trailer.       |       +----------|-----------+
+       | (including padding)  |       | Decrypt & MAC verify |
+       +----------|-----------+       +----------|-----------+
+                  |                              |
+        +---------|-----------+       +----------|-----------+
+        |    MAC generate &   |       |  Remove TLS header   |
+        |      Encrypt        |       |      & trailer.      |
+        +---------|-----------+       | (including padding)  |
+                  |                   +----------|-----------+
+                  |                              |
+                  ~                              ~
+                  |                              |
+                  V                              V
+            TLSCiphertext                  TLSPlaintext
+
+Supported Versions
+^^^^^^^^^^^^^^^^^^
+
+* TLS 1.2
+* TLS 1.3
+* DTLS 1.2
 
 Device Features and Capabilities
 ---------------------------------
@@ -701,6 +761,8 @@ PDCP related configuration parameters are defined in ``rte_security_pdcp_xform``
 
 DOCSIS related configuration parameters are defined in ``rte_security_docsis_xform``
 
+TLS record related configuration parameters are defined in ``rte_security_tls_record_xform``
+
 
 Security API
 ~~~~~~~~~~~~
diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index ab44bbe0f0..04872ec1a0 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -314,6 +314,10 @@ rte_security_capability_get(void *ctx, struct rte_security_capability_idx *idx)
 						RTE_SECURITY_PROTOCOL_MACSEC) {
 				if (idx->macsec.alg == capability->macsec.alg)
 					return capability;
+			} else if (idx->protocol == RTE_SECURITY_PROTOCOL_TLS_RECORD) {
+				if (capability->tls_record.ver == idx->tls_record.ver &&
+				    capability->tls_record.type == idx->tls_record.type)
+					return capability;
 			}
 		}
 	}
diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index c9cc7a45a6..54c32c1147 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -597,6 +597,98 @@ struct rte_security_docsis_xform {
 	/**< DOCSIS direction */
 };
 
+/** Implicit nonce length to be used with AEAD algos in TLS 1.2 */
+#define RTE_SECURITY_TLS_1_2_IMP_NONCE_LEN 4
+/** Implicit nonce length to be used with AEAD algos in TLS 1.3 */
+#define RTE_SECURITY_TLS_1_3_IMP_NONCE_LEN 12
+/** Implicit nonce length to be used with AEAD algos in DTLS 1.2 */
+#define RTE_SECURITY_DTLS_1_2_IMP_NONCE_LEN 4
+
+/** TLS version */
+enum rte_security_tls_version {
+	RTE_SECURITY_VERSION_TLS_1_2,	/**< TLS 1.2 */
+	RTE_SECURITY_VERSION_TLS_1_3,	/**< TLS 1.3 */
+	RTE_SECURITY_VERSION_DTLS_1_2,	/**< DTLS 1.2 */
+};
+
+/** TLS session type */
+enum rte_security_tls_sess_type {
+	/** Record read session
+	 * - Decrypt & digest verification.
+	 */
+	RTE_SECURITY_TLS_SESS_TYPE_READ,
+	/** Record write session
+	 * - Encrypt & digest generation.
+	 */
+	RTE_SECURITY_TLS_SESS_TYPE_WRITE,
+};
+
+/**
+ * TLS record session options
+ */
+struct rte_security_tls_record_sess_options {
+	/** Disable IV generation in PMD
+	 *
+	 * * 1: Disable IV generation in PMD. When disabled, IV provided in rte_crypto_op will be
+	 *      used by the PMD.
+	 *
+	 * * 0: Enable IV generation in PMD. When enabled, PMD generated random value would be used
+	 *      and application is not required to provide IV.
+	 */
+	uint32_t iv_gen_disable : 1;
+};
+
+/**
+ * TLS record protocol session configuration.
+ *
+ * This structure contains data required to create a TLS record security session.
+ */
+struct rte_security_tls_record_xform {
+	/** TLS record version. */
+	enum rte_security_tls_version ver;
+	/** TLS record session type. */
+	enum rte_security_tls_sess_type type;
+	/** TLS record session options. */
+	struct rte_security_tls_record_sess_options options;
+	union {
+		/** TLS 1.2 parameters. */
+		struct {
+			/** Starting sequence number. */
+			uint64_t seq_no;
+			/** Implicit nonce to be used for AEAD algos. */
+			uint8_t imp_nonce[RTE_SECURITY_TLS_1_2_IMP_NONCE_LEN];
+		} tls_1_2;
+
+		/** TLS 1.3 parameters. */
+		struct {
+			/** Starting sequence number. */
+			uint64_t seq_no;
+			/** Implicit nonce to be used for AEAD algos. */
+			uint8_t imp_nonce[RTE_SECURITY_TLS_1_3_IMP_NONCE_LEN];
+			/**
+			 * Minimum payload length (in case of write sessions). For shorter inputs,
+			 * the payload would be padded appropriately before performing crypto
+			 * transformations.
+			 */
+			uint32_t min_payload_len;
+		} tls_1_3;
+
+		/** DTLS 1.2 parameters */
+		struct {
+			/** Epoch value to be used. */
+			uint16_t epoch;
+			/** 6B starting sequence number to be used. */
+			uint64_t seq_no;
+			/** Implicit nonce to be used for AEAD algos. */
+			uint8_t imp_nonce[RTE_SECURITY_DTLS_1_2_IMP_NONCE_LEN];
+			/** Anti replay window size to enable sequence replay attack handling.
+			 * Anti replay check is disabled if the window size is 0.
+			 */
+			uint32_t ar_win_sz;
+		} dtls_1_2;
+	};
+};
+
 /**
  * Security session action type.
  */
@@ -634,6 +726,8 @@ enum rte_security_session_protocol {
 	/**< PDCP Protocol */
 	RTE_SECURITY_PROTOCOL_DOCSIS,
 	/**< DOCSIS Protocol */
+	RTE_SECURITY_PROTOCOL_TLS_RECORD,
+	/**< TLS Record Protocol */
 };
 /* >8 End enumeration of rte_security_session_protocol. */
 
@@ -651,6 +745,7 @@ struct rte_security_session_conf {
 		struct rte_security_macsec_xform macsec;
 		struct rte_security_pdcp_xform pdcp;
 		struct rte_security_docsis_xform docsis;
+		struct rte_security_tls_record_xform tls_record;
 	};
 	/**< Configuration parameters for security session */
 	struct rte_crypto_sym_xform *crypto_xform;
@@ -1217,6 +1312,17 @@ struct rte_security_capability {
 			/**< DOCSIS direction */
 		} docsis;
 		/**< DOCSIS capability */
+		struct {
+			enum rte_security_tls_version ver;
+			/**< TLS record version. */
+			enum rte_security_tls_sess_type type;
+			/**< TLS record session type. */
+			uint32_t ar_win_size;
+			/**< Maximum anti replay window size supported for DTLS 1.2 record read
+			 * operation. Value of 0 means anti replay check is not supported.
+			 */
+		} tls_record;
+		/**< TLS record capability */
 	};
 
 	const struct rte_cryptodev_capabilities *crypto_capabilities;
@@ -1280,6 +1386,10 @@ struct rte_security_capability_idx {
 		struct {
 			enum rte_security_macsec_alg alg;
 		} macsec;
+		struct {
+			enum rte_security_tls_version ver;
+			enum rte_security_tls_sess_type type;
+		} tls_record;
 	};
 };
 
-- 
2.25.1

[PATCH v2 3/5] security: support extra padding with TLS

[Anoob Joseph]

In TLS record write protocol (encrypt), application may request for
extra padding in addition to the default padding which ensures that
crypto payload is aligned to block size. This is required to hide
the size of the traffic from an observer.

Extend the usage of ``rte_crypto_op.aux_flags`` to allow users to
provide extra padding in units of 8B. It is an optional feature and any
device that supports the same can declare so by making use of
corresponding capability.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 lib/cryptodev/rte_crypto.h  | 14 +++++++++++++-
 lib/security/rte_security.h | 16 ++++++++++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index 9b8d0331a4..7b8f2bdc6d 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -99,8 +99,20 @@ struct rte_crypto_op {
 			/**< operation session type */
 			uint8_t aux_flags;
 			/**< Operation specific auxiliary/additional flags.
-			 * These flags carry additional information from the
+			 * These flags carry additional information from/to the
 			 * operation. Processing of the same is optional.
+			 *
+			 * The flags are defined as RTE_CRYPTO_OP_AUX_FLAGS_* and would be set by
+			 * PMD for application consumption when the status is
+			 * RTE_CRYPTO_OP_STATUS_SUCCESS. In case of errors, the value of this
+			 * field is undefined.
+			 *
+			 * With TLS record offload (RTE_SECURITY_PROTOCOL_TLS_RECORD),
+			 * application may provide the extra padding required for the plaintext
+			 * provided. This field can be used for passing the same in units of 8B. The
+			 * value would be set by application for PMD consumption.
+			 *
+			 * @see struct rte_security_tls_record_sess_options
 			 */
 			uint8_t reserved[2];
 			/**< Reserved bytes to fill 64 bits for
diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 54c32c1147..89e61e10ad 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -636,6 +636,22 @@ struct rte_security_tls_record_sess_options {
 	 *      and application is not required to provide IV.
 	 */
 	uint32_t iv_gen_disable : 1;
+	/** Enable extra padding
+	 *
+	 *  TLS allows user to pad the plain text to hide the actual size of the record. This is
+	 *  required to achieve traffic flow confidentiality in case of TLS/DTLS flows. This padding
+	 *  is in addition to the default padding performed by PMD (which ensures ciphertext is
+	 *  aligned to block size).
+	 *
+	 *  On supported devices, application may pass the required additional padding via
+	 *  ``rte_crypto_op.aux_flags`` field.
+	 *
+	 * 1 : Enable extra padding of the plain text provided. The extra padding value would be
+	 *     read from ``rte_crypto_op.aux_flags``.
+	 *
+	 * 0 : Disable extra padding
+	 */
+	uint32_t extra_padding_enable : 1;
 };
 
 /**
-- 
2.25.1

[PATCH v2 4/5] security: support TLS record lifetime notification

[Anoob Joseph]

From: Vidya Sagar Velumuri <vvelumuri@marvell.com>

Support TLS record lifetime notification. TLS record sessions may need
to be renegotiated after a specific number of records are processed. For
devices that are capable of tracking lifetime, application may request
to do so by configuring the lifetime parameters in session. Upon soft
expiry, PMD will set the notification in `rte_crypto_op.aux_flags` field.

Hard expiration of the session would mean any subsequent crypto
operation would fail.

Extend `rte_crypto_op.aux_flags` to support all sessions that may have a
lifetime notification and include TLS record lifetime under the same.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 lib/cryptodev/rte_crypto.h  | 11 ++++++++++-
 lib/security/rte_security.h | 18 ++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index 7b8f2bdc6d..9fe3e3d529 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -64,9 +64,18 @@ enum rte_crypto_op_sess_type {
 	RTE_CRYPTO_OP_SECURITY_SESSION	/**< Security session crypto operation */
 };
 
+/* Auxiliary flags related to crypto operation */
+#define RTE_CRYPTO_OP_AUX_FLAGS_SESS_SOFT_EXPIRY (1 << 0)
+/**< Session soft expiry limit has been reached. Applicable for any session that has a soft lifetime
+ * feature supported.
+ *
+ * @see rte_security_ipsec_lifetime
+ * @see rte_security_tls_record_lifetime
+ */
+
 /* Auxiliary flags related to IPsec offload with RTE_SECURITY */
 
-#define RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY (1 << 0)
+#define RTE_CRYPTO_OP_AUX_FLAGS_IPSEC_SOFT_EXPIRY RTE_CRYPTO_OP_AUX_FLAGS_SESS_SOFT_EXPIRY
 /**< SA soft expiry limit has been reached */
 
 /**
diff --git a/lib/security/rte_security.h b/lib/security/rte_security.h
index 89e61e10ad..00d2a29a01 100644
--- a/lib/security/rte_security.h
+++ b/lib/security/rte_security.h
@@ -654,6 +654,22 @@ struct rte_security_tls_record_sess_options {
 	uint32_t extra_padding_enable : 1;
 };
 
+/**
+ * Configure soft and hard lifetime of a TLS record session
+ *
+ * Lifetime of a TLS record session would specify the maximum number of packets that can be
+ * processed. TLS record processing operations would start failing once hard limit is reached.
+ *
+ * Soft limits can be specified to generate notification when the TLS record session is approaching
+ * hard limits for lifetime. This would result in a warning returned in ``rte_crypto_op.aux_flags``.
+ */
+struct rte_security_tls_record_lifetime {
+	/** Soft expiry limit in number of packets */
+	uint64_t packets_soft_limit;
+	/** Hard expiry limit in number of packets */
+	uint64_t packets_hard_limit;
+};
+
 /**
  * TLS record protocol session configuration.
  *
@@ -666,6 +682,8 @@ struct rte_security_tls_record_xform {
 	enum rte_security_tls_sess_type type;
 	/** TLS record session options. */
 	struct rte_security_tls_record_sess_options options;
+	/** TLS record session lifetime. */
+	struct rte_security_tls_record_lifetime life;
 	union {
 		/** TLS 1.2 parameters. */
 		struct {
-- 
2.25.1

[PATCH v2 5/5] cryptodev: add details of datapath handling of TLS records

[Anoob Joseph]

From: Vidya Sagar Velumuri <vvelumuri@marvell.com>

TLS/DTLS record processing requires content type to be provided per
packet (for record write operation). Extend usage of reserved fields in
rte_crypto_op for the same purpose.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Signed-off-by: Vidya Sagar Velumuri <vvelumuri@marvell.com>
---
 doc/guides/prog_guide/rte_security.rst | 12 ++++++++++++
 doc/guides/rel_notes/release_23_11.rst |  6 ++++++
 lib/cryptodev/rte_crypto.h             | 25 ++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/doc/guides/prog_guide/rte_security.rst b/doc/guides/prog_guide/rte_security.rst
index f90dee5df0..7a25a7e649 100644
--- a/doc/guides/prog_guide/rte_security.rst
+++ b/doc/guides/prog_guide/rte_security.rst
@@ -453,6 +453,18 @@ Protocol. The TLS Record Protocol provides connection security that has two basi
                   V                              V
             TLSCiphertext                  TLSPlaintext
 
+TLS and DTLS header formation (in record write operation) would depend on
+type of content. It is a per packet variable and would need to be handled by
+the same session. Application may pass this info to a cryptodev performing
+lookaside protocol offload by passing the same in ``rte_crypto_op.param1``.
+
+In record read operation, application is required to preserve any info it may
+need from the TLS/DTLS header (such as content type and sequence number) as the
+cryptodev would remove the header and padding as part of the lookaside protocol
+processing. With TLS 1.3, the actual content type is part of the trailer (before
+padding) and would be stripped by the PMD. For applications that may need this
+info, PMD would return the value in ``rte_crypto_op.param1`` field.
+
 Supported Versions
 ^^^^^^^^^^^^^^^^^^
 
diff --git a/doc/guides/rel_notes/release_23_11.rst b/doc/guides/rel_notes/release_23_11.rst
index 250735efa9..663d1e9cf9 100644
--- a/doc/guides/rel_notes/release_23_11.rst
+++ b/doc/guides/rel_notes/release_23_11.rst
@@ -83,6 +83,12 @@ New Features
   Similar to out of place processing support for lookaside security session, added
   the same support for inline ingress security session.
 
+* **Added support for TLS and DTLS record in rte_security.**
+
+  Added TLS and DTLS record transform for rte_security session and added
+  enhancements to ``rte_crypto_op`` fields to handle all datapath requirements
+  of TLS and DTLS. The support is added for TLS 1.2, TLS 1.3 and DTLS 1.2.
+
 * **Updated ipsec_mb crypto driver.**
 
   Added support for digest encrypted to AESNI_MB asynchronous crypto driver.
diff --git a/lib/cryptodev/rte_crypto.h b/lib/cryptodev/rte_crypto.h
index 9fe3e3d529..34aacd9312 100644
--- a/lib/cryptodev/rte_crypto.h
+++ b/lib/cryptodev/rte_crypto.h
@@ -123,7 +123,30 @@ struct rte_crypto_op {
 			 *
 			 * @see struct rte_security_tls_record_sess_options
 			 */
-			uint8_t reserved[2];
+			union {
+				struct {
+					uint8_t content_type;
+					/**< Content type. The field can act both as input
+					 * and output.
+					 *
+					 * As input, for passing message type in case of record
+					 * write (encrypt) operation. Applicable for,
+					 * 1. TLS 1.2
+					 * 2. TLS 1.3
+					 * 3. DTLS 1.2
+					 *
+					 * As output, for returning message type in case of record
+					 * read (decrypt) operation. Applicable for,
+					 * 1. TLS 1.3
+					 *
+					 * Message types are listed as RTE_TLS_TYPE_* and
+					 * RTE_DTLS_TYPE_*.
+					 */
+				} tls_record;
+				/**< TLS record */
+			} param1;
+			/**< Additional per operation parameter 1. */
+			uint8_t reserved[1];
 			/**< Reserved bytes to fill 64 bits for
 			 * future additions
 			 */
-- 
2.25.1

RE: [PATCH v2 0/5] add TLS record processing security offload

[Akhil Goyal]

Hi Harry,

Do you have any further comments on this series?

Regards,
Akhil

> -----Original Message-----
> From: Anoob Joseph <anoobj@marvell.com>
> Sent: Tuesday, October 3, 2023 4:19 PM
> To: Thomas Monjalon <thomas@monjalon.net>; Akhil Goyal
> <gakhil@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Harry
> van Haaren <harry.van.haaren@intel.com>
> Cc: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; dev@dpdk.org; Olivier Matz
> <olivier.matz@6wind.com>; Vidya Sagar Velumuri <vvelumuri@marvell.com>
> Subject: [PATCH v2 0/5] add TLS record processing security offload
> 
> Add Transport Layer Security (TLS) and Datagram Transport Layer Security
> (DTLS). The protocols provide communications privacy for L4 protocols
> such as TCP & UDP.
> 
> TLS (and DTLS) protocol is composed of two layers,
> 1. TLS Record Protocol
> 2. TLS Handshake Protocol
> 
> While TLS Handshake Protocol helps in establishing security parameters
> by which client and server can communicate, TLS Record Protocol provides
> the connection security. TLS Record Protocol leverages symmetric
> cryptographic operations such as data encryption and authentication for
> providing security to the communications.
> 
> Cryptodevs that are capable of offloading TLS Record Protocol may
> perform other operations like IV generation, header insertion, atomic
> sequence number updates and anti-replay window check in addition to
> cryptographic transformations.
> 
> In record write operations, message content type is a per packet field
> which is used in constructing the TLS header. One session is expected
> to handle all types of content types and so, 'rte_crypto_op.aux_flags'
> is used for passing the same.
> 
> Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and
> DTLS 1.2.
> 
> Changes in v2:
> - Replaced CCS with CHANGE_CIPHER_SPEC (Harry)
> - Split patches to clarify lifetime tracking and notification of the
>   same in rte_crypto_op (Harry)
> - Use 8 bits reserved space in rte_crypto_op to pass content type from
>   application to PMD (in case of record write) and from PMD to
>   application (in case of of record read). TLS 1.3 has content type as
>   part of trailer which would be encrypted and would be removed by PMD.
> - Updated documentation (Harry)
> 
> Akhil Goyal (1):
>   net: add headers for TLS/DTLS packets
> 
> Anoob Joseph (2):
>   security: add TLS record processing
>   security: support extra padding with TLS
> 
> Vidya Sagar Velumuri (2):
>   security: support TLS record lifetime notification
>   cryptodev: add details of datapath handling of TLS records
> 
>  doc/api/doxy-api-index.md              |   2 +
>  doc/guides/prog_guide/rte_security.rst |  74 +++++++++++++
>  doc/guides/rel_notes/release_23_11.rst |   6 ++
>  lib/cryptodev/rte_crypto.h             |  50 ++++++++-
>  lib/net/meson.build                    |   2 +
>  lib/net/rte_dtls.h                     |  61 +++++++++++
>  lib/net/rte_tls.h                      |  48 +++++++++
>  lib/security/rte_security.c            |   4 +
>  lib/security/rte_security.h            | 144 +++++++++++++++++++++++++
>  9 files changed, 388 insertions(+), 3 deletions(-)
>  create mode 100644 lib/net/rte_dtls.h
>  create mode 100644 lib/net/rte_tls.h
> 
> --
> 2.25.1

RE: [PATCH v2 0/5] add TLS record processing security offload

[Van Haaren, Harry]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, October 4, 2023 11:51 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Cc: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; dev@dpdk.org; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Anoob Joseph <anoobj@marvell.com>; Matz, Olivier
> <olivier.matz@6wind.com>; Vidya Sagar Velumuri <vvelumuri@marvell.com>
> Subject: RE: [PATCH v2 0/5] add TLS record processing security offload
> 
> Hi Harry,
> 
> Do you have any further comments on this series?

No additional comments here, thanks for rework Anoob.

Series-Acked-By: Harry van Haaren <harry.van.haaren@intel.com>

<snip>

RE: [PATCH v2 0/5] add TLS record processing security offload

[Akhil Goyal]

> Subject: [PATCH v2 0/5] add TLS record processing security offload
> 
> Add Transport Layer Security (TLS) and Datagram Transport Layer Security
> (DTLS). The protocols provide communications privacy for L4 protocols
> such as TCP & UDP.
> 
> TLS (and DTLS) protocol is composed of two layers,
> 1. TLS Record Protocol
> 2. TLS Handshake Protocol
> 
> While TLS Handshake Protocol helps in establishing security parameters
> by which client and server can communicate, TLS Record Protocol provides
> the connection security. TLS Record Protocol leverages symmetric
> cryptographic operations such as data encryption and authentication for
> providing security to the communications.
> 
> Cryptodevs that are capable of offloading TLS Record Protocol may
> perform other operations like IV generation, header insertion, atomic
> sequence number updates and anti-replay window check in addition to
> cryptographic transformations.
> 
> In record write operations, message content type is a per packet field
> which is used in constructing the TLS header. One session is expected
> to handle all types of content types and so, 'rte_crypto_op.aux_flags'
> is used for passing the same.
> 
> Support for TLS record protocol is added for TLS 1.2, TLS 1.3 and
> DTLS 1.2.
> 
> Changes in v2:
> - Replaced CCS with CHANGE_CIPHER_SPEC (Harry)
> - Split patches to clarify lifetime tracking and notification of the
>   same in rte_crypto_op (Harry)
> - Use 8 bits reserved space in rte_crypto_op to pass content type from
>   application to PMD (in case of record write) and from PMD to
>   application (in case of of record read). TLS 1.3 has content type as
>   part of trailer which would be encrypted and would be removed by PMD.
> - Updated documentation (Harry)
> 
> Akhil Goyal (1):
>   net: add headers for TLS/DTLS packets
> 
> Anoob Joseph (2):
>   security: add TLS record processing
>   security: support extra padding with TLS
> 
> Vidya Sagar Velumuri (2):
>   security: support TLS record lifetime notification
>   cryptodev: add details of datapath handling of TLS records
> 
>  doc/api/doxy-api-index.md              |   2 +
>  doc/guides/prog_guide/rte_security.rst |  74 +++++++++++++
>  doc/guides/rel_notes/release_23_11.rst |   6 ++
>  lib/cryptodev/rte_crypto.h             |  50 ++++++++-
>  lib/net/meson.build                    |   2 +
>  lib/net/rte_dtls.h                     |  61 +++++++++++
>  lib/net/rte_tls.h                      |  48 +++++++++
>  lib/security/rte_security.c            |   4 +
>  lib/security/rte_security.h            | 144 +++++++++++++++++++++++++
>  9 files changed, 388 insertions(+), 3 deletions(-)
>  create mode 100644 lib/net/rte_dtls.h
>  create mode 100644 lib/net/rte_tls.h
> 
Series Applied to dpdk-next-crypto
Updated comments with minor formatting across the patches. Please check.

Thanks.