WIP: implemented encrypted backup BIP138#109
Conversation
695d392 to
90fe3c7
Compare
|
in 7552892, |
| // CPubKey is either compressed (33 bytes) or uncompressed (65 bytes) | ||
| // In both cases, bytes 1-32 are the x-coordinate | ||
| uint256 result; | ||
| if (pubkey.size() >= 33) { |
There was a problem hiding this comment.
I'm not knowledgeable enough about the codebase, but is there some case where a CPubKey.size() could be 0 or 32?
it looks to me this if is confusing, because if it can resolve to false, we return a wrong (zeroized) key.
| * Handles: | ||
| * - Extended public keys (xpubs): extracts the pubkey and returns x-coordinate | ||
| * - Compressed public keys (33 bytes): strips the prefix byte | ||
| * - X-only public keys (32 bytes): returns as-is |
There was a problem hiding this comment.
it's not what the function return, in that case it seems returning a zeroized key
| // Require "m" prefix for BIP-xxxx paths | ||
| if (path_str.empty() || path_str[0] != 'm') { | ||
| return util::Error{Untranslated("Derivation path must start with 'm'")}; | ||
| } |
| return util::Error{Untranslated("Truncated derivation path data")}; | ||
| } | ||
|
|
||
| uint8_t child_count = data[pos++]; |
There was a problem hiding this comment.
uint8_t child_count = data[pos++];
if (child_count == 0) {
return util::Error{Untranslated("Child count must be > 0")};
}
|
@pythcoiner thanks for the feedback, I plan to update this soon(tm). |
|
i'm working on a branch that is based on this, I'd like to to have it's test passing on the test vectors generated from the rust implem: pythcoiner#2 |
|
@Sjors is there a reason to implement |
90fe3c7 to
e69813c
Compare
To keep the latter from becoming even more complex. Both binaries are shipped in the Bitcoin Core distribution, so it shouldn't be a problem. The core functionality Rebased and updated test vectors, doing some additional refactoring now... |
e69813c to
d837001
Compare
|
I moved more functionality to I added more test vectors from the BIP. Some vectors still to use BIPXXX, which I changed to BIP388 pending bitcoin/bips#1951 (comment). While comparing this implementation with the BIP, my agent found a mismatch between prose and test vectors. Commit 9fa12f7 changes the test vectors, but we could also change the prose - I haven't thought about it deeply. |
8dc383f to
d1016ab
Compare
|
Got rid of 65c4a4e in favor of some trivial serialization code in We're now actually using I also cleaned up 17a25e6 to make better use of existing crypto code (8330771 + e57aae0). Misc. other cleanups in the first couple of commits. I plan to refine the commits in this PR further another time, as that should give me a better sense which parts of the BIP are potentially problematic (i.e. actually tedious to implement, not just bad slop on my end). |
d1016ab to
1e89b29
Compare
|
I changed 8330771 -> f3c5471 to instead have I added a test vector (see commit message) to clarify |
1e89b29 to
80f46c9
Compare
80f46c9 to
ad79520
Compare
|
Bunch more cleanup, e.g.:
|
ad79520 to
b354c23
Compare
|
I implemented the descriptor format that I suggested in bitcoin/bips#1951 (comment). Also added a Also implemented the TLV change suggested here: https://github.com/bitcoin/bips/pull/1951/changes#r3304541662
Also pushed more cleanup:
|
b354c23 to
915eaed
Compare
|
Repaired one of the intermediate commits that didn't pass CI. |
915eaed to
ed40eea
Compare
|
Updated to track the latest draft BIP a thttps://github.com/bitcoin/bips/pull/1951/changes/99217c680f31792e576838d6879365303e7f7cbf. I was able to drop the customization commits d5200e2, e4c7077 and 1153d52. Still missing support for multiple records (inside the plain-text is good enough). So I updated 3567cd0 -> 359b956 with a proposed change. Other than that, this branch should match the BIP (minus BIP388 support). |
ed40eea to
d866388
Compare
Add two new structs used for creating requests and getting response when importing descriptors. These structs replace the UniValue-based interface of ProcessDescriptorImport() in a subsequent commit.
Rename ProcessDescriptorImport() to ImportDescriptor() and replace its UniValue-based arguments and return value with the ImportDescriptorRequest and ImportDescriptorResult structs introduced in the previous commit. Add ProcessDescriptorsImport() to handle wallet locking and rescanning over a vector of ImportDescriptorRequest items. Add ProcessUniValueDescriptor() to translat from the UniValue arguments used by the importdescriptors RPC into ImportDescriptorRequest. This allows a next commit to extract ImportDescriptor() and ProcessDescriptorsImport() out of the RPC code so they can be used by other future interfaces. Lowers the minimum timestamps to 0 instead of 1.
….cpp Extract ImportDescriptor() and ProcessDescriptorsImport() out of backup.cpp and move it into imports.cpp so other future interfaces can use them without needing to know about RPC code. The commit can be reviewed with the --color-moved=dimmed-zebra option for an easier review.
BIP-380 specifies that descriptors can use either ' or h as the hardened indicator. ParseHDKeypath only supported the former. This prepares for using ParseHDKeypath with paths extracted from descriptors which typically use 'h' for shell-escaping convenience.
Introduces WalletDescriptorInfo struct and DescriptorInfoToUniValue() helper to avoid code duplication when serializing descriptor metadata to UniValue. Refactors listdescriptors RPC to use the new helper.
Expose descriptor public-key collection that can omit keys observable from spent outputs. This lets encrypted-backup code derive recipient keys from non-observable descriptor xpubs without adding a separate BIP138-specific descriptor API.
Extract and normalize eligible descriptor BIP32 public key expressions for BIP138 encrypted backups. Extended public keys that are not directly observable from descriptor spends are converted to x-only keys, while literal pubkeys, directly observable xpubs, and the BIP341 NUMS point are excluded. For MuSig2 descriptors, participant xpubs remain eligible because the on-chain aggregate key does not reveal them. Add a test vector for aggregate derivation from bare participant xpubs; this should be upstreamed to the BIP vectors.
Add functions to compute the decryption secret and individual secrets as specified in BIP138. The decryption secret is derived from sorted public keys, and individual secrets allow any keyholder to decrypt. Functions added: - ComputeDecryptionSecret(): Hash sorted keys to derive decryption secret - ComputeIndividualSecret(): Hash a single key to derive its individual secret - ComputeAllIndividualSecrets(): Compute XOR'd secrets for all keys Includes test vectors from the BIP specification.
Add functions for encoding and decoding BIP138 derivation paths as count-prefixed arrays of big-endian child indices. Encode paths in deterministic lexicographic order. Use the existing ParseHDKeypath helper for textual path parsing in tests. Includes test vectors from the BIP specification.
Add functions for encoding/decoding individual secrets as specified in BIP138:
- EncodeIndividualSecrets: encode sorted secrets to binary format
- DecodeIndividualSecrets: decode from binary format
Individual secrets allow any keyholder to derive the decryption secret using:
decryption_secret = ci XOR sha256("BIP_XXXX_INDIVIDUAL_SECRET" || pi)
Includes test vectors from the BIP specification.
Add functions for encoding and decoding BIP138 content type metadata. Content types define what kind of data is in the encrypted payload: - TYPE 0x01 encodes a 2-byte big-endian BIP number. - TYPE 0x02 encodes vendor-specific content with CompactSize length. Decode unknown TYPE values below 0x80 by skipping their length-prefixed data. Reject TYPE 0x00 as reserved and TYPE values 0x80 or above as unsupported upgrade-required content.
Add the BIP138 ChaCha20-Poly1305 algorithm identifier, nonce size, tag size, and test coverage for the AEAD operation used by encrypted backups. The tests exercise a random roundtrip and the BIP test vectors against the existing AEADChaCha20Poly1305 helper with an empty AAD.
Add binary and base64 encoding, backup creation, and decryption helpers for BIP138 encrypted backups. Encrypted payloads are encoded as CONTENT followed by CompactSize plaintext length and plaintext inside the ciphertext. CONTENT is a single metadata field describing the plaintext. Add full-backup test vectors and roundtrip tests covering encode, decode, descriptor-derived encryption, descriptor-derived decryption, base64 encoding, and wrong-key failure.
Adds two new commands to bitcoin-wallet tool: encryptbackup: Creates an encrypted backup of all wallet descriptors. Outputs base64-encoded backup to stdout. Requires: -wallet=<name> decryptbackup: Decrypts a backup using a provided extended public key (xpub/tpub). Reads base64 backup from stdin, outputs JSON to stdout. Requires: -pubkey=<xpub> The output is compatible with the importdescriptors RPC. The decryption only requires an xpub that was used in the original wallet. In a recovery scenario, the user derives this from their seed phrase at a known derivation path (e.g., m/84'/0'/0'). Includes functional test demonstrating the full roundtrip.
Display unencrypted metadata from a BIP-xxxx encrypted backup: - Format version - Number of recipients - Encryption algorithm - Derivation paths (if present)
To include derivation path in backup header.
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.
d866388 to
6467f0c
Compare
|
Rebased and see bitcoin/bips#1951 (comment). |
Implements bitcoin/bips#1951
DO NOT USE YET
Backups generated with this code may become inaccessible if the BIP changes!
There's no guarantee this will ever make it into Bitcoin Core, and I might not maintain this branch.
It's also entirely vibe coded. Despite much back and forth with Claude and its buddy Codex, I have not yet thoroughly reviewed every commit it wrote.
Relevant upstream PRs:
ParseHDKeypathto supporth