BIP138: Compact Encryption Scheme for Non-seed Wallet Data#1951
BIP138: Compact Encryption Scheme for Non-seed Wallet Data#1951pythcoiner wants to merge 12 commits into
Conversation
2a6e241 to
d9d02ff
Compare
|
thanks for the review! will address comments tmr! |
In general nonce reuse is unsafe because if you make multiple backups over time, e.g. as you add more transaction labels, you would be reusing the nonce with different message. By including the However it still seems unwise to mess with cryptographic standards. It doesn't seem worth the risk for saving 32 bytes on something that's going to be at least a few hundred bytes for a typical multisig. |
|
Concept ACK, seems adjacent to how some lightning tools enable users to recover SCB's with just their seed to identify and decrypt the backup. Makes sense for descriptors to have something similar. |
1e4ca34 to
3b6b6ad
Compare
|
Concept ACK |
|
(not yet finish addressing comments) |
|
Hi @pythcoiner, By coincidence, two weeks ago I started working on a proposal for a "Standard Encrypted Wallet Payload" to be placed inside an "Encrypted Envelope". The "Wallet Payload" contains descriptors and metadata but can also act as a full wallet backup including transactions, UTXOs and addresses. The proposal is very much a work in progress. I only just found this discussion so am reading through it to compare it to my proposal. The descriptor backup in the "Wallet Payload" of my proposal seems to have some overlap with the BIP proposed here. If there is too much overlap I may reconsider progressing with my proposal. As mentioned, my proposal is very much a work in progress but the wallet payload proposal can be found here: https://gist.github.com/KeysSoze/7109a7f0455897b1930f851bde6337e3 Maybe jump to the test vector section to see what a basic backup of a descriptor and some meta data would look like prior to encryption. https://gist.github.com/KeysSoze/7109a7f0455897b1930f851bde6337e3#test-vectors As my proposal is designed to be modular and extensible the encryption envelopes may be extended to offer Multiparty Encryption and Authentication. See: I have already started documenting an encryption envelope that uses AES-256-GCM and password protection: https://gist.github.com/KeysSoze/866d009ccd082edf6802df240154b20d I have not written a reference implementation yet but there are well established python and Rust libraries for CBOR and COSE that should make implementing the BIPs relatively simple. |
ab0d14d to
2ce692d
Compare
Hi @KeysSoze, this work seems more related/parallel to the But I've adopted a slightly different approach by simply using JSON. FYI we already implemented this wallet backup format in Liana wallet and I plan to work on a BIP proposal relatively soon. |
|
Thanks for the updates! I think all I need now, is something like this (with better terminology): diff --git a/bip-0138.md b/bip-0138.md
index 2aa276b..db6b783 100644
--- a/bip-0138.md
+++ b/bip-0138.md
@@ -267,5 +267,5 @@ Implementations MUST reject empty payloads.
defined in `ENCRYPTION` where `PAYLOAD` is encoded following this format:
-`CONTENT` `LENGTH` `PLAINTEXT` (`PADDING`)
+`CONTENT` `LENGTH` `PLAINTEXT` (`CONTENT` `LENGTH` `PLAINTEXT` ... ) (`PADDING`)
`LENGTH`: variable-length integer representing the length of `PLAINTEXT` in bytes. It MUST
@@ -274,8 +274,6 @@ be present.
`PLAINTEXT`: the `LENGTH` bytes of payload data.
-`PADDING`: OPTIONAL bytes after `PLAINTEXT`, up to the end of the decrypted `PAYLOAD`.
-Parsers MUST consume exactly `LENGTH` bytes of `PLAINTEXT` and MUST ignore everything after
-it. These bytes are reserved for size padding (see Padding) and/or vendor-specific data,
-the same way trailing bytes after `CIPHERTEXT` are reserved and ignored.
+`PADDING`: OPTIONAL bytes after the final `PLAINTEXT`, up to the end of the decrypted
+`PAYLOAD`. Parsers MUST consume exactly `LENGTH` bytes of each `PLAINTEXT`.
#### Padding
@@ -308,12 +306,11 @@ All variable-length integers are encoded as
#### Content
-`CONTENT` is a variable length field defining the type of `PLAINTEXT` being encrypted,
-it follows this format:
+`CONTENT` is a variable length field defining the type of the following `PLAINTEXT`.
+It follows this format:
`TYPE` (`LENGTH`) `DATA`
-`CONTENT` is a single `TYPE (LENGTH) DATA` triple, one blob describing `PLAINTEXT`.
-It is not a sequence of entries; it is immediately followed by the payload `LENGTH`
-and `PLAINTEXT`.
+Each `CONTENT` field is a single `TYPE (LENGTH) DATA` triple, one blob describing the
+`PLAINTEXT` item immediately following it.
`TYPE`: 1-byte unsigned integer identifying how to interpret `DATA`.
@@ -341,6 +338,6 @@ the remaining payload bytes.
For an unknown `TYPE` less than `0x80`, parsers MUST consume its `LENGTH` bytes of
-`DATA`, treat the content type as unknown, and continue with the payload `LENGTH`
-and `PLAINTEXT`.
+`DATA`, treat the content type as unknown, consume the following payload `LENGTH` and
+`PLAINTEXT`, and continue.
For an unknown `TYPE` greater than or equal to `0x80`, parsers MUST reject theI think the test vectors lack coverage for rejecting an all-zero nonce. I updated Sjors/bitcoin#109 with the latest changes, plus my remaining wish list item. Though I also want to take a closer look at my implementation to see if it reveals any issues with the proposal. |
hum, I'm not strongly against that, but i dont see an usecase in the current context:
In fine, I'm happy to add it if there is concrete examples (I may have missed obvious usecase btw) |
|
I prefer to keep each piece of content simple, so e.g. I would have a one blob with the BIP388 policy (or BIP380 descriptors), one blob with BIP329 transaction labels, and maybe a partially signed transaction for a recovery scenario. By allowing multiple pieces of content (in a single encrypted blob), it's easy to expand later. I don't believe in the approach of BIP129 of trying to define a format to store everything in a single JSON blob (not opposed to it either).
I don't think you can add support for multiple pieces of content in a single encrypted blob later, you'd have to break compatibility. |
|
@Sjors I've added what you suggested |
|
|
||
| | Value | Definition | | ||
| |:-------|:---------------------------------------| | ||
| | 0x00 | Reserved | |
There was a problem hiding this comment.
Claude suggested the following to make padding work with multiple items:
| Value | Definition |
|:-------|:---------------------------------------|
-| 0x00 | Reserved |
+| 0x00 | End of content items; padding follows |
| 0x01 | BIP Number (big-endian uint16) |
| 0x02 | Vendor-Specific Opaque Tag |(and elsewhere don't say 0x00: parsers MUST reject the payload.)
There was a problem hiding this comment.
it's also how I've implemented it, I'll update the spec
There was a problem hiding this comment.
Let me know when that's done and I'll give it another try on the implementation side.
When walking the CONTENT/LENGTH/PLAINTEXT items of a decrypted payload, stop at the first 0x00 TYPE byte and treat the remaining bytes as padding, rather than rejecting the payload. This lets encoders zero-fill a payload up to a padding bucket (BIP138 "Padding") without breaking decryption. This is kept as a separate commit because the BIP138 text is not yet unambiguous: the content-type table still lists 0x00 as "reject", while the payload section describes the first 0x00 as the end of the item sequence. See bitcoin/bips#1951 for the proposed clarification. Other malformed content (e.g. an unknown TYPE >= 0x80) is still rejected.
|
I updated Sjors/bitcoin#109 with your changes in 56deb75; pretty much the same code since I already anticipate the change. I added a commit that implements my suggested change in how to handle padding in #1951 (review), but I'm happy to switch to a different approach. |
|
Something I have in mind is to optionally pad the list of individual secrets with decoy entries, random values indistinguishable from a real individual secret. The padding could round the count up to fixed buckets, like start at 5 then double on overflow (5 -> 10 -> 20) |
|
and on the same topic, as derivation paths are optionals maybe we should state in the spec that for decryption, if common derivation path are not present in the list, the implem should iterate over the list of derivation paths for account 0-10. |
I like that idea, assuming it doesn't add too much overhead relative to the descriptor size. Could round up to Fibonacci numbers >= 3, so most simple setups have a good anonymity set.
BIP87 mainnet account 0 seems like a good default to check (
If the user is expecting testnet / signet, they should just specify that ( If you have access to the seed then we can suggest a fairly wide range to scan. But when you need to fetch an xpub from an external device, and possibly deal with a permission prompt on that device, I would stick to |
right we must be sure to use only derivation paths that signing devices let user fetch w/o prompting for validation edit: I've asked claude to look at source code of these devices it seems none of the devices prompt user for confirmation if we restrict to these derivation paths: |
|
Ok, that's good news. We should lobby them for adding But there's also air-gapped setups to consider. So I still suspect it's better to try However this gets too nitty-gritty for a BIP I think? |
I think for airgap devices, the encrypted payload should be passed to the device via sd card and decrypted on device, or have a way to fetch a list of xpub like i offer here |
Tested today fetching m/87h/... using async-hwi cli with USB devices:
So I think it's fine to try fetch m/87h/... from device but last, and prompt user if fails/hang |
I'll have it implemented in rust anyway, so we can reference the logic w/o much details imo |
Interesting idea, but I wouldn't want to depend on that.
This uses
As long as it's presented as practical tips, rather than part of the specification, that's fine. |
Currently only allowed with explicit prompt ( Tracked here, I should fit it before the next release. |
|
ping @benma |
|
doing some test, fetching all those xpubs (60) could be quite long:
(test done using this tool) |
|
@pythcoiner that seems fine, waiting two minutes to recover your lost life savings :-) |
|
@Sjors I'm thinking about something: I order to make it easy to decrypt for QRCode airgaped devices, we could encrypt a dummy payload (like the hash of the "real" payload) and place it before the "real" payload, this way a coordinator could truncate after the first payload and send the "light" envelope via QR in order the signing device can try to find which key match, w/o carrying the "big" payload via QR |
|
@pythcoiner that sounds overly complicated. The air gapped device could just decrypt and re-encrypt what it wants to share. Since the data is not authenticated, there shouldn't be a downside to that. |
|
With Cove I implemented encryption for wallets and metadata, but since it needed to be re-encrypted after each user change (anytime a user changed a label or locked/unlocked a utxo) I was a bit more worried about the 96-bit nonce. My solution was to derive a new key from the main key using |
|
@praveenperera we've talked in the past about having a deterministic nonce, see here
what exactly were you worried about? in your scheme, is the salt random? |
|
Ya I didn't want to do deterministic nonce either to easy to fuck up. My worry, not sure if its warranted but, in Cove things get re-encrypted and uploaded on all user changes, to keep all backups in sync. So even tho chances of a 96-bit collision is very small, its still theoretically possible if the user does a lot of changes. Like changing label names a lot (manually or with import/export from other wallets), or with lock/unlocking UTXOs because that also changes the labels. Also this was a bigger deal in Cove because the encrypted package contains private keys and metadata (encryption key is derived from passkey using PRF), so slightly different circumstances. |
well, I’m not used to this kind of math, but I don’t think it’s meaningfully probable, if it were, ChaCha20-Poly1305 would probably have used a larger nonce size |
That's what XChaCha is but then if you use it you technically, have to fill out the paperwork for the Apple app store. Again chances are small, it made sense for cove thought I'd share my solution, I'll leave it up to people better versed in crypto stuff to decide if the tradeoffs are worth it or not. |
This is a bip for encrypted backup, an encryption scheme for bitcoin wallet related metadata.
Mailing list post: https://groups.google.com/g/bitcoindev/c/5NgJbpVDgEc