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

ZCA-011 Private addresses and information of protected transactions exposed through log files #1504

Closed
coinspect opened this Issue Oct 10, 2016 · 8 comments

Comments

@coinspect

coinspect commented Oct 10, 2016

Private addresses and Information of protected transactions including plaintext of memo fields are logged to ~/.zcash/*/debug.log, a log file in persistent storage that is not necessarily encrypted.

https://github.com/zcash/zcash/blob/master/src/wallet/asyncrpcoperation_sendmany.cpp#L113-L119

https://github.com/zcash/zcash/blob/master/src/wallet/asyncrpcoperation_sendmany.cpp#L758-L769

2016-10-10 16:05:38 async rpc opid-ae0e3e97-adde-438d-a06f-e09ee2183f6d finished (status=success, tx=CTransaction(hash=4a7c170761, ver=2, vin.size=0, vout.size=0, nLockTime=0)
)
2016-10-07 18:51:55 ThreadRPCServer method=z_sendmany
2016-10-07 18:51:55 opid-26d2c: found unspent note (txid=0419b1e5b1, vjoinsplit=0, ciphertext=0, amount=0.50, memo=0000000000)
2016-10-07 18:51:55 opid-26d2c: found unspent note (txid=0419b1e5b1, vjoinsplit=0, ciphertext=1, amount=0.1784, memo=f600000000)
2016-10-07 18:51:55 opid-26d2c: spending 0.3001 to send 0.30 with fee 0.0001
2016-10-07 18:51:55  -  transparent input: 0.00 (to choose from)
2016-10-07 18:51:55  -      private input: 0.6784 (to choose from)
2016-10-07 18:51:55  - transparent output: 0.00
2016-10-07 18:51:55  -     private output: 0.30
2016-10-07 18:51:55  -                fee: 0.0001
2016-10-07 18:51:55 opid-26d2c: spending note (txid=0419b1e5b1, vjoinsplit=0, ciphertext=0, amount=0.50)
2016-10-07 18:51:55 opid-26d2c: spending note (txid=0419b1e5b1, vjoinsplit=0, ciphertext=1, amount=0.1784)
2016-10-07 18:51:55 opid-26d2c: generating note for change (amount=0.3783)
2016-10-07 18:51:55 opid-26d2c: creating joinsplit at index 0 (vpub_old=0.00, vpub_new=0.0001, in[0]=0.50, in[1]=0.1784, out[0]=0.30, out[1]=0.3783)

https://github.com/zcash/zcash/blob/master/src/wallet/rpcdump.cpp#L308
https://github.com/zcash/zcash/blob/master/src/wallet/rpcdump.cpp#L312

2016-10-07 16:03:30 Skipping import of tmXUrzGp2d15quthkdoYrsxreE6vtg3GKYs (key already present)
2016-10-07 16:03:30 Skipping import of zaddr ztPqrWMZctcpDRQhFLokoa2n2mQHDFYJ5EMvMSzGJKSjbaNBP5JvKxav34XGfKy1ecrvd93dETMp1Fdih4qJ6NGqSAnHqht (key already present)
2016-10-07 16:03:30 Skipping import of zaddr ztgryfVice57QzdyLHk82wkrDQeUKzFwzKqhQZJyt4xUsXw1akYYry5WeNNAB7u8xpFMWpTmdppWsWNiBwTSfiDVXzmGLc3 (key already present)
2016-10-07 16:03:30 Rescanning last 61 blocks
@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Oct 11, 2016

Contributor

Current logging of z_sendmany is for debugging purposes.

We can add a switch (e.g. #if 0) to turn this into dead code or comment out. I don't think we should simply delete the code as it is useful / needed for ongoing development of z_sendmany.

@daira Users will want to at least know the status of operations. How much more / what type of information should we log?

Contributor

bitcartel commented Oct 11, 2016

Current logging of z_sendmany is for debugging purposes.

We can add a switch (e.g. #if 0) to turn this into dead code or comment out. I don't think we should simply delete the code as it is useful / needed for ongoing development of z_sendmany.

@daira Users will want to at least know the status of operations. How much more / what type of information should we log?

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 12, 2016

Contributor

Let's add a -debug=zrpc option which covers logging in the z_* calls.

Contributor

daira commented Oct 12, 2016

Let's add a -debug=zrpc option which covers logging in the z_* calls.

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Oct 12, 2016

Contributor

@daira PR #1523 updates z_* log calls to use 'zrpc'. Is the z payment address shown above ok? What about the memo field being logged?

Contributor

bitcartel commented Oct 12, 2016

@daira PR #1523 updates z_* log calls to use 'zrpc'. Is the z payment address shown above ok? What about the memo field being logged?

zkbot pushed a commit that referenced this issue Oct 14, 2016

zkbot
Auto merge of #1523 - bitcartel:master_1504_z_logging, r=bitcartel
Refine LogPrint debugging for z_* rpc calls

For #1504 ZCA011
@nathan-at-least

This comment has been minimized.

Show comment
Hide comment
@nathan-at-least

nathan-at-least Oct 17, 2016

Contributor

Let's add a note in security-warnings.md that debug level logging is potentially dangerous, and also logging payment z-addrs may be a security leak for some use cases.

Contributor

nathan-at-least commented Oct 17, 2016

Let's add a note in security-warnings.md that debug level logging is potentially dangerous, and also logging payment z-addrs may be a security leak for some use cases.

@str4d str4d added the documentation label Oct 17, 2016

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 17, 2016

Contributor

I'm unconvinced that memo fields should ever be logged, even behind a flag. Users might send secrets through them.

Contributor

daira commented Oct 17, 2016

I'm unconvinced that memo fields should ever be logged, even behind a flag. Users might send secrets through them.

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 17, 2016

Contributor

Also note that memo fields are a communication between two parties, and they will be logged if either party has -debug=zrpc enabled, even if the other party does not know about or has not consented to that. Although we can't prevent people from deliberately recording contents memo fields (indeed we don't want to because that would preclude some use cases), we shouldn't provide any feature that if accidentally enabled could cause another user's privacy to be compromised without their knowledge or consent.

Contributor

daira commented Oct 17, 2016

Also note that memo fields are a communication between two parties, and they will be logged if either party has -debug=zrpc enabled, even if the other party does not know about or has not consented to that. Although we can't prevent people from deliberately recording contents memo fields (indeed we don't want to because that would preclude some use cases), we shouldn't provide any feature that if accidentally enabled could cause another user's privacy to be compromised without their knowledge or consent.

@bitcartel

This comment has been minimized.

Show comment
Hide comment
@bitcartel

bitcartel Oct 20, 2016

Contributor

@daira How about -debug=zrpcunsafe or -debug=zrpcunsafememo for just logging the memo field?

Contributor

bitcartel commented Oct 20, 2016

@daira How about -debug=zrpcunsafe or -debug=zrpcunsafememo for just logging the memo field?

@daira

This comment has been minimized.

Show comment
Hide comment
@daira

daira Oct 20, 2016

Contributor

That would be fine, yes.

Contributor

daira commented Oct 20, 2016

That would be fine, yes.

@bitcartel bitcartel added the has PR label Oct 20, 2016

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1584 - bitcartel:1504_zca_011_logging, r=daira
More granular control over logging of z_* calls

Closes #1504

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1584 - bitcartel:1504_zca_011_logging, r=daira
More granular control over logging of z_* calls

Closes #1504

zkbot pushed a commit that referenced this issue Oct 21, 2016

zkbot
Auto merge of #1584 - bitcartel:1504_zca_011_logging, r=daira
More granular control over logging of z_* calls

Closes #1504

zkbot pushed a commit that referenced this issue Oct 22, 2016

zkbot
Auto merge of #1584 - bitcartel:1504_zca_011_logging, r=daira
More granular control over logging of z_* calls

Closes #1504

@zkbot zkbot closed this in #1584 Oct 22, 2016

@daira daira added the memo fields label Oct 23, 2016

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