Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ALTCP support #63

Open
HamzaHajeir opened this issue May 31, 2023 · 17 comments
Open

ALTCP support #63

HamzaHajeir opened this issue May 31, 2023 · 17 comments

Comments

@HamzaHajeir
Copy link

Hi there
LWIP offers ALTCP APIs (Application Layered TCP) which offers a seamless shift into TLS support with their ALTCP_TLS.

The corresponding header files cannot be found by the compiler.

I use Arduino core.

What can I do to enable ALTCP?

With thanks,
Hamza Hajeir
@mcspr
Copy link
Collaborator

mcspr commented Jun 16, 2023

See our build instructions, makefiles & cmake

In both cases, .c files can be omitted from builder and .h are only partially installed to esp8266/Arduino include dirs. For the first case, you would have to make changes to file lists in makefiles/Makefile.lwip2 or lwip-builder.cmake

iirc header installation is manual in makefile case, where d-a-v would filter out unused stuff like apps, sockets, altcp, etc. before doing a commit at esp8266/Arduino. Should probably install everything to avoid further issues, since I just noticed we have a bug with mismatching struct layout when using auto-ip feature because of that >_<

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jun 19, 2023 via email

@mcspr
Copy link
Collaborator

mcspr commented Jun 19, 2023

Note of the version difference, most likely upstream v2.1.3 already has those
For cherry picking anything else, add it as patches/*.patch?

To modify build flags for the lib
https://github.com/d-a-v/esp82xx-nonos-linklayer/blob/master/glue-lwip/arduino/lwipopts.h

Unused functions are ignored in the resulting build

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jun 20, 2023 via email

@mcspr
Copy link
Collaborator

mcspr commented Jun 21, 2023

No reason why not; isn't the whole idea of altcp is just extra api on top of existing one? so, if we internally or otherwise don't use it, nothing changes or raw api callers start to behave differently

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jun 22, 2023 via email

@d-a-v
Copy link
Owner

d-a-v commented Jul 11, 2023

A side discussion: I'm seeing less activity for the Arduino core of 8266,

Because it's just working ?

I personally was and still am busy with other project.
As @mcspr said, no reason why it couldn't be integrated (header files and linkable code in library).
Did altcp work in your test ?

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jul 12, 2023 via email

@d-a-v
Copy link
Owner

d-a-v commented Jul 18, 2023

Is ALTCP requiring MbedTLS ?
We don't have it yet on esp8266.
We are using BearSSL instead. Would ALTCP be able to use something else than MbedTLS ?

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jul 18, 2023 via email

@d-a-v
Copy link
Owner

d-a-v commented Jul 18, 2023

Supporting MbedTLS on esp8266 Arduino is quite an amount of work.
Are you expecting to use this repository with something else on esp8266 other than the arduino core ?

@HamzaHajeir
Copy link
Author

HamzaHajeir commented Jul 19, 2023 via email

@HamzaHajeir
Copy link
Author

Hi

I think if BearSSL API is similar to mbedTLS API regarding SSL, there's a possibility to copy and tweak altcp_tls_mbedtls.c.

@d-a-v
Copy link
Owner

d-a-v commented Aug 7, 2023

OK then,
if you're using makefiles to build lwip2, you can try with this patch.
You would have to create a lwip2-src/src/apps/altcp_tls2/ (or whatever name you wish to use) and put your tweaks in there.

diff --git a/glue-lwip/arduino/lwipopts.h b/glue-lwip/arduino/lwipopts.h
index bcbda45..1f43e5e 100644
--- a/glue-lwip/arduino/lwipopts.h
+++ b/glue-lwip/arduino/lwipopts.h
@@ -1499,7 +1499,7 @@
  * See @ref altcp_api
  */
 #if !defined LWIP_ALTCP || defined __DOXYGEN__
-#define LWIP_ALTCP                      0
+#define LWIP_ALTCP                      1
 #endif
 
 /** LWIP_ALTCP_TLS==1: enable TLS support for altcp API.
@@ -1510,6 +1510,9 @@
 #if !defined LWIP_ALTCP_TLS || defined __DOXYGEN__
 #define LWIP_ALTCP_TLS                  0
 #endif
+#if !defined LWIP_ALTCP_TLS2 || defined __DOXYGEN__
+#define LWIP_ALTCP_TLS2                 1
+#endif
 
 /**
  * @}
diff --git a/makefiles/Makefile.lwip2 b/makefiles/Makefile.lwip2
index 6a4ccbd..1e504e1 100644
--- a/makefiles/Makefile.lwip2
+++ b/makefiles/Makefile.lwip2
@@ -10,6 +10,8 @@ OBJ = \
        $(patsubst %.c,$(BUILD)/%.o,$(wildcard api/*.c)) \
        $(patsubst %.c,$(BUILD)/%.o,$(wildcard apps/sntp/*.c)) \
        $(patsubst %.c,$(BUILD)/%.o,$(wildcard apps/mdns/*.c)) \
+ $(patsubst %.c,$(BUILD)/%.o,$(wildcard apps/altcp_tls2/*.c)) \
+ $(patsubst %.c,$(BUILD)/%.o,$(wildcard apps/http/*.c)) \
 #      $(subst ../../lwip2-contrib-src/,contrib/, \
 #              $(patsubst %.c,$(BUILD)/%.o,$(wildcard ../../lwip2-contrib-src/apps/ping/*.c)))

@mcspr
Copy link
Collaborator

mcspr commented Aug 7, 2023

LWIP_ALTCP_TLS_MBEDTLS
LWIP_ALTCP_TLS_BEARSSL
?

I think the idea is LWIP_ALTCP_TLS enables TLS, another flag selects implementation. Plus some code to #error out when building with both

@d-a-v
Copy link
Owner

d-a-v commented Aug 7, 2023

This one is defined and used in in altcp_tls_mbedtls*
It should be indeed added (or LWIP_ALTCP_TLS_BEARSSL) in lwipopts.h

lwip2-src/src/include/lwip/apps/altcp_tls_mbedtls_opts.h:#ifndef LWIP_ALTCP_TLS_MBEDTLS
lwip2-src/src/include/lwip/apps/altcp_tls_mbedtls_opts.h:#define LWIP_ALTCP_TLS_MBEDTLS                        0

@mcspr
Copy link
Collaborator

mcspr commented Aug 7, 2023

Don't forget that bssl also has some changes so we don't trigger watchdog timer

~/.p/p/f/t/s/s/bearssl ((b024386d))> git grep 'yield('
src/ec/ec_prime_i15.c:  optimistic_yield(10000);
src/ec/ec_prime_i31.c:        optimistic_yield(10000);
src/inner.h:  extern void optimistic_yield(uint32_t);
src/inner.h:  #define optimistic_yield(ignored)
src/rsa/rsa_i15_priv.c: optimistic_yield(10000);
src/rsa/rsa_i15_priv.c: optimistic_yield(10000);
src/rsa/rsa_i15_priv.c: optimistic_yield(10000);
src/rsa/rsa_i31_priv.c: optimistic_yield(10000);
src/rsa/rsa_i31_priv.c: optimistic_yield(10000);
src/rsa/rsa_i31_priv.c: optimistic_yield(10000);

Besides modifying our mbedtls port in a similar way, there are MBEDTLS_*_ALT that are sort-of the thing we might be looking for to avoid the issue.
(doc does not seem to mention anything when searching for either yield or context switching related keywords)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mcspr @d-a-v @HamzaHajeir