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

Revise mechanism for obtaining secrets from Trezor #1508

Open
andrewkozlik opened this issue Mar 2, 2021 · 6 comments
Open

Revise mechanism for obtaining secrets from Trezor #1508

andrewkozlik opened this issue Mar 2, 2021 · 6 comments
Labels
feature Product related issue visible for end user R&D Research and development team related

Comments

@andrewkozlik
Copy link
Contributor

andrewkozlik commented Mar 2, 2021

Applications that need to obtain a secret value from Trezor can currently use the CipherKeyValue message, see SLIP11. The password manager (SLIP16) uses this to decrypt the file containing the password metadata and to decrypt the individual passwords. The CipherKeyValue system uses a very generic confirmation dialog on Trezor and the displayed strings, such as "Unlock encrypted storage?" or "Unlock github.com for user Satoshi Nakamoto?", are involved in the encryption/decryption process. One problem with this is that it is not well suited for translations or, to be more precise, for switching between languages and for changing the message in the future.

image image

Another use-case we encountered recently is obtaining private keys for Decred staking, see #1249 (comment). For staking the wallet needs to be online. Since the staking private keys cannot be misused to cause coin loss, a desktop wallet could generate them independently of Trezor. However, it's useful for the keys to be recoverable from the seed, so Trezor could be used as a source of some secret, which is handed over to the desktop application (after user confirmation). The desktop application can then derive the private keys from that secret. In addition it would be useful for Trezor to conduct the same derivation process on the device to derive the corresponding public keys, so that when it's creating a staking ticket it may verify that it owns the voting rights to the ticket.

Let's gather other possible use-cases for the above feature in this issue and decide whether we want to redesign the current mechanism or introduce an additional mechanism for obtaining secret values from Trezor.

@tsusanka tsusanka added this to the 21.05 milestone Mar 15, 2021
@tsusanka
Copy link
Contributor

We also use this with labeling and I am pretty sure the headline "Encrypt value" confuses some users.

emu00000000

@prusnak
Copy link
Member

prusnak commented Mar 15, 2021

Let's discuss this together with FIDO2 extensions: hmac-secret, txAuthSimple and txAuthGeneric

@tsusanka tsusanka added P1 High feature Product related issue visible for end user labels Mar 18, 2021
@alex-jerechinsky alex-jerechinsky modified the milestones: 21.05, 21.06 May 11, 2021
@tsusanka tsusanka modified the milestones: 21.06, 21.07 May 20, 2021
@tsusanka tsusanka removed this from the 21.07 milestone Jun 23, 2021
@matejcik matejcik added the R&D Research and development team related label Oct 22, 2021
@hynek-jina hynek-jina removed the HIGH label May 6, 2022
@Hannsek Hannsek added this to the 🌍 Translations milestone Mar 28, 2023
@andrewkozlik
Copy link
Contributor Author

andrewkozlik commented Feb 19, 2024

In order to make use of translations in the dialogs and to avoid strange prompts, such as "Encrypt value, Enable labeling?", Trezor needs to understand what secret it is being asked to release. We could define a fairly generic message type with an enum to determine the requested secret. Based on the enum value and optional parameters Trezor would:

  1. determine whether a confirmation dialog needs to be displayed,
  2. optionally display a specific dialog, making use of translations, and
  3. determine how to compute the secret from the optional parameters.

Three types of secret that could be requested come to mind:

  • A 32-byte symmetric encryption key obtained from SLIP-21, e.g. to encrypt and decrypt account and transaction labels.
  • The result of encrypting or decrypting a value using a key obtained from SLIP-21, e.g. a password.
  • An extended private key (XPRV) of a particular BIP-32 subtree, e.g. a special XPRV used only for staking a particular cryptocurrency or an XPRV of a coin that is no longer supported by Trezor, so that users can access their assets using another wallet. The available BIP-32 subtrees would be strictly whitelisted.

The obvious disadvantage of this system over SLIP-11 is that in order to support a new feature the firmware would need to be updated with a new dialog and processing instructions telling it how to obtain the result.

message GetSecret {
    optional SecretType secret_type = 1;  // The type of secret that Trezor is being asked to release.
    enum SecretType {
        GetLabelingKey = 1;         // The encryption key used for account and transaction labels. Parameters: none. No user confirmation.
        GetPasswordDbKey = 2;       // The encryption key used for the password database. Parameters: none. No user confirmation.
        EncryptPassword = 3;        // Encrypt a password. Parameters: service, user name, password. No user confirmation.
        DecryptPassword = 4;        // Decrypt a password. Parameters: service, user name, password. User confirmation required.
    }
    repeated bytes parameters = 2;
}

message Secret {
    optional bytes secret = 1;  // A symmetric key, ciphertext or plaintext, depending on the requested secret type.
}

message GetPrivateKey {
    repeated uint32 address_n = 1;         // BIP-32 path to derive the key from the master node.
    optional string ecdsa_curve_name = 2;  // ECDSA curve name to use.
}

message PrivateKey {
    optional string xprv = 1;  // Extended private key.
}

The purpose of this proposal is to define an easily extendable API. At this moment we are only interested in implementing GetLabelingKey. Other types, including the GetPrivateKey message can be added later as needed.

@andrewkozlik
Copy link
Contributor Author

@matejcik what do you think about the proposal above? The main advantage over the existing CipherKeyValue scheme is in shifting the responsibility for the displayed message onto Trezor, i.e. enabling translations. In actual fact, right now we just need to implement the GetLabelingKey function, which should be silent. So the implementation effort on Trezor side is fairly minimal and Suite could start using it right away instead of doing 2 migrations:

  1. CipherKeyValue with dialog -> silent CipherKeyValue now (metadata encryption v2 (confirm-less labeling) trezor-suite#9130).
  2. Silent CipherKeyValue -> GetSecret some time in the future.

I personally don't have any preference about whether we should do it now or later.
cc @matejkriz, @tsusanka and @mroz22

@matejkriz
Copy link
Member

If we migrate "labeling" to silent CipherKeyValue, the main advantage (enabling translations) is not relevant because there will be nothing to translate.

SLIP-11 is more flexible from Suite perspective, so I believe there is zero motivation from our side for SLIP-21 for "labeling".

Only if you are sure that we will switch to GetSecret some time in the future because of other reasons, we can consider to do it now to save the second migration.

@matejcik
Copy link
Contributor

@andrewkozlik I like the array of parameters, but the enum for secret types seems too limiting. Instead we could do a "symbolic name" for the secret_type parameter, something like trezor::labeling for labeling, etc.

(Then we could grandfather in the old prompts, so that "Enable labeling?" becomes a well-defined secret type instead of a raw string. Of course that would only make sense to make it backwards-compatible with SLIP11, and it won't work for password manager)


GetPrivateKey seems like its own thing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Product related issue visible for end user R&D Research and development team related
Projects
Status: No status
Development

No branches or pull requests

9 participants