1536104 - OTR: Allow the user to disable logging of encrypted conversations.

Status: RESOLVED FIXED

(Chat Core :: Security: OTR, enhancement)

Description

[Kai Engert [:KaiE:]]

By default, Thunderbird records chat conversations on disk, and they can be reviewed at a later time using the "previous conversation" view.

We need to decide, if OTR messages shall be recorded on disk, too, or if they should be excluded. Potentially, it might be useful to offer a preference to switch the recording on and off. If we do, we must decide what the default should be.

OTR means "off-the-record", so it might be reasonable to exclude OTR messages from being recorded by default.

Comment 1

[Kai Engert [:KaiE:]]

See also: https://github.com/arlolra/ctypes-otr/issues/49

Comment 2

[Magnus Melin [:mkmelin]]

For the large masses, having the logs is probably quite ok. If we want to use otr by default when possible, not having logs could annoy a lot of users.

Maybe logs should be recorded by default, but there could be a per-conversation UI element (button?) to easily remove that conversation when it's over? Also maybe a hidden pref to always remove logs for conversations when they end. This would cover an activist panic-button scenario.

Comment 3

[Kai Engert [:KaiE:]]

I'm OK with "log by default", given that we target a large set of users, and given that we don't require OTR by default.

I agree we should have a pref for disabling logging of encrypted OTR conversations.

Once you have written it to disk, a forensics analysis can probably recover it from disk. (Probably with a much higher likelyhood than based on temporary files created and things in RAM.)

If the pref is set, instead of removing logs after the conversation ended, I'd prefer not to log them at all. (The computer might crash, then you don't have a change to remove it.)

The conversation can be shown on screen during the active conversation (even if we skip logging to disk).

Comment 4

[Kai Engert [:KaiE:]]

Let's use this bug to implement the pref, and the code to support it. Changing summary.

I suggest we add another checkbox to the existing account specific OTR prefs.

Comment 5

[Kai Engert [:KaiE:]]

Attachment: 1536104-v1.patch

The difficult part is, how do we disable logging?

I'm attaching an initial patch, which seems to work, and which disables logging for active encrypted conversations. (I'll work on the pref, and UI for the pref, in the next patch iteration.)

This patch uses the least of amount of changes I could come up with.

(I had investigated other approaches, but they seemed more complicated, and required more changes. Ideally, we'd attach an attribute "disable Logging" to each received and sent OTR message. However, the message objects don't seem to survive through the entire processing. E.g., the "new-text" notification seems to be decoupled from the initial object. It would probably be necessary to pass attributes from the initial message along, while they are being processed by individual chat protocol's code, and include attributes when calling the writeMessage function. There's risk that an implementation might skip that. I think it's better if the code to disable logging doesn't require changes to the individual chat transport code.)

Comment 6

[Kai Engert [:KaiE:]]

Attachment: 1536104-v2.patch

includes pref UI

Comment 7

[Kai Engert [:KaiE:]]

Attachment: 1536104-v3.patch

lint

Comment 8

[Kai Engert [:KaiE:]]

Attachment: Bug 1536104 - OTR: Allow the user to disable logging of encrypted conversations. r=clokep

Depends on D32839

Comment 9

[Kai Engert [:KaiE:]]

Attachment: demo-nolog.patch

just a test

Comment 10

[Kai Engert [:KaiE:]]

Florian, could you please review the updated patch in phab? TIA

Comment 11

[Kai Engert [:KaiE:]]

Florian, let me reply to your comment here, because I'm not sure if you get notifications in phab.

You said about the OTRDefaults module:

This JS module seems pointless. I'm not sure if you created it this way because you have plans for something in the future, but looking at what's present in the current patch, this would be better done with default preference values.

Is it possible to use "default preference values" for "per account" preferences? If yes, do you know how I'd define that?

I added the OTRDefaults module, because I didn't know how to define default values for per-account preferences. I need to query the preference from both inside the OTR module, and also from other chat code. I'd like to avoid hardcoding the pref value in multiple places, that might be confusing. Another alternative is to embed the default values inside the OTR code, and have the other chat code import the OTR.jsm module, if you prefer that.

Comment 12

[Florian Quèze [:florian]]

(In reply to Kai Engert (:kaie:) from comment #11)

Is it possible to use "default preference values" for "per account" preferences?

These don't seem to be per-account defaults, they seem to have the same value globally.
So I'm just saying OTRDefaults.allowMsgLog() can be replaced with Services.prefs.getBoolPref("chat.otrAllowMsgLog") and the default can be stored around https://searchfox.org/comm-central/rev/cfdeb6eb89f7ed5939701dbcace185b20b96accf/mail/app/profile/all-thunderbird.js#717

Comment 13

[Kai Engert [:KaiE:]]

Florian, thanks, I changed the patch as you suggested, new patch in phab.

Comment 14

[Kai Engert [:KaiE:]]

Updated again

Comment 15

[Kai Engert [:KaiE:]]

Attachment: 1536104-final.patch

Comment 16

[Kai Engert [:KaiE:]]

Comment on attachment 9072517 [details] [diff] [review]
1536104-final.patch

Needed for OTR feature

Comment 17

[Kai Engert [:KaiE:]]

FYI, I have tested this patch on comm-beta.

I had tested earlier revisions with comm-central, but cannot test it with latest comm-central, because of bug 1559790

Comment 18

[Jorg K (CEST = GMT+2)]

Yes, you can ;-)

Comment 19

[Kai Engert [:KaiE:]]

Thanks Jörg! Works in comm-central.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/809b8d802ea7
OTR: Allow the user to disable logging of encrypted conversations. r=florian

Comment 21

[Jorg K (CEST = GMT+2)]

TB 68 beta 2:
https://hg.mozilla.org/releases/comm-beta/rev/abaa168f4d3cf5cff8065e7f25a38d5d7ef8f671