blockchain-certificates / BlockcertsFramework-iOS

An iOS wallet for viewing, validating, and sharing certs
MIT License
20 stars 15 forks source link

iOS keychain seed fix required #33

Closed kimdhamilton closed 7 years ago

kimdhamilton commented 7 years ago

From @kimdhamilton on August 23, 2017 16:45

Problem

I noticed that the keys generated from a mnemonic in the iOS implementation differ from other Blockcerts implementations (android and python). I verified that the android and python implementations match the values from the BIP39 online calculator, which in turn matches the official BIP39 test vectors.

Example test case

In https://iancoleman.github.io/bip39/, input the following:

mnemonic: "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon art"

(no passphrase)

The first couple of BIP44 output addresses are:

1KBdbBJRVYffWHWWZ1moECfdVBSEnDpLHi
1EiJMaaahrhpbhgaNzMeUe1ZoiXdbBhWhR

But in the Blockcerts iOS testcases, we generate the following:

        XCTAssertEqual(firstAddress, "1GJetgbaGvD3ztwF5bh8AKb2RCCkizxus")
        XCTAssertEqual(secondAddress, "14YGtBBBxGdxkGN1XYM5yNPFAhy7TzGCMW")

Solution

I isolated the source of the difference to generation of derived keychain in Keychain.swift.

If we change:

        keychain = BTCKeychain(seed: mnemonic.data)
        accountKeychain = keychain.derivedKeychain(withPath: "m/44'/0'/0'")

To:

        keychain = mnemonic.keychain
        accountKeychain = keychain.derivedKeychain(withPath: "m/44'/0'/0'/0")

Then the expected results are generated.

The problem is that mnemonic.data is not actually a seed; it looks like just an encoding of the word indices concatenated with the password if provided. This doesn't have the proper entropy to be used as a keychain seed.

Problem

I am not yet sure the best way to make this change without existing wallets losing ownership of their already-issued certificates. I am thinking through this

Copied from original issue: blockchain-certificates/wallet-iOS#32

kimdhamilton commented 7 years ago

Fixed keychain initialization issue in e6462ac81b767f079e848943191ab3917f380db2

Created https://github.com/blockchain-certificates/wallet-iOS/issues/33 to track backcompat fix, which will be needed when features are added to the wallet enabling strong ownership