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

Message signing (CIP-8) #72

Merged
merged 31 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
53f8338
remove irrelevant TODOs
janmazak Oct 4, 2023
0106dac
remove features from Nano S
janmazak Jun 20, 2023
58a34d5
add ledger_app.toml
janmazak Dec 12, 2023
98cf06e
refactor: rename credentials
janmazak Oct 6, 2023
e4aaf1a
refactor: rename a local variable
janmazak Oct 7, 2023
3b863de
refactor: remove poolIdPath from ctx
janmazak Oct 7, 2023
93e9e61
refactor: remove poolKeyHash from ctx
janmazak Oct 7, 2023
0601322
refactor: rename local function
janmazak Oct 7, 2023
9d8024c
refactor: certificates in tx hash builder
janmazak Oct 7, 2023
61114c9
refactor: separate extended credential types
janmazak Oct 7, 2023
5ee6fac
refactor: certificates in signTx
janmazak Oct 11, 2023
fa22922
conway: new key derivation schemas
janmazak Oct 5, 2023
15e901f
conway: add tx body items
janmazak Oct 11, 2023
219fbf9
update version
janmazak Dec 16, 2023
e086ad5
feature: add cbor set tag 258
janmazak Jan 6, 2024
62a72e4
increase max URL length
janmazak Jan 6, 2024
89d1ed0
fix: withdrawal canonical ordering
janmazak Jan 16, 2024
f0c4192
update bech32 prefixes
janmazak Jan 16, 2024
44f1c32
update changelog
janmazak Nov 10, 2023
8b0896f
audit fixes
janmazak Feb 2, 2024
dd4cc8d
feature: message signing (CIP-8)
janmazak Jan 30, 2024
bdf24b0
refactor: fix naming for payment part
janmazak Feb 19, 2024
0281c1f
update: token list with decimals
janmazak Feb 25, 2024
d42d012
fix: audit
janmazak Mar 17, 2024
01ae387
fix: fuzzing
janmazak Mar 17, 2024
8d4c344
fix: fuzzing2
janmazak Mar 22, 2024
8ffc2aa
fix: msg signing ui bug
janmazak Apr 9, 2024
e1c04f8
update: token registry
janmazak Apr 9, 2024
a432ca7
update version
janmazak Apr 9, 2024
8849dee
Merge remote-tracking branch 'origin-ledgerhq/develop' into msg-signing
janmazak Apr 22, 2024
6f20e60
fix: bech32 prefix for stake pool
janmazak Apr 25, 2024
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
39 changes: 38 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,42 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).


## [6.0.3](TBD) - [TBD]
## [7.1.0](TBD) - [TBD]

Message signing (CIP-8)

### Added

- support for basic message signing (CIP-8, CIP-30)

### Changed

- usage of chunks of maximum allowed size is now enforced (datums and reference scripts in outputs)
- TODO updated list of native tokens recognized by the app with correct decimal places


## [7.0.2](TBD) - [TBD]

Conway era

### Added

- export of Conway-era keys (DReps, Constitutional Committee Hot and Cold keys)
- Conway era transaction body items (new certificates, voting procedures, treasury, donation)
- optional CBOR tag 258 in CDDL sets
- reduced features on Nano S (since Ledger app v7, due to memory limits)

### Changed

- updated list of native tokens recognized by the app with correct decimal places
- increased max. URL and DNS name length to 128

### Fixed

- bug in checking canonical ordering of withdrawals


## [6.1.2](https://github.com/LedgerHQ/app-cardano/compare/v5.0.0...LedgerHQ:nanos_2.1.0_6.1.2_sdk_2.1.0-12) - [October 25th 2023]

Support for CIP-36 voting

Expand All @@ -15,11 +50,13 @@ Support for CIP-36 voting
- export of vote keys (1694'/1815'/...)
- support for CIP-36 voting (signing of vote-cast fragments with 1694 keys)
- support for CIP-36 registrations (in transaction auxiliary data)
- support for the Stax device

### Changed

- API for Catalyst voting registration (it is still possible to use CIP-15 in auxiliary data)
- updated list of native tokens recognized by the app with correct decimal places
- multidelegation allowed (as used by Lace, i.e. stake keys do not need to end with 0 as address_index)


## [5.0.0](https://github.com/LedgerHQ/app-cardano/compare/4.1.2...LedgerHQ:nanos_2.1.0_5.0.0) - [October 11th 2022]
Expand Down
46 changes: 44 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@
#*******************************************************************************

APPNAME = "Cardano ADA"
APPVERSION_M = 6

APPVERSION_M = 7
APPVERSION_N = 1
APPVERSION_P = 2
APPVERSION_P = 1
APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)"

ifeq ($(BOLOS_SDK),)
Expand Down Expand Up @@ -120,6 +121,31 @@ else
DEFINES += PRINTF\(...\)=
endif

# restricted features for Nano S
# but not in DEVEL mode where we usually want to test all features with HEADLESS
ifeq ($(TARGET_NAME), TARGET_NANOS)
ifneq ($(DEVEL), 1)
APP_XS = 1
else
APP_XS = 0
endif
else
APP_XS = 0
endif

ifeq ($(APP_XS), 1)
DEFINES += APP_XS
else
# features not included in the Nano S app
DEFINES += APP_FEATURE_OPCERT
DEFINES += APP_FEATURE_NATIVE_SCRIPT_HASH
DEFINES += APP_FEATURE_POOL_REGISTRATION
DEFINES += APP_FEATURE_POOL_RETIREMENT
DEFINES += APP_FEATURE_BYRON_ADDRESS_DERIVATION
DEFINES += APP_FEATURE_BYRON_PROTOCOL_MAGIC_CHECK
endif
# always include this, it's important for Plutus users
DEFINES += APP_FEATURE_TOKEN_MINTING

##################
# Dependencies #
Expand Down Expand Up @@ -196,5 +222,21 @@ format:
size: all
$(GCCPATH)arm-none-eabi-size --format=gnu bin/app.elf

##############
# Device-specific builds
##############

nanos: clean
BOLOS_SDK=$(NANOS_SDK) make

nanosp: clean
BOLOS_SDK=$(NANOSP_SDK) make

nanox: clean
BOLOS_SDK=$(NANOX_SDK) make

stax: clean
BOLOS_SDK=$(STAX_SDK) make

# import generic rules from the sdk
include $(BOLOS_SDK)/Makefile.rules
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ We recommend using the containerized build. See [Getting started](doc/build.md)

### Loading the app

`make load`

Builds and loads the application into the connected device. Make sure to close the Ledger app on the device before running the command.

Most common reason for a failed loading is the app taking too much space. Check `make size` (should be below 140K or so).
We recommend using the [Ledger VS Code plugin](https://marketplace.visualstudio.com/items?itemName=LedgerHQ.ledger-dev-tools) to load the app on a device.

### Debug version

Expand All @@ -27,7 +23,7 @@ also comment out

DEFINES += RESET_ON_CRASH

and then run `make clean load`.
The debug version is too big to fit on Nano S, but works on Speculos.

### Setup

Expand Down Expand Up @@ -78,6 +74,10 @@ _Before merging a PR, one should make sure that:_
* `make clean load` runs without errors and warnings (except those reported for nanos-secure-sdk repo) for development build (see Debug version above)
* `make analyze` does not report errors or warnings

## Running tests

All the tests are initiated from the accompanying [ledgerjs package](https://github.com/vacuumlabs/ledgerjs-cardano-shelley) (see what [commands to run](https://github.com/vacuumlabs/ledgerjs-cardano-shelley?tab=readme-ov-file#tests)). You have to make sure that the version of ledgerjs correspond to the app version, otherwise some tests with fail (possibly resulting in odd errors) or test coverage will be incomplete.

## How to get a transaction body computed by Ledger

Ledger computes a rolling hash of the serialized transaction body, but the body itself is ordinarily not available. It is possible to acquire it from the development build by going through the following steps:
Expand Down
2 changes: 1 addition & 1 deletion doc/build.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
- Install Docker
- Pull the required containers as discussed in https://github.com/LedgerHQ/ledger-app-builder/ (lite container is sufficient for a C build):

`sudo docker pull ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-lite:latest`
`docker pull ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-lite:latest`

## Compiling the app

Expand Down
11 changes: 11 additions & 0 deletions doc/features.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Features (not) available on specific Ledger devices

Nano S has a very limited space for storing applications. It is not enough to fit all Cardano features there, so some of them are only available on Nano S+ and other more spacious Ledger devices (e.g. Nano X and Stax).

The features not supported on Nano S, Cardano app version 7 and above:
* pool registration and retirement
* signing of operational certificates
* computation of native script hashes
* details in Byron change outputs (only the address is shown)

Details can be found in [Makefile](../Makefile) and in the code (search for compilation flags beginning with `APP_FEATURE_`).
6 changes: 2 additions & 4 deletions doc/ins_get_public_keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Get an extended public key (i.e., public key + chain code) for a given BIP32 pat

It is also possible to ask for a confirmation for exporting several keys (if the paths describing the keys are not suspicious, they won't be shown to the user and no further confirmation is required).

The allowed derivation paths correspond to wallet keys (accounts, spending paths, staking paths) and pool cold keys, as described in
The allowed derivation paths correspond to wallet keys (accounts, payment paths, staking paths) and pool cold keys, as described in
- [CIP 1852 - HD Wallets for Cardano](https://cips.cardano.org/cips/cip1852/);
- [CIP 1853 - HD Stake Pool Cold Keys for Cardano](https://cips.cardano.org/cips/cip1853/).

Expand Down Expand Up @@ -80,8 +80,6 @@ Concatenation of `pub_key` and `chain_code` representing the extended public key
- Ledger might impose more restrictions, see implementation of `policyForGetExtendedPublicKey` in [src/securityPolicy.c](../src/securityPolicy.c) for details
- calculate extended public key
- respond with extended public key

**TODOs**
- ❓(IOHK): Should we also support BTC app like token validation? (Note: Token validation is to prevent concurrent access to the Ledger by two different host apps which could confuse user into performing wrong actions)
- ❓(IOHK): Should we support permanent app setting where Ledger forces user to acknowledge public key retrieval before sending it to host? (Note: probably not in the first version of the app)
- ❓(IOHK): Should there be an option to show the public key on display? Is it useful in any way? (Note: probably not)
6 changes: 3 additions & 3 deletions doc/ins_sign_stake_pool_registration.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ P2 = `0x36`
|relay format | 1 | `RELAY_SINGLE_HOST_NAME=0x01` |
|isPortGiven | 1 | `ITEM_INCLUDED_NO=0x01` or `ITEM_INCLUDED_YES=0x02` |
|port | 2 | Big endian; included if and only if isPortGiven is `ITEM_INCLUDED_YES`
|dns name | variable | byte buffer, max size 64
|dns name | variable | byte buffer, max size 128

|Field| Length | Comments|
|-----|--------|---------|
|relay format | 1 | `RELAY_MULTIPLE_HOST_NAME=0x02` |
|dns name | variable | byte buffer, max size 64
|dns name | variable | byte buffer, max size 128


---
Expand All @@ -175,7 +175,7 @@ P2 = `0x37`
|-----|--------|---------|
|includeMetadata | 1 | `ITEM_INCLUDED_NO=0x01` or `ITEM_INCLUDED_YES=0x02` |
|metadata hash | 32 | byte buffer; only if includeMetadata is `ITEM_INCLUDED_YES`
|metadata url | variable | byte buffer, max size 64; only if includeMetadata is `ITEM_INCLUDED_YES`
|metadata url | variable | byte buffer, max size 128; only if includeMetadata is `ITEM_INCLUDED_YES`


---
Expand Down
44 changes: 31 additions & 13 deletions doc/ins_sign_tx.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# Sign Transaction

Note: this is somewhat incomplete (Babbage and Conway era elements are not described in detail) and some parts might be outdated. We strongly recommend to use [ledgerjs for Cardano](https://github.com/vacuumlabs/ledgerjs-cardano-shelley) for signing transactions. Check its latest API to find out what is supported.

**Description**

Given transaction inputs and transaction outputs, fee, ttl, staking certificates, reward withdrawals, metadata hash, validity interval start, and mint, construct and sign a transaction.
Given transaction inputs and transaction outputs, fee, ttl, staking certificates, reward withdrawals, metadata hash, validity interval start, mint, Plutus (Babbage) additional transaction body elements, and Conway additional elements, construct and sign a transaction.

Due to Ledger constraints and potential security implications (parsing errors), Cardano Ledger app uses a custom format for streaming the transaction to be signed. The main rationale behind not streaming directly the (CBOR-encoded) cardano raw transaction to Ledger is the following:
1) The app needs to support BIP44 change address outputs (Ledger should not display user's own change addresses to the user as this degrades UX).
Expand All @@ -13,7 +15,7 @@ Due to Ledger constraints and potential security implications (parsing errors),
**SignTx Limitations**

- Output address size is limited to 128 bytes (single APDU). (Note: IOHK is fine with address size limit of 100 bytes)
- Addresses that are not shown to the user are base addresses with spending path `m/1852'/1815'/account'/{0,1}/changeIndex` and the standard stake key `m/1852'/1815'/account'/2/0`, where values of `account` and `changeIndex` are limited (for now, `0 <= account <= 100` and `0 <= changeIndex <= 1 000 000`). This makes it feasible to brute-force all change addresses in case an attacker manages to modify change address(es). (As the user does not confirm change addresses, it is relatively easy to perform MITM attack).
- Addresses that are not shown to the user are base addresses with payment key path `m/1852'/1815'/account'/{0,1}/changeIndex` and the standard stake key `m/1852'/1815'/account'/2/0`, where values of `account` and `changeIndex` are limited (for now, `0 <= account <= 100` and `0 <= changeIndex <= 1 000 000`). This makes it feasible to brute-force all change addresses in case an attacker manages to modify change address(es). (As the user does not confirm change addresses, it is relatively easy to perform MITM attack).
- Only transactions with at least one input will be signed (this provides protection against certificate replays and transaction replays on different networks).

**Communication protocol non-goals:**
Expand Down Expand Up @@ -233,7 +235,23 @@ Optional.

### Certificate

We support 4 types of certificates in ordinary transactions (signing mode `SIGN_TX_SIGNINGMODE_ORDINARY_TX` in the initial APDU message): stake key registration, stake key deregistration, stake delegation, and stake pool retirement. We support 3 types in multisig transactions (signing mode `SIGN_TX_SIGNINGMODE_MULTISIG_TX` in the initial APDU message): stake key registration, stake key deregistration, and stake delegation.
We support the following certificate types in ordinary transactions (signing mode `SIGN_TX_SIGNINGMODE_ORDINARY_TX` in the initial APDU message):
* CERTIFICATE_STAKE_REGISTRATION = 0,
* CERTIFICATE_STAKE_DEREGISTRATION = 1,
* CERTIFICATE_STAKE_DELEGATION = 2,
* CERTIFICATE_STAKE_POOL_RETIREMENT = 4,
* CERTIFICATE_STAKE_REGISTRATION_CONWAY = 7,
* CERTIFICATE_STAKE_DEREGISTRATION_CONWAY = 8,
* CERTIFICATE_VOTE_DELEGATION = 9,
* CERTIFICATE_AUTHORIZE_COMMITTEE_HOT = 14,
* CERTIFICATE_RESIGN_COMMITTEE_COLD = 15,
* CERTIFICATE_DREP_REGISTRATION = 16,
* CERTIFICATE_DREP_DEREGISTRATION = 17,
* CERTIFICATE_DREP_UPDATE = 18,

For signing mode `SIGN_TX_SIGNINGMODE_MULTISIG_TX`, everything from the above list except `CERTIFICATE_STAKE_POOL_RETIREMENT` is allowed.

For signing mode `SIGN_TX_SIGNINGMODE_PLUTUS_TX`, everything from the above list is allowed.

In addition, a transaction using `SIGN_TX_SIGNINGMODE_POOL_REGISTRATION_OPERATOR` or `SIGN_TX_SIGNINGMODE_POOL_REGISTRATION_OWNER` as the signing mode contains a single certificate for stake pool registration which must not be accompanied by other certificates or by withdrawals (due to security concerns about cross-witnessing data between them). This certificate is processed by a state sub-machine. Instructions for this sub-machine are given in P2; see [Stake Pool Registration](ins_sign_stake_pool_registration.md) for the details on accepted P2 values and additional APDU messages needed.

Expand All @@ -242,41 +260,41 @@ In addition, a transaction using `SIGN_TX_SIGNINGMODE_POOL_REGISTRATION_OPERATOR
| P1 | `0x06` |
| P2 | (unused / see [Stake Pool Registration](ins_sign_stake_pool_registration.md)) |

**Data for CERTIFICATE_TYPE_STAKE_REGISTRATION**
**Data for CERTIFICATE_STAKE_REGISTRATION**

|Field| Length | Comments|
|-----|--------|---------|
|Output type| 1 | `CERTIFICATE_TYPE_STAKE_REGISTRATION=0x00`|
|Output type| 1 | `CERTIFICATE_STAKE_REGISTRATION=0x00`|
|Stake credential| variable | See stake credential explained above|

**Data for CERTIFICATE_TYPE_STAKE_DEREGISTRATION**
**Data for CERTIFICATE_STAKE_DEREGISTRATION**

|Field| Length | Comments|
|-----|--------|---------|
|Output type| 1 | `CERTIFICATE_TYPE_STAKE_DEREGISTRATION=0x01`|
|Output type| 1 | `CERTIFICATE_STAKE_DEREGISTRATION=0x01`|
|Stake credential| variable | See stake credential explained above|

**Data for CERTIFICATE_TYPE_STAKE_DELEGATION**
**Data for CERTIFICATE_STAKE_DELEGATION**

|Field| Length | Comments|
|-----|--------|---------|
|Output type| 1 | `CERTIFICATE_TYPE_STAKE_DELEGATION=0x02`|
|Output type| 1 | `CERTIFICATE_STAKE_DELEGATION=0x02`|
|Stake credential| variable | See stake credential explained above|
|Pool key hash| 28 | Hash of staking pool public key|

**Data for CERTIFICATE_TYPE_STAKE_POOL_REGISTRATION**
**Data for CERTIFICATE_STAKE_POOL_REGISTRATION**

|Field| Length | Comments|
|-----|--------|---------|
|Output type| 1 | `CERTIFICATE_TYPE_STAKE_POOL_REGISTRATION=0x03`|
|Output type| 1 | `CERTIFICATE_STAKE_POOL_REGISTRATION=0x03`|

This only describes the initial certificate message. All the data for this certificate are obtained via a series of additional APDU messages; see [Stake Pool Registration](ins_sign_stake_pool_registration.md) for the details.

**Data for CERTIFICATE_TYPE_STAKE_POOL_RETIREMENT**
**Data for CERTIFICATE_STAKE_POOL_RETIREMENT**

|Field| Length | Comments|
|-----|--------|---------|
|Output type| 1 | `CERTIFICATE_TYPE_STAKE_POOL_RETIREMENT=0x04`|
|Output type| 1 | `CERTIFICATE_STAKE_POOL_RETIREMENT=0x04`|
|Stake key path| variable | BIP44 path. See [GetExtPubKey call](ins_get_public_keys.md) for a format example |
|Pool key hash| 28 | Hash of staking pool public key|

Expand Down
22 changes: 18 additions & 4 deletions fuzzing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ else()
add_link_options($ENV{LIB_FUZZING_ENGINE})
endif()

add_compile_options(-g)


set(SDK_PATH ${BOLOS_SDK})
Expand Down Expand Up @@ -77,6 +78,8 @@ set(CARDANO_SOURCE
${CARDANO_PATH}/src/securityPolicy.c
${CARDANO_PATH}/src/signCVote.c
${CARDANO_PATH}/src/signCVote_ui.c
${CARDANO_PATH}/src/signMsg.c
${CARDANO_PATH}/src/signMsg_ui.c
${CARDANO_PATH}/src/signOpCert.c
${CARDANO_PATH}/src/signTx.c
${CARDANO_PATH}/src/signTxCVoteRegistration.c
Expand Down Expand Up @@ -140,11 +143,21 @@ add_compile_definitions(
HAVE_ECDSA
HAVE_EDDSA
HAVE_HASH
HAVE_SHA224
HAVE_SHA256
HAVE_SHA3

# include all app features, incl. those removed from Nano S
APP_FEATURE_OPCERT
APP_FEATURE_NATIVE_SCRIPT_HASH
APP_FEATURE_POOL_REGISTRATION
APP_FEATURE_POOL_RETIREMENT
APP_FEATURE_BYRON_ADDRESS_DERIVATION
APP_FEATURE_BYRON_PROTOCOL_MAGIC_CHECK
APP_FEATURE_TOKEN_MINTING
)

set(SOURCE
set(SOURCE
${UX_SOURCE}
${CARDANO_SOURCE}
./src/os_mocks.c
Expand All @@ -160,13 +173,14 @@ set(harnesses
deriveNativeScriptHash_harness
getPublicKeys_harness
signCVote_harness
signMsg_harness
signOpCert_harness
signTx_harness
)

foreach(harness IN LISTS harnesses)
add_executable(${harness}
add_executable(${harness}
./src/${harness}.c
)
)
target_link_libraries(${harness} PUBLIC cardano)
endforeach()
endforeach()
2 changes: 2 additions & 0 deletions fuzzing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ deriveAddress_harness
deriveNativeScriptHash_harness
getPublicKeys_harness
signCVote_harness
signMsg_harness
signOpCert_harness
signTx_harness
```
Expand All @@ -36,4 +37,5 @@ Since there is an already existing corpus, to start fuzzing with it simply do `.


## Notes

For more context regarding fuzzing check out the app-boilerplate fuzzing [README.md](https://github.com/LedgerHQ/app-boilerplate/blob/master/fuzzing/README.md)
Loading
Loading