espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13.3k stars 7.35k forks source link

STARTTLS support / inband connection upgades for WiFiSecureClient #9099

Open dirkx opened 8 months ago

dirkx commented 8 months ago

Related area

Protocols such as SMTP, XMPP, IMAP, FTP, IRC, posgress, mysql, nntp, lmtp, sieve and ldap

Hardware specification

Whole ESP32 range

Is your feature request related to a problem?

Protocols such as SMTP, XMPP, IMAP, FTP, IRC, posgress, mysql, nntp, lmtp, sieve and ldap allow for a connection to be started in the clear; and then, after some in-the-clear, capability exchange; an 'upgrade' to SSL or TLS.

This means that you start the connection in the clear; as a normal WiFiClient like connection. At some point conclude that you can request SSL, and only then start the SSL or TLS negotiation.

This is currently not possible with WiFiClientSecure as it goes straight into negotiation after the TCP connect.

Describe the solution you'd like

The option to do the connect in two phases; with a startSSL/startTLS (e.g. as in https://www.openssl.org/docs/man1.0.2/man1/openssl-s_client.html its starttls flag (or the equivalent in stunnel, socat, etc)).

Follows the stunnel/socat/openssl standard.

Describe alternatives you've considered

Replacing all of WifiSecureConnect or writing this in raw mbedtls.

Additional context

No response

I have checked existing list of Feature requests and the Contribution Guide

lbernstone commented 8 months ago

Is this an Arduino standard, or something you would see implemented as an entirely new class?

dirkx commented 8 months ago

Given how very common this type of SSL/TLS protocol is (especially also for IoT devices) and given that WiFiClientSecure is a very natural place to do this - I would suggest to accomplish this as per this patch - as a backward compatible 'extra' in the existing WiFiClientSecure class.

I.e. only have this activate if the user calls the

    client.setPlainStart();

as per PR-9100. So it becomes easy to merge into existing protocol code. Because the downside of a new sort of class is that it is harder to switch. And that is often what needs to happen in code - e.g. based on a user configuration or based on a service-discovery. And if this were a separate class - the application would have to create two code-paths (or 4); rather than just a simple (if (setting | capability-reported-bysever) ... client.setPlainStart(); ... client.StartTLS();) on the same class.

lbernstone commented 8 months ago

On first look into the code, it seems like it should be possible to pass in an existing WiFiClient on a new object, attempt to initialize ssl_client on that same socket, and then mark the WiFiClient as not connected. Does that make more sense than having it a method of WiFiClient? You (or the protocol library) will need to handle the negotiation to get to the point it is ready to start SSL, and the TLS itself will need to be a straight forward tunnel.

dirkx commented 8 months ago

That was more or less my first approach for EmailSender and XMPP0-client The downside to that is that you generally need to support several variations in your code - especially if the end user can configure it. The problem then becomes that you end up with fairly complex code - e.g. one WifiClientSecure path - and another with WifiClient that later becomes WifiClientSecure. But that means that you cannot easily pass reference to the main object around (or need keep some extra state) as you call protocol specific things which need to work for both WifiCLient and Secure.

And this is commonly needed - as most of these protocols are 'identical' in SSL/TLS upgraded mode and plain-text mode; i.e. the same command (but for perhaps a few extra ones).

Hence this approach - which makes the code look exactly like WifiClientSecure (which is really what you are doing and want to do) — merely with an extra flag you use in some cases.

I.e all existing code stays as it it - there are just 2 lines for this in the initial negotiation near/where the lines are for that specific protocol. I.e. old

 WiFiClientSecure foo
 foo.connect(…)
 Do normal welcome stuff / header parsing of protocol
 while(1)
             Do your stuff

becomes:

 WiFiClientSecure foo
 If some config/DNS/etc 
      foo.setPlainStart();
 foo.connect(…)
      Do normal welcome stuff / header parsing of protocol
 If we need to give the command for upgrade to SSL/TLS                                   
      foo.StartTLS();
 while(1)
      Do your stuff