Update bip-0038.mediawiki #29

Fix some erroneous statements in the description of the math used for encryption/decryption of EC-Multiplied keys/addresses

See also PR #27.

The UTF-8 stuff needs some fixes too. @MidnightLightning would you be willing to add those fixes into your patch set?

In particular, it should be explicitly specified that the password is converted to Unicode Normalization Form C before being encoded in UTF-8. See http://www.unicode.org/faq/normalization.html and http://tools.ietf.org/html/rfc3987. And the set of test vectors should include:

1. A sequence normalized with NFC.
2. A character normally encoded using UTF-16 surrogates, to demonstrate that the surrogates should be replaced by the proper code point (especially relevant for javascript implementations). Ie, the encoding should be UTF-8, not CESU-8.
3. Inclusion of a password with an embedded null, to demonstrate that the null is encoded as a zero byte, not Modified UTF-8.

An example of such an input passphrase is: "\u03D2\u0301\u0000\U10400\U1F4A9" which should get encoded as '\xcf\x93\x00\xf0\x90\x90\x80\xf0\x9f\x92\xa9' (13 bytes) before being further processed.

EDIT: breaking down the parts of the suggested passphrase:

1. U+03D2 U+0301 which should get normalized to U+03D3 under NFC (http://www.unicode.org/faq/normalization.html#6)
2. U+0000 which should get encoded as 0x00 (Modified UTF-8 would serialize this as 0xC0 0x80)
3. U+10400 which is an astral character which is represented in UTF-16 (Java/JavaScript) as U+D801 U+DC00 but which should not be encoded as six bytes (as CESU-8 would do; see https://en.wikipedia.org/wiki/CESU-8#Examples)
4. U+1F4A9 which repeats the "astral characters" test as http://mathiasbynens.be/notes/javascript-unicode#poo-test suggests

Sanity check:

$ python
>>> import unicodedata
>>> unicodedata.normalize('NFC', u'\u03D2\u0301\u0000\U00010400\U0001F4A9').encode('utf-8')
'\xcf\x93\x00\xf0\x90\x90\x80\xf0\x9f\x92\xa9'

Sure, I can add that in. Can you generate the full test vector for that input passphrase, and I'll do the same to make sure they agree?

I don't actually have a full bip38 implementation; I just have https://github.com/cscott/bip38-cracker that I've been hacking on (for a reddit contest). So it's actually a little easier for me to verify a test vector than to generate one. So if you generate and post a test vector I can plug it in quick and check it out.

Otherwise it will take me a little longer, as I'll have to write (or carefully audit) a bit of code to do the generation. But I'll can do it if necessary.

All right, can you check me on this:

• Password of \u03D2\u0301\u0000\U00010400\U0001F4A9; GREEK UPSILON WITH HOOK, COMBINING ACUTE ACCENT, NULL, DESERET CAPITAL LETTER LONG I, PILE OF POO
• Unencrypted key to encrypt: 5Jajm8eQ22H3pGWLEVCXyvND8dQZhiQhoLJNKjYXk9roUFTMSZ4
• Public address of private key: 16ktGzmfrurhbhi6JGqsMWf7TyqK9HNAeF
• Encrypted key: 6PRW5o9FLp4gJDDVqJQKJFTpMvdsSGJxMYHtHaQBF3ooa8mwD69bapcDQn

Do you get that same Encrypted Key as I do? I'm on a Mac and having problems getting Casascius' Address Utility (which is the reference implementation cited in the BIP) to allow proper Unicode character entry on my system (running with Mono); I'll see if I can verify that on a PC if I get a chance. But for this test, I'm using a Node.js Javascript implementation, so the input password is actually using the related surrogate pairs to define the string literal being input into the function and the unorm library to perform the NFC normalization:

var pass = new Buffer(unorm.nfc("\u03D2\u0301\u0000\uD801\uDC00\uD83D\uDCA9"), 'utf8');
console.log(pass.toString('hex')); // cf9300f0909080f09f92a9

I added this test case to my own obj-c bip38 implementation and it passed: https://github.com/voisine/zincwallet/blob/a334859dbd720e1293509f30d98b69324670cdc9/ZincWalletTests/ZincWalletTests.m#L177

Also, I independently found the same typos from your commit and included fixes in pull request #47 before I saw this pr.

This needs a rebase to the current version before it can be merged. If the changes are already in the document through the other patch, we can close it.

Okay, rebased this update. There's some discrepancy between ownerentropy vs ownersalt; you thought the derivation of passfactor was incorrect, while I thought the seedb key derivation was incorrect. I'll double-check my code, and can you check yours to make sure the proper use of entropy/salt is used in that definition?

Added in the UTF normalization test case; I haven't tried it with lot/sequence numbers, nor figured out what the private key would be in hex format, so doesn't quite match the syntax of the other test cases yet; if anyone's able to provide them before I have a chance to figure them out, I'll add them in.

I changed step 2 in ec multiply decryption in order for passfactor derivation to match passfactor creation in step 4 of intermediate code generation.

the decryption key for seedb is derivedhalf2, which is generated using passpoint, addresshash, and ownerentropy, so your edit is also correct.

@voisine is this ready for merging?

ACK, this is a good edit. I'll make another PR to add mention about the need for unicode normalization of the password.