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

B2CA-1619: Blind Signing friction #645

Merged
merged 4 commits into from
Sep 24, 2024
Merged
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
2 changes: 2 additions & 0 deletions src/common_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <stdbool.h>
#include <stdint.h>
#include "ui_logic.h"

void ui_idle(void);
void ui_warning_blind_signing(void);
Expand All @@ -24,6 +25,7 @@ void ui_191_switch_to_question(void);

// EIP-712
void ui_712_start(void);
void ui_712_start_unfiltered(void);
void ui_712_switch_to_message(void);
void ui_712_switch_to_sign(void);

Expand Down
2 changes: 2 additions & 0 deletions src_bagl/ui_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ extern const ux_flow_step_t* const ux_display_privacy_shared_secret_flow[];

extern const ux_flow_step_t* ux_approval_tx_flow[15];

extern const ux_flow_step_t ux_warning_blind_signing_warn_step;

#endif // _UI_FLOW_H_
53 changes: 49 additions & 4 deletions src_bagl/ui_flow_signMessage712.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
#include "ui_logic.h"
#include "shared_context.h" // strings
#include "common_ui.h"
#include "ui_flow.h" // ux_warning_blind_signing_warn_step

enum { UI_712_POS_REVIEW, UI_712_POS_END };
static uint8_t ui_pos;
static bool filtered;

// forward declaration for the BAGL step
static void ui_712_start_flow(void);

static void dummy_cb(void) {
switch (ui_712_next_field()) {
Expand Down Expand Up @@ -72,6 +77,15 @@ UX_STEP_CB(
&C_icon_validate_14,
"Approve",
});
UX_STEP_CB(
ux_712_step_approve_risky,
pbb,
_approve_cb(),
{
&C_icon_validate_14,
"Accept risk",
"and sign",
});
UX_STEP_CB(
ux_712_step_reject,
pb,
Expand All @@ -80,6 +94,15 @@ UX_STEP_CB(
&C_icon_crossmark,
"Reject",
});

UX_STEP_INIT(
ux_712_warning_blind_signing_jump_step,
NULL,
NULL,
{
ui_712_start_flow();
}
);
// clang-format on

UX_FLOW(ux_712_flow,
Expand All @@ -89,18 +112,40 @@ UX_FLOW(ux_712_flow,
&ux_712_step_approve,
&ux_712_step_reject);

void ui_712_start(void) {
ux_flow_init(0, ux_712_flow, NULL);
UX_FLOW(ux_712_flow_unfiltered,
&ux_712_step_review,
&ux_712_step_dynamic,
&ux_712_step_dummy,
&ux_712_step_approve_risky,
&ux_712_step_reject);

UX_FLOW(ux_712_warning_blind_signing_flow,
&ux_warning_blind_signing_warn_step,
&ux_712_warning_blind_signing_jump_step);

static void ui_712_start_flow(void) {
ux_flow_init(0, filtered ? ux_712_flow : ux_712_flow_unfiltered, NULL);
ui_pos = UI_712_POS_REVIEW;
}
void ui_712_start(void) {
filtered = true;
ui_712_start_flow();
}

void ui_712_start_unfiltered(void) {
filtered = false;
ux_flow_init(0, ux_712_warning_blind_signing_flow, NULL);
}

void ui_712_switch_to_message(void) {
ux_flow_init(0, ux_712_flow, &ux_712_step_dynamic);
ux_flow_init(0, filtered ? ux_712_flow : ux_712_flow_unfiltered, &ux_712_step_dynamic);
ui_pos = UI_712_POS_REVIEW;
}

void ui_712_switch_to_sign(void) {
ux_flow_init(0, ux_712_flow, &ux_712_step_approve);
ux_flow_init(0,
filtered ? ux_712_flow : ux_712_flow_unfiltered,
filtered ? &ux_712_step_approve : &ux_712_step_approve_risky);
ui_pos = UI_712_POS_END;
}

Expand Down
7 changes: 6 additions & 1 deletion src_features/signMessageEIP712/commands_712.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,19 @@
* @param[in] success whether the command was successful
*/
static void apdu_reply(bool success) {
bool home = true;

if (success) {
apdu_response_code = APDU_RESPONSE_OK;
} else {
if (apdu_response_code == APDU_RESPONSE_OK) { // somehow not set
apdu_response_code = APDU_RESPONSE_ERROR_NO_INFO;
}
if (eip712_context != NULL) {
home = eip712_context->go_home_on_failure;
}
eip712_context_deinit();
ui_idle();
if (home) ui_idle();
}
}

Expand Down
1 change: 1 addition & 0 deletions src_features/signMessageEIP712/context_712.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ bool eip712_context_init(void) {
// Since they are optional, they might not be provided by the JSON data
explicit_bzero(eip712_context->contract_addr, sizeof(eip712_context->contract_addr));
eip712_context->chain_id = 0;
eip712_context->go_home_on_failure = true;

struct_state = NOT_INITIALIZED;

Expand Down
1 change: 1 addition & 0 deletions src_features/signMessageEIP712/context_712.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ typedef struct {
uint8_t contract_addr[ADDRESS_LENGTH];
uint64_t chain_id;
uint8_t schema_hash[224 / 8];
bool go_home_on_failure;
} s_eip712_context;

extern s_eip712_context *eip712_context;
Expand Down
21 changes: 17 additions & 4 deletions src_features/signMessageEIP712/ui_logic.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ typedef struct {
typedef struct {
bool shown;
bool end_reached;
uint8_t filtering_mode;
e_eip712_filtering_mode filtering_mode;
uint8_t filters_to_process;
uint8_t field_flags;
uint8_t structs_to_review;
Expand Down Expand Up @@ -154,13 +154,26 @@ void ui_712_set_value(const char *str, size_t length) {
/**
* Redraw the dynamic UI step that shows EIP712 information
*/
void ui_712_redraw_generic_step(void) {
bool ui_712_redraw_generic_step(void) {
if (!ui_ctx->shown) { // Initialize if it is not already
ui_712_start();
if ((ui_ctx->filtering_mode == EIP712_FILTERING_BASIC) && !N_storage.dataAllowed &&
!N_storage.verbose_eip712) {
// Both settings not enabled => Error
ui_error_blind_signing();
apdu_response_code = APDU_RESPONSE_INVALID_DATA;
eip712_context->go_home_on_failure = false;
return false;
}
if (ui_ctx->filtering_mode == EIP712_FILTERING_BASIC) {
ui_712_start_unfiltered();
} else {
ui_712_start();
}
ui_ctx->shown = true;
} else {
ui_712_switch_to_message();
}
return true;
}

/**
Expand Down Expand Up @@ -652,7 +665,7 @@ bool ui_712_feed_to_display(const void *field_ptr,

// Check if this field is supposed to be displayed
if (last && ui_712_field_shown()) {
ui_712_redraw_generic_step();
if (!ui_712_redraw_generic_step()) return false;
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src_features/signMessageEIP712/ui_logic.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ unsigned int ui_712_reject();
void ui_712_set_title(const char *str, size_t length);
void ui_712_set_value(const char *str, size_t length);
void ui_712_message_hash(void);
void ui_712_redraw_generic_step(void);
bool ui_712_redraw_generic_step(void);
void ui_712_flag_field(bool show,
bool name_provided,
bool token_join,
Expand Down
5 changes: 4 additions & 1 deletion src_features/signTx/logic_signTx.c
Original file line number Diff line number Diff line change
Expand Up @@ -585,10 +585,13 @@ uint16_t finalizeParsing(void) {
ui_idle();
io_seproxyhal_touch_tx_ok();
} else {
#ifdef HAVE_BAGL
// If blind-signing detected, start the warning flow beforehand
if (tmpContent.txContent.dataPresent) {
ui_warning_blind_signing();
} else {
} else
#endif
{
start_signature_flow();
}
}
Expand Down
61 changes: 29 additions & 32 deletions src_nbgl/ui_approve_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,20 +167,23 @@ static uint8_t setTagValuePairs(void) {
return nbPairs;
}

static void reviewCommon(void) {
void ux_approve_tx(bool fromPlugin) {
explicit_bzero(&pairsList, sizeof(pairsList));
explicit_bzero(&tx_approval_context, sizeof(tx_approval_context));
tx_approval_context.fromPlugin = fromPlugin;

uint64_t chain_id = get_tx_chain_id();
if (chainConfig->chainId == ETHEREUM_MAINNET_CHAINID && chain_id != chainConfig->chainId) {
tx_approval_context.displayNetwork = true;
} else {
tx_approval_context.displayNetwork = false;
}

pairsList.nbPairs = setTagValuePairs();
pairsList.pairs = pairs;
nbgl_operationType_t op = TYPE_TRANSACTION;

#if API_LEVEL >= 19
if (tmpContent.txContent.dataPresent) {
op |= BLIND_OPERATION;
}
#endif
uint32_t buf_size = SHARED_BUFFER_SIZE / 2;
if (tx_approval_context.fromPlugin) {
uint32_t buf_size = SHARED_BUFFER_SIZE / 2;
char op_name[sizeof(strings.common.fullAmount)];
plugin_ui_get_id();

Expand All @@ -198,36 +201,30 @@ static void reviewCommon(void) {
op_name,
(pluginType == EXTERNAL ? "on " : ""),
strings.common.toAddress);
} else {
snprintf(g_stax_shared_buffer, buf_size, REVIEW("transaction"));
snprintf(
g_stax_shared_buffer + buf_size,
buf_size,
tmpContent.txContent.dataPresent ? BLIND_SIGN("transaction") : SIGN("transaction"));
}

nbgl_useCaseReview(op,
if (tmpContent.txContent.dataPresent) {
nbgl_useCaseReviewBlindSigning(TYPE_TRANSACTION,
&pairsList,
get_tx_icon(),
g_stax_shared_buffer,
NULL,
g_stax_shared_buffer + buf_size,
NULL,
reviewChoice);
} else {
nbgl_useCaseReview(TYPE_TRANSACTION,
&pairsList,
get_tx_icon(),
g_stax_shared_buffer,
NULL,
g_stax_shared_buffer + buf_size,
reviewChoice);
} else {
nbgl_useCaseReview(
op,
&pairsList,
get_tx_icon(),
REVIEW("transaction"),
NULL,
tmpContent.txContent.dataPresent ? BLIND_SIGN("transaction") : SIGN("transaction"),
reviewChoice);
}
}

void ux_approve_tx(bool fromPlugin) {
memset(&tx_approval_context, 0, sizeof(tx_approval_context));

tx_approval_context.fromPlugin = fromPlugin;
tx_approval_context.displayNetwork = false;

uint64_t chain_id = get_tx_chain_id();
if (chainConfig->chainId == ETHEREUM_MAINNET_CHAINID && chain_id != chainConfig->chainId) {
tx_approval_context.displayNetwork = true;
}

reviewCommon();
}
19 changes: 1 addition & 18 deletions src_nbgl/ui_blind_signing.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "feature_signTx.h"
#include "ui_nbgl.h"
#include "apdu_constants.h"
#include "context_712.h"

static void ui_error_blind_signing_choice(bool confirm) {
if (confirm) {
Expand All @@ -13,14 +14,6 @@ static void ui_error_blind_signing_choice(bool confirm) {
}
}

static void ui_warning_blind_signing_choice(bool confirm) {
if (confirm) {
io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, true);
} else {
start_signature_flow();
}
}

void ui_error_blind_signing(void) {
nbgl_useCaseChoice(&C_Warning_64px,
"This transaction cannot be clear-signed",
Expand All @@ -29,13 +22,3 @@ void ui_error_blind_signing(void) {
"Reject transaction",
ui_error_blind_signing_choice);
}

void ui_warning_blind_signing(void) {
nbgl_useCaseChoice(&C_Warning_64px,
"Blind signing ahead",
"This transaction's details are not fully verifiable. If you sign it, you "
"could lose all your assets.",
"Back to safety",
"Continue anyway",
ui_warning_blind_signing_choice);
}
9 changes: 5 additions & 4 deletions src_nbgl/ui_message_signing.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
#include <stdbool.h>
#include "ui_signing.h"

#define TEXT_MESSAGE "message"
#define TEXT_TYPED_MESSAGE "typed " TEXT_MESSAGE
#define TEXT_REVIEW_EIP712 REVIEW(TEXT_TYPED_MESSAGE)
#define TEXT_SIGN_EIP712 SIGN(TEXT_TYPED_MESSAGE)
#define TEXT_MESSAGE "message"
#define TEXT_TYPED_MESSAGE "typed " TEXT_MESSAGE
#define TEXT_REVIEW_EIP712 REVIEW(TEXT_TYPED_MESSAGE)
#define TEXT_SIGN_EIP712 SIGN(TEXT_TYPED_MESSAGE)
#define TEXT_BLIND_SIGN_EIP712 BLIND_SIGN(TEXT_TYPED_MESSAGE)

void ui_typed_message_review_choice(bool confirm);

Expand Down
Loading
Loading