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.

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

Move BearSSL from STACK_PROXY to a real, thunked 2nd stack #5168

Merged
merged 33 commits into from
Nov 15, 2018

Conversation

earlephilhower
Copy link
Collaborator

@earlephilhower earlephilhower commented Sep 26, 2018

This is still WIP, but it introduces a new bit to the BearSSL client so I wanted to get feedback early.
-edit 10/27-

No longer WIP

BearSSL needs 5.6KB of stack for its own uses due to allocating large local variables in the encryption routines. To make this happen originally, I hacked almost every function in the BearSSL library to use a #define macro to allocate local variables on a SW managed stack.

This meant that almost every function was edited, sometimes significantly (replacing a structure with a pointer-to-structure). This is painful and error prone, especially as new functions are added or existing ones modified.

To work around this I've written a simple "thunking" wrapper in assembly that changes the ESP8266's stack pointer itself to point to a local (heap-allocated) 2nd stack for function calls and then back to the main one on return. It's pretty simple conceptually, and effectively moves all BearSSL locals to a private stack without requiring any code changes in the library itself.

The assembly wrapper is maybe 20 lines of code and reasonably readable, and only a small subset of bearssl calls need to be wrapped due to stack depth (basically those calls related to actual SSL engine processing). Because the stack pointer itself is changed, not only the called function, but every function in the library that the main SSL function calls will use this secondary space.

There may be impact on the postmortem, and potentially interrupts (but I don't think it's really an issue, depends on where the IRQ call stack is located). (Interestingly enough, the postmortem still works mostly, but doesn't understand to skip stuff from the top of the BSSL stack until the bottom of the CONT stack...

@earlephilhower
Copy link
Collaborator Author

@Adam5Wu Here's the changes I was suggesting. Let me know your thoughts on where the thunks should live...

When a crash occurs while in the second stack, dump the BSSL stack and
then also the stack that it was called from (either cont or sys).
@earlephilhower
Copy link
Collaborator Author

BearSSL stack now decodes properly followed by the main stack

Connecting to NOBABIES
...
WiFi connected
IP address: 
192.168.1.138
Waiting for NTP time sync: .
Current time: Tue Oct  2 04:05:03 2018
Number of CA certs read: 150
Attempting to fetch https://www.github.com/...
Trying: www.github.com:443...

Exception (29):
epc1=0x40207aa6 epc2=0x00000000 epc3=0x00000000 excvaddr=0x00000000 depc=0x00000000

>>>stack>>>

ctx: bearssl 
sp: 3fff1a08 end: 3fff2008 offset: 01a0
3fff1ba8:  00000000 00000000 00000000 dfd9bb00  
3fff1bb8:  00000008 00000000 00000001 95573847  
3fff1bc8:  e852ab0a 6496901a d11e8e36 3fff7b14  
3fff1bd8:  3ffe8b08 3ffef384 3fff1c08 4020ac6c  
3fff1be8:  3fff0c2c 3fff0c6c 00000001 00000000  
3fff1bf8:  3ffef1bc 00000000 3ffef1b8 40207a98  
3fff1c08:  4020ce58 00000000 000003e8 700da1d5  
3fff1c18:  00000000 00000000 00000000 00000000  
....
3fff1fc8:  00000250 00000005 deadbeef 3ffef204  
3fff1fd8:  00000008 00000414 3fff220c 402205ed  
3fff1fe8:  000000db deadbeef deadbeef deadbeef  
3fff1ff8:  deadbeef 00000000 3fff0944 40201078  

ctx: cont 
sp: 3ffffe50 end: 3fffffc0 offset: 0000
3ffffe50:  00000008 00000000 00000001 40209bc5  
3ffffe60:  203a7073 78383025 646e6520 3025203a  
3ffffe70:  6f207838 65736666 25203a74 0a783430  
....
3fffffa0:  3fffdad0 00000000 3ffef358 4020b79c  
3fffffb0:  feefeffe feefeffe 3ffe8508 40100739  
<<<stack<<<
 ets Jan  8 2013,rst cause:2, boot mode:(1,6)
 ets Jan  8 2013,rst cause:4, boot mode:(1,6)
wdt reset

And decodes with the standard exception decoder to the right thing:


Exception 29: StoreProhibited: A store referenced a page mapped with an attribute that does not permit stores
PC: 0x40207aa6: SPIFFSCertStoreFile::open(bool) at /home/earle/Arduino/BearSSL_CertStore1/BearSSL_CertStore1.ino line 104
EXCVADDR: 0x00000000

Decoding stack results
0x4020ac6c: fs::FS::open(char const*, char const*) at /home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266/FS.cpp line 207
0x40207a98: SPIFFSCertStoreFile::open(bool) at /home/earle/Arduino/BearSSL_CertStore1/BearSSL_CertStore1.ino line 104
0x40207b7d: BearSSL::CertStore::findHashedTA(void*, void*, unsigned int) at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/CertStoreBearSSL.cpp line 148
0x40227bbd: br_x509_minimal_run at src/x509/x509_minimal.c line 1217
0x40228446: xm_append at src/x509/x509_minimal.c line 285
0x40222311: br_ssl_hs_client_run at src/ssl/ssl_hs_client.c line 1863
0x402204b3: br_ssl_engine_recvrec_ack at src/ssl/ssl_engine.c line 652
0x40220108: jump_handshake at src/ssl/ssl_engine.c line 1081
0x402205ed: br_ssl_engine_recvrec_ack at src/ssl/ssl_engine.c line 1206
0x40209bc5: BearSSL::WiFiClientSecure::_run_until(unsigned int, bool) at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp line 494
0x40209dd4: BearSSL::WiFiClientSecure::_wait_for_handshake() at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp line 518
0x40209fb7: BearSSL::WiFiClientSecure::_connectSSL(char const*) at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp line 884
0x4020b710: esp_yield() at /home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 91
0x4020176b: delay at /home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_wiring.c line 51
0x40208e11: WiFiClient::connect(IPAddress, unsigned short) at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/include/ClientContext.h line 136
0x4020a091: BearSSL::WiFiClientSecure::connect(char const*, unsigned short) at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.cpp line 215
0x402076f9: fetchURL(BearSSL::WiFiClientSecure*, char const*, unsigned short, char const*) at /home/earle/Arduino/BearSSL_CertStore1/BearSSL_CertStore1.ino line 156
0x40201174: br_thunk_add_ref at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/BearSSLThunks.c line 43
0x401004e8: malloc at /home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266/umm_malloc/umm_malloc.c line 1672
0x40201174: br_thunk_add_ref at /home/earle/Arduino/hardware/esp8266com/esp8266/libraries/ESP8266WiFi/src/BearSSLThunks.c line 43
0x40207983: loop() at /home/earle/Arduino/BearSSL_CertStore1/BearSSL_CertStore1.ino line 247
0x4020b79c: loop_wrapper() at /home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266/core_esp8266_main.cpp line 125

earlephilhower and others added 5 commits October 1, 2018 18:08
The thunk code needs to be visible to the core routines, so move it to the
cores/esp8266 directory.  Probably need to refactor the stack setup and the
bearssl portion to avoid dependency on bearssl libs in cores/esp8266
@earlephilhower earlephilhower changed the title WIP - Move BearSSL from a hackish STACK_PROXY to a real, thunked 2nd stack Move BearSSL from STACK_PROXY to a real, thunked 2nd stack Oct 27, 2018
@earlephilhower
Copy link
Collaborator Author

This has been refactored per the discussion on gitter and is ready for review.

@earlephilhower earlephilhower added this to the 2.5.0 milestone Oct 27, 2018
Make stack_thunks generic, remove bearssl include inside of cores/esp8266.

Allocate the stack on a WiFiServerSecure object creation to avoid
fragmentation since we will need to allocate the stack to do any
connected work, anyway.

A stress test is now included which checks the total BearSSL second
stack usage for a variety of TLS handshake and certificate options
from badssl.org.
@d-a-v
Copy link
Collaborator

d-a-v commented Oct 31, 2018

👍 Thank you for making BearSSL rock-stable ! 👍

Run a series of SSL connection and transmission tests that stress
BearSSL and its stack usage to the device tests.

Modify device tests to include a possible SPIFFS generation and
upload when a make_spiffs.py file is present in a test directory.
@earlephilhower
Copy link
Collaborator Author

@pornin made BearSSL stable. I just make sure we don't make it unstable in the core.

The latest push includes the stress test as part of the device test suite. It was a simpler hack running the device tests than I though it would be.

Update to use the merged master branch of bearssl.  Should have no code
changes.
Copy link
Collaborator

@devyte devyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@devyte
Copy link
Collaborator

devyte commented Nov 15, 2018

Merging this to get some exposure.

@devyte devyte merged commit 2f43807 into esp8266:master Nov 15, 2018
@earlephilhower earlephilhower deleted the thunks branch November 18, 2020 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants