bip138: address review

bip-0138.md

@@ -9,6 +9,7 @@
   License: BSD-2-Clause
   Discussion: https://delvingbitcoin.org/t/a-simple-backup-scheme-for-wallet-accounts/1607
               https://groups.google.com/g/bitcoindev/c/5NgJbpVDgEc/m/TtGK9sF9BgAJ
+              https://github.com/bitcoin/bips/pull/1951
 ```
 
 ## Introduction
@@ -16,7 +17,7 @@
 ### Abstract
 
 This BIP defines a compact encryption scheme for **output script descriptors** (BIP-0380),
-**wallet policies** (BIP-0388), **labels** (BIP-0329), and **wallet backup metadata** (BIP-0139).
+**wallet policies** (BIP-0388), and other wallet metadata.
 The payload must not contain any private key material.
 
 Users can store encrypted backups on untrusted media or cloud services without leaking
@@ -38,18 +39,24 @@ in all copies.
 
 Losing the **wallet descriptor** (or **wallet policy**) is almost as catastrophic as
 losing the seed itself. The seed lets you sign, but the descriptor maps you to your coins.
-For multisig or miniscript wallets, keys alone won't help: without the descriptor, you
+For multisig or miniscript wallets, keys alone are insufficient: without the descriptor, you
 can't reconstruct the script.
 
+Multisig setups can be designed to retain access to coins even if one of the
+seeds is lost, for example a 3-of-3 MuSig2 keypath with a script path that lets
+2-of-3 recover. But when one seed is lost, its public key may also be lost,
+rendering the coins inaccessible. This risk is not intuitive.
+
 Offline storage of descriptors has two practical obstacles:
 
 1. **Descriptors are hard to store offline.**  
    Descriptors can be much longer than a 12/24-word seed. Paper and steel backups
-   become impractical or error-prone.
+   become impractical or error-prone, while USB sticks and other offline digital
+   storage solutions degrade over time and are unlikely to survive flood or fire.
 
 2. **Online redundancy carries privacy risk.**  
-   USB drives, phones, and cloud storage solve the length problem but expose your
+   Phones and cloud storage solve the length problem but expose your wallet
-   wallet structure. Plaintext descriptors leak your pubkeys and script details.
+   structure. Plaintext descriptors leak your pubkeys and script details.
    Cloud storage is often unencrypted, and even cloud encryption could be compromised,
    depending on (often opaque) implementation details. Its security also reduces to
    that of the weakest device with cloud access. Each copy increases the attack surface.
@@ -57,7 +64,7 @@ Offline storage of descriptors has two practical obstacles:
 This BIP therefore proposes an **encrypted**, compact backup format that:
 
 * can be **safely stored in multiple places**, including untrusted online services,  
-* can be **decrypted only by intended holders** of specified public keys,
+* can be **decrypted only by intended holders** of specified _public_ keys,
 
 See the original [Delving post](https://delvingbitcoin.org/t/a-simple-backup-scheme-for-wallet-accounts/1607/31)
 for more background.
@@ -65,8 +72,12 @@ for more background.
 ### Expected properties
 
 * **Encrypted**: safe to store with untrusted cloud providers or backup services  
-* **Access controlled**: only designated cosigners can decrypt  
+* **Access controlled**:
-* **Easy to implement**: it should not require any sophisticated tools/libraries.  
+  * only designated cosigners can decrypt, using their xpub
+  * can't be decrypted using public keys revealed on chain
+* **Easy to implement**:
+  * it should not require any sophisticated tools/libraries
+  * it should not require access to private key material, which secure signing systems may not yield
 * **Vendor-neutral**: works with any hardware signer
 
 ### Scope
@@ -76,6 +87,8 @@ scheme also works for labels (BIP-0329) and other wallet metadata like
 wallet backup metadata (BIP-0139).
 
 Private key material MUST be removed before encrypting any payload.
+This is because the use of _public key_ material for decryption makes the
+scheme unsuitable for storing private key material.
 
 ## Specification
 
@@ -88,10 +101,13 @@ Note: in the following sections, the operator ⊕ refers to the bitwise XOR oper
   in particular it supports single-signature descriptors ($n = 1$). Each $p_i$
   is the x-only-normalized root public key of one *allowed* key expression
   (see [Descriptor key requirements](#descriptor-key-requirements)).
-- Let $s$ = sha256(sha256("BIP138_DECRYPTION_SECRET") | sha256("BIP138_DECRYPTION_SECRET") | $p_1$ | $p_2$ | ... | $p_n$)
+- Let $s$ = tagged_hash("BIP138_DECRYPTION_SECRET", $p_1$ | $p_2$ | ... | $p_n$)
-- Let $s_i$ = sha256(sha256("BIP138_INDIVIDUAL_SECRET") | sha256("BIP138_INDIVIDUAL_SECRET") | $p_i$)
+- Let $s_i$ = tagged_hash("BIP138_INDIVIDUAL_SECRET", $p_i$)
 - Let $c_i$ = $s$ ⊕ $s_i$
 
+where tagged_hash(tag, m) = sha256(sha256(tag) | sha256(tag) | m) is the tagged
+hash construction from [BIP340](bip-0340.mediawiki#design).
+
 Because $s$ and $s_i$ use distinct domain-separation tags, $s \neq s_i$ and
 therefore $c_i$ is never the all-zero string.
 
@@ -175,7 +191,7 @@ the encoder MUST generate a new one.
 In order to decrypt the payload of a backup, the owner of a certain public key p
 computes:
 
-* let $s_i$ = sha256(sha256("BIP138_INDIVIDUAL_SECRET") ‖ sha256("BIP138_INDIVIDUAL_SECRET") ‖ $p$)
+* let $s_i$ = tagged_hash("BIP138_INDIVIDUAL_SECRET", $p$)
 * for each `individual_secret_i` generate `reconstructed_secret_i` =
 `individual_secret_i` ⊕ `si`
 * for each `reconstructed_secret_i` process $payload$ =
@@ -228,8 +244,8 @@ at least classify these:[^common-derivation-paths]
 - BitBox02: `m/44h/<coin>h/{0..9}h`, `m/87h/<coin>h/{0..9}h`
 - Ledger: `m/87h/<coin>h/{0..9}h`
 
-If fetching an xpub for one of these common paths fails or times out,
+If fetching an xpub for one of these common paths fails or times out and
-implementations SHOULD warn the user.
+decryption fails, implementations SHOULD warn the user.
 
 [^derivation-optional]: **Why are derivation paths optional?**
     When standard derivation paths are used, they are easily discoverable, making
@@ -312,15 +328,15 @@ Implementations MUST reject empty payloads.
 `CIPHERTEXT` is the encrypted data resulting from encryption of `PAYLOAD` with algorithm
 defined in `ENCRYPTION` where `PAYLOAD` is encoded following this format:
 
-`CONTENT_TYPE` `LENGTH` `PLAINTEXT` (`CONTENT_TYPE` `LENGTH` `PLAINTEXT` ...) (`PADDING`)
+`CONTENT_TYPE` `CONTENT_LENGTH` `CONTENT` (`CONTENT_TYPE` `CONTENT_LENGTH` `CONTENT` ...) (`PADDING`)
 
-`LENGTH`: variable-length integer representing the length of `PLAINTEXT` in bytes. It MUST
+`CONTENT_LENGTH`: variable-length integer representing the length of `CONTENT` in bytes.
-be present.
+It MUST be present.
 
-`PLAINTEXT`: the `LENGTH` bytes of payload data.
+`CONTENT`: the `CONTENT_LENGTH` bytes of payload data.
 
-`PADDING`: OPTIONAL bytes after the final `PLAINTEXT`, up to the end of the decrypted
+`PADDING`: OPTIONAL bytes after the final `CONTENT`, up to the end of the decrypted
-`PAYLOAD`. Parsers MUST consume exactly `LENGTH` bytes of each `PLAINTEXT`. The padding
+`PAYLOAD`. Parsers MUST consume exactly `CONTENT_LENGTH` bytes of each `CONTENT`. The padding
 bytes are reserved for size padding (see Padding) and/or vendor-specific data, the same
 way trailing bytes after `CIPHERTEXT` are reserved and ignored.
 
@@ -333,7 +349,7 @@ for mutable payloads such as BIP-329 or BIP-0139.[^padding-privacy]
 
 Padding is OPTIONAL and is an encoder concern only: decoders ignore the `PADDING` bytes and
 MUST NOT validate any bucket. Because the padding sits inside `CIPHERTEXT`, the
-`ENCRYPTED_PAYLOAD` `LENGTH` reveals only the padded size, not the real `PLAINTEXT` size.
+`ENCRYPTED_PAYLOAD` `LENGTH` reveals only the padded size, not the real `CONTENT` size.
 
 Encoders that pad SHOULD zero-fill `PAYLOAD` up to the smallest bucket `S_n` that fits it,
 with `S_0 = 10240` (10 KiB) and `S_n = floor(S_0 * (5 / 4)^n)`. Payloads of 10 KiB or less
@@ -344,7 +360,7 @@ unnecessary for static ones such as BIP-380 descriptors and BIP-388 wallet polic
 [^padding-privacy]: **Why pad inside the ciphertext?**
     Padding the encrypted payload, rather than appending bytes after the backup, hides the
     real payload size from anyone who parses the backup, not just from someone who measures
-    the file size. The size field a parser sees is the padded `LENGTH`.
+    the file size. The size field a parser sees is the padded `ENCRYPTED_PAYLOAD` `LENGTH`.
 
 #### Integer Encodings
 
@@ -353,18 +369,18 @@ All variable-length integers are encoded as
 
 #### Content Type
 
-`CONTENT_TYPE` is a variable length field defining the type of the `PLAINTEXT` that follows
+`CONTENT_TYPE` is a variable length field defining the type of the `CONTENT` that follows
 it. It follows this format:
 
-`TYPE` (`LENGTH`) `DATA`
+`TYPE` (`TYPE_LENGTH`) `TYPE_PARAMS`
 
-A `PAYLOAD` carries one or more `CONTENT_TYPE LENGTH PLAINTEXT` items, each `CONTENT_TYPE`
+A `PAYLOAD` carries one or more `CONTENT_TYPE CONTENT_LENGTH CONTENT` items, each `CONTENT_TYPE`
-describing the `PLAINTEXT` immediately following it. The sequence ends at the first
+describing the `CONTENT` immediately following it. The sequence ends at the first
 `TYPE` byte equal to `0x00`, or at the end of the `PAYLOAD`, whichever comes first; all
 remaining bytes are `PADDING`. A `PAYLOAD` MUST contain at least one item (see
 [Payload Size Limits](#payload-size-limits)).
 
-`TYPE`: 1-byte unsigned integer identifying how to interpret `DATA`.
+`TYPE`: 1-byte unsigned integer identifying how to interpret `TYPE_PARAMS`.
 
 | Value  | Definition                             |
 |:-------|:---------------------------------------|
@@ -373,36 +389,36 @@ remaining bytes are `PADDING`. A `PAYLOAD` MUST contain at least one item (see
 | 0x02   | Vendor-Specific Opaque Tag             |
 | 0x03   | String                                 |
 
-`LENGTH`: variable-length integer representing the length of `DATA` in bytes.
+`TYPE_LENGTH`: variable-length integer representing the length of `TYPE_PARAMS` in bytes.
 
-For all `TYPE` values except `0x01`, `LENGTH` MUST be present.[^length-required]
+For all `TYPE` values except `0x01`, `TYPE_LENGTH` MUST be present.[^length-required]
 
-[^length-required]: **Why is `LENGTH` mandatory?**
+[^length-required]: **Why is `TYPE_LENGTH` mandatory?**
-    `LENGTH` is what lets a parser skip a content type it does not implement and still
+    `TYPE_LENGTH` is what lets a parser skip a content type it does not implement and still
-    read the items that follow; without it, the parser cannot know where `DATA` ends.
+    read the items that follow; without it, the parser cannot know where `TYPE_PARAMS` ends.
     A string note placed before a descriptor, for example, must not stop an
     implementation that only handles descriptors from recovering that descriptor. A
-    type whose `DATA` is always empty still carries a zero `LENGTH` for this reason.
+    type whose `TYPE_PARAMS` is always empty still carries a zero `TYPE_LENGTH` for this reason.
     `0x01` is exempt because every implementation handles it, so it is never skipped
     blindly.
 
-`DATA`: variable-length field whose encoding depends on `TYPE`.
+`TYPE_PARAMS`: variable-length field whose encoding depends on `TYPE`.
 
 For `TYPE` values defined above:
 - 0x00: parsers MUST stop reading content items and treat the remaining bytes as
   padding.
-- 0x01: `LENGTH` MUST be omitted and `DATA` is a 2-byte big-endian unsigned integer
+- 0x01: `TYPE_LENGTH` MUST be omitted and `TYPE_PARAMS` is a 2-byte big-endian unsigned integer
   representing the BIP number that defines it.
-- 0x02: `DATA` MUST be `LENGTH` bytes of opaque, vendor-specific data.
+- 0x02: `TYPE_PARAMS` MUST be `TYPE_LENGTH` bytes of opaque, vendor-specific data.
-- 0x03: `DATA` MUST be empty, and the following `PLAINTEXT` is the string itself,
+- 0x03: `TYPE_PARAMS` MUST be empty, and the following `CONTENT` is the string itself,
   which MUST be valid UTF-8.
 
-For all `TYPE` values except `0x01`, parsers MUST reject `CONTENT_TYPE` if `LENGTH` exceeds
+For all `TYPE` values except `0x01`, parsers MUST reject `CONTENT_TYPE` if `TYPE_LENGTH` exceeds
 the remaining payload bytes.
 
-For an unknown `TYPE` less than `0x80`, parsers MUST consume its `LENGTH` bytes of
+For an unknown `TYPE` less than `0x80`, parsers MUST consume its `TYPE_LENGTH` bytes of
-`DATA`, treat the content type as unknown, consume the following payload `LENGTH` and
+`TYPE_PARAMS`, treat the content type as unknown, consume the following payload
-`PLAINTEXT`, and continue with the next item.
+`CONTENT_LENGTH` and `CONTENT`, and continue with the next item.
 
 For an unknown `TYPE` greater than or equal to `0x80`, parsers MUST reject the
 payload.[^type-upgrade]
@@ -414,7 +430,7 @@ payload.[^type-upgrade]
 
 #### BIP380 Descriptor Backup Content
 
-When `CONTENT_TYPE` is `TYPE = 0x01` with `DATA = 0x017c` (BIP380), `PLAINTEXT` is a
+When `CONTENT_TYPE` is `TYPE = 0x01` with `TYPE_PARAMS = 0x017c` (BIP380), `CONTENT` is a
 UTF-8 BIP380 descriptor backup. It is either a single bare descriptor string, or
 a JSON descriptor backup document (for several descriptors) when the first
 character is `{`.
@@ -457,7 +473,7 @@ wallets MAY use this value as a scanning hint.
 
 #### BIP388 Wallet Policy Backup Content
 
-When `CONTENT_TYPE` is `TYPE = 0x01` with `DATA = 0x0184` (BIP388), `PLAINTEXT` is a
+When `CONTENT_TYPE` is `TYPE = 0x01` with `TYPE_PARAMS = 0x0184` (BIP388), `CONTENT` is a
 UTF-8 JSON wallet policy backup document. A BIP388 wallet policy is a descriptor
 template plus a vector of key information items, as defined in BIP388.
 

bip-0138/test_vectors/content_type.json

@@ -60,7 +60,7 @@
     "content": "0201"
   },
   {
-    "description": "Invalid proprietary (LENGTH exceeds payload)",
+    "description": "Invalid proprietary (TYPE_LENGTH exceeds payload)",
     "valid": false,
     "content": "020500"
   },