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

[Feature] Add SeedXOR implementation #43

Open
kdmukai opened this issue Jul 6, 2021 · 37 comments
Open

[Feature] Add SeedXOR implementation #43

kdmukai opened this issue Jul 6, 2021 · 37 comments
Labels
enhancement New feature or request

Comments

@kdmukai
Copy link
Contributor

kdmukai commented Jul 6, 2021

see: https://seedxor.com/

Concept by nvk/Coldcard. Split a 24-word seed into 2+ 24-word seeds via bitwise XOR. Each resulting seed is completely valid but the "true" wallet can only be recovered with all n seeds.

An interesting security option especially for our unique flow of scanning seed QR codes. After creating the 2+ new seeds as QR codes, scanning all n would take almost no additional time. Since each seed is valid in their own right, decoy funds can be deposited on them. This way if an attacker steals or forces someone to give up their seed, they'll only get the decoy funds without realizing that the main stash is XOR'ed with n+ other seeds.

Certainly not a mainstream use case, but adds a fascinating security wrinkle for the rare edge cases that might want it.

Coldcard implementation here:
https://github.com/Coldcard/firmware/blob/0661b80cbdd079816f138b6f1fd64955d5048d7c/shared/xor_seed.py

@kdmukai kdmukai changed the title [Feature] Add SeedXOR support [Feature] Add SeedXOR implementation Jul 6, 2021
@SeedSigner SeedSigner added the enhancement New feature or request label Aug 16, 2021
@david-bakin
Copy link

david-bakin commented Nov 27, 2021

Not sure why you don't consider it a mainstream use case - it could be one as an alternative to seed words + passphrase - just depends on user's preference on how to handle 2-part secrets to access wallet - and as pointed out, works really well with SS (cross reference to #17, specifically this comment.

@openoms
Copy link

openoms commented Nov 27, 2021

Agree with aiming to make the seed XOR usage more mainstream.
See how it is by far the simplest solution in creating a secure redundant 2of3 setup from a single key: https://github.com/openoms/bitcoin-tutorials/blob/master/backups/seedxor.md

@dipunm
Copy link

dipunm commented May 31, 2022

Seed XOR for me is interesting because for inheritance, most of the multi-part security management can be done offline without a computer at all, which has two benefits:

  1. It is something that can be turned into a printable step by step guide for an inheritor to follow.
  2. It avoids needing to discuss security tradeoffs about air-gapped computers, viruses, etc. until the last word (which is where you want to load the words into a wallet anyway)

Once the seed XOR stuff is done, you have a single key and no multi-sig complications which gives the inheritor the freedom to sweep the funds, and also the opportunity to learn to use Bitcoin from the shallow end of the pool.

Something like: https://www.dipunmistry.co.uk/secure-bitcoin-for-dummies/btc-vault/0.primer/ is what I have been thinking about.

That said, CREATING seedxors can take a while even with a guide like: https://github.com/dipunm/seedxor, it took me 45 minutes to trial run generating 2 backup keys that XOR to give me my private key.

I'd say that is where the most value comes from for a seed signer. Turn dice rolls (or PRNG) into SeedXOR sets in a provably fair way.

@jdlcdl
Copy link

jdlcdl commented Nov 22, 2022

I'm currently playing with this in it's own "more_entropy" branch over here

It's a new advanced setting, "more entropy" which enables a new button to press (under add passphrase) before a seed is finalized.
In this "More entropy" menu, I've added 4 reversible operations to be performed on the pending seed's entropy. They are:

  • Invert bits (basically xor with 0xFFs)
  • Reverse bits (all 128 or 256 bits in reverse order).
  • Reverse bytes (all 16 or 32 bytes in reverse order, endian-ness toggle)
  • Seed XOR (which currently only works if another seed is already loaded).

In each case, user is brought back immediately to Finalize the seed, and can view the new fingerprint. From there, onto Done, Set passphrase, or More entropy.

For the first 3 ops, you can do any number of them in any order and if you've done them an even number of times, you get to your original key. Same goes for Seed XOR, do it with the same key 2x and you're back to the start... but this is not the case when mixing with the other ops.

I figure it's 3 more bits of un-written entropy for the bit-fiddling ops plus 1 more bit for each printed seed-qr found on your body... more wrinkles for sure.

I'll chime in here again when I think this is close to ready.

(added) It might seem dangerous, but nobody is forced to use it. If you never use any of it (because nobody else supports it), a seedsigner user would still benefit 3 more bits of entropy per seed, w/ each xor, as long as the choices exist. ie: "Load A, reverse bits, finalize. Load B, invert_bits, xor A, reverse_bytes, reverse_bits, xor A, invert bits, finalize."

@jdlcdl
Copy link

jdlcdl commented Dec 9, 2022

I've made more progress on seed xor in my forked 'more_entropy' branch linked above.

For now, it's available when "more entropy" is enabled and visible when finalizing a seed via the "More entropy" link.
One of the options is SeedXor (if, and only if another seed of same mnemonic length has already been loaded). Selecting SeedXor shows the list of available seeds; user can go back or can apply xor, landing back in the "more entropy" screen... where they can reverse the process or seed-xor with other seeds, until they decide to finalize.

Still to do:
To tear seed xor out of my arguably cringy "More entropy" branch, into a seed_xor branch of it's own, and make it available in the seed finalization process, along with "Done", and "Set passphrase"; perhaps with it's own settings entry too? At the same time, I'll look to accomplishing it with as few changes to existing base classes as possible, unlike "more entropy" ;)

added: an image to run through reference coldcard test vector.
SeedXOR_ColdCardTestVectors

@dipunm
Copy link

dipunm commented Dec 9, 2022

This is amazing, thank you for working on this. I want to download this fork and test it but will likely not be able to get to it until next year. 🫡

@jdlcdl
Copy link

jdlcdl commented Dec 10, 2022

@dipunm, your feedback will be much appreciated.

I've had a humbling experience coming up to speed with git and github lately, but I'm trying to learn to use branches, and doing my best to reserve from pushing until each are in "non-crashing somewhat-working" order.

I keep an "integration" branch somewhat current when I've made meaningful progress on any of the other branches, treating integration as if it were a dev branch of multiple enhancements. Leaving this on my pi2 by default, I use it to remind me of all I have not finished, with hopes the bugs will nag me into eventual fixes, leading to "Done.". Currently it has "more_entropy", "screensaver_blankscreen", "prudently_paranoid_settings" and "kdmukai/initial_multilanguage". At the very top of each branch's README.md file, I've added notes to explain what each branch is about.

Thanks for chiming in with your encouraging words!

[added]: I admit that was an intentional shameless plug for my integration branch above. Of the first 3 branches integrated, they're me kicking the wheels, expressing ideas, and taking a shot at implementation... but to me and what I use them for, they're close enough to "Done." w/o external interest. The nagging issue that is most pressing in my mind is moving forward with Keith's multilanguage branch... so there, my interest in that matter has been stated!

@jdlcdl
Copy link

jdlcdl commented Jan 2, 2023

Since yesterday, I'm starting to reform my opinions on this. Thanks to hard lessons that we have yet to learn via lukedashjr as well as a share from keith in the telegram group that reads like "The tldr is: if you think SeedXOR is a good idea, wait until I tell you about multisig!".

Perhaps XOR or shamir is still interesting for splitting shares and storing them distanced for a single private key w/ a utxo that is held long term. I'm recently leaning hard against it for a bip39 seedqr/phrase that is used to create a bip32 hd wallet.

I had a great deal of fun playing with my more_entropy branch, because it's a way to have a recorded backup that is not the complete solution to spend funds. In a single screen, a couple clicks can alter the entropy of a seed in a way that it can easily be done by computer or by hand and it's reversible, but each mutation is really just a single bit (did the user rev-bits/rev-bytes/invert-bits or did they not). You can then SeedXOR (128-256 more bits) and do it again for 3 more bits, and on and on. BUT, simply adding a passphrase is even more efficient, and already a standard; from one keyboard screen, up up left left click buys us 6-7 bits per character and we only clicked 5 times... that's pretty hard to beat.

On SeedXOR, Keith opined that it might be the invention of the necessity to augment security of backups for seeds that are locked in a non-seedsigner hardware device, this makes perfect sense. But multisig m of n where m is greater than 1 seems much safer... hard to screw-up... and no more difficult (actually much easier) than SeedXOR. With SeedXOR, there are more ways to mess it up than to do it correctly:

  • we can safely use different encodings for SeedXOR to do it right, but if we mix them up between seeds while doing it by hand, we'd get the wrong answer every time.
  • we can be creative to achieve an ugly m of n with SeedXOR, but it's much easier to screw that up too, in a way that a normal user will fail to find the right seed but the sophisticated attacker would find it trivially.
  • giving shares to multiple individuals who "feel safe" that their share is necessary to build the real seed and don't realize that the other individuals could simply backup the final seed or even an intermediate seed, then later remove their own share to learn the shares of the others.

Given the concerns relayed in my prudently_paranoid_setttings branch, I'm not even sure that there is a "right" way to do single-sig, or a "right" way to do 1 of n multisig... with or without a passphrase, and SeedXOR doesn't fix this without motivating a surprise attack with no motivation to keep the rightful owner alive/conscious.

@dipunm
Copy link

dipunm commented Jan 2, 2023

@jdlcdl it's a shame that you have found yourself disheartened by recent events (possibly other feedback too?)

I however stand undeterred by the SeedXOR idea. Multisig is getting easier to be honest, I agree that with little direction around how to use seed xor, it is easy to make mistakes, however multisig is not exempt from this at all.

SeedXOR does have benefits over passphrases:

  • Higher likeliness of a high entropy nth part (of course I could create a seed of 10101010... And that would have low entropy but someone would be intentionally cheating... Low entropy passphrases are definitely more likely)
  • Each part is a valid key (can be used to make decoy wallets)

Compared to multisig, on the seed signer the benefits are lower as the device has temporary memory, but IMO, SeedXOR is best used as a backup tool used with a single hardware wallet that stores the combined key in a secure element.

What happened to Luke is unfortunate, but not a reason to abandon single sig completely, nor a reason to stop innovating to improve the ways we handle it.

I am however biased as I have been spending a better part of a year thinking about SeedXOR and how it can be used in good self custody schemes.

Many will not want to use multisig, and options should exist for them. It isn't objectively proven that multisig is best for everyone. SeedXOR can make the trade off of an involved setup, but a simple usage (when used with a hardware wallet), and the benefit of no physical record of the key in its entirety (only copy exists on a security hardened device).

Even with a seed signer, it can still help to create multi-part emergency backups and inheritance setups while the owner holds a single copy of the combined key on their person or somewhere they consider to be safe without having to worry about losing or damaging it (because you have a multi-part recovery plan).

I agree that the options such as "reverse the bits" and so on are potentially risky as it is very easy to put yourself in a position where you have to remember the exact configuration and that is objectively bad -- not that the feature is objectively bad, just the situation itself. Time and time again we tell ourselves that "I'll remember" and forget the details at the best of times.

@jdlcdl
Copy link

jdlcdl commented Jan 2, 2023

it's a shame that you have found yourself disheartened by recent events (possibly other feedback too?)

I'm mostly disheartened as a result of the role-playing games that I imagine in my head when trying to put adversarial thought to whatever I'm planning to do. But it's only the lack of feedback that would add to this... never feedback that makes me question a previous lack of consideration. After imagining what might have happened to luke recently, single sig scares the hell out of me, whereas I would have argued in the past that depending on how it's done, it can be done right and it's not my job to tell others what they must not do. If we have just a bunch of private keys that are offline, future mistakes we make while signing on a utxo from some device that we're not aware is compromised won't put our other keys at risk, but with an hdwallet, this wouldn't be the case. I will not remove or fail to maintain what I have already completed of SeedXOR in my more-entropy branch, it remains in my integration branch too (shameless plug once again for blankscreen, multilanguage, paranoid-settings improvements which I'm still a believer of), and if there is interest from others then I most certainly would do the work to tear SeedXOR out into it's own branch... in the spirit of serving popular wants and needs... and I'll just include my current thoughts at that time so the user is aware of whatever my feelings have evolved to.

I am however biased as I have been spending a better part of a year thinking about SeedXOR and how it can be used in good self custody schemes.

I will admit that it was your video from a london meetup which introduced me to this concept, and inspired me... and that XOR has been a binary operation very close to my heart ever since I discovered its magic. I look forward to you sharing more of your thoughts on this.

Thank you for chiming in once again, and Happy New Year to you and yours!

@jdlcdl
Copy link

jdlcdl commented Jan 4, 2023

@dipunm I wrote:

if there is interest from others then I most certainly would do the work to tear SeedXOR out into it's own branch.

Today I checked my crystal ball... it's still cloudy, buses still exist and I cross the street frequently; tomorrow is not promised.

I've created a seed_xor branch in my repo here and this is a hat-tip to you, good sir!

You'd have to enable "Seed XOR" in advanced settings, then load at least one seed and finalize it.
Then, you'd need to load another seed of the same mnemonic length in order to have the option to "Seed XOR" just underneath "Add Passphrase" when finalizing.

At this time, you know my current feelings, so I have no intention to make this a pull request. Also, since I do not believe in asking permission, nor do I believe in begging forgiveness, for anything that it one's "right", consider this branch "out in the wild" if you want to pursue it. I'll certainly pay attention while I'm here and I'm always open to be persuaded.

This is a slightly less cringy implementation than more_entropy, but it still includes one new seed_xor() method in the models.seed.Seed class which would require careful peer review; at least it's not 5 new methods in that class plus 4 new helper functions in a new helper module. I thought of packing it all into the "view" but it felt more appropriate to be done in the "model".

Feel free to check it out.
... and while you're there, please take a look at "integration" too (hint hint hint hint hint) where my other hard work is featured... How's your french? Give it a shot please and let me know what you think. I'm chomping at that bit, for my own personal family/friend reasons.

;)

@dipunm
Copy link

dipunm commented Jan 4, 2023

Awesome. As I said, I would have time in the new year so I will find that time asap!

Hopefully I can get playing this weekend. Also my french is ... Non-existent 😅 sorry. Can't help there.

@KyleOfTheCorn
Copy link

I'm surprised no one has mentioned privacy as a reason for wanting to stick with a singlesig setup. Telling users to just use multisig actually harms their privacy, as it reveals that they're using a multisig (until Taproot usage significantly increases), and they will lack the ability to use privacy-preserving tools in wallets (like Sparrow Wallet).

@dipunm
Copy link

dipunm commented Jan 12, 2023

I will be attempting to find time this weekend to test this feature 😍

@jdlcdl
Copy link

jdlcdl commented Jan 12, 2023

I will be attempting to find time this weekend to test this feature heart_eyes

@dipunm, you have no idea how appreciative I will be to have your feedback. I've worked really hard (hopefully smart too) and I suspect you'll be the first to have played with any of my branches.

To save you some time, might I suggest the following setup, which is what I've learned (the hard way) works best for me when running someone else's branch.

If you are not already setup for seedsigner development, follow the standard instructions for setting up a development environment on your RPi. It can be found in https://github.com/SeedSigner/seedsigner/blob/dev/docs/manual_installation.md. Make sure that it works, that you can ssh in, that you can turn on/off systemd seedsigner-service, and that you can run seedsigner manually from the commandline, etc.

Then, to avoid polluting your fresh seedsigner development environment, consider the following steps:

  • Create a new virtualenv just for this type of alpha testing. This way, you'll know that your normal seedsigner-env environment remains ready for YOU to advance with your own development. You can then switch back with a quick "workon seedsigner-env" which will default after a boot or the next time you login. Also, maybe let your eyes get used to seeing how this changes your shell prompt... just as we do with our seed fingerprints.
    mkvirtualenv --python=python3 seedsigner-alpha-env

  • Don't pollute your freshly cloned ~/seedsigner repo. Instead, test others' code in something like ~/seedsigner-alpha. This way, cleaning-up is a quick "rm -rf ~/seedsigner-alpha" away.
    git clone https://github.com/jdlcdl/seedsigner.git seedsigner-alpha

  • Change directory into the new seedsigner-alpha repo.
    cd ~/seedsigner-alpha

  • Checkout the branch that you want to test. You can see the available branches by navigating to my github page and selecting the "branches" select-menu towards the upper-left of the page (default is "dev"). I do my best to make brief edits at the very top of the README file for each of my branches; it's a short explanation of what the branch is about.
    git checkout seed_xor

  • Whether it's a virgin virtualenv that has never been used, or if you're testing yet-another-branch from another developer and you do this all the time... get in the habit of making sure the environment is up-to-date for the branch that you are about to test.
    pip3 install -r requirements.txt

  • Make sure you've killed any running instance of seedsigner, then change directory and get to testing!
    cd src
    python main.py

  • I already know you know this... but... NO REAL SEEDS, and don't forget to switch to 'testnet'!

If you have any questions along the way... I WILL JUMP HIGH to help you out as quickly as I can.

Thanks in advance!

<3


P.S. I'll take this opportunity to once-again shamelessly plug my "integration" branch, which contains all of my working enhancement branches (whether they're good ideas or not). If testing this one, you'll want to read the babel/ readme file because you need to compile the binary message catalogs; it has new requirements too (a newer PIL library). It contains the following branches (w/added advanced settings):

  • kdmukai/initial_multilanguage with my exploration between mid-nov to mid-dec 2022, which is as internationalized as done can be "Done!" with 100% french and most of spanish translated -- except for my own added screens because I dont want to pollute the messages.pot catalog... for now.
  • screensaver_blankscreen will keep your pi much cooler when left on for hours during development/testing,
  • more_entropy replaces seed_xor while finalizing a seed... you can tweak the seed in a few reversible ways, plus XOR,
  • prudently_paranoid_settings allows you to disable seed_backup, address_explorer, and export_xpub, and others in a way where once disabled, it's truly "disabled" and would force you to discard loaded seeds before re-enabling anything that could allow seedsigner to leak privacy or security data (think as if you'd scanned your seedbackup quickly then locked it back in the safe... so now you can relax, refill coffee, take a w/c break, get distracted w/o seedsigner showing anybody your secrets were they to get a hold of your seedsigner while it was still powered on),
  • issue_294_passphrase is a bug or a feature depending on how you look at it. bip39 allows leading or trailing spaces in a passphrase, or only spaces... while seedsigner strips them, meaning you couldn't recover an hd wallet that had been setup previously with leading/trailing/only spaces (which would be a terrible practice but is still valid bip39). With this addition, you CAN practice this horrible practice, and it would show you your strange spaces as blocks during review.

@dipunm
Copy link

dipunm commented Jan 17, 2023

ooh wee, I did not expect setup to be so involved!

I have managed to get ssh access over USB, now I can take steps to install the software. The information you have provided is very useful so thank you for that.

I will keep you posted on my progress as I find time to test.

@dipunm
Copy link

dipunm commented Jan 17, 2023

Okay! I have played with the UX, I haven't done any correctness checks, but I'm happy to get direction on what you want me to focus on re: testing.

My initial thoughts are:

  • SeedXor next to passphrase is sensible (adding extra entropy to a new seed) BUT...
  • There is no way to retroactively combine 2 or more seeds (nor is there a way to later apply a passphrase to be fair)
  • You can't recursively keep creating new seeds (this would be the ultimate fast seedxor experience IMO)
  • This forces you to "prepare" N-1 seeds, and remember to apply seedXOR on the Nth seed which requires focus to do and is likely to be a point of mistakes or frustration. I know we can delete or even re-enter the last seed if we forget, but you can see how the whole thing can be laborious and this extra thing to track is likely to cause frustration.

The icon may want a change, it looks a bit like a virus to me, I like the idea of two polarised filters overlapping, although there isn't much space or colour to work with here either.

This definitely works functionally, enabling SeedXOR through advanced is easy and was intuitive to find.

Regarding the recursive idea, I was thinking:

  • Create seed
  • User should write down and then verify 12/24 words
  • choose Seed XOR when asked about passphrase
  • go to menu asking how they'd like to create their next seed
  • loop until user chooses DONE

Note to self: to test: how does the UI handle mixed 12/24 word seeds?

@dipunm
Copy link

dipunm commented Jan 17, 2023

re: icon, I see we are dealing with characters.

Here are some suggestions:

  • ⧉ (U+29C9)
  • ⊕ (U+2295)
  • ⨁ (U+2A01)
  • 𑗕 (U+115D5)

@jdlcdl
Copy link

jdlcdl commented Jan 17, 2023

With hopes of providing a tight feedback loop for you @dipunm, I will respond quickly (but no code changes yet).

You said:

There is no way to retroactively combine 2 or more seeds (nor is there a way to later apply a passphrase to be fair)

I'm assuming that this would mean having the seed shares already loaded, then combining them via xor, perhaps from the tools menu... and being able to add a passphrase on top of the resulting xor-ed seed before "Done".

I am not proposing a solution here... but given that no more code were written to address this, I'm thinking that the same could be done currently by:

  • load each seed share and "Done" (they would all need to be of the same mnemonic length) then
  • if using 12 word shares: load "abandon "*11 + "about" in order to prime the pending seed as 16 - 0x00 bytes, then "Seed XOR" against each of the loaded shares, then "Add Passphrase" and/or "Done".
  • else if using 24 word parts: load "abandon "*23 + "art" in order to prime the pending seed as 32 - 0x00 bytes, then "Seed XOR" against each of the loaded shares, then "Add Passphrase" and/or "Done".
  • seedsigner would then have all of your seed shares loaded AND the last seed would be the resulting seed. You'd be able to SeedBackup for the seed words and/or SeedQR.

"Yuck!!!" to that process! I will give it some thought, with the help of your other feedback above, to find a better solution.

You said:

You can't recursively keep creating new seeds (this would be the ultimate fast seedxor experience IMO)

I'm assuming that this would mean loading but NOT finalizing any of the seed shares, instead selecting "Seed XOR" on all but the last one, in order to load the next... then selecting "Add Passphrase" and/or "Done" such that the seedsigner has ONLY the resulting seed loaded into the "Seeds" menu at the end of the process. I agree, this would be slick! I will give this some thought.

You said:

This forces you to "prepare" N-1 seeds, and remember to apply seedXOR on the Nth seed.

This is correct given the current implementation. As well, you've got to remember to SeedXOR against each of the shares once and only once (or at least an odd number of times for each), before finalizing. I can also see the "Yuck" in that.

You also gave me some examples of a better icon. I will see what is available in the current fonts, leaning towards the one which most clearly represents "XOR", as opposed to "BioHazard".

Thank you for the suggested ideal flow, I will keep this in mind while playing with changes mentioned above.

As for the flow that includes "reviewing seed words": I would expect this whenever seedsigner has "created" a new seed which would happen either by a photo, dice rolls, or calculating last word once in those flows...
BUT are you sure you would want to be pushed through reviewing seed words for every result of a "Seed XOR" operation??? This might be useful the very first time, but user could also just use the SeedBackup once loaded. Every time after that, it would be unnecessary, so I'm going to assume that scanning or entering seed words while loading in the recursive XOR manner that you describe will NOT need the seed review step.

Also, as for UI of how it deals with seed parts of different mnemonic length, currently "Seed XOR" would not be available when finalizing unless you already had another seed previously loaded of the same length that you are about to finalize. If you had both 12 and 24 word seeds loaded as shares, the current implementation would only offer the choice to XOR against compatible length seed shares.

Thank you for all of your feedback @dipunm!

@jdlcdl
Copy link

jdlcdl commented Jan 17, 2023

Note to self:

I've given lots of thought about using "prudently_paranoid_settings" to convince seedsigner that when a prudently paranoid user disables a setting, that they really truly intentionally meant "Disable that damned setting!"... so that if an attacker were to recover a powered-on loaded seedsigner next to the rightful owner's corpse, seedsigner would NOT so easily give up all of its secrets after the owner's last wishes were in-fact "Please don't!".

If I were to implement reviewing seed words during "Seed XOR" against loaded seeds, then an attacker could load 0x00 bytes (11x abandon + about, or 23x abandon + art) and then use SeedXOR to retrieve any of the seed words which had been previously loaded. I should give this extra prudent thought, assuming that I remain a true-believer of prudently_paranoid_settings, which IS the case presently. In other words... prudently_paranoid_settings would need to avoid reviewing seed-words during SeedXOR flow if "SeedBackup" were disabled (as well as assert SeedBackup==enabled within the review seed words "view" to protect against a future bug that forgets to avoid doing so).

@dipunm
Copy link

dipunm commented Jan 17, 2023

BUT are you sure you would want to be pushed through reviewing seed words for every result of a "Seed XOR" operation??? This might be useful the very first time, but user could also just use the SeedBackup once loaded. Every time after that, it would be unnecessary, so I'm going to assume that scanning or entering seed words while loading in the recursive XOR manner that you describe will NOT need the seed review step.

Oh, to be clear, I wasn't suggesting that we FORCE anyone to review, the UX is currently set up fine where the user is given the option to verify the seed words and they can choose to skip it if they want.

I will test and give comment about how the 12/24 word seed behaviour is. I suspect that I will have nothing to comment on other than a job well done because of course we cannot mix 12 and 24 word seeds and this is very much an edge case that I expect no person in their right mind would even discover :)... who makes both 12 and 24 word seeds?!?!

@dipunm
Copy link

dipunm commented Jan 17, 2023

Okay did a bit more testing.

I created a random key with a passphrase then another and I was offered to SeedXOR with it. What should I be expecting here? I think keys with passphrases should be excluded just like mismatching word counts are.

Further, it is technically incorrect because it shows the fingerprint WITH passphrase which I imagine is NOT what we are combining:

12 words        3f7114eb
+ passphrase    42cf8657

24 words        142f6440
+ passphrase    09486d0d

12 words        ef664f12
+ 42cf8657 <--- INCORRECT

@dipunm
Copy link

dipunm commented Jan 17, 2023

Going to "add passphrase" then back = Finalize seed page
Going to "SeedXor" then back = Verify Backup page.

@dipunm
Copy link

dipunm commented Jan 17, 2023

I noticed that if you seedXOR with something, you will see a done option and then you have to go to Backup Seed > View Seed Words in order to see the new words.

I had a little think about this and this seems perfectly fine to me. In some cases you may never want to or need to see the combined key, you just get an XPUB, receive to it and later you can trust that combining the two+ seeds will give you the same private key in the future.

@dipunm
Copy link

dipunm commented Jan 17, 2023

It is interesting that SeedSigner doesn't attempt to help you choose a valid final word, nor does it let you continue if you have an invalid final word.

The reason I checked for this was when I saw this example: https://embit.rocks/#/api/bip39?id=mnemonic_to_bytes it occurred to me that if ever seed signer DOES allow invalid checksums, then you will likely want to ignore the checksum when converting from words to bytes.

Fortunately, there is nothing to do here since none of the settings allowed me to create a mnemonic with an invalid checksum.

That also tells me that I shouldn't suggest any methods of last word selection that doesn't guarantee a valid checksum. I have in the past suggested that you could choose a word within range (i.e. last word = 3A_, choose a word between inject -> invest) -- although this technique preserves the original entropy, it is likely to be more problematic than I thought when it comes to restoring.

@jdlcdl
Copy link

jdlcdl commented Jan 18, 2023

Four posts above this one, @dipunm wrote:

I think keys with passphrases should be excluded just like mismatching word counts are.

Further, it is technically incorrect because it shows the fingerprint WITH passphrase which I imagine is NOT what we are combining:

This is a great point! I can see that this could be confusing.

To review:

SeedXOR IS combining the initial entropy only of two or more BIP-39 mnemonic phrases, then it's adding the necessary checksum bits to result in a new valid BIP-39 mnemonic phrase.

That is, SeedXOR:

  • is NOT combining two or more BIP-39 seeds (the 512 bit result of pbkdf2("sha512", mnemonic-phrase, "mnemonic"+passphrase, 2048) which is later fed into BIP-32 to create a master wallet private-key and chain-code)
  • is NOT combining two or more BIP-32 wallets which is the same as saying it is NOT combining master-xprvs nor master-xpubs nor key-identifiers nor fingerprints.

In my implementation of SeedXOR, there are two and only two three and only three edge-cases checked (I can imagine a 3rd now, which I'll mention below) where SeedXOR of two seeds is NOT allowed and an exception would be raised:

  • you cannot SeedXOR two seeds of different mnemonic lengths (more accurately, of 2 or more different lengths of entropy bits) because this would imply clipping the longer one to the length of the shorter one. ie: entropy bits would be lost... Yikes! Else it would require filling the shorter one to the length of the longer one with something other than entropy... Yikes!
  • you cannot SeedXOR one seed against itself (more accurately, the same entropy bits against themselves) because this would result in all 0x00 bytes ie: a mnemonic of "abandon "*11 + "about", or "abandon "*23 + "art"... Yikes! While reversible, still a bad idea.

Similarly, an equally bad idea, but NOT yet a defined edge-case where an exception would be raise is: This has been implemented and will be pushed in the next commit to my seed_xor branch.

  • allowing SeedXOR against a seed's inversion, which would result in all 0xff bytes, ie: "zoo "*11 + "wrong" or "zoo "*23 + "vote"... Yikes!

This raises some interesting confusions for the user.

  • The same mnemonic-phrase used with different passphrases (empty is different than non-empty) will create seeds with different fingerprints, which would raise an exception if combined via SeedXOR. They might wonder "Why can't I SeedXOR these unique fingerprints?".
  • While very improbable, two different mnemonic-phrases, given billions of different passphrase attempts, can have the same fingerprint even though they are different and do NOT represent the same BIP-32 hd wallets, and could safely be combined via SeedXOR. They might wonder "Why am I being allowed to SeedXOR the same seed with itself?".

You make a very good, completely convincing point that we should NOT offer the possibility to combine via SeedXOR any seeds with a passphrase, simply to avoid this possible confusion for the user. It would NOT stop them from using SeedXOR, it would instead force them to reload their seeds again, this time w/o passphrase, better organizing their own thoughts and understanding while also avoiding unnecessary confusion.
[Update]: This has been implemented and will be pushed in the next commit to my seed_xor branch.

GREAT FEEDBACK @dipunm!!! Thank you!

@jdlcdl
Copy link

jdlcdl commented Jan 18, 2023

Four posts above this one, @dipunm wrote:

Going to "add passphrase" then back = Finalize seed page
Going to "SeedXor" then back = Verify Backup page.

If this is true, and I trust you on this, but I WILL verify,
then
and I have verified that you've found yet another bug in that second line... they should both go back to Finalize!

Good job ;)

[UPDATE]: this is resolved.

@jdlcdl
Copy link

jdlcdl commented Jan 18, 2023

@dipunm, you are a star!

I have work to do, thank you for all of your feedback so far!

@jdlcdl
Copy link

jdlcdl commented Jan 18, 2023

Four posts above this one, @dipunm in self-reflection opined:

That also tells me that I shouldn't suggest any methods of last word selection that doesn't guarantee a valid checksum. I have in the past suggested that you could choose a word within range (i.e. last word = 3A_, choose a word between inject -> invest) -- although this technique preserves the original entropy, it is likely to be more problematic than I thought when it comes to restoring.

I'm not so convinced about this yet, and it's not an opinion that I've formed at-all on my own, it comes from a discussion I recall by Mr. Wuille in regards to the bech32 encoding.

There are times when error-detection is preferred over error-correction, and there are also times when error-correction is preferred over error-detection; it's about weighing the trade-offs. You might not want to error-correct an improperly copied address because if you got it wrong, funds would be sent to the wrong address and lost forever; you would instead want to motivate the user to identify their mistake and correct it on their own. Other times, like recovering a seed for the rightful owner, you might want to lean towards error-correction, as you mention you've done in the past. I think that both methods are valid opinions in this case, and I could also see leaning towards your previous method since it hints to the user that a guess is about to be made.

@dipunm
Copy link

dipunm commented Jan 18, 2023

Maybe this week I will run through some tests with a low entropy (000... or 111...) key to see how the software behaves. I'll also try to create duplicate keys to see how SeedSigner behaves there too.

IMO low entropy seeds are bad for many purposes, least of all seed xor. I think they should be caught and handled at the key input stage similarly to how inputting keys with invalid checksums are met with a warning and no way to continue.

I agree that people shouldn't mix low entropy with their keys but:

  • low entropy XOR high entropy XOR medium entropy = high entropy

The final key is always as secure as the highest entropy key*.

The reason not to use low entropy is simply because a low entropy key is useless and worst case it is misleading.

You shouldn't use a low entropy key as part of a multi-part setup and, since any one key can be considered the master key, you definitely shouldn't use the low entropy key as the master key.

  • Clearly if low + high = high, then it is possible for high + high = low. 🤔

@jdlcdl
Copy link

jdlcdl commented Jan 18, 2023

  • Clearly if low + high = high, then it is possible for high + high = low. thinking

32 bytes of high entropy are 256 bits of entropy.
The inversion of that entropy is every bit just as high... but they're also highly correlated, even if reverse-correlated.
You most certainly wouldn't want to SeedXOR them together... you'd get 0xff*32... the lowest of low entropy.

I agree with your thoughts that there might be something to be done to protect users from, or at least warn users about, low-entropy seeds.

;)

jdlcdl pushed a commit to jdlcdl/seedsigner that referenced this issue Jan 18, 2023
* Cannot SeedXOR against a seed's inversion.
* Cannot SeedXOR against a seed w/ passphrase.
jdlcdl pushed a commit to jdlcdl/seedsigner that referenced this issue Jan 18, 2023
* Not even offered to SeedXOR against itself.
* Cannot SeedXOR against a seed's inversion.
* Cannot SeedXOR against a seed w/ passphrase.
jdlcdl pushed a commit to jdlcdl/seedsigner that referenced this issue Jan 19, 2023
    * cleanup: get_seedxor_seeds() replaces redundant cut/pastes
    * bugfix: back from SeedXOR to Finalize, not Scan
@jdlcdl
Copy link

jdlcdl commented Jan 19, 2023

Six posts above this one, I've outlined the 4 edge cases that are checked in this implementation:

  • cannot SeedXOR two seeds whose mnemonic lengths differ, because entropy would be lost, and irreversible.
  • cannot SeedXOR a seed against itself, because new entropy would be all zeros.
  • cannot SeedXOR against any seed with a passphrase; too confusing, just reload same mnemonic w/o passphrase.
  • cannot SeedXOR a seed against its inversion, because new entropy would be all ones... but THIS IS NEW, and it's sort of difficult to test.

In order to test that last case, we can create an inversion of any seed's entropy by doing SeedXOR against 0xFF bytes.

I've included 2 standard SeedQRs which are 12 and 24 word seeds with all 0xFF bytes, so that you can invert any seed (load the SeedQR that matches your mnemonic length, and use SeedXOR to invert it), and then verify that this SeedXOR implementation will raise an Exception if you try to SeedXOR against any seed's inversion.

  • 25x25 SeedQR has 12 mnemonic words:
    "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo wrong"
    zoo_x_11_plus_wrong

  • 29x29 SeedQR has 24 mnemonic words:
    "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo vote"
    zoo_x_23_plus_vote

@jdlcdl
Copy link

jdlcdl commented Sep 3, 2023

@pythcoiner recently mentioned that it would be nice to apply seed_xor to seeds that were already loaded on seedsigner.

So Im keeping a note here. I believe that while the below ux is less-than-frictionless, the intended functionality could be achieved by scanning a SeedQR representing all 0x00 entropy bytes then applying seed_xor against a loaded seed, resulting in itself, followed by applying seed_xor against another.

12 words, 16 - 0x00 bytes
'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about'
abandon_11_about

24 words, 32 - 0x00 bytes
'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art'
abandon_23_art

@GregTonoski
Copy link

  • cannot SeedXOR a seed against its inversion, because new entropy would be all ones... but THIS IS NEW, and it's sort of difficult to test.

What is unique risk about that edge case?

@jdlcdl
Copy link

jdlcdl commented Oct 22, 2023

  • cannot SeedXOR a seed against its inversion, because new entropy would be all ones... but THIS IS NEW, and it's sort of difficult to test.

What is unique risk about that edge case?

If SeedXOR is used combine a seed against it's inversion, user would end up with all 1s as entropy, giving a mnemonic with all words as "zoo" except for the last one.

It's very similar to using SeedXOR to combine a seed against itself, user ends up with all except last word being "abandon".

@deficruncher
Copy link

deficruncher commented Apr 10, 2024

My 2 cents about current implementation and UX concerns.

As I understand in more_entropy branch XORing can be applied in a seed finalize seed. I feel like that is not appropriate place for that feature.

To me it seems that more fitting place would be in: "seeds -> create a seed -> New Seed (XOR) -> Select seed length" along "New Seed (DICE)" and "New Seed (CAMERA)". After selecting it we would have "Select first seed" screen and "Select second seed" with the list of loaded seeds of selected length.

That way it's more explicit that XOR makes new seeds from other already existing and proper seeds. And all XOR inputs are on the seed signer to export if needed! (Unlike the more_entropy approach in which seeds are modified in finalize step). Also that way it would be impossible to XOR seed with itself (because on the "select second seed" the first choice would be filtered out).

As a side note: imho those "revert/invert/reverse" bits are a distraction. Those basically add a layer of poor "security by obscurity": each such operation adds 1 bit of entropy. Imho not worth the added complexity and UX overhead.

@jdlcdl
Copy link

jdlcdl commented Apr 13, 2024

My 2 cents ...

Thank you for all of this input.

The reversible byte-and-bit operations in more_entropy would never be appropriate in seedsigner, but I threw them in there because they are operations (like seedxor) that can be done by hand if working with a backup that had been stored as 16 or 32 hex bytes of entropy. I totally agree that they're just an additional 3 bits combined (did user do each of them or did they not), and if mixed with xor it can become extended again and again (into a super footgun). Those are not "secure" solutions at all, they're simply hand-workable and reversible obscurity.

As for your input on ui for xor, thank you especially for this. Between what exists now, @dipunm's input above, and yours above, there are 3 unique ways of looking at a possible SeedXOR implementation and ui presentation. I still think I prefer @dipunm's (load them in series, only finalize when complete) but I can see that your proposal is closer to what is already there (in that it uses seeds that have already been fully loaded into seedsigner).

In the end, I don't suspect that at this time, SeedXOR is planned for inclusion into seedsigner, so I may be slow about future commits to this branch. I suspect that as more signing hardware and the rest of the industry decide to treat SeedXOR as a common standard, that it may change and be attractive for seedsigner too. As it stands, seedsigner has a bias to do the simplest, safest, best-practices for users as a priority (if a better solution already exists... do that instead of less ideal solution) and as a secondary bias, seedsigner wants to be able to recover wallets that others have implemented so that users can be saved (without enticing users to continue with less-than best-practices solutions).

Thanks again for taking the time to look at the branch and to follow-up with your much-appreciated written feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants