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

[realtek-ambz] Enable Mbed-TLS for MD5 hashing, remove Polar SSL #156

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

hn
Copy link
Contributor

@hn hn commented Aug 16, 2023

Set Mbed-TLS as default for MD5 hashing, remove Polar SSL. The latter has given wrong results on at least one MCU and is generally deprecated.

Closes issue #142.

@kuba2k2
Copy link
Member

kuba2k2 commented Aug 17, 2023

Well, I think I found the issue. The PolarSSL MD5 context in LT is defined as follows:

typedef struct {
	unsigned long total[2];	  /*!< number of bytes processed  */
	unsigned long state[4];	  /*!< intermediate digest state  */
	unsigned char buffer[64]; /*!< data block being processed */
} md5_context;

whereas the context in PolarSSL library is this:

typedef struct
{
    uint32_t total[2];          /*!< number of bytes processed  */
    uint32_t state[4];          /*!< intermediate digest state  */
    unsigned char buffer[64];   /*!< data block being processed */

    unsigned char ipad[64];     /*!< HMAC: inner padding        */
    unsigned char opad[64];     /*!< HMAC: outer padding        */
}
md5_context;

Notice the missing ipad and opad fields, which make the structure smaller (thus, esphome allocates less memory for the context, and we have a memory corruption).

@kuba2k2 kuba2k2 changed the title Enable Mbed-TLS for MD5 hashing, remove Polar SSL [realtek-ambz] Enable Mbed-TLS for MD5 hashing, remove Polar SSL Aug 17, 2023
@kuba2k2 kuba2k2 merged commit ccf21b4 into libretiny-eu:master Aug 17, 2023
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.

2 participants