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

wizard should not log secrets #8124

Closed
accumulator opened this issue Jan 4, 2023 · 2 comments
Closed

wizard should not log secrets #8124

accumulator opened this issue Jan 4, 2023 · 2 comments

Comments

@accumulator
Copy link
Member

No description provided.

@accumulator
Copy link
Member Author

fixed in 495f51e

@accumulator accumulator changed the title android qml: wizard don't log secrets wizard don't log secrets Jan 10, 2023
@SomberNight SomberNight changed the title wizard don't log secrets wizard should not log secrets Mar 16, 2023
@SomberNight
Copy link
Member

Here is a log excerpt for a 2fa wallet creation:

 47.33 | D | gui.qml.qeqr | QR requested for otpauth://totp/Electrum 2FA wallet_23?secret=&digits=6
 47.38 | D | util.profiler | QEQRImageProvider.requestImage 0.0468 sec
 47.39 | D | plugins.trustedcoin.qml.Plugin | create remote key
 47.39 | W | gui.qml.qeapp | file:///home/user/wspace/electrum/electrum/plugins/trustedcoin/qml/ShowConfirmOTP.qml:16: ReferenceError: requestNewSecret is not defined (exception occurred during delayed function evaluation)
 48.45 | D | gui.qml.qeqr | QR requested for otpauth://totp/Electrum 2FA wallet_23?secret=XBJVVKF7XVDGIU2N&digits=6
 48.50 | D | util.profiler | QEQRImageProvider.requestImage 0.0532 sec
 61.46 | I | n/network | fee_histogram [[1.0, 20540]]
 61.46 | D | gui.qml.qenetwork | fee histogram updated: [[1.0, 20540]]
 65.61 | D | plugins.trustedcoin.qml.Plugin | check OTP, shortId=df23b2b7469bee384ba8e9efb85963d9a1f5206479adfd01c36778aa9a614475, otp=234759
 66.52 | D | plugins.trustedcoin.qml.Plugin | OTP verify success
 66.52 | W | gui.qml.qeapp | OTP verify success
 67.14 | W | gui.qml.qeapp | file:///home/user/wspace/electrum/electrum/plugins/trustedcoin/qml/ShowConfirmOTP.qml:16: ReferenceError: requestNewSecret is not defined
 67.14 | D | wizard | wizard current: {'2fa_email': 'asd@asd.com', 'keystore_type': 'haveseed', 'seed': '<sensitive value removed>', 'seed_extend': False, 'seed_extra_words': '<sensitive value removed>', 'seed_type': '2fa', 'seed_variant': 'electrum', 'trustedcoin_keepordisable': 'keep', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
 67.14 | D | wizard | view=trustedcoin_show_confirm_otp
 67.14 | D | plugins.trustedcoin.qml.Plugin | OTP secret accepted, creating keystores
 67.14 | D | wizard | resolve_next view is wallet_password
 67.15 | D | wizard | wizard stack:
0: {}
1: {'wallet_name': 'wallet_23'}
2: {'seed_type': '2fa_segwit', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
3: {'seed_type': '2fa_segwit', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
4: {'keystore_type': 'haveseed', 'seed_type': '2fa_segwit', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
5: {'keystore_type': 'haveseed', 'seed': '<sensitive value removed>', 'seed_extend': False, 'seed_extra_words': '<sensitive value removed>', 'seed_type': '2fa', 'seed_variant': 'electrum', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
6: {'keystore_type': 'haveseed', 'seed': '<sensitive value removed>', 'seed_extend': False, 'seed_extra_words': '<sensitive value removed>', 'seed_type': '2fa', 'seed_variant': 'electrum', 'trustedcoin_keepordisable': 'keep', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
7: {'2fa_email': 'asd@asd.com', 'keystore_type': 'haveseed', 'seed': '<sensitive value removed>', 'seed_extend': False, 'seed_extra_words': '<sensitive value removed>', 'seed_type': '2fa', 'seed_variant': 'electrum', 'trustedcoin_keepordisable': 'keep', 'wallet_name': 'wallet_23', 'wallet_type': '2fa'}
 67.15 | W | gui.qml.qeapp | next view: wallet_password
 71.92 | W | gui.qml.qeapp | Finished new wallet wizard
 71.92 | I | wizard | Creating wallet from wizard data
 71.92 | D | wizard | {'2fa_email': 'asd@asd.com', 'encrypt': True, 'keystore_type': 'haveseed', 'password': '123456', 'seed': 'image decade quit finger alpha hurt curious minute party identify lock permit', 'seed_extend': False, 'seed_extra_words': '', 'seed_type': '2fa', 'seed_variant': 'electrum', 'trustedcoin_keepordisable': 'keep', 'wallet_name': 'wallet_23', 'wallet_type': '2fa', 'x1/': {'derivation': 'm/0h', 'pw_hash_version': 1, 'root_fingerprint': '5e04608c', 'type': 'bip32', 'xprv': 'tprv8cGAshAZKMBTN8PPVdkC4THgfDYiCEw97FoWV2Xn6UHEJ8oLt6i3uwJzgCLNSYrctjJY6VaQSsdcfofG6n1wZU3bmCKj66K3zhYDxZ2t8RB', 'xpub': 'tpubD8xD27CoTis8FbRBPHQnTrwoEF4eMa83gZQHmYa5Wk5d8d47WVXe6RvrrMZJKwaNHZNtLF7pTZkZ2t7hSodT7PPPJSWNWv8XSAzTCCkYB3T'}, 'x2/': {'derivation': 'm/1h', 'pw_hash_version': 1, 'root_fingerprint': '5e04608c', 'type': 'bip32', 'xprv': None, 'xpub': 'tpubD8xD27CoTis8JwSFZtEsTQcAnbvTcrJDwK71LYTncUwxMFUtxnmvH6BCnvu7Em4Vrd7Di1xHpP6hJmFNVxBzv6o8vH2PxDJpGMDZbJBQZyL'}, 'x3/': {'derivation': 'm', 'pw_hash_version': 1, 'root_fingerprint': 'd53a5124', 'type': 'bip32', 'xprv': None, 'xpub': 'tpubD6NzVbkrYhZ4Wr6o6NYjqFpyQFepQF8vV92z9fHH5P978Du96PoTTe9mVm46DCJB7VwbQe72wBgNYEe3jRMA9iisCeC3uiq4YmaurvHwYYq'}}
 71.92 | I | storage.WalletStorage | wallet path /home/user/.electrum/testnet/wallets/wallet_23
 71.92 | D | wizard | creating keystore from 2fa seed

At 48.45, the TOTP base secret is logged.
At 71.92, the seed, seed_extra_words, and xprvs are logged.

Logging is not enabled by default, but still, we should really try to be careful.
I personally use a debug build in production, and would like that to be ~safe... :P

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

No branches or pull requests

2 participants