Conversation
Remove sed command for modifying Makefile in apollo-lib installation step.
There was a problem hiding this comment.
Pull request overview
This PR migrates the project’s crypto usage from PolarSSL to mbedTLS and updates build/docs accordingly.
Changes:
- Replaced PolarSSL includes/APIs with mbedTLS equivalents for AES, SHA1/SHA256, MD5, ARC4, and HMAC.
- Updated linking/build pipeline to install and link against mbedTLS libraries in CI and the Makefile.
- Removed the
SKIP64macro and updated PFD-related code/macros to use direct field values.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| source/util.c | Switches HMAC-SHA1 implementation to mbedtls_md_* helpers. |
| source/rifrap.c | Migrates AES ECB + SHA1 calls to mbedTLS. |
| source/psv_resign.c | Migrates AES + SHA1 usage to mbedTLS contexts. |
| source/psv_ps2.c | Migrates ARC4 usage to mbedTLS ARC4 APIs. |
| source/ps2classic.c | Migrates AES + SHA1 usage to mbedTLS. |
| source/ps1card.c | Migrates AES CBC + SHA256 to mbedTLS. |
| source/pfd_util.c | Migrates AES operations to mbedTLS. |
| source/pfd.c | Migrates AES + HMAC-SHA1 to mbedTLS and removes SKIP64 usage. |
| source/exec_cmd.c | Migrates MD5 hashing to mbedTLS MD5 context. |
| include/types.h | Removes SKIP64 macro. |
| include/pfd_internal.h | Updates size/offset macros to no longer use SKIP64. |
| docs/README.md | Documentation update from PolarSSL dependency to mbedTLS. |
| README.md | Documentation update from PolarSSL dependency to mbedTLS. |
| Makefile | Replaces -lpolarssl with mbedTLS libraries. |
| .github/workflows/build.yml | Adds CI step to build/install mbedTLS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mbedtls_md_init(&sha1); | ||
| mbedtls_md_setup(&sha1, mbedtls_md_info_from_type(MBEDTLS_MD_SHA1), 1); | ||
|
|
||
| sha1_hmac_starts(&sha1, key, key_length); | ||
| mbedtls_md_hmac_starts(&sha1, key, key_length); | ||
| while ((n = fread(buf, 1, sizeof(buf), fp)) > 0) | ||
| sha1_hmac_update(&sha1, buf, n); | ||
| sha1_hmac_finish(&sha1, output); | ||
| mbedtls_md_hmac_update(&sha1, buf, n); | ||
| mbedtls_md_hmac_finish(&sha1, output); | ||
|
|
||
| memset(&sha1, 0, sizeof(sha1_context)); | ||
| mbedtls_md_free(&sha1); |
There was a problem hiding this comment.
calculate_file_hmac_hash doesn't check the return codes from mbedtls_md_setup / mbedtls_md_hmac_*. If any of these calls fail, the function can still return 0 and/or leave output undefined, which can silently produce incorrect HMACs. Capture and validate each mbedTLS return value, and on failure ensure the md context is freed and the file is closed before returning an error (keeping the function's existing 0/-1 style contract).
| mbedtls_md_init(&sha1); | ||
| mbedtls_md_setup(&sha1, mbedtls_md_info_from_type(MBEDTLS_MD_SHA1), 1); | ||
|
|
||
| sha1_hmac_starts(&sha1, ctx->real_hash_key, PFD_HASH_KEY_SIZE); | ||
| mbedtls_md_hmac_starts(&sha1, ctx->real_hash_key, PFD_HASH_KEY_SIZE); | ||
|
|
||
| while (current_entry_index < SKIP64(ctx->hash_table->num_reserved)) { | ||
| while (current_entry_index < (ctx->hash_table->num_reserved)) { | ||
| entry = &ctx->entry_table->entries[current_entry_index]; | ||
| sha1_hmac_update(&sha1, (const u8 *)entry->file_name, PFD_ENTRY_NAME_SIZE); | ||
| sha1_hmac_update(&sha1, entry->key, PFD_ENTRY_DATA_SIZE); | ||
| current_entry_index = SKIP64(entry->additional_index); | ||
| mbedtls_md_hmac_update(&sha1, (const u8 *)entry->file_name, PFD_ENTRY_NAME_SIZE); | ||
| mbedtls_md_hmac_update(&sha1, entry->key, PFD_ENTRY_DATA_SIZE); | ||
| current_entry_index = (entry->additional_index); | ||
| } | ||
|
|
||
| sha1_hmac_finish(&sha1, hash); | ||
| mbedtls_md_hmac_finish(&sha1, hash); | ||
|
|
||
| memset(&sha1, 0, sizeof(sha1_context)); | ||
| mbedtls_md_free(&sha1); |
There was a problem hiding this comment.
pfd_calculate_entry_hash ignores return codes from mbedtls_md_setup and the HMAC start/update/finish calls. If setup or any HMAC step fails, hash may be incorrect/uninitialized but the function still returns success. Please check each mbedTLS return value, and bail out with mbedtls_md_free(&sha1) and an error return when any step fails.
No description provided.