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

Rework blind signing policies #628

Merged
merged 5 commits into from
Aug 13, 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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [1.11.2](https://github.com/ledgerhq/app-ethereum/compare/1.11.1...1.11.2) - 2024-08-13

### Added

- Blind-signing setting

### Changed

- Simplified blind-signing warnings on Flex & Stax
- Restored blind-signing warning screen from < 1.11.0 on Nano devices

## [1.11.1](https://github.com/ledgerhq/app-ethereum/compare/1.11.0...1.11.1) - 2024-07-26

### Fixed
Expand Down Expand Up @@ -38,6 +49,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Ledger Flex support

### Removed

- (clone) Flare
- (clone) Flare Coston
- (clone) Eth Goerli
Expand Down
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ include ./makefile_conf/chain/$(CHAIN).mk

APPVERSION_M = 1
APPVERSION_N = 11
APPVERSION_P = 1
APPVERSION_P = 2
APPVERSION = $(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)

# Application source files
Expand Down Expand Up @@ -156,7 +156,6 @@ DISABLE_STANDARD_APP_FILES = 1
########################################

DEFINES += CHAINID_COINNAME=\"$(TICKER)\" CHAIN_ID=$(CHAIN_ID)
DEFINES += BUILD_YEAR=\"$(shell date +%Y)\"

# Enabled Features #
include makefile_conf/features.mk
Expand Down
7 changes: 5 additions & 2 deletions client/src/ledger_app_clients/ethereum/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,25 @@


class SettingID(Enum):
BLIND_SIGNING = auto()
VERBOSE_ENS = auto()
VERBOSE_EIP712 = auto()
NONCE = auto()
VERBOSE_EIP712 = auto()
DEBUG_DATA = auto()


def get_device_settings(firmware: Firmware) -> list[SettingID]:
if firmware == Firmware.NANOS:
return [
SettingID.BLIND_SIGNING,
SettingID.NONCE,
SettingID.DEBUG_DATA,
]
return [
SettingID.BLIND_SIGNING,
SettingID.VERBOSE_ENS,
SettingID.VERBOSE_EIP712,
SettingID.NONCE,
SettingID.VERBOSE_EIP712,
SettingID.DEBUG_DATA,
]

Expand Down
3 changes: 2 additions & 1 deletion src/common_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
#include <stdint.h>

void ui_idle(void);
void ui_warning_contract_data(void);
void ui_warning_blind_signing(void);
void ui_error_blind_signing(void);
void ui_display_public_eth2(void);
void ui_display_privacy_public_key(void);
void ui_display_privacy_shared_secret(void);
Expand Down
6 changes: 2 additions & 4 deletions src/handle_swap_sign_transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,8 @@ void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t

if (N_storage.initialized != 0x01) {
internalStorage_t storage;
storage.contractDetails = 0x00;
storage.initialized = 0x01;
storage.displayNonce = 0x00;
storage.contractDetails = 0x00;
explicit_bzero(&storage, sizeof(storage));
storage.initialized = true;
nvm_write((void*) &N_storage, (void*) &storage, sizeof(internalStorage_t));
}

Expand Down
9 changes: 1 addition & 8 deletions src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -548,14 +548,7 @@ __attribute__((noreturn)) void coin_main(libargs_t *args) {

if (!N_storage.initialized) {
internalStorage_t storage;
storage.contractDetails = false;
storage.displayNonce = false;
#ifdef HAVE_EIP712_FULL_SUPPORT
storage.verbose_eip712 = false;
#endif
#ifdef HAVE_DOMAIN_NAME
storage.verbose_domain_name = false;
#endif
explicit_bzero(&storage, sizeof(storage));
storage.initialized = true;
nvm_write((void *) &N_storage, (void *) &storage, sizeof(internalStorage_t));
}
Expand Down
1 change: 1 addition & 0 deletions src/shared_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ typedef struct bip32_path_t {
} bip32_path_t;

typedef struct internalStorage_t {
bool dataAllowed;
bool contractDetails;
bool displayNonce;
#ifdef HAVE_EIP712_FULL_SUPPORT
Expand Down
2 changes: 0 additions & 2 deletions src/ui_callbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,4 @@ unsigned int io_seproxyhal_touch_eth2_address_ok(const bagl_element_t *e);
unsigned int io_seproxyhal_touch_privacy_ok(const bagl_element_t *e);
unsigned int io_seproxyhal_touch_privacy_cancel(const bagl_element_t *e);

void ui_warning_contract_data(void);

void io_seproxyhal_send_status(uint32_t sw);
8 changes: 6 additions & 2 deletions src_bagl/common_ui.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,12 @@ void ui_idle(void) {
ux_flow_init(0, ux_idle_flow, NULL);
}

void ui_warning_contract_data(void) {
ux_flow_init(0, ux_blind_signing_flow, NULL);
void ui_error_blind_signing(void) {
ux_flow_init(0, ux_error_blind_signing_flow, NULL);
}

void ui_warning_blind_signing(void) {
ux_flow_init(0, ux_warning_blind_signing_flow, NULL);
}

void ui_display_public_eth2(void) {
Expand Down
160 changes: 79 additions & 81 deletions src_bagl/ui_flow.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,24 @@

// Reuse the strings.common.fullAmount buffer for settings displaying.
// No risk of collision as this buffer is unused in the settings menu
#define SETTING_VERBOSE_DOMAIN_NAME_STATE (strings.common.fullAmount + (BUF_INCREMENT * 0))
#define SETTING_VERBOSE_EIP712_STATE (strings.common.fullAmount + (BUF_INCREMENT * 1))
#define SETTING_BLIND_SIGNING_STATE (strings.common.fullAmount + (BUF_INCREMENT * 0))
#define SETTING_VERBOSE_DOMAIN_NAME_STATE (strings.common.fullAmount + (BUF_INCREMENT * 1))
#define SETTING_DISPLAY_NONCE_STATE (strings.common.fullAmount + (BUF_INCREMENT * 2))
#define SETTING_DISPLAY_DATA_STATE (strings.common.fullAmount + (BUF_INCREMENT * 3))
#define SETTING_VERBOSE_EIP712_STATE (strings.common.fullAmount + (BUF_INCREMENT * 3))
#define SETTING_DISPLAY_DATA_STATE (strings.common.fullAmount + (BUF_INCREMENT * 4))

#define BOOL_TO_STATE_STR(b) (b ? ENABLED_STR : DISABLED_STR)

static void display_settings(const ux_flow_step_t* const start_step);
static void switch_settings_blind_signing(void);
#ifdef HAVE_DOMAIN_NAME
static void switch_settings_verbose_domain_name(void);
#endif // HAVE_DOMAIN_NAME
static void switch_settings_display_data(void);
static void switch_settings_display_nonce(void);
#ifdef HAVE_EIP712_FULL_SUPPORT
static void switch_settings_verbose_eip712(void);
#endif // HAVE_EIP712_FULL_SUPPORT
#ifdef HAVE_DOMAIN_NAME
static void switch_settings_verbose_domain_name(void);
#endif // HAVE_DOMAIN_NAME

//////////////////////////////////////////////////////////////////////
// clang-format off
Expand Down Expand Up @@ -70,6 +72,26 @@ UX_FLOW(ux_idle_flow,
FLOW_LOOP);

// clang-format off
UX_STEP_CB(
ux_settings_flow_blind_signing_step,
#ifdef TARGET_NANOS
bnnn_paging,
#else
bnnn,
#endif
switch_settings_blind_signing(),
{
#ifdef TARGET_NANOS
.title = "Blind signing",
.text =
#else
"Blind signing",
"Enables transaction",
"blind signing",
#endif
SETTING_BLIND_SIGNING_STATE
});

#ifdef HAVE_DOMAIN_NAME
UX_STEP_CB(
ux_settings_flow_verbose_domain_name_step,
Expand All @@ -83,19 +105,6 @@ UX_STEP_CB(
});
#endif // HAVE_DOMAIN_NAME

#ifdef HAVE_EIP712_FULL_SUPPORT
UX_STEP_CB(
ux_settings_flow_verbose_eip712_step,
bnnn,
switch_settings_verbose_eip712(),
{
"Raw messages",
"Displays raw content",
"from EIP712 messages",
SETTING_VERBOSE_EIP712_STATE
});
#endif // HAVE_EIP712_FULL_SUPPORT

UX_STEP_CB(
ux_settings_flow_display_nonce_step,
#ifdef TARGET_NANOS
Expand All @@ -116,6 +125,19 @@ UX_STEP_CB(
SETTING_DISPLAY_NONCE_STATE
});

#ifdef HAVE_EIP712_FULL_SUPPORT
UX_STEP_CB(
ux_settings_flow_verbose_eip712_step,
bnnn,
switch_settings_verbose_eip712(),
{
"Raw messages",
"Displays raw content",
"from EIP712 messages",
SETTING_VERBOSE_EIP712_STATE
});
#endif // HAVE_EIP712_FULL_SUPPORT

UX_STEP_CB(
ux_settings_flow_display_data_step,
#ifdef TARGET_NANOS
Expand Down Expand Up @@ -147,17 +169,19 @@ UX_STEP_CB(
// clang-format on

UX_FLOW(ux_settings_flow,
&ux_settings_flow_blind_signing_step,
#ifdef HAVE_DOMAIN_NAME
&ux_settings_flow_verbose_domain_name_step,
#endif // HAVE_DOMAIN_NAME
&ux_settings_flow_display_nonce_step,
#ifdef HAVE_EIP712_FULL_SUPPORT
&ux_settings_flow_verbose_eip712_step,
#endif // HAVE_EIP712_FULL_SUPPORT
&ux_settings_flow_display_nonce_step,
&ux_settings_flow_display_data_step,
&ux_settings_flow_back_step);

static void display_settings(const ux_flow_step_t* const start_step) {
strlcpy(SETTING_BLIND_SIGNING_STATE, BOOL_TO_STATE_STR(N_storage.dataAllowed), BUF_INCREMENT);
strlcpy(SETTING_DISPLAY_DATA_STATE,
BOOL_TO_STATE_STR(N_storage.contractDetails),
BUF_INCREMENT);
Expand All @@ -182,6 +206,10 @@ static void toggle_setting(volatile bool* setting, const ux_flow_step_t* ui_step
display_settings(ui_step);
}

static void switch_settings_blind_signing(void) {
toggle_setting(&N_storage.dataAllowed, &ux_settings_flow_blind_signing_step);
}

static void switch_settings_display_data(void) {
toggle_setting(&N_storage.contractDetails, &ux_settings_flow_display_data_step);
}
Expand All @@ -204,76 +232,46 @@ static void switch_settings_verbose_domain_name(void) {

//////////////////////////////////////////////////////////////////////
// clang-format off
UX_STEP_NOCB(
ux_blind_signing_warning_step,
pbb,
{
&C_icon_warning,
#ifdef TARGET_NANOS
"Transaction",
"not trusted",
#else
"This transaction",
"cannot be trusted",
#endif
});
#ifndef TARGET_NANOS
UX_STEP_NOCB(
ux_blind_signing_text1_step,
nnnn,
UX_STEP_CB(
ux_error_blind_signing_step,
bnnn_paging,
ui_idle(),
{
"Your Ledger cannot",
"decode this",
"transaction. If you",
"sign it, you could",
"Error",
"Blind signing must be enabled in Settings",
});
UX_STEP_NOCB(
ux_blind_signing_text2_step,
nnnn,
#else
UX_STEP_CB(
ux_error_blind_signing_step,
pnn,
ui_idle(),
{
"be authorizing",
"malicious actions",
"that can drain your",
"wallet.",
&C_icon_crossmark,
"Blind signing must be",
"enabled in Settings",
});
#endif

UX_STEP_NOCB(
ux_blind_signing_link_step,
nn,
{
"Learn more:",
"ledger.com/e8",
});
UX_STEP_CB(
ux_blind_signing_accept_step,
ux_warning_blind_signing_warn_step,
pbb,
start_signature_flow(),
{
&C_icon_validate_14,
#ifdef TARGET_NANOS
"Accept risk",
"and review",
#else
"Accept risk and",
"review transaction",
#endif
});
UX_STEP_CB(
ux_blind_signing_reject_step,
pb,
report_finalize_error(),
{
&C_icon_crossmark,
"Reject",
&C_icon_warning,
"Blind",
"signing",
});
UX_STEP_INIT(
ux_warning_blind_signing_jump_step,
NULL,
NULL,
{
start_signature_flow();
}
);
// clang-format on

UX_FLOW(ux_blind_signing_flow,
&ux_blind_signing_warning_step,
#ifndef TARGET_NANOS
&ux_blind_signing_text1_step,
&ux_blind_signing_text2_step,
#endif
&ux_blind_signing_link_step,
&ux_blind_signing_accept_step,
&ux_blind_signing_reject_step);
UX_FLOW(ux_error_blind_signing_flow, &ux_error_blind_signing_step);
UX_FLOW(ux_warning_blind_signing_flow,
&ux_warning_blind_signing_warn_step,
&ux_warning_blind_signing_jump_step);
4 changes: 3 additions & 1 deletion src_bagl/ui_flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

extern const ux_flow_step_t* const ux_idle_flow[];

extern const ux_flow_step_t* const ux_blind_signing_flow[];
extern const ux_flow_step_t* const ux_error_blind_signing_flow[];

extern const ux_flow_step_t* const ux_warning_blind_signing_flow[];

extern const ux_flow_step_t* const ux_settings_flow[];

Expand Down
Loading
Loading