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

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----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