From: Anoob Joseph <anoobj@marvell.com>
To: "Van Haaren, Harry" <harry.van.haaren@intel.com>
Cc: Hemant Agrawal <hemant.agrawal@nxp.com>,
"dev@dpdk.org" <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
Date: Wed, 20 Sep 2023 11:51:54 +0000 [thread overview]
Message-ID: <PH0PR18MB4672910A66F10B0F788518ECDFF9A@PH0PR18MB4672.namprd18.prod.outlook.com> (raw)
In-Reply-To: <PH8PR11MB68036ABE92D59B59B85F665DD7F9A@PH8PR11MB6803.namprd11.prod.outlook.com>
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
next prev parent reply other threads:[~2023-09-20 11:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 7:17 [RFC PATCH 0/3] add TLS record processing security offload Anoob Joseph
2023-08-11 7:17 ` [RFC PATCH 1/3] net: add headers for TLS/DTLS packets Anoob Joseph
2023-09-20 9:22 ` Van Haaren, Harry
2023-08-11 7:17 ` [RFC PATCH 2/3] security: add TLS record processing Anoob Joseph
2023-09-20 9:23 ` Van Haaren, Harry
2023-09-20 11:51 ` Anoob Joseph [this message]
2023-09-21 8:38 ` Van Haaren, Harry
2023-09-21 10:55 ` Anoob Joseph
2023-09-21 11:01 ` Van Haaren, Harry
2023-08-11 7:17 ` [RFC PATCH 3/3] cryptodev: add details of datapath handling of TLS records Anoob Joseph
2023-09-20 9:24 ` Van Haaren, Harry
2023-09-20 9:22 ` [RFC PATCH 0/3] add TLS record processing security offload Van Haaren, Harry
2023-10-03 10:48 ` [PATCH v2 0/5] " Anoob Joseph
2023-10-03 10:48 ` [PATCH v2 1/5] net: add headers for TLS/DTLS packets Anoob Joseph
2023-10-03 10:48 ` [PATCH v2 2/5] security: add TLS record processing Anoob Joseph
2023-10-03 10:48 ` [PATCH v2 3/5] security: support extra padding with TLS Anoob Joseph
2023-10-03 10:48 ` [PATCH v2 4/5] security: support TLS record lifetime notification Anoob Joseph
2023-10-03 10:48 ` [PATCH v2 5/5] cryptodev: add details of datapath handling of TLS records Anoob Joseph
2023-10-04 10:51 ` [PATCH v2 0/5] add TLS record processing security offload Akhil Goyal
2023-10-04 15:44 ` Van Haaren, Harry
2023-10-09 20:08 ` Akhil Goyal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=PH0PR18MB4672910A66F10B0F788518ECDFF9A@PH0PR18MB4672.namprd18.prod.outlook.com \
--to=anoobj@marvell.com \
--cc=dev@dpdk.org \
--cc=gakhil@marvell.com \
--cc=harry.van.haaren@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=jerinj@marvell.com \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=olivier.matz@6wind.com \
--cc=thomas@monjalon.net \
--cc=vvelumuri@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).