Skip to content

Commit

Permalink
Merge pull request #72 from vacuumlabs/msg-signing
Browse files Browse the repository at this point in the history
Message signing (CIP-8)
  • Loading branch information
yogh333 authored May 29, 2024
2 parents 7534d00 + 6f20e60 commit 605d709
Show file tree
Hide file tree
Showing 86 changed files with 4,994 additions and 1,385 deletions.
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

0 comments on commit 605d709

Please sign in to comment.