make tx keys available to the user

[moneromooo-monero]

They are also stored in the cache file, to be retrieved using
a new get_tx_key command.

[iamsmooth]

This seems problematic from a security perspective.

1. The tx private keys are being stored in the cache file, which is not encrypted.
2. The tx private keys are being sent over the wallet RPC, which currently has no security at all.
3. The tx private keys are sent over RPC and stored even if the user has no need for this feature at all.

Also (not a security issue)

1. As with the earlier payments change, the tx private keys won't be stored in the cache file unless the transactions were proceed after the code change was made. This will lead to confusion (the other one certainly has).

If I recall correctly (I could be wrong) there is no private information going over the RPC currently, so this is a significant (de?) evolution of the security model from having the private key information stay contained within the wallet (encrypted key file and process memory).

Suggestions

1. Make this functionality optional (command line option) and disabled by default
2. Remove the tx_keys from default RPCs, use new RPCs that are only enabled if the feature is enabled. Alternatively options on the requests to enable, but only allowed if the feature is enabled.
3. Encrypt the cache file (but this will not be backward compatible unless encrypted vs. unencrypted cache files are recognized)
4. Note somewhere the restriction that keys in the cache file are not populated unless the transaction was proceed with the option enabled. Maybe a rescan feature to populate it,

[iamsmooth]

Alternative to item 3 above would be a separate (encrypted) tx keys file. That would address the compatibility issue.

[luigi1111]

Numbers under "This seems problematic from a security perspective."

1. I was unaware the storage destination for this data was not encrypted. Agree Tx private keys should be encrypted. Orthogonal, but what else is stored in this file? (more directly, why isn't this file encrypted?)
2. I don't have useful input on the RPC stuff.
3. Dropping first part (see 2.), I don't necessarily agree with "even if the user has no need for this feature at all". You have no need of it unless you have need of it; i.e., a dispute arises. AFAIK, storing r is the only way a sender can prove he/she sent payment to X. X is the only other party that can prove this, and presumably they won't be forthcoming with the necessary information (their viewkey OR the key derivation for that tx).

1 under (not a security issue):
I thought of this as well. Looks like presently it would just return "No tx key found for this txid".

Based on my 3, I think the new functionality should be on by default. (thoughts?)

I do not understand "Maybe a rescan feature to populate it".

Thanks for your input! I think topic this warrants more discussion in general.

[iamsmooth]

@luigi1111

Perhaps my mistake. I thought it was possible to extract the key from a previous existing transaction but I didn't check that and it may well not be the case.

Anyway, I think @moneromooo-monero has said he plans to encrypt the file now. There is other potentially private data in there, such as the record of all your transactions. That's not as bad as key data but its still not something a privacy-oriented coin should leave unencrypted.

BTW there is even more data in the simplewallet.log file, but I assume at some point we will clean that up and only put relevant "log" type messages there not info about every single tx (unless in debug mode of course)

[iamsmooth]

To clarify, if it is indeed the case that there is no method to recover the key then we should indeed store it (encrypted).