search

arduino-libraries / ArduinoBearSSL

Port of BearSSL to Arduino
MIT License
86 stars 49 forks source link

Possible error in BEAR_SSL_CLIENT_IBUF_SIZE calculation #79

Closed imatrisciano closed 11 months ago

imatrisciano commented 11 months ago

In BearSSLClient.h line 40-45 the values for BEAR_SSL_CLIENT_OBUF_SIZE and BEAR_SSL_CLIENT_IBUF_SIZE are calculated.

https://github.com/arduino-libraries/ArduinoBearSSL/blob/5c278698e451035701723f344f4bd2bf9a157da7/src/BearSSLClient.h#L40-L46

By writing so, BEAR_SSL_CLIENT_OBUF_SIZE will be computed to be 512+85 = 597. I would expect BEAR_SSL_CLIENT_IBUF_SIZE to be 8192 + 85 + 325 - 597 = 8005 But by using defines the code for BEAR_SSL_CLIENT_IBUF_SIZE actually compiles to 8192 + 85 + 325 - 512 + 85 = 8175

I don't know if this is intended behaviour. If it's not, things can be fixed by just adding some parenthesis 

#ifndef BEAR_SSL_CLIENT_OBUF_SIZE
#define BEAR_SSL_CLIENT_OBUF_SIZE (512 + 85)
#endif

#ifndef BEAR_SSL_CLIENT_IBUF_SIZE
#define BEAR_SSL_CLIENT_IBUF_SIZE (8192 + 85 + 325 - BEAR_SSL_CLIENT_OBUF_SIZE)
#endif
aentinger commented 11 months ago

Hi @imatrisciano ☕ 👋

Thank you for pointing this out. You are correct, this is a bug. I've looked up the required minimum buffer sizes for BearSSL from its API documentation and it definitely makes more sense with the parenthesis in place. It's noteworthy, that alltogether even a bigger buffer size is recommended (BR_SSL_BUFSIZE_INPUT should be 16709, BR_SSL_BUFSIZE_OUTPUT should be 16469), the current size is what it is due to this library initially targetting the ArduinoCore-samd platform, where the maximum available TOTAL memory is 32k - and you just can't spend already 32k on the BearSSL buffers.