Skip to content

Commit

Permalink
[clean] Better RP/user displayed on NFC with makeCredential and getAs…
Browse files Browse the repository at this point in the history
…sertion
  • Loading branch information
lpascal-ledger committed Nov 4, 2024
1 parent d580c19 commit 16e4ba8
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 41 deletions.
6 changes: 4 additions & 2 deletions include/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,12 @@ static const uint8_t FIDO_AID[FIDO_AID_SIZE] = {0xA0, 0x00, 0x00, 0x06, 0x47, 0x
253 // Should be 256, stax-rc4 MCU only support 255, so use 253 + 2 for now here
#define EXT_ENC_DEFAULT_LE 65536

#define NAME_BUFFER_SIZE 65

typedef struct global_s {
char buffer_20[20];
char buffer1_65[65];
char buffer2_65[65];
char buffer1_65[NAME_BUFFER_SIZE];
char buffer2_65[NAME_BUFFER_SIZE];
char display_status[131];
bool is_nfc;
bool is_getNextAssertion;
Expand Down
10 changes: 3 additions & 7 deletions include/nfc_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,9 @@ bool nfc_io_is_response_pending(void);

/*
* Sends a previously prepared response through NFC, then (if successful) displays a status screen
* (using `app_nbgl_status`). Depending on `display_infos`, this screen will contain additional
* information such as the relying party name and/or the user credential.
*
* @param display_infos If the displayed status screen should contain RP/user information or not.
* (using `app_nbgl_status`).
*/
int nfc_io_send_prepared_response(bool display_infos);
int nfc_io_send_prepared_response();

#else
static inline void nfc_io_set_le(uint32_t le __attribute__((unused))) {
Expand All @@ -46,8 +43,7 @@ static inline bool nfc_io_is_response_pending(void) {
return false;
}

static inline int nfc_io_send_prepared_response(bool display_infos) {
UNUSED(display_infos);
static inline int nfc_io_send_prepared_response() {
return -1;
}
#endif
3 changes: 0 additions & 3 deletions src/ctap2/get_assertion/get_assertion_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,6 @@ static int build_and_encode_getAssertion_response(uint8_t *buffer,
cbip_add_string(&encoder, credData->userStr, credData->userStrLen);
}

// While we're at it, copying user name on display buffer
ctap2_display_copy_username(credData->userStr, credData->userStrLen);

PRINTF("Adding user to response %.*H\n", credData->userIdLen, credData->userId);
}

Expand Down
4 changes: 4 additions & 0 deletions src/ctap2/make_credential/make_credential.c
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ static int parse_makeCred_authnr_user(cbipDecoder_t *decoder, cbipItem_t *mapIte
ctap2RegisterData->userIdLen,
ctap2RegisterData->userId);
}

return 0;
}

Expand Down Expand Up @@ -435,6 +436,9 @@ void ctap2_make_credential_handle(u2f_service_t *service, uint8_t *buffer, uint1
goto exit;
}

// RP & user decoded, we can store them into display buffer for future usage
ctap2_copy_info_on_buffers();

// Handle cryptographic algorithms
status = process_makeCred_authnr_keyCredParams(&decoder, &mapItem);
if (status != 0) {
Expand Down
2 changes: 0 additions & 2 deletions src/ctap2/make_credential/make_credential_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ void ctap2_make_credential_ux(void) {
ctap2_register_data_t *ctap2RegisterData = globals_get_ctap2_register_data();
ctap2UxState = CTAP2_UX_STATE_MAKE_CRED;

ctap2_copy_info_on_buffers();

UX_WAKE_UP();

#if defined(HAVE_BAGL)
Expand Down
2 changes: 1 addition & 1 deletion src/ctap2/processing.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void send_cbor_response(u2f_service_t *service, uint32_t length) {
status = "Login request signed";
}
nfc_io_set_response_ready(SW_NO_ERROR, length, status);
nfc_io_send_prepared_response(true);
nfc_io_send_prepared_response();
} else if (CMD_IS_OVER_U2F_CMD) {
io_send_response_pointer(responseBuffer, length, SW_NO_ERROR);
} else {
Expand Down
39 changes: 28 additions & 11 deletions src/globals.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,40 @@ uint8_t responseBuffer[IO_APDU_BUFFER_SIZE];

#include "string_utils.h"

static void copy_name_in_buffer65(char *buffer, const char *name, uint8_t nameLength) {
bool name_too_long = (nameLength >= NAME_BUFFER_SIZE);
if (name_too_long) {
nameLength = NAME_BUFFER_SIZE - 4;
memcpy(buffer, name, nameLength);
// Appending '...' at the end of the name, to highlight it was truncated
buffer[nameLength] = '.';
buffer[nameLength + 1] = '.';
buffer[nameLength + 2] = '.';
buffer[nameLength + 3] = '\0';
} else {
memcpy(buffer, name, nameLength);
buffer[nameLength] = '\0';
}
}

static void ctap2_display_copy_username(const char *name, uint8_t nameLength) {
copy_name_in_buffer65(g.buffer2_65, name, nameLength);
}

static void ctap2_display_copy_rp(const char *name, uint8_t nameLength) {
copy_name_in_buffer65(g.buffer1_65, name, nameLength);

}

void ctap2_copy_info_on_buffers(void) {
ctap2_register_data_t *ctap2RegisterData = globals_get_ctap2_register_data();

// TODO show that rp.id is truncated if necessary
uint8_t len = MIN(sizeof(g.buffer1_65) - 1, ctap2RegisterData->rpIdLen);
memcpy(g.buffer1_65, ctap2RegisterData->rpId, len);
g.buffer1_65[len] = '\0';
ctap2_display_copy_rp(ctap2RegisterData->rpId, ctap2RegisterData->rpIdLen);

// TODO show that user.id is truncated if necessary
if (ctap2RegisterData->userStr) {
uint8_t nameLength = MIN(ctap2RegisterData->userStrLen, sizeof(g.buffer2_65) - 1);

memcpy(g.buffer2_65, ctap2RegisterData->userStr, nameLength);
g.buffer2_65[nameLength] = '\0';
ctap2_display_copy_username(ctap2RegisterData->userStr, ctap2RegisterData->userStrLen);
} else {
uint8_t nameLength = MIN(ctap2RegisterData->userIdLen, (sizeof(g.buffer2_65) - 1) / 2);

format_hex(ctap2RegisterData->userId, nameLength, g.buffer2_65, sizeof(g.buffer2_65));
}
}
Expand All @@ -66,7 +83,7 @@ void truncate_pairs_for_display(bool large) {
}

void prepare_display_status(bool clean_buffer) {
if (!g.is_nfc || clean_buffer) {
if (clean_buffer) {
PRINTF("NO NFC or cleaning, so no display status for buffer1_65 '%s' and buffer2_65 '%s'\n",
g.buffer1_65,
g.buffer2_65);
Expand Down
11 changes: 3 additions & 8 deletions src/nfc_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ bool nfc_io_is_response_pending(void) {
return nfc_data_ready;
}

int nfc_io_send_prepared_response(bool display_infos) {
int nfc_io_send_prepared_response() {
if (!nfc_data_ready) {
return io_send_sw(SW_WRONG_DATA);
}
Expand Down Expand Up @@ -81,13 +81,8 @@ int nfc_io_send_prepared_response(bool display_infos) {
}

int ret = io_send_response_pointer(responseBuffer + start, size, sw);
if (sw == SW_NO_ERROR && nfc_status != NULL) {
if (display_infos) {
ctap2_copy_info_on_buffers();
}
if (!g.is_getNextAssertion) {
app_nbgl_status(nfc_status, true, ui_idle);
}
if (sw == SW_NO_ERROR && nfc_status != NULL && !g.is_getNextAssertion) {
app_nbgl_status(nfc_status, true, ui_idle);
}

return ret;
Expand Down
8 changes: 4 additions & 4 deletions src/u2f_processing.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ static int u2f_handle_apdu_enroll(const uint8_t *rx, uint32_t data_length, const

nfc_io_set_response_ready(sw, length, "Registration details\nsent");

return nfc_io_send_prepared_response(false);
return nfc_io_send_prepared_response();
} else if (CMD_IS_OVER_U2F_USB) {
u2f_message_set_autoreply_wait_user_presence(&G_io_u2f, true);
}
Expand Down Expand Up @@ -405,7 +405,7 @@ int u2f_handle_apdu(uint8_t *rx, int rx_length) {
if (!CMD_IS_OVER_U2F_NFC) {
return io_send_sw(SW_INS_NOT_SUPPORTED);
}
return nfc_io_send_prepared_response(false);
return nfc_io_send_prepared_response();

default:
PRINTF("unsupported\n");
Expand All @@ -419,14 +419,14 @@ int u2f_handle_apdu(uint8_t *rx, int rx_length) {

case 0x11:
PRINTF("NFCCTAP_GETRESPONSE\n");
return nfc_io_send_prepared_response(false);
return nfc_io_send_prepared_response();

case FIDO2_NFC_INS_APPLET_DESELECT:
PRINTF("unsupported\n");
return io_send_sw(SW_INS_NOT_SUPPORTED);

case 0xc0:
return nfc_io_send_prepared_response(false);
return nfc_io_send_prepared_response();

default:
PRINTF("unsupported\n");
Expand Down
6 changes: 3 additions & 3 deletions src/ui_shared_nbgl.c
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ static void tickerCallback(void) {
}

void app_nbgl_status(const char *message, bool is_success, nbgl_callback_t on_quit) {
if (is_success) {
if (g.is_nfc && is_success) {
// Truncate display buffers for small police (hence `false`) then format them into the
// display buffer (which is then used in `centeredInfo.text3`)
truncate_pairs_for_display(false);
Expand Down Expand Up @@ -310,8 +310,8 @@ void app_nbgl_status(const char *message, bool is_success, nbgl_callback_t on_qu
.centeredInfo.style = LARGE_CASE_INFO,
.centeredInfo.text1 = message,
.centeredInfo.text2 = NULL,
.centeredInfo.text3 = NULL,
/* .centeredInfo.text3 = g.display_status[0] == 0 ? NULL : &g.display_status[0], */
/* .centeredInfo.text3 = NULL, */
.centeredInfo.text3 = g.display_status[0] == 0 ? NULL : &g.display_status[0],
.tapActionText = NULL,
.tapActionToken = QUIT_TOKEN,
.topRightStyle = NO_BUTTON_STYLE,
Expand Down

0 comments on commit 16e4ba8

Please sign in to comment.