search

mjg-foundation / passport2-monero

v2.x.x series of firmware for Passport, rebuilt for monero
Other
28 stars 3 forks source link

Create Monero Seeds #4

Open mjg-foundation opened 1 year ago

mjg-foundation commented 1 year ago

The first step in replacing bitcoin with monero in this project will be creating monero seeds. Users should be able to:

  1. Erase passport
  2. Create a new monero seed or restore from seed words
  3. Read seed words back from passport
  4. (optional) switch between firmwares without needing to erase

Here are the places to start work:

Much of the functionality needed is probably available in extmod/trezor/crypto/monero/. I'll start the bounty off at 2 XMR, and hope more can flow in from the community.

bounty: 2 XMR

donttracemebruh commented 1 year ago

I'm offering an additional 2 XMR to the bounty

kayabaNerve commented 1 year ago

Do Foundation devices support alloc? I do see they're no_std.

mjg-foundation commented 1 year ago

Do Foundation devices support alloc? I do see they're no_std.

Alloc functionality is available in C micropython interfaces using m_new and m_del, see extmod/foundation/modfoundation-ur.h for example

kayabaNerve commented 1 year ago

Yeah, I'm asking regarding Rust.

kayabaNerve commented 1 year ago

It looks like https://github.com/rust-embedded/embedded-alloc could be introduced to offer Rust alloc. Would that be unacceptable?

To clarify, I'm fine with Rust yet I've never worked on embedded devices with it. I have no idea the larger implications of defining an allocator on Rust's side. I don't see why it'd be an issue, but it may significantly increase code size to unacceptable degrees or may break C expectations or...

mjg-foundation commented 1 year ago

Yeah, I'm asking regarding Rust.

I haven't worked much with our rust libs, so I would err on the side of safety. This isn't intended to be an oxidation project, unless it so happens that way and it works better.

kayabaNerve commented 1 year ago

https://docs.rs/monero-serai/0.1.4-alpha/monero_serai/

There is the question of if Rust should be used here. I'd say yes, if it works. monero-serai offers all the necessary functionality discussed on the passport side, without known security issues (its TX creation code is currently non-standard, but the passport would solely sign TXs).

I'm also willing to work on integrating monero-serai. I'd, personally, refuse to touch the Monero codebase for this. While libringct alone (the TX signing code) would be fine, moving over multisig would require moving over wallet2 which is a monolith without praise. Solely acknowledgement it does what it needs to.

Then if we look for what other projects did, Trezor rewrote vast swaths of Monero in Python:

https://github.com/trezor/trezor-firmware/blob/master/core/src/apps/monero/xmr/bulletproof.py

monero-serai would offer constant time versions with secure memory handling (though I'm unsure how much of a concern that is given its a HW wallet).

TL;DR I legitimately believe monero-serai would be a solid candidate for the Foundation, if usable. I'd be willing to work on this. If you'd rather Python work be applied, or the Monero C/C++ code, I will respect that. I just am not personally interested in picking up such work, and will leave this open to other developers.

kayabaNerve commented 1 year ago

Sorry if these messages were a bit too aggressive/forward :sweat_smile: They were just meant to be to the point. I'd love to see Monero on Foundation and wouldn't mind doing what I can :)

mjg-foundation commented 1 year ago

https://docs.rs/monero-serai/0.1.4-alpha/monero_serai/

There is the question of if Rust should be used here. I'd say yes, if it works. monero-serai offers all the necessary functionality discussed on the passport side, without known security issues (its TX creation code is currently non-standard, but the passport would solely sign TXs).

I'm also willing to work on integrating monero-serai. I'd, personally, refuse to touch the Monero codebase for this. While libringct alone (the TX signing code) would be fine, moving over multisig would require moving over wallet2 which is a monolith without praise. Solely acknowledgement it does what it needs to.

Then if we look for what other projects did, Trezor rewrote vast swaths of Monero in Python:

https://github.com/trezor/trezor-firmware/blob/master/core/src/apps/monero/xmr/bulletproof.py

monero-serai would offer constant time versions with secure memory handling (though I'm unsure how much of a concern that is given its a HW wallet).

TL;DR I legitimately believe monero-serai would be a solid candidate for the Foundation, if usable. I'd be willing to work on this. If you'd rather Python work be applied, or the Monero C/C++ code, I will respect that. I just am not personally interested in picking up such work, and will leave this open to other developers.

Whatever works for you! I'm not gonna gatekeep the codebase, and if you produce a PR that signs monero transactions correctly and robustly (with relatively clean coding style and structure), I'll approve it.

mjg-foundation commented 1 year ago

I'm still new to rust, and have not used it at all professionaly, but I'm interested in the stability and productivity gains it can bring here. I just advise against oxidating too much at a time to the point where it becomes prohibitive to review

kayabaNerve commented 1 year ago

Totally understand. I'm still reviewing if monero-serai is even feasible in a no_std environment (which is of independent interest to me). The last thing I want to do is submit monero-serai for seeds, then hear the rest of it is unusable and it can't do signing.

kayabaNerve commented 1 year ago

@mjg-foundation I managed to build monero-serai, a Rust library offering:

for the Passport (no_std, its cortex-m chip). Since it's no_std, I did have to remove the net code (and with it the scanning/TX creation code). Nothing removed should be relevant to on-passport usage though.

https://github.com/kayabaNerve/passport2-monero demos it being bound and is able to successfully generate seeds (re: binding, not any updated UI).

Regarding the choice of library, it's just meant to be a well-designed, modern Monero library. I could get all the Python code from Trezor, which has been used in this env (HW wallets) before, yet am biased towards monero-serai due to being the author. I also know I can trust monero-serai yet haven't personally reviewed Trezor's work. On a practical note, I'd note how I assume the Rust to be more performant/featured (primarily re: multisig).

If you're concerned about security (as monero-serai is relatively new), there are known issues. Luckily enough, the know issues line up with the code removed to fit in no-std. This does mean monero-serai isn't an immediate candidate for pairing up on the laptop/desktop/phone side though. I have a developer working on those, and then we're planning to have monero-serai audited as well (hopefully over the next few months).

I'll follow up when the UI binding is done. I have to grab a device with a camera... Then, you're more than welcome to accept my PR, or decline it as sure, it's notably (and unexpectedly) increasing the carcinization. I would not be offended and have no plans to beg/complain for payment if you don't believe it's the right fit.

mjg-foundation commented 1 year ago

Wow, that was fast! Make sure to go through DEVELOPMENT, and ensure that just build color and just build mono work, just clean ing in between. Use the simulator to test out UI binding, and let me know when it's ready to test!

By test, I mean run on hardware and make sure it isn't totally broken.

It sounds like seraphis relies on a future version of monero, so this issue would be in limbo until seraphis is made official, as far as proper testing. I'll look into seraphis, and start a "discussion" in github about the options of seraphis vs ringct. This could turn into a huge debate, but it's important we do things right.

mjg-foundation commented 1 year ago

@kayabaNerve I think I misinterpreted your comments. So we've got seraphis support on deck but this bounty will continue for RingCT?

mjg-foundation commented 1 year ago

I'm missreading seria as seraphis 😅, carry on

mjg-foundation commented 1 year ago

After re-reading with an eye adjustment, that all sounds very good. If you have devs working on mobile, consider taking bounty #3 , or assisting with it. Barring any objections, I'm fine with moving forward with your approach.

mjg-foundation commented 1 year ago

@Monero-HackerIndustrial mentioned that we have an opportunity here to establish standards for broad compatibility among monero wallets for seed creation, unsigned transaction formatting, and signing, similar to the BIPs

kayabaNerve commented 1 year ago

The seed here is the never-formally specified Monero seed algorithm. The only alternative is Polyseed, which is formally defined, yet also only implemented into Polyseed (with a PR for monero-serai). While it's a much better format, it's hard to recommend it for use here since Monero proper doesn't support it (though it is possible to convert a Polyseed to a seed).

As for the rest of those items, I'd point discussion to #6.

kayabaNerve commented 1 year ago

Working on stash.py, almost all of this is fundamentally incompatible with Monero. The amount of code for fingerprints/xpubs/accounts has to be completely slashed.

Also, the Monero seed won't fit into storage. It's at minimum 100 bytes. Only the master key will.

While I've updated stash.py, it's completely breaking to most other components. At this time, I don't want to slash and burn the entire codebase to correct for that.

kayabaNerve commented 1 year ago

Should be ready for my testing in a simulator. If the needed UX steps, detailed in the original post work, I'll submit this for testing on a real device (not there yet).

mjg-foundation commented 1 year ago

As mentioned on twitter, I would be very happy if the data stored in the SE can be used by both the original firmware and this firmware, so users wouldn't need to erase when switching or buy a second device. Up to you to decide feasibility.

mjg-foundation commented 1 year ago

Also do feel free to slash and burn by commenting sutff out, I understand that most of the functionality will need to be rewritten. The UI can be left with nothing but the setup process, main page, hardware management stuff (brightness, auto shutdown, FCC info, etc), backups, and advanced -> view seed words

mjg-foundation commented 1 year ago

@kayabaNerve someone pointed out to me that trezor has a working, python-based, GPL3 monero cold signing library. This could be a more efficient way to work with what we've already got. Thoughts?

vdo commented 1 year ago

I add 1XMR to the bounty

kayabaNerve commented 1 year ago

@mjg-foundation

https://github.com/mjg-foundation/passport2-monero/issues/4#issuecomment-1518426008

Then if we look for what other projects did, Trezor rewrote vast swaths of Monero in Python:

https://github.com/trezor/trezor-firmware/blob/master/core/src/apps/monero/xmr/bulletproof.py

monero-serai would offer constant time versions with secure memory handling (though I'm unsure how much of a concern that is given its a HW wallet).

TL;DR I legitimately believe monero-serai would be a solid candidate for the Foundation, if usable. I'd be willing to work on this. If you'd rather Python work be applied, or the Monero C/C++ code, I will respect that. I just am not personally interested in picking up such work, and will leave this open to other developers.

I discussed this a while ago. My reasons for monero-serai, besides personal bias, are features/presumed performance.

I'd also note: 1) monero-serai will be audited. I don't believe Trezor's work has. 2) I personally am not a fan of Trezor's flow.

https://github.com/trezor/trezor-firmware/tree/master/core/src/apps/monero/signing

They use a builder pattern, which is fine, yet I question the quality of a 10-step process. This is likely simply since I'm unused to their library and haven't fully seen it/its flow.

3) Their messages aren't minimal.

https://github.com/trezor/trezor-firmware/blob/master/core/src/trezor/messages.py#L3852-L3872

This mirrors a struct in wallet2, and has an address, yet also decoded data about the address. This introduces an inconsistency possibility which needs to be checked when decoding. Ideally, these fields aren't decoded at all, and simply set if needed (since their communication isn't valuable when they must be independently derived anyways to be verified).

The library overall, at first glance, does seem comprehensive and well written though. I wouldn't directly object to its usage.

mjg-foundation commented 1 year ago

@kayabaNerve Any progress on this? I'm excited to get it compiling on my machine, and curious what the binary size is

Monero-HackerIndustrial commented 1 year ago

I didn't notice this issue so commenting now. This one should be pretty straightforward for me to help with.

Monero seed word generation using diceware (built with monero python library).

https://github.com/Monero-HackerIndustrial/MoneroDice-WalletGen I am going to change the KDF to match the one that polyseed uses.

capture_xpub: derive whatever key or data is needed to generate new subaddresses

There is an address and subaddress index. It is already built into the monero python library.

predictive_utils.py: use monero word dictionary, will need to replace modfoundation-bip39.h with a monero equivalent I started this process from monerosigner. https://github.com/Monero-HackerIndustrial/PortableMoneroQR/tree/main/wordlist

Most of the functions that you are asking for are already built in to the monero python library. If you don't mind having the monero python library be a dependency I can help add those changes pretty simply.

Monero-HackerIndustrial commented 1 year ago

To follow up on the trezor library singing question. I believe that there were some hardware constraints with how signing needed to happen on a trezor that dictated some of the design decisions.

Transaction signing was not a big constraint on monerosigner since I could use a monerod node in offline to sign with it. It would be very nice to have a well documented python example of raw tx signing. I will take a look at the trezor code to see if I can use it to make an example python signing.

mjg-foundation commented 1 year ago

@Monero-HackerIndustrial Sounds good, if you can get monero-python into the firmware. It may be most efficient to not directly add it as a dependency, but bring over specific necessary files

kayabaNerve commented 1 year ago

I haven't had the time to further work on this, so I'm fine stepping to the side.

Monero-HackerIndustrial commented 1 year ago

I'm setting aside some time to take a look at this Friday. I can bring over the needed specific files. For passport, do you support other language seed phrases other than English?

I planned on bringing over the other language wordlist regardless.

mjg-foundation commented 1 year ago

I'm setting aside some time to take a look at this Friday. I can bring over the needed specific files. For passport, do you support other language seed phrases other than English?

I planned on bringing over the other language wordlist regardless.

We don't, but feel free to!

Monero-HackerIndustrial commented 1 year ago

For the route of cutting out the non needed portions of the monero python library. How do I tag those files for licenses?

Do I just add: SPDX-License-Identifier: {$SPDX_license_name} And a reference to the original file at the top of the document?

Monero-HackerIndustrial commented 1 year ago

I started working on creating a portable version of the seed library called pureSeed.py

I'll eventually add some tools for converting different seed types (Trezor, Ledger, Mymonero & Polyseed). Making it a separate repo then will move over the completed version into a pr.

Couple of questions on the existing passport codebase.

In passport2-monero/simulator/sim_modules/passport/__init__.py I see :

  SPDX-FileCopyrightText: © 2020 Foundation Devices, Inc. <hello@foundationdevices.com>
# SPDX-License-Identifier: GPL-3.0-or-later
#
# __init__.py - Simulated replacements for some Foundation code
#

import sys
from urandom import randint, seed
from utime import ticks_ms

IS_SIMULATOR = True
IS_COLOR = sys.argv[6] == 'color'
IS_DEV = True
HAS_FUEL_GAUGE = False

class Noise:
    AVALANCHE = 1
    MCU = 2
    SE = 4
    ALS = 8
    ALL = AVALANCHE | MCU | SE | ALS

    def __init__(self):
        v = ticks_ms()
        # print('Initialize RNG with seed = {}'.format(v))
        seed(v)

    def random_bytes(self, buf, _source):
        for i in range(len(buf)):
            buf[i] = randint(0, 255)

common.py contains this variable:

# Avalanche noise source
noise = None

I see a noise c file and matching header file. I see the function inside there bool noise_get_random_bytes(uint8_t sources, void* buf, size_t buf_len);

I am guessing that the simulated functions are to make debugging easier on a simulator. How does the device generate that entropy? How is common.noise.random_bytes(seed, common.noise.ALL) actually called from the python code on a device? Is there a cytpe loader or is that already exposed in the device environment?

Also a couple of notes on current seed generation:


    # Hash to mitigate any potential bias in RNG sources
    seed = trezorcrypto.sha256(seed).digest()

I would advocate for having some KDF to generate the key from the random source. In the bitcoin space there is a standard under bip39 for raw entropy to seed. There is no equivalent standard for monero. Here is a thread on creating a standard for monero KDF seeds: I can take care of standardizing it for you.

Is there any constraints for using sha512 instead of sha256 for the KDF on the monero version? I was thinking of using this pattern for the seed:

#the new way uses the key derivation of
#https://github.com/diybitcoinhardware/embit/blob/2bf81739eb5f01f8ad59d23c492fd9d9564eed48/src/embit/bip39.py#L86
PBKDF2_ROUNDS = 2048
#password used for the salt (a sha256sum )
password = hashlib.sha256(dice_rolls.encode()).digest()
entropy_bytes  = hashlib.pbkdf2_hmac(
        "sha512",
        dice_rolls.encode("utf-8"),
        password,
        PBKDF2_ROUNDS,
        32,
    )

hex = entropy_bytes

hex = hex.hex()
s = Seed(hex)
phrase = s.phrase
public_address = s.public_address()

This is basically the same as bip39 I use the same #of iterations but changed PBKDF2-HMAC-SHA256 to PBKDF2-HMAC-SHA512?

Excuse my ignorance on how the foundation device creates entropy. I am just getting started looking at the code and could be wrong. I just didn't see a KDF for randomness to seed. Is there a reason for this ? (Is the source of randomness trusted enough should you still run a KDF?)

Thank you in advance! (FYI None of these questions are blockers for me, I am working on porting over the wordlists but would like to understand the device better).

mjg-foundation commented 1 year ago

Someone deleted their bounty of 2 XMR, bringing this down to 3 XMR total.

donttracemebruh commented 1 year ago

Someone deleted their bounty of 2 XMR, bringing this down to 3 XMR total.

I'm assuming this is referring to the bounty I offered. I don't know why but my comment dissapeared. When I logged into github the reply magically reappeared. Not sure how that's possible.

To be clear, I'm still offering the bounties I originally pledged for this and the other open issues.

detherminal commented 1 year ago

Should prs be opened directly to main branch?

mjg-foundation commented 1 year ago

Should prs be opened directly to main branch?

Yep!

mjg-foundation commented 1 year ago

I started working on creating a portable version of the seed library called pureSeed.py

I'll eventually add some tools for converting different seed types (Trezor, Ledger, Mymonero & Polyseed). Making it a separate repo then will move over the completed version into a pr.

Couple of questions on the existing passport codebase.

In passport2-monero/simulator/sim_modules/passport/__init__.py I see :

 SPDX-FileCopyrightText: © 2020 Foundation Devices, Inc. <hello@foundationdevices.com>
# SPDX-License-Identifier: GPL-3.0-or-later
#
# __init__.py - Simulated replacements for some Foundation code
#

import sys
from urandom import randint, seed
from utime import ticks_ms

IS_SIMULATOR = True
IS_COLOR = sys.argv[6] == 'color'
IS_DEV = True
HAS_FUEL_GAUGE = False

class Noise:
   AVALANCHE = 1
   MCU = 2
   SE = 4
   ALS = 8
   ALL = AVALANCHE | MCU | SE | ALS

   def __init__(self):
       v = ticks_ms()
       # print('Initialize RNG with seed = {}'.format(v))
       seed(v)

   def random_bytes(self, buf, _source):
       for i in range(len(buf)):
           buf[i] = randint(0, 255)

common.py contains this variable:

# Avalanche noise source
noise = None

I see a noise c file and matching header file. I see the function inside there bool noise_get_random_bytes(uint8_t sources, void* buf, size_t buf_len);

I am guessing that the simulated functions are to make debugging easier on a simulator. How does the device generate that entropy? How is common.noise.random_bytes(seed, common.noise.ALL) actually called from the python code on a device? Is there a cytpe loader or is that already exposed in the device environment?

Also a couple of notes on current seed generation:


    # Hash to mitigate any potential bias in RNG sources
    seed = trezorcrypto.sha256(seed).digest()

I would advocate for having some KDF to generate the key from the random source. In the bitcoin space there is a standard under bip39 for raw entropy to seed. There is no equivalent standard for monero. Here is a thread on creating a standard for monero KDF seeds: I can take care of standardizing it for you.

Is there any constraints for using sha512 instead of sha256 for the KDF on the monero version? I was thinking of using this pattern for the seed:

#the new way uses the key derivation of
#https://github.com/diybitcoinhardware/embit/blob/2bf81739eb5f01f8ad59d23c492fd9d9564eed48/src/embit/bip39.py#L86
PBKDF2_ROUNDS = 2048
#password used for the salt (a sha256sum )
password = hashlib.sha256(dice_rolls.encode()).digest()
entropy_bytes  = hashlib.pbkdf2_hmac(
        "sha512",
        dice_rolls.encode("utf-8"),
        password,
        PBKDF2_ROUNDS,
        32,
    )

hex = entropy_bytes

hex = hex.hex()
s = Seed(hex)
phrase = s.phrase
public_address = s.public_address()

This is basically the same as bip39 I use the same #of iterations but changed PBKDF2-HMAC-SHA256 to PBKDF2-HMAC-SHA512?

Excuse my ignorance on how the foundation device creates entropy. I am just getting started looking at the code and could be wrong. I just didn't see a KDF for randomness to seed. Is there a reason for this ? (Is the source of randomness trusted enough should you still run a KDF?)

Thank you in advance! (FYI None of these questions are blockers for me, I am working on porting over the wordlists but would like to understand the device better).

Sorry I forgot to come around to this. The simulator C files mock out the device functionality, so yeah on the device the avalanche rng is used. I'm not a hardware guy and I joined foundation pretty late in passport's development, so I'm not too familiar with the design decisions and some of the more permanent code like seed generation. The only limitation on the seed is what entropy can be stored in the secure element. IIRC we store entropy and parse it into a seed on demand, but I can double check that tomorrow (I'm shitposting from the gym rn)

Edit: If you look at the call to SecretStash.encode(seed_bits), and the encode function in stash.py, you can see what's being saved and how it's encoded.