Proposal: Standardize Format of Encrypted Private Key File

[samsends]

A standardized encrypted key file format would simplify cross-service access to accounts. For instance, desktop wallets could read the same key files placed in a .stellar directory, and web-platforms could store the encrypted key file (without the password) and then decrypt client side.

I am aware that work was done to support mnemonic key file storage which is awesome. A key file format that easily supports switching encryption/kdf algorithms and respective parameters is useful too. In some use cases, faster decryption might be desirable and as compute power increases we may need to increase encryption strength.

I propose that we adapt Ethereum's https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition to meet the needs of Stellar. @bartekn mentioned in stellar/stellar-protocol#48 that he votes to use a similar definition to https://github.com/ethereum/wiki/wiki/Web3-Secret-Storage-Definition for encrypting private keys.

Suggested changes: Removal of the UUID field in the EncryptedKeyJSON (perhaps replace with hint?) and changing CipherParams to be a map[string]interface{} (more extensible). Original definitions here: https://github.com/ethereum/go-ethereum/blob/master/accounts/keystore/key.go#L81

// EncryptedKeyJSON ...
type EncryptedKeyJSON struct {
    Crypto  CryptoJSON `json:"crypto"`
    Account string     `json:"account"`
    Version int        `json:"version"`
}

// CryptoJSON ...
type CryptoJSON struct {
    Cipher       string                 `json:"cipher"`
    CipherText   string                 `json:"ciphertext"`
    CipherParams map[string]interface{} `json:"cipherparams"`
    KDF          string                 `json:"kdf"`
    KDFParams    map[string]interface{} `json:"kdfparams"`
    MAC          string                 `json:"mac"`
}

Test-containing repo: https://github.com/tensortask/stellar-keyfile Generated key file:

{
  "crypto": {
    "cipher": "aes-128-ctr",
    "ciphertext": "62e3c8c15245be6a7323302965caf19b8bd18aa22a2cd5ec7d2c5508d39e4ad06fee910ffc112a04ba3c09547bcee1525857ea085cef3120",
    "cipherparams": {
      "iv": "e8d9a67b7007c8bc198525644cbb4b88"
    },
    "kdf": "scrypt",
    "kdfparams": {
      "keyLen": 32,
      "n": 262144,
      "p": 1,
      "r": 8,
      "salt": "a07860088e95b225d1b67a79ed9462246675922576702ea7718bf13cba1216e3"
    },
    "mac": "353f8ed42ae06884bee688f9e5a0afad668b8155939c296c7a8adcfe8d965cff"
  },
  "account": "GBGRTRIJ6M6XXTPROIPA5I4SFTKXWCOE3B67B2IZCG3S7TYKGKJIGKO6",
  "version": 1
}

[samsends]

func TestEncryptKey(t *testing.T) {
    seed := "SBVUFF3GCYNJE2OBGFACJEPKLNSOYJFE622EDNCQFS2PDGTZE3QZSGAR"
    password := "testPassword"
    // lol, do not use these values.

    encKey, err := skf.EncryptKey(seed, password)
    if err != nil {
        t.Error(err)
    }
    fn := fmt.Sprintf("%s.json", encKey.Account)
    enc.Save(fn)

    priv, err := skf.Load(fn, password)
    if err != nil {
        t.Error(err)
    }
    if priv != seed {
        t.Errorf("Failure to derive original private key! got: %s, want: %s.", priv, seed)
    }
}

results in:

GBGRTRIJ6M6XXTPROIPA5I4SFTKXWCOE3B67B2IZCG3S7TYKGKJIGKO6.json

{
  "crypto": {
    "cipher": "aes-128-ctr",
    "ciphertext": "75b857954f8c24b64cce7c24c072673d1ac65a90c8e6a0dd340b23d9f710a4fef182772124d73523816648fa35eb3f917d3a439c919721d4",
    "cipherparams": {
      "iv": "3503456829bd06bb04f1e4f9c3df9182"
    },
    "kdf": "scrypt",
    "kdfparams": {
      "keyLen": 32,
      "n": 262144,
      "p": 1,
      "r": 8,
      "salt": "9674c2497f3011accf1504b81044b282f51fdaee5b9d7614e9f36fd1c5f14850"
    },
    "mac": "82436ec4b605e59a654c051b284c3fd692ee14009c585f0d1417c9b44216fbcd"
  },
  "account": "GBGRTRIJ6M6XXTPROIPA5I4SFTKXWCOE3B67B2IZCG3S7TYKGKJIGKO6",
  "version": 1
}

Test passes, original private key is derivable.

[samsends]

@bartekn, should I write a draft/PR?

[MisterTicot]

I think @istrau2 implementation is better, because uses the same cryptographic protocol than Stellar:

• https://github.com/stellarport/stellar-keystore

Introducing new dependencies opens more attack surface. So, if we really have to make a standard, I would recommend using it...

However...

This is basically pushing people to share around their private keys, which's is simply a wrong design. A basic concept of asymmetric cryptography is that the private key must not be shared whatever the way you ship it, hence the name.

I wrote properly secured solution for cross-wallet applications: CosmicLink. You can check the validity of the original idea.

[istrau2]

Yeah, if we are to standardize, let's please use the one that we have at stellarport. For a number of reasons:

1) As mentioned above, we use the same dependencies as the stellar-sdk does 2) It is already used by tens of thousands of people. 3) It is third party audited for security: https://github.com/stellarport/stellar-keystore/blob/master/audit.pdf by https://github.com/dchest author of https://github.com/dchest/tweetnacl-js

[samsends]

Hey @MisterTicot, thanks for the feedback. You bring up good points. I 100% agree that @istrau2's usage of NaCl (salsa20) as a default encryption algorithm is superior.

What is being proposed is the format of the encryption key, not the algorithms/implementation. Supporting multiple encryption/kdf parameters increases extensibility and allows existing keyfile formats (including stellarport's) to work with the standard.

Secondly, this is absolutely not "pushing people to share their private keys". The goal of this proposal is to ultimately reduce the friction of going from generated keypairs to encrypted ones.

The author of ED25519 writes, "The best attacks known actually cost more than 2^140 bit operations on average" (Bernstein et al, pg. 2). Which is comparable to the security analysis on Salsa20 conducted by the EU. Additionally this format authenticates with HMAC (MAC part of the struct) to protect against adaptive chosen cipher-text attacks... In short, if the user picks a good and unique password, the encryption security should be comparable with stellar private keys.

CosmicLink is undeniably VERY cool and I think it is extremely useful when used with a metamask-like plugin. But, IMHO, there is also a use-case for easily importing/exporting private keys in a secure manner.

Just FYI, StellarX seems to be storing encrypted keys server-side... it would be great if someone from their team could weigh in on this decision.

[MisterTicot]

@sbsends Your first sentence in this thread is:

A standardized encrypted key file format would simplify cross-service access to accounts.

This means that your initial motivation for implementing this standard is to make it easier for users to log-in various service with the same secret key.

My point is that this goes against cryptography good practices so I advice against it: Secret keys must not be shared then standards must not provide standard ways to do so.

I then explained that this use case (cross-service access to accounts) is actually well handled by CosmicLinks.

I hope this clarify my point.

[samsends]

Thank's for the clarification!

[samsends]

I think I see the breakdown. We both have different use cases in mind. I'm working on a CLI where it would be very awkward to redirect to a wallet between commands in the terminal. The flipside is that the user would have some trust in the application if they installed it. I understand the benefit of CosmicLink on the web. Thanks for weighing in @MisterTicot. Good points.

[MisterTicot]

I'm not trying to throw the whole thing away: for example it is a good practice that services offers standard backups.

It is legit that users want to keep control over their secret keys rather than totally delegate their storing to a service.

In case of shutdown, one could load its backup into another service - but the safe way to migrate is to transfer funds to a new account. Ledger teach it right: in case of emergency you can reload your seed from paper backup but then you must move all your funds to a new address.

So: my point is not to dismiss the proposal altogether, but to insist that its scope (use cases) must be well defined. IMHO a standard must comes in support to good practices, not against them.

We have to be clear on that, because actually a lot of dev seems to think that asking peoples secrets keys is the industry standard and this is educating peoples the wrong way.

Even if this standard state 3 times, and in bold, that keystore should be used only with one service except in case of emergency - and that services must make it clear -, I can easily imagine that some devs will happily invite their users to share them around for user-friendliness purpose.

But what kind of friendliness is that, given that this kind of misuses invariably ends up with big losses & drama ?

This is our responsibility to teach the basic rules that makes our funds safe in this space, and I think a keystore standard - if it has to exist - must be particularly strong on that point.

[samsends]

So: my point is not to dismiss the proposal altogether, but to insist that its scope (use cases) must be well defined. IMHO a standard must comes in support to good practices, not against them.

You are 100% right that it is better to create a separate account per service. Sorry for the misunderstanding at first.

To recap: standards > no-standards encryption > no-encryption single account per service > sharing private keys Tx signers are useful for cross-application functionality

Re tx-signers: Do you think it makes sense to adapt Metamask to XLM rather than starting from scratch? The Stellar project seems to be dead.

Re encrypted key storage: We could add a service/scope field to the keyfile OR actually use the service/scope in the KDF -- meaning the end user would have to supply the correct service/scope to unlock the key (forcing good habits). Let me know your thoughts on this.

[theaeolianmachine]

@bartekn are you fundamentally opposed to a draft being written up for this? I honestly think that there will be debate on the details, but overall the standardization of this seems like a good thing.