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

driver: add uqmi backend #149

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blocktrron
Copy link
Contributor

This commit adds a backend to interface with an eUICC connected to a compatible modem using the uqmi application.

This allows OpenWrt to manage an eUICC including the download and installation of SIM profiles.

This was previously not possible with most modems, as the APDU operations were aborted due to exceeding the timeout imposed on the AT interface by the modem firmware.

Tested-on: Quectel EC25 / Quectel EP06 / Quectel RG520N

This commit adds a backend to interface with an eUICC connected to a
compatible modem using the uqmi application.

This allows OpenWrt to manage an eUICC including the download and
installation of SIM profiles.

This was previously not possible with most modems, as the APDU
operations were aborted due to exceeding the timeout imposed
on the AT interface by the modem firmware.

Tested-on: Quectel EC25 / Quectel EP06 / Quectel RG520N

Signed-off-by: David Bauer <[email protected]>
Copy link
Contributor

@septs septs left a comment

Choose a reason for hiding this comment

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

this is safe from command injection attacks?

see https://owasp.org/www-community/attacks/Command_Injection

@blocktrron
Copy link
Contributor Author

@septs Where do you see the potential for Command injection? APDU data is generated by the existing lpac infrastructure and all binary data potentially landing in the system shell is formatted only in hex-chars here:

https://github.com/estkme-group/lpac/pull/149/files#diff-ec954341e60cb55be4a97cf02c58237678cc535c127029fe5a6c95fd228c34f2R125

The buffer is fixed-size 2048 byte long, which fits the maximum tansport size. However, it currently does not check if tx_len fits in there, I will fix this.

To give a it of background: uqmi is a standalone tool for modem-control on OpenWrt, so it sadly does not have a linkable library we could use and make the detour over the shell. This is set to change, as uqmi is being rewritten to provide an rpc interface. For the time being however, uqmi is the best way to interact with eUICCs in modems with small flash constraints like the ones targeted by OpenWrt.

Still looking forward for your feedback!

return -1;
}

fp = popen(final_command, "r");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should avoid system(3) or popen(3) because they pass commandline to shell and then shell will do some extra things like wordsplit, glob expansion. It doesn't execute as what we write.

Copy link
Contributor

Choose a reason for hiding this comment

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

client_id, logic_channel, buf);
uqmi_execute_command(cmd, buf, sizeof(buf));

json = strstr(buf, "{");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's unnecessary since it has been covered by cJSON_Parse().

Comment on lines +123 to +126
for (int i = 0; i < tx_len; i++)
{
snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "%02X", (uint8_t)(tx[i] & 0xFF));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

builtin bin to hex string function, plz use euicc_hexutil_bin2hex

lpac/euicc/hexutil.h

Lines 4 to 8 in 9e0612b

int euicc_hexutil_hex2bin_r(uint8_t *output, uint32_t output_len, const char *str, uint32_t str_len);
int euicc_hexutil_hex2bin(uint8_t *output, uint32_t output_len, const char *str);
int euicc_hexutil_bin2hex(char *output, uint32_t output_len, const uint8_t *bin, uint32_t bin_len);
int euicc_hexutil_gsmbcd2bin(uint8_t *output, uint32_t output_len, const char *str, uint32_t padding_to);
int euicc_hexutil_bin2gsmbcd(char *output, uint32_t output_len, const uint8_t *binData, uint32_t length);

Comment on lines +50 to +52
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DLPAC_WITH_APDU_UQMI")
target_sources(euicc-drivers PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/apdu/uqmi.c)

Copy link
Contributor

@septs septs Aug 30, 2024

Choose a reason for hiding this comment

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

Suggested change
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DLPAC_WITH_APDU_UQMI")
target_sources(euicc-drivers PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/apdu/uqmi.c)
if(LPAC_WITH_APDU_UQMI)
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DLPAC_WITH_APDU_UQMI")
target_sources(euicc-drivers PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/apdu/uqmi.c)
endif()

uQMI is optional and not available on all platforms

@z3ntu
Copy link
Contributor

z3ntu commented Aug 30, 2024

Is there any difference to #131 apart from using uqmi instead of libqmi? The functionality should be the same, right?

@septs
Copy link
Contributor

septs commented Aug 30, 2024

Is there any difference to #131 apart from using uqmi instead of libqmi? The functionality should be the same, right?

uQMI is OpenWrt specific

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.

4 participants