[RFC,2/3] security: add TLS record processing
Checks
Commit Message
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(+)
Comments
> -----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
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
> -----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>
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>
> -----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>
@@ -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
---------------------------------
@@ -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;
}
}
}
@@ -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;
};
};