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

POC of wrapped + sync transaction review #111

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
ifeq ($(BOLOS_SDK),)
$(error Environment variable BOLOS_SDK is not set)
endif

DISABLE_UI = 1
include $(BOLOS_SDK)/Makefile.defines

########################################
Expand All @@ -34,7 +34,19 @@ APPVERSION_P = 0
APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"

# Application source files
APP_SOURCE_PATH += src
APP_SOURCE_PATH += src sdk
ifeq ($(TARGET_NAME),TARGET_NANOS)
DEFINES += HAVE_NBGL NBGL_USE_CASE NBGL_STEP
DEFINES += TARGET_NANOS=1
DEFINES += HAVE_BAGL
DEFINES += BAGL_WIDTH=128 BAGL_HEIGHT=32
DISABLE_STANDARD_BAGL_UX_FLOW = 1
SDK_SOURCE_PATH += lib_bagl
APP_SOURCE_PATH += sdk_lib_nbgl/src sdk_lib_ux_stax

INCLUDES_PATH += sdk_lib_nbgl/include sdk_lib_ux_stax
CFLAGS += -Wno-macro-redefined
endif

# Application icons following guidelines:
# https://developers.ledger.com/docs/embedded-app/design-requirements/#device-icon
Expand Down
2 changes: 1 addition & 1 deletion ledger_app.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[app]
build_directory = "./"
sdk = "C"
devices = ["nanos", "nanox", "nanos+", "stax"]
devices = ["stax"]

[tests]
unit_directory = "./unit-tests/"
Expand Down
224 changes: 224 additions & 0 deletions sdk/io.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
/*****************************************************************************
* (c) 2021 Ledger SAS.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*****************************************************************************/

#include <stdint.h>
#include <string.h>

#include "os.h"
#include "io.h"
#include "write.h"

#ifdef HAVE_SWAP
#include "swap.h"
#endif

// TODO: Temporary workaround, at some point all status words should be defined by the SDK and
// removed from the application
#define SW_OK 0x9000
#define SW_WRONG_RESPONSE_LENGTH 0xB000

// Avoid conflict with SDK version
//uint8_t G_io_seproxyhal_spi_buffer[IO_SEPROXYHAL_BUFFER_SIZE_B];

/**
* Variable containing the length of the APDU response to send back.
*/
static uint32_t G_output_len = 0;

/**
* IO state (READY, RECEIVING, WAITING).
*/
static io_state_e G_io_state = READY;

#ifdef HAVE_BAGL
void io_seproxyhal_display(const bagl_element_t *element)
{
io_seproxyhal_display_default(element);

Check warning

Code scanning / CodeQL

Implicit function declaration Warning

Function call implicitly declares 'io_seproxyhal_display_default'.
}
#endif // HAVE_BAGL

// This function can be used to declare a callback to SEPROXYHAL_TAG_TICKER_EVENT in the application
void app_ticker_event_callback(void) {}

uint8_t io_event(uint8_t channel)
{
(void) channel;

switch (G_io_seproxyhal_spi_buffer[0]) {
case SEPROXYHAL_TAG_BUTTON_PUSH_EVENT:
#ifdef HAVE_BAGL
UX_BUTTON_PUSH_EVENT(G_io_seproxyhal_spi_buffer);
#endif // HAVE_BAGL
break;
case SEPROXYHAL_TAG_STATUS_EVENT:
if (G_io_apdu_media == IO_APDU_MEDIA_USB_HID && //
!(U4BE(G_io_seproxyhal_spi_buffer, 3) & //
SEPROXYHAL_TAG_STATUS_EVENT_FLAG_USB_POWERED)) {
THROW(EXCEPTION_IO_RESET);
}
__attribute__((fallthrough));
case SEPROXYHAL_TAG_DISPLAY_PROCESSED_EVENT:
#ifdef HAVE_BAGL
UX_DISPLAYED_EVENT({});
#endif // HAVE_BAGL
#ifdef HAVE_NBGL
UX_DEFAULT_EVENT();
#endif // HAVE_NBGL
break;
#ifdef HAVE_NBGL
case SEPROXYHAL_TAG_FINGER_EVENT:
UX_FINGER_EVENT(G_io_seproxyhal_spi_buffer);
break;
#endif // HAVE_NBGL
case SEPROXYHAL_TAG_TICKER_EVENT:
app_ticker_event_callback();
UX_TICKER_EVENT(G_io_seproxyhal_spi_buffer, {});
break;
default:
UX_DEFAULT_EVENT();
break;
}

if (!io_seproxyhal_spi_is_status_sent()) {
io_seproxyhal_general_status();
}

return 1;
}

uint16_t io_exchange_al(uint8_t channel, uint16_t tx_len)
{
switch (channel & ~(IO_FLAGS)) {
case CHANNEL_KEYBOARD:
break;
case CHANNEL_SPI:
if (tx_len) {
io_seproxyhal_spi_send(G_io_apdu_buffer, tx_len);

if (channel & IO_RESET_AFTER_REPLIED) {
halt();
}

return 0;
}
else {
return io_seproxyhal_spi_recv(G_io_apdu_buffer, sizeof(G_io_apdu_buffer), 0);
}
default:
THROW(INVALID_PARAMETER);
}

return 0;
}

void io_init()
{
// Reset length of APDU response
G_output_len = 0;
G_io_state = READY;
}

int io_recv_command()
{
int ret = -1;

switch (G_io_state) {
case READY:
G_io_state = RECEIVED;
// IO_CONTINUE_RX needed in case an APDU is received during a sync NBGL call
// so that we will process it at the next io_recv_command() call.
// Note that this needs a small change in io_exchange().
// local definition for cases where it's not yet implemented in the SDK
#ifndef IO_CONTINUE_RX
#define IO_CONTINUE_RX 0
#endif
ret = io_exchange(CHANNEL_APDU | IO_CONTINUE_RX, G_output_len);
break;
case RECEIVED:
G_io_state = WAITING;
ret = io_exchange(CHANNEL_APDU | IO_ASYNCH_REPLY, G_output_len);
G_io_state = RECEIVED;
break;
case WAITING:
G_io_state = READY;
ret = -1;
break;
}

return ret;
}

int io_send_response_buffers(const buffer_t *rdatalist, size_t count, uint16_t sw)
{
int ret = -1;

G_output_len = 0;
if (rdatalist && count > 0) {
for (size_t i = 0; i < count; i++) {
const buffer_t *rdata = &rdatalist[i];

if (!buffer_copy(rdata,
G_io_apdu_buffer + G_output_len,
sizeof(G_io_apdu_buffer) - G_output_len - 2)) {
return io_send_sw(SW_WRONG_RESPONSE_LENGTH);
}
G_output_len += rdata->size - rdata->offset;
if (count > 1) {
PRINTF("<= FRAG (%u/%u) RData=%.*H\n", i + 1, count, rdata->size, rdata->ptr);
}
}
PRINTF("<= SW=%04X | RData=%.*H\n", sw, G_output_len, G_io_apdu_buffer);
}
else {
PRINTF("<= SW=%04X | RData=\n", sw);
}

write_u16_be(G_io_apdu_buffer, G_output_len, sw);
G_output_len += 2;

#ifdef HAVE_SWAP
// If we are in swap mode and have validated a TX, we send it and immediately quit
if (G_called_from_swap && G_swap_response_ready) {
PRINTF("Swap answer is processed. Send it\n");
if (io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, G_output_len) == 0) {
swap_finalize_exchange_sign_transaction(sw == SW_OK);
}
else {
PRINTF("Unrecoverable\n");
os_sched_exit(-1);
}
}
#endif // HAVE_SWAP

switch (G_io_state) {
case READY:
ret = -1;
break;
case RECEIVED:
#if 0 // enforce io_send_xxx to be synchronous
G_io_state = READY;
ret = 0;
break;
#endif
case WAITING:
ret = io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, G_output_len);
G_output_len = 0;
G_io_state = READY;
break;
}

return ret;
}
94 changes: 94 additions & 0 deletions sdk/nbgl_sync.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
#include "nbgl_use_case.h"
#include "nbgl_sync.h"
#include "nbgl_wrapper.h"
#include "io.h"
#include "ledger_assert.h"
#include "os.h"

static sync_nbgl_ret_t ret;
static bool ended;


static void choice_callback(bool confirm) {
if (confirm) {
ret = NBGL_SYNC_RET_SUCCESS;
} else {
ret = NBGL_SYNC_RET_REJECTED;
}

ended = true;
}

static void quit_callback(void) {
ret = NBGL_SYNC_RET_SUCCESS;
ended = true;
}

static void sync_nbgl_init(void) {
ended = false;
ret = NBGL_SYNC_RET_ERROR;
}

static sync_nbgl_ret_t sync_nbgl_wait(void) {
int apdu_state = G_io_app.apdu_state;
while (!ended) {

// Check for a apdu_state change.
// This would means an APDU was received.
// In such case immediately stop the loop.
// We could expose another version that would ignore such APDU event.
// Anyway, it will be handled only at next app call of io_recv_command().
if (apdu_state == APDU_IDLE && G_io_app.apdu_state != APDU_IDLE) {
return NBGL_SYNC_RET_RX_APDU;
}
Comment on lines +41 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure to understand the need to end the loop in this case.
New APDU should be ignored because a transaction is already being processed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a proposal to interrupt sync nbgl calls like nbgl_useCaseStatus, or nbgl_useCaseHome during which there is no transaction being processed.
But this might indeed disappear as we might want to respond to the APDU only after the timeout of nbgl_useCaseStatus and I'm not sure we will make a sync version of nbgl_useCaseHome

// - Looping on os_io_seph_recv_and_process(0);
// - This will send a general_status and then wait for an event.
// - Upon event reception this will call io_seproxyhal_handle_event()
// - On some case this will call io_event() which usually forward the
// event to the UX lib.
os_io_seph_recv_and_process(0);
}

return ret;
}

sync_nbgl_ret_t sync_nbgl_useCaseTransactionReview(
const nbgl_layoutTagValueList_t *tagValueList,
const nbgl_icon_details_t *icon,
const char *reviewTitle,
const char *reviewSubTitle, /* Most often this is empty, but sometimes indicates a path / index */
const char *finish_page_text /*unused on Nano*/)
{
sync_nbgl_init();
nbgl_useCaseTransactionReview(tagValueList,
icon,
reviewTitle,
reviewSubTitle,
finish_page_text,
choice_callback);
return sync_nbgl_wait();
}

sync_nbgl_ret_t sync_nbgl_useCaseAddressReview(
const char *address,
const nbgl_icon_details_t *icon,
const char *reviewTitle,
const char *reviewSubTitle)
{
sync_nbgl_init();

nbgl_useCaseAddressReview(address,
icon,
reviewTitle,
reviewSubTitle,
choice_callback);

return sync_nbgl_wait();
}

sync_nbgl_ret_t sync_nbgl_useCaseStatus(const char *message, bool isSuccess)
{
sync_nbgl_init();
nbgl_useCaseStatus(message, isSuccess, quit_callback);

Check warning

Code scanning / CodeQL

Call to a function with one or more incompatible arguments Warning

Calling
nbgl_useCaseStatus
: argument
isSuccess
of type
bool
is incompatible with parameter
bool isSuccess
.
return sync_nbgl_wait();
}
23 changes: 23 additions & 0 deletions sdk/nbgl_sync.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include "nbgl_use_case.h"

typedef enum {
NBGL_SYNC_RET_SUCCESS,
NBGL_SYNC_RET_REJECTED,
NBGL_SYNC_RET_RX_APDU,
NBGL_SYNC_RET_ERROR
} sync_nbgl_ret_t;

sync_nbgl_ret_t sync_nbgl_useCaseTransactionReview(
const nbgl_layoutTagValueList_t *tagValueList,
const nbgl_icon_details_t *icon,
const char *reviewTitle,
const char *reviewSubTitle, /* Most often this is empty, but sometimes indicates a path / index */
const char *finish_page_text /*unused on Nano*/);

sync_nbgl_ret_t sync_nbgl_useCaseAddressReview(
const char *address,
const nbgl_icon_details_t *icon,
const char *reviewTitle,
const char *reviewSubTitle);

sync_nbgl_ret_t sync_nbgl_useCaseStatus(const char *message, bool isSuccess);
Loading
Loading