patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9823, altcp_tls_mbedtls.c: add...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9823: altcp_tls_mbedtls.c: add restartable feature

Submitter:  Giuseppe Modugno <giusloq>
Submitted:  Thu 27 Jun 2019 09:57:43 AM UTC
   
 
Category:  apps Priority:  5 - Normal
Status:  None Privacy:  Public
Assigned to:  None Open/Closed:  Open
Planned Release:  None

Jump to the original submission

Tue 15 Oct 2019 10:30:09 AM UTC, comment #8: 

commentaire #7 :

> OK, so for your setup, tcp_poll (500ms) might be enough. For the software implementations, where the restartable feature should just be a "yield, let other connections be serived, too", it is clearly too slow as the CPU might do nothing for 500ms.


Just have time to read this link. And you're right, this is only relevant on SW implementation.

The timeout value must be configurable at compile time to allow tunning according application needs.

Additionally, restartable ECP isn't available in still maintained 2.7 branch. So lwIP ALTCP should take care of that.

David GIRAULT <dgirault>
Tue 15 Oct 2019 10:07:19 AM UTC, comment #7: 

OK, so for your setup, tcp_poll (500ms) might be enough. For the software implementations, where the restartable feature should just be a "yield, let other connections be serived, too", it is clearly too slow as the CPU might do nothing for 500ms.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 15 Oct 2019 10:02:24 AM UTC, comment #6: 


commentaire #5 :

>
> comment #1:
> > Hi Giuseppe,
> >
> > I think the handling of the ALTCP_MBEDTLS_FLAGS_CRYPTO_IN_PROGRESS flag must be made by the altcp_mbedtls_lower_poll() function. This allow reuse existing cyclic TCP timer. And ensure your new function isn't called too much.
>
> As Simon already observed, poll mechanism call rate will be too slow.
>
>
> > Anyway, this patch require that the ECP calculus are made in HW (or that sw implementation can stop at some point).
>
> I couldn't get your point. If ECP calculus are made in HW, I think they would be much more fast and restartable wouldn't be useful.


We currently use the STSafe crypto HW module and it take 400ms min to achieve ECDSA signature check during TLS negotiation.

This is why I think the poll API may be enough. CPU (STM32F479) is completely free to do other thing during this time. Too fast polling will waste CPU cycles.

I don't check basic ECP operations timings yet, so I can't know if it's a lot faster or not.


David GIRAULT <dgirault>
Fri 11 Oct 2019 07:59:16 AM UTC, comment #5: 


comment #1:

> Hi Giuseppe,
>
> I think the handling of the ALTCP_MBEDTLS_FLAGS_CRYPTO_IN_PROGRESS flag must be made by the altcp_mbedtls_lower_poll() function. This allow reuse existing cyclic TCP timer. And ensure your new function isn't called too much.


As Simon already observed, poll mechanism call rate will be too slow.


> Anyway, this patch require that the ECP calculus are made in HW (or that sw implementation can stop at some point).


I couldn't get your point. If ECP calculus are made in HW, I think they would be much more fast and restartable wouldn't be useful.

Giuseppe Modugno <giusloq>
Fri 11 Oct 2019 07:57:03 AM UTC, comment #4: 

comment #3:

> Wait, just found out I haven't been following mbedTLS development:
>
> https://tls.mbed.org/kb/development/restartable-ecc


If you read my original post, you will see the same link.


> ECC SW algorithms can be made restartable. In that case, we'd need a "call me again directly" as it's a "yield" thing, not a "I'll finish in the background" thing. Here, using tcp_poll would definitively not work!


Indeed my proposal is to arm an already expired timer that would be called by the superloop (sys_check_timeouts) in a short time.

Giuseppe Modugno <giusloq>
Wed 09 Oct 2019 05:42:34 PM UTC, comment #3: 

Wait, just found out I haven't been following mbedTLS development:

https://tls.mbed.org/kb/development/restartable-ecc

ECC SW algorithms can be made restartable. In that case, we'd need a "call me again directly" as it's a "yield" thing, not a "I'll finish in the background" thing. Here, using tcp_poll would definitively not work!

Simon Goldschmidt <goldsimon>
Group administrator
Wed 09 Oct 2019 05:35:54 PM UTC, comment #2: 

The question is whether our poll interval (500ms) is fast enough for a decent TLS performance...

A SW implementation that would 'yield' would be good as well to help sharing processor power amongst other connections, but I don't think that's gonna happen soon with mbedTLS...

Simon Goldschmidt <goldsimon>
Group administrator
Wed 09 Oct 2019 10:01:33 AM UTC, comment #1: 

Hi Giuseppe,

I think the handling of the ALTCP_MBEDTLS_FLAGS_CRYPTO_IN_PROGRESS flag must be made by the altcp_mbedtls_lower_poll() function. This allow reuse existing cyclic TCP timer. And ensure your new function isn't called too much.

Anyway, this patch require that the ECP calculus are made in HW (or that sw implementation can stop at some point).


David GIRAULT <dgirault>
Thu 27 Jun 2019 09:57:43 AM UTC, original submission:  

mbedTLS features an option named MBEDTLS_ECP_RESTARTABLE[1] that is very useful in NO_SYS=1 platforms and when ECP calculus takes a long time (as in many modern MCUs without crypto engine). I'm using a Cortex_M3 LPC1768 and the TLS handshake takes around 5-10 seconds.

During this period, in cooperative multitasking, altcp_mbedtls_lower_recv_process() blocks other tasks (except interrupts). The restartable option splits the long calculus in steps and gives the possibility to run other tasks.

Unfortunately altcp_tls_mbedtls isn't compatible with restartable option. Indeed, the return value MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS isn't managed.

I tried to add restartable option, please look at the patch. I trigger an already expired timer when mbedtls_handshake() returns CRYPTO_IN_PROGRESS. In the timer handler I call altcp_mbedtls_lower_recv_process() again. If the calculus is yet in progress, the timer is triggered again.

This patch works in my case, but it is experimental and mainly sub-optimal for the following reasons.

1. There's one timer for each TLS session. An optimization could be using a single timer for all TLS sessions, but this means that altcp_tls_mbedtls must trace all TLS sessions (a list?). In my application I have only one TLS client session, so one timer per session isn't so bad.

2. sys_timeout() could assert if we are out of timers.

3. MBEDTLS_ERR_SSL_CRYPTO_IN_PROGRESS can be returned by mbedtls_read() and mbedtls_write() too. I think this happens only occasionally when, during a session, one peer decides to renegotiate something. I don't know how to manage this situation.

[1] https://tls.mbed.org/kb/development/restartable-ecc

Giuseppe Modugno <giusloq>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #47139:  patch.diff added by giusloq (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by dgirault (Posted a comment)
  • -email is unavailable- added by giusloq (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-06-27 giusloq Attached File- Added patch.diff, #47139

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code