[patch #9823] altcp_tls_mbedtls.c: add restartable feature

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
URL:
  <https://savannah.nongnu.org/patch/?9823>

                 Summary: altcp_tls_mbedtls.c: add restartable feature
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: giusloq
            Submitted on: Thu 27 Jun 2019 09:57:43 AM UTC
                Category: apps
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email:
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

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




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Thu 27 Jun 2019 09:57:43 AM UTC  Name: patch.diff  Size: 2KiB   By:
giusloq

<http://savannah.nongnu.org/patch/download.php?file_id=47139>

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #1, patch #9823 (project lwip):

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



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #2, patch #9823 (project lwip):

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

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #3, patch #9823 (project lwip):

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!

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #4, patch #9823 (project lwip):

[comment #3 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.


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #5, patch #9823 (project lwip):


[comment #1 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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #6, patch #9823 (project lwip):


[comment #5 commentaire #5 :]
>
> [comment #1 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.



    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #7, patch #9823 (project lwip):

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.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9823] altcp_tls_mbedtls.c: add restartable feature

David GIRAULT-2
Follow-up Comment #8, patch #9823 (project lwip):

[comment #7 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.


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9823>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel