search

Zondax / ledger-oasis

Oasis app for Ledger Nano S and X
Apache License 2.0
3 stars 6 forks source link

Invalid entity metadata signing payloads #82

Closed tjanez closed 3 years ago

tjanez commented 3 years ago

Hi,

I've been testing the newly added entity metadata signing support and its fixes (I used the latest dev branch, commit 59307e4).

Steps to reproduce

cd $(mktemp -d)
OASIS_CORE_LEDGER_VERSION=1.2.0
wget https://github.com/oasisprotocol/oasis-core-ledger/releases/download/v${OASIS_CORE_LEDGER_VERSION}/oasis_core_ledger_${OASIS_CORE_LEDGER_VERSION}_linux_amd64.tar.gz
tar -xf oasis_core_ledger_${OASIS_CORE_LEDGER_VERSION}_linux_amd64.tar.gz
export LEDGER_SIGNER_PATH="$PWD/oasis_core_ledger_${OASIS_CORE_LEDGER_VERSION}_linux_amd64/ledger-signer"
git clone https://github.com/oasisprotocol/metadata-registry-tools.git
cd metadata-registry-tools
git checkout tjanez/test-ledger-signer
make build
# Open Oasis app on Ledger.
make test-cli-ledger

NOTE: The tjanez/test-ledger-signer branch disables oasis-registry entity update's built-in validation: https://github.com/oasisprotocol/metadata-registry-tools/commit/f1fe88e412ece6f2bd22abd781a8e53e2be4b4fd to test validation on Ledger's app.

Too long entity name is accepted, but when confirming, nothing is shown on Ledger's screen

Excerpt from make test-cli-ledger's output:

+ /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/oasis-registry/oasis-registry entity update --signer.dir ./ --signer.backend plugin --signer.plugin.name ledger --signer.plugin.path /tmp/tmp.9WyFkMxZDE/oasis_core_ledger_1.2.0_linux_amd64/ledger-signer /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/tests/fixtures/entity-ledger/update-name-too-long.json
You are about to sign the following entity metadata descriptor:
  Version: 1
  Serial:  5
  Name:    This is some tooooooooooooooo long entity name (51)
  URL:     https://my.entity/url
  Email:   my@entity.org
  Keybase: my_keybase_handle
  Twitter: my_twitter_handle

You may need to review the transaction on your device if you use a hardware-based signer plugin...

This is how the too long name is shown on Ledger's screen: IMG_20210204_130221 IMG_20210204_130227

After signing the transaction, Entity metadata update fails with:

ts=2021-02-08T11:06:10.343509169Z level=error module=cmd/entity caller=entity.go:122 msg="failed to update metadata" err="bad signed entity metadata: entity name too long (length: 51 max: 50)"

A too long name should be rejected similar to how an invalid version is rejected, e.g.:

ts=2021-02-08T11:02:58.197079641Z level=error module=cmd/entity caller=entity.go:115 msg="failed to sign metadata" err="signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: Invalid v (version) value"

Too long URL, Email, Keybase handle, Twitter handle

These all fail with the same error:

signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: unexpected CBOR error

Excerpts from make test-cli-ledger's output:

+ /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/oasis-registry/oasis-registry entity update --signer.dir ./ --signer.backend plugin --signer.plugin.name ledger --signer.plugin.path /tmp/tmp.9WyFkMxZDE/oasis_core_ledger_1.2.0_linux_amd64/ledger-signer /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/tests/fixtures/entity-ledger/update-url-too-long.json
You are about to sign the following entity metadata descriptor:
  Version: 1
  Serial:  5
  Name:    My entity name
  URL:     https://my.entity/url/this/is/a/toooooooooo/long/url/of/length/65
  Email:   my@entity.org
  Keybase: my_keybase_handle
  Twitter: my_twitter_handle

You may need to review the transaction on your device if you use a hardware-based signer plugin...
ts=2021-02-08T11:06:12.086585054Z level=error module=cmd/entity caller=entity.go:115 msg="failed to sign metadata" err="signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: unexpected CBOR error"
+ /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/oasis-registry/oasis-registry entity update --signer.dir ./ --signer.backend plugin --signer.plugin.name ledger --signer.plugin.path /tmp/tmp.9WyFkMxZDE/oasis_core_ledger_1.2.0_linux_amd64/ledger-signer /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/tests/fixtures/entity-ledger/update-email-too-long.json
You are about to sign the following entity metadata descriptor:
  Version: 1
  Serial:  5
  Name:    My entity name
  URL:     https://my.entity/url
  Email:   a.toooo.long.email@address.33.com
  Keybase: my_keybase_handle
  Twitter: my_twitter_handle

You may need to review the transaction on your device if you use a hardware-based signer plugin...
ts=2021-02-08T11:06:13.904556447Z level=error module=cmd/entity caller=entity.go:115 msg="failed to sign metadata" err="signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: unexpected CBOR error"
+ /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/oasis-registry/oasis-registry entity update --signer.dir ./ --signer.backend plugin --signer.plugin.name ledger --signer.plugin.path /tmp/tmp.9WyFkMxZDE/oasis_core_ledger_1.2.0_linux_amd64/ledger-signer /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/tests/fixtures/entity-ledger/update-keybase-too-long.json
You are about to sign the following entity metadata descriptor:
  Version: 1
  Serial:  5
  Name:    My entity name
  URL:     https://my.entity/url
  Email:   my@entity.org
  Keybase: aTooooooooLongKeybaseHandleWith33
  Twitter: my_twitter_handle

You may need to review the transaction on your device if you use a hardware-based signer plugin...
ts=2021-02-08T11:06:15.611534031Z level=error module=cmd/entity caller=entity.go:115 msg="failed to sign metadata" err="signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: unexpected CBOR error"
+ /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/oasis-registry/oasis-registry entity update --signer.dir ./ --signer.backend plugin --signer.plugin.name ledger --signer.plugin.path /tmp/tmp.9WyFkMxZDE/oasis_core_ledger_1.2.0_linux_amd64/ledger-signer /tmp/tmp.9WyFkMxZDE/metadata-registry-tools/tests/fixtures/entity-ledger/update-twitter-too-long.json
You are about to sign the following entity metadata descriptor:
  Version: 1
  Serial:  5
  Name:    My entity name
  URL:     https://my.entity/url
  Email:   my@entity.org
  Keybase: my_keybase_handle
  Twitter: aTooooooooLongTwitterHandleWith33

You may need to review the transaction on your device if you use a hardware-based signer plugin...
ts=2021-02-08T11:06:17.32602767Z level=error module=cmd/entity caller=entity.go:115 msg="failed to sign metadata" err="signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: unexpected CBOR error"

I'm not familiar with how CBOR parsing is done on the Ledger but I would be curious if it would be possible to decode such CBOR values and report that they are invalid?

E.g. something similar that is done for invalid version field values:

signature/signer/plugin: failed to sign: ledger: failed to sign message: ledger/oasis: failed to sign: Invalid v (version) value
jleni commented 3 years ago

Test and close

emmanuelm41 commented 3 years ago

Lola confirmed we have fixed it and we have a test that shows it. We can close the issue with the PR