Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aezeed: add new package implementing the aezeed cipher seed scheme #773

Merged
merged 4 commits into from
Mar 2, 2018

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Feb 23, 2018

In this PR, we add a new package implementing the aezeed cipher
seed scheme (based on aez).

This is a new scheme developed that aims to overcome the
two major short comings of BIP39: a lack of a version, and a lack of a
wallet birthday. A lack a version means that wallets may not
necessarily know how to re-derive addresses during the recovery
process. A lack of a birthday means that wallets don’t know how far
back to look in the chain to ensure that they derive all the proper
user addresses. Additionally, BIP39 use a very weak KDF. We use
scrypt with modern parameters (n=32768, r=8, p=1). A set of benchmarks has
been added, on my laptop I get about 100ms per attempt):

⛰ go test -run=XXX -bench=.

goos: darwin
goarch: amd64
pkg: github.com/lightningnetwork/lnd/aezeed
BenchmarkToMnenonic-4     	      10	 102287840 ns/op
BenchmarkFromMnenonic-4   	      10	 105874973 ns/op
PASS
ok  	github.com/lightningnetwork/lnd/aezeed	3.036s

Aside from addressing the shortcomings of BIP 39 a cipher seed
can: be upgraded, and have it's password changed,

Sample seed:

ability dance scatter raw fly dentist bar nominee exhaust wine snap super cost case coconut ticket spread funny grain chimney aspect business quiz ginger

Plaintext aezeed encoding

The aezeed scheme addresses these two drawbacks and adds a number of
desirable features. First, we start with the following plaintext seed:

1 byte internal version || 2 byte timestamp || 16 bytes of entropy

The version field is for wallets to be able to know how to re-derive
the keys of the wallet.

The 2 byte timestamp is expressed in Bitcoin Days Genesis, meaning that
the number of days since the timestamp in Bitcoin’s genesis block. This
allow us to save space, and also avoid using a wasteful level of
granularity. With the currently, this can express time up until 2188.

Finally, the entropy is raw entropy that should be used to derive
wallet’s HD root.

aezeed enciphering/deciperhing

Next, we’ll take the plaintext seed described above and encipher it to
procure a final cipher text. We’ll then take this cipher text (the
CipherSeed) and encode that using a 24-word mnemonic. The enciphering
process takes a user defined passphrase. If no passphrase is provided,
then the string “aezeed” will be used.

To encipher a plaintext seed (19 bytes) to arrive at an enciphered
cipher seed (33 bytes), we apply the following operations:

  • First we take the external version an append it to our buffer. The
    external version describes how we encipher. For the first version
    (version 0), we’ll use scrypt(n=32768, r=8, p=1) and aezeed.
  • Next, we’ll use scrypt (with the version 9 params) to generate a
    strong key for encryption. We’ll generate a 32-byte key using 5 bytes
    as a salt. The usage of the salt is meant to make the creation of
    rainbow tables infeasible.
  • Next, the enciphering process. We use aez, modern AEAD with
    nonce-misuse resistance properties. The important trait we exploit is
    that it’s an arbitrary input length block cipher. Additionally, it
    has what’s essentially a configurable MAC size. In our scheme we’ll use
    a value of 8, which acts as a 64-bit checksum. We’ll encrypt with our
    generated seed, and use an AD of (version || salt).
  • Finally, we’ll encode this 33-byte cipher text using the default
    world list of BIP 39 to produce 24 english words.

Properties of the aezeed cipher seed

The aezeed cipher seed scheme has a few cool properties, notably:

  • The mnemonic itself is a cipher text, meaning leaving it in
    plaintext is advisable if the user also set a passphrase. This is in
    contrast to BIP 39 where the mnemonic alone (without a passrphase) may
    be sufficient to steal funds.
  • A cipherseed can be modified to change the passphrase. This
    means that if the users wants a stronger passphrase, they can decipher
    (with the old passphrase), then encipher (with a new passphrase).
    Compared to BIP 39, where if the users used a passphrase, since the
    mapping is one way, they can’t change the passphrase of their existing
    HD key chain.
  • A cipher seed can be upgraded. Since we have an external version,
    offline tools can be provided to decipher using the old params, and
    encipher using the new params. In the future if we change ciphers,
    change scrypt, or just the parameters of scrypt, then users can easily
    upgrade their seed with an offline tool.

// 24 mnemonic words.
EncipheredCipherSeedSize = 33

// CipherTextExpasnion is the number of bytes that will be added as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be CipherTextExpansion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@Roasbeef Roasbeef added this to the v0.4-beta milestone Feb 23, 2018
@jonathancross
Copy link
Contributor

This is fantastic! Exactly what we'd need to bring Electrum and BIP39 wallets into a common standard without compromising the benefits of each approach. Will you be submitting a BIP for this?

Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid set of changes! Excited to see this merged 💯

I really dig the simplicity of the cipher seed scheme and the properties it enables. I've left a couple comments, but they're mostly pretty minor.

// for the Bitcoin Days Genesis timestamp, and 16 bytes for entropy. It
// also governs how the cipher seed should be enciphered. In this
// version we take the deciphered seed, create a 5 byte salt, use that
// with an optional passphrase to genereata a 32-byte key (via scrypt),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: generate*

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// directly construct the HD seed. This scheme was created as the widely used
// schemes in the space lack two critical traits: a version byte, and a
// birthday timestamp. The version allows us to modify the details of the
// scheme in the future, and the birthday gives wallet a limit of how far back
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/wallet/wallets

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// scheme in the future, and the birthday gives wallet a limit of how far back
// in the chain they'll need to start scanning. We also add an external version
// to the enciphering plaintext seed. With this addition, seeds are able to be
// "upgraded" (to diff params, or entirely diff crypt), while maintain the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/maintain/maintaining

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


// To compute our "birthday", we'll first use the current time, then
// subtract that from the Bitcoin Genesis Date. We'll then convert that
// value to to days.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra "to"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

return nil
}

// decode attempts to decode an encode cipher seed instance into the target
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/encode/encoded

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -375,7 +375,7 @@ func mnenonicToCipherText(mnenonic *Mnenonic) [EncipheredCipherSeedSize]byte {
return cipherText
}

// FromMnemonic attempts to map the mnenonic to the original cipher text byte
// ToCipherSeed attempts to map the mnenonic to the original cipher text byte
// slice. Then we'll attempt to decrypt the ciphertext using aez with the
// passed passphrase, using the last 6 bytes of the ciphertext as a salt for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be 5 bytes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, we modified the params a bit recently.

// aez.
keyLen = 32

bitsPerWord = 11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment.

return nil, err
}
} else {
// Otherwise, we'll copy of the set of bytes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "of" isn't needed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// cipherText is the cached cipher text. This is the result of
// enciphering the above with aez, applying our versioning, salt, and
// associated data authentication scheme.
cipherText [EncipheredCipherSeedSize]byte
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be used other than setting it when computing the ciphertext. If it's meant to be a cache, then we'd want to check if it was set before trying to compute it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I had an initial use for it, but changed my mind towards the end. Updated to remove the field all together.

// Encipher maps the cipher seed to an aez ciphertext using an optional
// passphrase.
func (c *CipherSeed) Encipher(pass []byte) ([EncipheredCipherSeedSize]byte, error) {
return c.encipher(pass)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we cache the ciphertext here as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we'll want to check if a password was provided like in ToMnemonic. It might be better to move the check into encipher to avoid having to do it in both Encipher and ToMnemonic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@Roasbeef
Copy link
Member Author

@jonathancross once we get a bit of real world usage, we may submit a BIP. For now, it fits our use case precisely, so we'd rather include it immediately in the next release, rather than trying to make it a standard from the get go. Since we have an external version, if anything changes in the process, then we can provide an upgrade tool for anyone with an existing tool.

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super excited about this new seed format, it offers so many benefits/improvements over current BIP39, e.g. birthdays! Also cool to see some more example of the quick testing package in action :)

Biggest change is just a simple %s/mnenonic/mnemonic/g, plus some others that exist due to casing. I tried to comment on the important functions/variables/types that had the mispelling, but there be others.


// Next, we'll encode the serialized plaintext cipherseed into a buffer
// that we'll use for encryption.
seedBytes := bytes.NewBuffer(make([]byte, 0, DecipheredCipherSeedSize))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bytes.Buffer struct has a 64-byte bootstrap array. We can get rid of two allocations by just using

var seedBytes bytes.Buffer

key, nil, [][]byte{ad[:]}, CipherTextExpasnion, cipherSeed, nil,
)

// Finally, we'll pack the {version || ciphertext || salt| seed into a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment formatting needs fixing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignore this, was carried over from review i had started a few days ago

)

var (
scryptN = 32768
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add note that these constants are tied to external version 0?

InternalVersion uint8

// Birthday is the time that the seed was created. This is expressed as
// the number of dates since the timestamp in the Bitcoin genesis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dates -> days

}
} else {
// Otherwise, we'll copy the set of bytes.
copy(seed[:], (*entropy)[:])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary dereference? or is this our preferred syntax within lnd style guidelines?

aezeed/errors.go Outdated
ErrIncorrectVersion = fmt.Errorf("wrong seed version")

// ErrInvalidPass is returned if the user enters an invalid passphrase
// for a particular enciphered mnenonic.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mnenonic -> mnemonic


// Next, we'll slice off the salt from the pass cipher seed, and then
// snip off the end of the cipher seed, ignoring the version.
salt := cipherSeedBytes[len(cipherSeedBytes)-saltSize:]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use saltOffset constant described above here and next line

for _, word := range mnenonic {
// Using the reverse word map, we'll locate the index of this
// word within the word list.
index := uint64(reverseWordMap[word])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This operation assumes that every word of mnemonic is actually in the reversed word list. This seems like the logical place to verify that the provided words are valid, maybe returning ErrUnknownMnenomicWord if we don't know a particular one.


// NumMnenonicWords is the number of words that an encoded cipher seed
// will result in.
NumMnenonicWords = 24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NumMnenonicWords -> NumMnemonicWords

seed *CipherSeed
)

// BenchmarkFromMnenonic benchmarks the process of converting a cipher seed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost! BenchmarkToMnenonic -> BenchmarkToMnemonic

if err != nil {
b.Fatalf("unable to create mnenonic: %v", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add b.ReportAllocs() here as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

if err != nil {
b.Fatalf("unable to create seed: %v", err)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add b.ReportAllocs() here so that we can measure bytes/allocs per invocation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@ecdsa
Copy link

ecdsa commented Feb 28, 2018

interesting.
do you really need day granularity for the timestamp? you could use block_height//2016

@cfromknecht
Copy link
Contributor

@ecdsa tying the birthday to a particular block height would require the seed format to know about the parameters of the underlying chain. the rationale behind not doing so is that an aezeed can be used to secure keys on any/multiple chains without modification, while also being flexible enough to allow the same seed to add derivation paths for other currencies at a later time.

most importantly, using the block height in the birthday would require you to sync the [header] chain before creating the seed.

that aside, I believe your point is about potentially increasing the granularity? we definitely could increase it, though with day granularity we should be good until almost 2188. with 2-week intervals, it would approach the year 4251. note that this is only a bound on the birthday of the seed, not a bound on the seed's lifetime. IMO I think this is a happy medium in terms of granularity, has a super simple implementation, and works on any chain!

@Roasbeef
Copy link
Member Author

Roasbeef commented Mar 1, 2018

Alrighty, pushed out a fixup commit that adds the ability to detect if a word isn't in the original list, and also if the mnemonic was entered incorrectly.

@cfromknecht PTAL.

cfromknecht
cfromknecht previously approved these changes Mar 1, 2018
Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty, I really like this last set of changes. The addition of an external checksum gives us some integrity, as well as a way to distinguish invalid passwords from invalid mnemonics. LGTM conditioned on one last spelling correction! Long live aezeed ⚡️


// encipheredSeedSize is the size of the cipherseed before applying the
// external version, salt, and checksum for the final encoding.
encipheredSeedSize = DecipheredCipherSeedSize + CipherTextExpansion
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice additions!

// Before we attempt to decrypt the cipher seed, we'll mutate one of
// the word so it isn't actually in our final word list.
randIndex := rand.Int31n(int32(len(mnemonic)))
mnemonic[randIndex] = "kek"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

cipherSeed := cipherSeedBytes[1:saltOffset]
checksum := cipherSeedBytes[checkSumOffset:]

// Before we perform any crypto operations, we'll remote and verify the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove* and verify

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@dabura667
Copy link

Super cool.


// Encipher maps the cipher seed to an aez ciphertext using an optional
// passphrase.
func (c *CipherSeed) Encipher(pass []byte) ([EncipheredCipherSeedSize]byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the non-exported version of this method really needed?

// "aezeed" in place.
passphrase := pass
if len(passphrase) == 0 {
passphrase = []byte("aezeed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract the default passphrase into constant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// when enciphering the raw seed, giving us the equivalent of 40-bit MAC (as we
// check for a particular seed version). Using the external 4 byte checksum,
// we're able to ensure that the user input the correct set of words. Finally,
// the password in the scheme is optional. If not specified, an empty string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be "aezeed"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// we're able to ensure that the user input the correct set of words. Finally,
// the password in the scheme is optional. If not specified, an empty string
// will be used as the password. Otherwise, the addition of the password means
// that users can encrypt the saw "plaintext" seed under distinct passwords to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/saw/raw?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

// we're able to ensure that the user input the correct set of words. Finally,
// the password in the scheme is optional. If not specified, an empty string
// will be used as the password. Otherwise, the addition of the password means
// that users can encrypt the saw "plaintext" seed under distinct passwords to
Copy link
Contributor

@halseth halseth Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this... That the users can encrypt the plaintext unders distinct passwords, is that a desirable feature or not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this means they can replicate the same seed widely under distinct passwords. Each one when decrypted will allow then to restore their wallet.

One can even take an existing seed, and then produce a new one with a diff passphrase.

@cfromknecht
Copy link
Contributor

cfromknecht commented Mar 1, 2018

I thought about the case in which we may want to allow the user to just copy and paste the raw seed output (including the header and footer) and the need to make the seed easily parseable. I made another version that both writes and reads the seed entirely, the primary distinction is that each field is now just <num>:<word> instead of <num>. <word> which makes parsing simpler.

Some demo code in a playground is here: https://play.golang.org/p/nD4YLW_HW5o

This isn't necessary for this PR, but figured I'd make note of it here in case we want to add or come back to this later.

The new logic also is more general to allow us to try varying the number of columns. I still think 4 looks the best, but this should allow us to try out whatever combinations we want :)

Roasbeef added 4 commits March 1, 2018 17:10
In this commit, we add a new package implementing the aezeed cipher
seed scheme. This is a new scheme developed that aims to overcome the
two major short comings of BIP39: a lack of a version, and a lack of a
wallet birthday. A lack a version means that wallets may not
necessarily know *how* to re-derive addresses during the recovery
process. A lack of a birthday means that wallets don’t know how far
back to look in the chain to ensure that they derive *all* the proper
user addresses.

The aezeed scheme addresses these two drawbacks and adds a number of
desirable features. First, we start with the following plaintext seed:
{1 byte internal version || 2 byte timestamp || 16 bytes of entropy}.

The version field is for wallets to be able to know *how* to re-derive
the keys of the wallet.

The 2 byte timestamp is expressed in Bitcoin Days Genesis, meaning that
the number of days since the timestamp in Bitcoin’s genesis block. This
allow us to save space, and also avoid using a wasteful level of
granularity. With the currently, this can express time up until 2188.

Finally, the entropy is raw entropy that should be used to derive
wallet’s HD root.

Next, we’ll take the plaintext seed described above and encipher it to
procure a final cipher text. We’ll then take this cipher text (the
CipherSeed) and encode that using a 24-word mnemonic. The enciphering
process takes a user defined passphrase. If no passphrase is provided,
then the string “aezeed” will be used.

To encipher a plaintext seed (19 bytes) to arrive at an enciphered
cipher seed (33 bytes), we apply the following operations:
   * First we take the external version an append it to our buffer. The
external version describes *how* we encipher. For the first version
(version 0), we’ll use scrypt(n=32768, r=8, p=1) and aezeed.
  * Next, we’ll use scrypt (with the version 9 params) to generate a
strong key for encryption. We’ll generate a 32-byte key using 5 bytes
as a salt. The usage of the salt is meant to make the creation of
rainbow tables infeasible.
  * Next, the enciphering process. We use aezeed, modern AEAD with
nonce-misuse resistance properties. The important trait we exploit is
that it’s an *arbitrary input length block cipher*. Additionally, it
has what’s essentially a configurable MAC size. In our scheme we’ll use
a value of 4, which acts as a 32-bit checksum. We’ll encrypt with our
generated seed, and use an AD of (version || salt). We'll them compute a
checksum over all the data, using crc-32, appending the result to the
end.
  * Finally, we’ll encode this 33-byte cipher text using the default
world list of BIP 39 to produce 24 english words.

The `aezeed` cipher seed scheme has a few cool properties, notably:
   * The mnemonic itself is a cipher text, meaning leaving it in
plaintext is advisable if the user also set a passphrase. This is in
contrast to BIP 39 where the mnemonic alone (without a passphrase) may
be sufficient to steal funds.
   * A cipherseed can be modified to *change* the passphrase. This
means that if the users wants a stronger passphrase, they can decipher
(with the old passphrase), then encipher (with a new passphrase).
Compared to BIP 39, where if the users used a passphrase, since the
mapping is one way, they can’t change the passphrase of their existing
HD key chain.
  * A cipher seed can be *upgraded*. Since we have an external version,
offline tools can be provided to decipher using the old params, and
encipher using the new params. In the future if we change ciphers,
change scrypt, or just the parameters of scrypt, then users can easily
upgrade their seed with an offline tool.
  * We're able to verify that a user has input the incorrect passphrase,
and that the user has input the incorrect mnemonic independently.
In this commit we add a set of benchmarks to be able to measure the
enciphering and deciphering speed of the current scheme with the
current scrypt parameters.

On my laptop I get about 100ms per attempt:
⛰ go test -run=XXX -bench=.

goos: darwin
goarch: amd64
pkg: github.com/lightningnetwork/lnd/aezeed
BenchmarkToMnenonic-4     	      10	 102287840 ns/op
BenchmarkFromMnenonic-4   	      10	 105874973 ns/op
PASS
ok  	github.com/lightningnetwork/lnd/aezeed	3.036s
@Roasbeef Roasbeef merged commit 1ba3992 into lightningnetwork:master Mar 2, 2018
@ecdsa
Copy link

ecdsa commented Mar 2, 2018

@Roasbeef can you clarify the purpose of the version number? is it there only in order to be able to upgrade the cipher (and the key derivation remains unspecified, as in BIP43) , or will it also be used to specify the key derivation?

@Roasbeef
Copy link
Member Author

Roasbeef commented Mar 3, 2018

@ecdsa there're two versions: external and internal.

The external version governs how to decipher the cipher seed. So: key derivation parameters, checksum verification, salt. In the future if we upgrade any of these parameters, users can use an offline tool to "upgrade" their seed.

The internal version is for the wallet. It tells that wallet how to go about re-deriving all the addresses for the user. Unlike the external version (in my mental model at least), wallets don't need to agree on what this value is, or how it should be interpreted. So for example a version of 0 could mean to a wallet: "use bip44". While a version of 1 could mean: "use bip49 and bip84". If a wallet gets the deciphered cipher seed, and doesn't recognize the version, then it should reject it.

Copy link
Contributor

@jimpo jimpo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good. The design is sweet.

// With CipherSeedVersion we encipher as follows: we use
// scrypt(n=32768, r=8, p=1) to derive a 32-byte key from an optional
// user passphrase. We then encipher the plaintext seed using a value
// of tau (with aez) of 8-bytes (so essentially a 32-bit MAC). When
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tau of 4 bytes

// computing our checksum.
crcTable = crc32.MakeTable(crc32.Castagnoli)

// defaultPassphras is the default passphrase that will be used for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: defaultPassphrase

// Before we attempt to map the mnemonic back to the original
// ciphertext, we'll ensure that all the word are actually a part of
// the current default word list.
for _, word := range m {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @cfromknecht's comment to move this check into mnemonicToCipherText.

// re-enciphers the plaintext cipher seed into a brand new mnemonic. This can
// be used to allow users to re-encrypt the same seed with multiple pass
// phrases, or just change the passphrase on an existing seed.
func (m *Mnemonic) ChangePass(oldPass, newPass []byte) (Mnemonic, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to roll the salt here as well?

@JeremyRubin
Copy link

I know this is already merged, but I wanted to propose an alternative for the birthday mechanism which strengthens the 'exclusion proof' property.

If one uses a block hash instead of a clock-time, it makes it cryptographically impossible to pick a future birthdate. Picking a future time undermines the utility of the birthdate for proving that no transactions would be present before a certain time. However, even an honest seed-creator can't preclude themselves from creating a future-timed seed. This is because a big reorg could place transactions created after the creation of the seed to before the creation of the seed.

Using a blockhash instead of clock-time doesn't solve this problem, but it allows us to detect that this has occurred and then revert to a chain scan up to the most-recent-ancestor of the header hash used.

To keep this from adding overhead to the seed, what can be done is to only allow using every 144th (e.g. bitcoin-day) header hash. This can be specified using the same 2 byte index, then the client simply has to fetch the header-hash from the network and include it. One must be careful to ensure that it is indeed the same blockhash, otherwise no addresses will be found. This risk can be mitigated by not using the indexing trick above to save 30 bytes (storing the header with the seed directly) or by making use of the aez checksum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.