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

Update.setMD5() should downcase the MD5 string #8945

Closed
1 task done
jksemple opened this issue Nov 30, 2023 · 3 comments · Fixed by #8959
Closed
1 task done

Update.setMD5() should downcase the MD5 string #8945

jksemple opened this issue Nov 30, 2023 · 3 comments · Fixed by #8959
Labels
Area: Libraries Issue is related to Library support. Status: Solved

Comments

@jksemple
Copy link

jksemple commented Nov 30, 2023

Board

ESP32 ESP32S3 ESP8266 etc

Device Description

Seeedstudio ESP32S3 Sense but any Espressif device

Hardware Configuration

No

Version

latest master (checkout manually)

IDE Name

Arduino IDE

Operating System

Windows 10

Flash frequency

40Mhz

PSRAM enabled

yes

Upload speed

115200

Description

When requesting a firmware update from a remote OTA server using HttpUpdate the remote server returns an expected MD5 in header "x-MD5" as a 32 char hex string. If received HttpUpdate passes this to Update object for comparison with an internally computed MD5 calculated using MD5Builder.
Update stores the expected MD5 using UpdateClass::setMD5(). OTA servers built on .Net will send MD5 as uppercase hex strings by default. In UpdateClass::end() the actual MD5 of the received firmware is compared with the expected MD5 and it fails because of case difference.

UpdateClass::setMD5() should not assume lowercase hex characters and should downcase the expected MD5 before storing to the _target_md5 member variable.

This was raised earlier as Issue #4641 Uppercase MD5 gets rejected but was closed due to inaction.

Sketch

N/A

Debug Message

Done 856064 of 856656
Done 856656 of 856656
Done 856656 of 856656
[205243][E][HttpUpdate.cpp:445] runUpdate(): Update.end failed! (MD5 Check Failed)

[205246][E][HttpUpdate.cpp:359] handleUpdate(): Update failed

[205247][E][ssl_client.cpp:37] _handle_error(): [data_to_read():361]: (-76) UNKNOWN ERROR CODE (004C)
[205256][E][HTTPClient.cpp:408] disconnect(): tcp is closed

HTTP_UPDATE_FAILED Error (7): Update error: MD5 Check Failed

Other Steps to Reproduce

The problem goes away if the remote server downcases the value in the "x-MD5" header before returning it.

I have checked existing issues, online documentation and the Troubleshooting Guide

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@jksemple jksemple added the Status: Awaiting triage Issue is waiting for triage label Nov 30, 2023
@jksemple
Copy link
Author

As a more general point it is a bit inconsistent to render the device's MAC address as upper case hex and MD5 checksums as lower case hex

@P-R-O-C-H-Y P-R-O-C-H-Y added Area: Libraries Issue is related to Library support. Status: Needs investigation We need to do some research before taking next steps on this issue and removed Status: Awaiting triage Issue is waiting for triage labels Dec 4, 2023
@P-R-O-C-H-Y
Copy link
Member

Hi @jksemple, thank you for opening this issue once again. Feel free to open a PR with the proposed fix:
"UpdateClass::setMD5() should not assume lowercase hex characters and should downcase the expected MD5 before storing to the _target_md5 member variable."

@jksemple
Copy link
Author

jksemple commented Dec 5, 2023

Ha. You beat me to it.
Thanks for fixing

@VojtechBartoska VojtechBartoska added Status: Solved and removed Status: Needs investigation We need to do some research before taking next steps on this issue labels Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Libraries Issue is related to Library support. Status: Solved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants