LedgerHQ / ledger-device-rust-sdk

Rust SDK for Ledger device applications
Apache License 2.0
44 stars 29 forks source link

call to os_perso_derive_node_with_seed_key hangs #49

Closed siy closed 1 year ago

siy commented 1 year ago

I need to implement key pair derivation for Curve25519 using SLIP10 (rather than Bip44). When I'm trying to call os_perso_derive_node_with_seed_key() function, it just hangs the device. Below is the minimalistic code which reproduces the issue:

pub fn derive_private_key_curve25519() -> Result<[u8; 32], AppErrors> {
    unsafe {
        let mut seed = [0u8; 32];
        // m/44'/1022'/365'
        let path: [u32; 3] = [
            44u32 | 0x80000000u32,
            1022u32 | 0x80000000u32,
            365u32 | 0x80000000u32,
        ];

        os_perso_derive_node_with_seed_key(
            HDW_ED25519_SLIP10,
            CX_CURVE_Curve25519,
            path.as_ptr(),
            path.len() as c_uint,
            seed.as_mut_ptr() as *mut c_uchar,
            null_mut(),
            null_mut(),
            0,
        );

        return Ok(seed);
    }
}

The corresponding version in C looks like this:

uint8_t seed[32];
uint32_t path[] = {44 | 0x80000000, 1022 | 0x80000000, 365 | 0x80000000};

os_perso_derive_node_bip32_seed_key(HDW_ED25519_SLIP10, CX_CURVE_Ed25519, path, 3, seed, NULL, NULL, 0);

The os_perso_derive_node_bip32_seed_key is just an alias to os_perso_derive_node_with_seed_key defined in os_seed.h.

Any ideas why code above hangs?

yhql commented 1 year ago

Hi, did you try this code with speculos? Trying to know if you're actually getting a segfault. Does the C code you show hangs also or only the Rust one?

siy commented 1 year ago

Haven't tried it with speculos (will do). Haven't tried C version by miself, but there are a lot of very similar code in various apps: iota, sia or even in article here.

siy commented 1 year ago

Speculos fails with following report:

siy@bobryzen:~/CLionProjects/Ledger/speculos$ ./speculos.py ../../Radix/babylon-ledger-app/target/nanos/release/babylon-ledger-app
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
[*] speculos launcher revision: d94210d
[*] using SDK version 2.1 on nanos
[*] loading CXLIB from "/home/siy/CLionProjects/Ledger/speculos/speculos/cxlib/nanos-cx-2.1.elf"
[*] patching svc instruction at 0x126358
[*] patching svc instruction at 0x4000173c
 * Serving Flask app 'speculos.api.api' (lazy loading)
 * Environment: development
 * Debug mode: off
16:47:46.696:werkzeug:  * Running on all addresses.
   WARNING: This is a development server. Do not use it in a production deployment.
QPixmap::scaled: Pixmap is a null pixmap
16:47:46.697:werkzeug:  * Running on http://192.168.1.18:5000/ (Press CTRL+C to quit)
16:47:59.357:apdu: > 80120000
launcher: seed_key: unsupported curve

I have a feeling that I saw somewhere that such a combination of curve and derivation path is not supported by speculos.

siy commented 1 year ago

Also, tried C version, and it seems is working flawlessly in speculos.

siy commented 1 year ago

I made a small modification to the code and speculos started producing different result. Changed code (added seed):

pub fn derive_private_key_curve25519__() -> Result<[u8; 32], AppErrors> {
    unsafe {
        let mut seed = [0u8; 32];
        // m/44'/1022'/365'
        let path: [u32; 3] = [
            44u32 | 0x80000000u32,
            1022u32 | 0x80000000u32,
            365u32 | 0x80000000u32,
        ];
        let mut ed25519_seed: [u8; 12] = [
            b'R', b'a', b'd', b'i', b'x', b'B', b'a', b'b', b'y', b'l', b'o', b'n',
        ];

        os_perso_derive_node_with_seed_key(
            HDW_ED25519_SLIP10,
            CX_CURVE_Curve25519,
            path.as_ptr(),
            path.len() as c_uint,
            seed.as_mut_ptr() as *mut c_uchar,
            null_mut(),
            ed25519_seed.as_mut_ptr(),
            ed25519_seed.len() as c_uint,
        );

        return Ok(seed);
    }
}

Speculos output:

siy@bobryzen:~/CLionProjects/Ledger/speculos$ ./speculos.py ../../Radix/babylon-ledger-app/target/nanos/release/babylon-ledger-app
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
[*] speculos launcher revision: d94210d
[*] using SDK version 2.1 on nanos
[*] loading CXLIB from "/home/siy/CLionProjects/Ledger/speculos/speculos/cxlib/nanos-cx-2.1.elf"
[*] patching svc instruction at 0x126358
[*] patching svc instruction at 0x400017d8
 * Serving Flask app 'speculos.api.api' (lazy loading)
 * Environment: development
 * Debug mode: off
17:52:53.462:werkzeug:  * Running on all addresses.
   WARNING: This is a development server. Do not use it in a production deployment.
17:52:53.462:werkzeug:  * Running on http://192.168.1.18:5000/ (Press CTRL+C to quit)
QPixmap::scaled: Pixmap is a null pixmap
17:53:01.449:apdu: > aa12000000

Unhandled exception 2 at /home/siy/CLionProjects/Ledger/speculos/src/bolos/cx_ec.c:1433
Exiting.
yhql commented 1 year ago

Thanks for all the info! I had trouble noticing but I think this the difference between the Rust code and the C example:

The second one is incorrect to use for SLIP10 derivation, and is not supported in speculos indeed :) So you should use CX_CURVE_Ed25519 in Rust as well.

siy commented 1 year ago

Was about to write the same :) Thanks a lot!

siy commented 1 year ago

After the fix, the code shown above works in speculos, but hangs on the real device. Any ideas?

yhql commented 1 year ago

Check that the SDK version used with speculos (2.1 for Nano S given your example above) matches the version of your device, as a first check.

siy commented 1 year ago

Device (Nano S) has latest available firmware: Secure Element: 2.1.0 Microcontroller: 1.11 Bootloader: 0.11 Is it OK?

yhql commented 1 year ago

It is, and actually I'm not sure what would cause a hang on Nano S but not in speculos at this point :/

siy commented 1 year ago

By the way: I'm trying to setup speculos so it will use same seed phrase (12/18/24 words) as test device in order to ensure that tests will perform identically. Unfortunately after diggigng documentation and skimming sources I've not found way to set seed phrase in speculos. Any hints?

yhql commented 1 year ago

I think the --seed argument should do what you need, https://github.com/LedgerHQ/speculos/blob/master/speculos/main.py#L193

siy commented 1 year ago

Thanks! From the examples it looked like this argument is used for RNG init, rather than mnemonic setup.

siy commented 1 year ago

Question: might the issue be caused by compiler options with which C part of the SDK is compiled?

siy commented 1 year ago

OK, the issue was caused by invalid curve declaration in metadata. Simulator ignores metadata and supports all curves (not just ones declared for app) and works fine.

During investigation, I've found that following metadata declaration does not work:

[package.metadata.nanos]
curve = "secp256k1"
curve = "ed25519"
flags = "0x240"
icon = "icons/nanos_app_radix.gif"
icon_small = "icons/nanos_app_radix.gif"
path = "44'/1022'"
name = "Radix Babylon"

Attempt to build binary fails with

Duplicate key `curve` in table `package.metadata.nanos`

I need both curves to be present in the metadata. Any ideas?

yhql commented 1 year ago

OK I see, thanks for the explanation.

There's a PR available here https://github.com/LedgerHQ/cargo-ledger/pull/16 that I will merge today that handles curves as a list

[package.metadata.nanos]
curve = ["secp256k1", "ed25519"]
siy commented 1 year ago

Issue is fixed. Thanks for help!