Example: crypto_accelerator object extern #53

pbhide · 2022-08-29T19:21:06Z

This example demonstrates a definition of a crypto_accelerator
engine as an extern object and its use.

The example assumes that inline accelerators are at the end of
a pipeline and operate on the packet after it is deparsed.
The packet is recirculated after decryption so that the original
packet can be processed again.

On encryption, the packet is sent out after encrypt w/o a need
for recirculation.

The example shows the use of get_results() method in parser()
as well as MainControl()

thomascalvert-xlnx · 2022-09-26T20:31:09Z

examples/crypto-accelerator.p4

+
+    /// Operation
+    void encrypt<T>(in T enable_auth);
+    void decrypt<T>(in T enable_auth);


Following today's discussion, perhaps these should explicitly say these operations are asynchronous, e.g. mark_for_encrypt, encrypt_async, prime_encryption, etc.

All methods are async as per the assumption stated in the PR's description. In the proposed architecture the crypto accelerator is at the end of the pipeline. Since all methods are async, I think it is not necessary to name them as async. This also seems to be the convention with other externs such as drop_packet() or send_to_port() These actions take effect only after completing the execution of the current pipeline (including deparser).
See description of drop_packet here -
https://github.com/p4lang/pna/blob/main/pna.p4#L642

I can add more description to this effect in the comments.. but prefer not to change the names.
When we add sync method we should name it as such, since that does not fit current architecture.

changed the names as encrypt -> enable_encrypt, decrypt -> enable_decrypt and also added some comments to indicate that this operation happens after deparser.

fix compilation error

jfingerh · 2022-10-02T17:40:58Z

@pbhide Would it be reasonable to give a specific example of the behavior of encryption, by showing an example packet after it has been deparsed, but before the decrypt block begins processing the packet, and then also the modified contents of the packet just after the decrypt block finishes processing the packet?

Similarly for an encrypted packet.

jfingerh · 2022-10-02T17:42:16Z

@pbhide For IPSec decryption, would your proposal include the behavior that the decrypt block does the anti-replay attack detection check? Or would you propose that such a check must be done explicitly in P4 code somehow, before sending the packet to the decryption block?

If the decryption block does the anti-replay attack detection, would one of the reasons for failing to decrypt the packet be that this check failed?

jfingerh · 2022-10-02T17:43:29Z

@pbhide For decryption of packets that also have an message digest authentication, e.g. an IPsec AH header, does your proposal include that the decryption block would check the contents of this AH header, and one of the possible error conditions during decryption would be a failure of the authentication?

pbhide · 2022-10-03T15:50:23Z

@pbhide Would it be reasonable to give a specific example of the behavior of encryption, by showing an example packet after it has been deparsed, but before the decrypt block begins processing the packet, and then also the modified contents of the packet just after the decrypt block finishes processing the packet?

Similarly for an encrypted packet.

You mean show packet in the comments?
From programmers point of view ipsec_esp_decrypt() action adds a recirc header to the packet. set_auth_data_xxx apis may add some information to packet that is specific to target implementation.. so it cannot be show here. If you follow the parser path on recirc, that indicates all the header that are present after decrypt.

Similarly, if we look at ipsec_esp_encrypt() new headers (outer header of a tunnel) are added by this action. ICV and trailer that is specified by AES-GCM related rfc are added by crypto engine if instructed to do so by using set_icv_xxx methods.

pbhide · 2022-10-03T15:56:27Z

@pbhide For IPSec decryption, would your proposal include the behavior that the decrypt block does the anti-replay attack detection check? Or would you propose that such a check must be done explicitly in P4 code somehow, before sending the packet to the decryption block?

If the decryption block does the anti-replay attack detection, would one of the reasons for failing to decrypt the packet be that this check failed?

SA specific random number and extended sequence numbers are expected to be handled in the P4 code.

jfingerh · 2022-10-03T16:05:21Z

@pbhide wrote "You mean show packet in the comments?"

If there is a more convenient way to show such documentation, e.g. text and a figure in the PNA specification, or a separate document that is in this pna repository, that sounds like it might be much more convenient to write and maintain than trying to do ASCII art in comments. Whatever is easiest for you. If it is not in comments, the comments could mention where a P4 developer can go to get more details, e.g. "see section of document for more details".

jfingerh · 2022-10-03T16:08:43Z

@pbhide wrote "SA specific random number and extended sequence numbers are expected to be handled in the P4 code."

Do you mean that you expect P4 code to extract the sequence number, or the extended sequence number, and pass them as separate parameters to the decryption block?

Or do you mean that you expect the P4 developer to write P4 code that performs anti-replay attack protection, because the decryption block will not do this feature in any way?

pbhide · 2022-10-03T16:09:08Z

@pbhide For decryption of packets that also have an message digest authentication, e.g. an IPsec AH header, does your proposal include that the decryption block would check the contents of this AH header, and one of the possible error conditions during decryption would be a failure of the authentication?

This example assume ipsec ESP which provides both authentication and confidentiality, not AH.

jfingerh · 2022-10-03T16:12:12Z

In general, it would be good to be very precise about the "division of labor" for decrypting and encrypting IPsec packets between what you expect the P4 code to do, and what you expect the decryption/encryption block to do, including any packet modifications made by the block on its own, independent of P4 code, whether the decrypt block does anti-replay attack protection check or not, whether it does authentication checks or not, which options of IPsec ESP and/or AH you expect it will support, versus which it will not.

Of course these details might change in future versions, but it seems worth trying to at least give your expected answer to such questions.

pbhide · 2022-10-03T16:15:32Z

Of course these details might change in future versions, but it seems worth trying to at least give your expected answer to such questions.

I'll add this information in the comments where the extern object is defined to make it clear wrt assumed functionality of the hardware

…). The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method. The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.

…ution) - Comments to describe assumed crypto engine functionality - set_error_action() method to control disposition of packets on error

pbhide · 2022-10-08T16:26:41Z

@jafingerhut I have updated the comments in crypto-accelerator.p4 to provide the description/assumptions about crypto engine functionality. PTAL.

…). The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method. The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.

…mple, added comments about esn checking/update that was broughtup during the meeting

pbhide · 2022-10-12T03:02:52Z

@pbhide Would it be reasonable to give a specific example of the behavior of encryption, by showing an example packet after it has been deparsed, but before the decrypt block begins processing the packet, and then also the modified contents of the packet just after the decrypt block finishes processing the packet?

Similarly for an encrypted packet.

Added ESP packet format.. and text to indicate headers that are present before and after decrypt and encrypt

pbhide · 2022-10-17T17:54:07Z

@pbhide For decryption of packets that also have an message digest authentication, e.g. an IPsec AH header, does your proposal include that the decryption block would check the contents of this AH header, and one of the possible error conditions during decryption would be a failure of the authentication?

This example does not show AH based auth. But it can be done where P4 pipeline include outerIP header (clear the mutable fields etc), AH header when providing the auth data offset. The engine will compute and compare ICV over it.

jfingerh · 2022-10-17T19:39:44Z

examples/include/crypto-accelerator.p4

+/// specific headers like ESP, AH etc
+///
+/// Note that crypto accelerator does not modify the packet outside the payload area and ICV
+///     Any wire-protocol header, trailer add/remove is handled by P4 pipeline


Today there is no standard way for a P4 pipeline to add/remove trailers at the end of long pipelines. The best workaround for short packets is to completely parse every byte of the short packet, and add/remove headers at the end, but that is not a good solution for packets that can be MTU in size.

Does your solution propose new capabilities in PNA and/or P4 to add/remove trailers?

Alternately, one could avoid the need for handling trailers in P4 pipeline completely by bundling together trailer add/remove into the encrypt/decrypt block logic.

The architecture can do anything it wants with trailers, that's how ethernet trailers actually work.
But you could add some externs to manipulate them. Symmetric with packet_in, but for trailers.

There are multiple ways to add/manipulate trailers. This capability is useful in endpoints (PNA where protocols like ESP, RDMA do use trailers). One method can be - payload is defined as a variable length header. Parser can extract it (based on length computed from ipHeader.totalLen or any other means available). Deparser similarly can add it back to packet and then add trailer headers after it. This will not need any new support in P4 language.
Many targets and/or target compilers allow large extractions as long as control functions do not access the extracted bytes as in this case.

For targets that need trailer processing by the accelerator, more methods can be added to this extern object for handling the trailers.. but those will be target specific, so I have not shown them here.

"Many targets and/or target compilers allow large extractions as long as control functions do not access the extracted bytes as in this case." That might be so, but I haven't used any of them. I am aware of several targets that do not support this way of accessing the end of MTU-sized packets, so while the P4 language spec definitely supports what you say in the language, there are a fair number of targets that won't do that.

Accessing/using trailers is done by more than one protocols so it is possible to define target-specific externs to add/remove/modify trailers that are generically useful outside IPSec ESP. Since the inline accelerators are after deparser, it is expected that trailers are added before packet reaches the accelerator. For crypto accelerators, trailers may be just payload that will be encrypted.
From the division of work perspective, I did not want to include this capability in the accelerator. In case it is required for specific targets, we can define target-specific methods for it as mentioned in earlier comment.

pbhide · 2022-10-24T20:40:32Z

examples/include/crypto-accelerator.p4

+    AUTH_FAILURE = 1,
+    HW_ERROR = 2
+}
+


Add comments here

pbhide · 2022-10-24T20:41:27Z

examples/include/crypto-accelerator.p4

+    // set_error_action() indicates behavior on encoutering an error
+    // Default action is NO_ACTION i.e. report error via get_results()
+    // Certain actions may need action parameters, e.g send_to_port
+    void set_error_action<T>(crypto_error_action_e error_action, in T action_param);


call it drop_on_error() remove other options. Name set_error_behavior().. or on_error() ? crypto_error_action_e -> crypto_error_behavior_e

This method was added as a result of discussion to see if it makes sense. I am removing it for the following reasons -

As mentioned in the code comments, it is preferable to keep the accelerator agnostic to packet redirection (send to port), packet drop type of functions that are not related to crypto operation.

Also many existing targets do not support this functionality.

Any targets existing or future that support it, can add a target specific method for this.

apinski-cavium · 2022-10-24T20:42:42Z

examples/include/crypto-accelerator.p4

+*/
+
+/// Crypto accelerator Extern
+enum bit<8> crypto_algorithm_e {


Should these not be serializable enums?

apinski-cavium · 2022-10-24T20:49:25Z

examples/ipsec-acc.p4

+
+        // instruct engine to add icv after encrypted payload
+        ipsec_acc.set_icv_offset(ICV_AFTER_PAYLOAD);
+        ipsec_acc.set_icv_len((bit<32>)4); // Four bytes of ICV value.


Minor issue:
32w4 instead of the cast.

jfingerh · 2022-10-26T04:53:34Z

@pbhide Sorry, a couple of more questions:

For replay attack detection, are you thinking that this would be done in P4 code after the decryption block and packet recirculation? If so, that makes sense, and I would recommend that you mention this somewhere. If you meant before, then that also means before authentication, which would leave you open to attackers that, without knowing your crypto details at all, can make you update your replay attack detection state for packets never sent by the true sender, i.e. a cheap denial of service attack.

Are you planning to create a P4 program example that uses this extern, demonstrating a complete packet decryption code, with all of the things that the P4 should do before and after the decryption block is used? And similarly for packet encryption code? I think those would be very valuable examples, even if they do not demonstrate all of the options.

…). The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method. The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.

pbhide · 2022-10-30T23:32:03Z

@pbhide Sorry, a couple of more questions:

For replay attack detection, are you thinking that this would be done in P4 code after the decryption block and packet recirculation? If so, that makes sense, and I would recommend that you mention this somewhere. If you meant before, then that also means before authentication, which would leave you open to attackers that, without knowing your crypto details at all, can make you update your replay attack detection state for packets never sent by the true sender, i.e. a cheap denial of service attack.

Are you planning to create a P4 program example that uses this extern, demonstrating a complete packet decryption code, with all of the things that the P4 should do before and after the decryption block is used? And similarly for packet encryption code? I think those would be very valuable examples, even if they do not demonstrate all of the options.

I have mentioned that anti-replay checks are not shown in this example... This is not a complete ipsec example wrt P4 pipeline functions.. this has a few short-cuts to just show the usage of the extern. I don't plan to provide a complete working ipsec example here as this proposal is not yet accepted. It it gets accepted, we may invest in it.. but as such it is not necessary to demonstrate the use of the extern.

jfingerh · 2022-10-30T23:37:12Z

@pbhide wrote: "I have mentioned that anti-replay checks are not shown in this example... This is not a complete ipsec example wrt P4 pipeline functions.. this has a few short-cuts to just show the usage of the extern. I don't plan to provide a complete working ipsec example here as this proposal is not yet accepted. It it gets accepted, we may invest in it.. but as such it is not necessary to demonstrate the use of the extern."

Makes sense. I have created this issue to track the creation of an example P4 program that hopefully demonstrates all of the pieces of a working IPsec decryption case, plus IPsec encryption case, using the extern, once it is merged in: #72

jfingerh

While we may want to edit this before Christmas 2022, I think this is in a fine state to merge in as it is now, and fine-tune it with future small PRs as desired. Will discuss at 2022-Oct-31 PNA meeting to see if there is any objection. If no big objections, I hope to merge this as it is on its 7th commit.

thantry · 2022-10-31T20:27:16Z

LGTM

jafingerhut · 2022-11-01T00:08:32Z

We have a couple of approvals on this, and no objections mentioned, so merging it in. Feel free to create PRs for review with modifications to this, if desired, and they can be considered.

…). The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method. The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.

pbhide and others added 3 commits August 27, 2022 10:30

Example: crypto_accelerator extern

19d04ee

Update example crypto accelerator code to use methods to encrypt packet.

6968eb4

decrypt function and some externs

cdeccc1

thomascalvert-xlnx reviewed Sep 26, 2022

View reviewed changes

pbhide force-pushed the crypto_acc branch from 4298a1a to 52c7ae6 Compare September 30, 2022 22:17

separate out the crypto accelerator object in a separate file

b392e1f

fix compilation error

loalan mentioned this pull request Oct 3, 2022

Example: crypto inline method addition to crypto_accelerator object #59

Open

- Move crypto-accelerator.p4 to include dir (clean runp4tests.sh exec…

5aecaeb

…ution) - Comments to describe assumed crypto engine functionality - set_error_action() method to control disposition of packets on error

pbhide force-pushed the crypto_acc branch from 52c7ae6 to 5aecaeb Compare October 8, 2022 16:24

Added Ipsec ESP information and packet formats to explain current exa…

70bdefc

…mple, added comments about esn checking/update that was broughtup during the meeting

jfingerh reviewed Oct 17, 2022

View reviewed changes

pbhide commented Oct 24, 2022

View reviewed changes

apinski-cavium reviewed Oct 24, 2022

View reviewed changes

review comments

6ded306

jfingerh mentioned this pull request Oct 30, 2022

Create functionally correct example demonstrating IPsec encryption and decryption #72

Open

jfingerh approved these changes Oct 30, 2022

View reviewed changes

jafingerhut merged commit ebb2b6c into p4lang:main Nov 1, 2022

Example: crypto_accelerator object extern #53

Example: crypto_accelerator object extern #53

Conversation

pbhide commented Aug 29, 2022

Choose a reason for hiding this comment

pbhide Sep 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfingerh commented Oct 2, 2022

jfingerh commented Oct 2, 2022 • edited Loading

jfingerh commented Oct 2, 2022

pbhide commented Oct 3, 2022

pbhide commented Oct 3, 2022

jfingerh commented Oct 3, 2022

jfingerh commented Oct 3, 2022

pbhide commented Oct 3, 2022

jfingerh commented Oct 3, 2022

pbhide commented Oct 3, 2022

pbhide commented Oct 8, 2022

pbhide commented Oct 12, 2022

pbhide commented Oct 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jfingerh commented Oct 26, 2022

pbhide commented Oct 30, 2022

jfingerh commented Oct 30, 2022 • edited Loading

jfingerh left a comment

Choose a reason for hiding this comment

thantry commented Oct 31, 2022

jafingerhut commented Nov 1, 2022

pbhide Sep 26, 2022 •

edited

Loading

jfingerh commented Oct 2, 2022 •

edited

Loading

jfingerh commented Oct 30, 2022 •

edited

Loading