1674995 - Find a way to enforce that all composed email have encryption enabled initially, and require an opt out (don't rely on mail.identity.default.encryptionpolicy)

Status: ASSIGNED

(MailNews Core :: Security: OpenPGP, enhancement)

Description

[anonym]

Attachment: Don-t-reset-encryption-choice-when-disabling-encrypt.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0

Steps to reproduce:

I set mail.identity.default.encryptionpolicy to 2 and then added an OpenPGP key to an account.

Actual results:

The account has its encryptionpolicy set to 0.

Expected results:

The resulting account should have its encryptionpolicy set to the same as mail.identity.default.encryptionpolicy, i.e. 2 in my case.

The attached patch fixes the bug for me.

Comment 1

[anonym]

Oops, this should probably be filed under "MailNews Core" → "Security: OpenPGP", but it seems I cannot fix that now. Sorry!

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9185428 [details] [diff] [review]
Don-t-reset-encryption-choice-when-disabling-encrypt.patch

Review of attachment 9185428 [details] [diff] [review]:
-----------------------------------------------------------------

I see, looks good! r=mkmelin

Comment 3

[Kai Engert (:KaiE:)]

I would like to verify this patch prior to checkin.

Comment 4

[Kai Engert (:KaiE:)]

I'm not convinced this change is good.

When composing a new message, we'll check the account settings. If neither S/MIME nor OpenPGP is configured, we ignore the encryption policy.

With this patch, if a user creates a new account, but the user does not setup any security mechanism (does not configured S/MIME certificate, does not configure OpenPGP key), then the preferences pane will claim that encryption is required - but we will not enforce that when sending messages.

I think that's confusing.

Because setting the pref cannot be used to enforce the sending of encrypted messages, I think this isolated change is useless.

In addition, when switching from a different account to this new account, the code reads the encryptionpolicy setting. That means it will enable encryption, again, despite the account not being prepared to send encrypted. That's confusing, too.

Comment 5

[Kai Engert (:KaiE:)]

I agree that the first part of the patch is removing unnecessary duplicate code, that removal is fine.

Comment 6

[Kai Engert (:KaiE:)]

IMHO, I'd consider the pref to be part of the implementation. The code assumes this value to be the default, that matches the implementation. Unless the user configures the details of an encryption technology, setting the value to nonzero cannot work.

Comment 7

[Magnus Melin [:mkmelin]]

Obviously when irrelevant such pref values wouldn't be used. There more misuse elsewhere in the e2e prefs as well. It's separate concerns: what the value is and whether it's enabled or not - disabled vs value. Setting disabled/enabled shouldn't change the value - since it's separate things.

Comment 8

[Kai Engert (:KaiE:)]

Is there any useful scenario that this patch enables? I don't see any with this patch alone.

It seems the existing code protects the user from creating a broken configuration, and this patch suggest to avoid that protection.

Comment 9

[Magnus Melin [:mkmelin]]

Yes, per the bug description. You, or your admin, might have set the default for all profiles to be encryptionpolicy=2 (always encrypt). That would be reset to 0 (don't encrypt) without this patch, for new identities. Of course in the setup process also the key/cert need to be selected, but it's not respecting the configured default policy so that once you have the key setup, it would require encryption.

Comment 10

[anonym]

With this patch, if a user creates a new account, but the user does not setup any security mechanism (does not configured S/MIME certificate, does not configure OpenPGP key), then the preferences pane will claim that encryption is required - but we will not enforce that when sending messages.

But that setting is disabled/"greyed out", which should be interpreted as "not relevant", so it seems fine to me. If you disagree, hiding the whole "Default settings for sending messages" section when no security mechanism is configured should fix your concerns, right?

Because setting the pref cannot be used to enforce the sending of encrypted messages, I think this isolated change is useless.

What I really want with this change, which it achieves nicely, is to make encryption opt-out. Having to opt-in for security is error-prone (more below). I don't want to forbid sending plaintext email.

In addition, when switching from a different account to this new account, the code reads the encryptionpolicy setting. That means it will enable encryption, again, despite the account not being prepared to send encrypted. That's confusing, too.

As far as I understand, you just need two accounts with different encryptionpolicy to trigger this confusion, so it is orthogonal to my patch and a separate bug that already affects Thunderbird. Am I misunderstanding you?

Is there any useful scenario that this patch enables? I don't see any with this patch alone.

My scenario concerns Tails, a privacy-focused Linux distribution that ships Thunderbird, and mkmelin's explanation matches our situation exactly. Without this patch, users have to manually remember/know to change this policy after setting up OpenPGP, or remember to always opt-in to encryption for each email they send. That is a lot of room for error to send an email that was intended as encrypted in plaintext instead, which can be life-threatening for some of our users. So for them this is a serious UX regression in the default setup compared to how it worked with Enigmail.

Comment 11

[Kai Engert (:KaiE:)]

Sorry for now following up earlier, I cannot believe it's been more than 2.5 years...
(There's just too many things needing attention.)

Your explanation makes sense.

You're asking for a configuration mechanism to request:
"Enable encryption for all messages in composer by default. Never send out an unencrypted message, unless the user has explicitly demanded to disable encryption."

I think it would be good to have that as a global pref, rather than trying to use other prefs to trick thunderbird into the behavior.

In bug 1796631 we have recently added some global prefs related to email encryption.

I wonder if that should get amended.

If the recently added "automatically enable encryption when possible" is off (and maybe it should be converted into a radio button?),
we could enable two alternative choices:

• Initially enable encryption when composing an email, sending unencrypted messages requires a manual action to disable encryption
• The Settings per account and identity control whether an email will be initially set to use or not use encryption

Comment 12

[anonym]

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

Sorry for now following up earlier, I cannot believe it's been more than 2.5 years...
(There's just too many things needing attention.)

No problem, I know how it is. :)

You're asking for a configuration mechanism to request:
"Enable encryption for all messages in composer by default. Never send out an unencrypted message, unless the user has explicitly demanded to disable encryption."

Not really, we already have this with the per-identity pref mail.identity.idX.encryptionpolicy set to 2. For new accounts/identities it defaults to 0 (plaintext), and what I want is to be able to change that default to 2, which is what my patch makes possible.

I think it would be good to have that as a global pref, rather than trying to use other prefs to trick thunderbird into the behavior.

I don't get the "trick" part. There are many mail.identity.default prefs that sets defaults for some of the prefs of new accounts/identities, so it seems like a commonly used convention in Thunderbird.

My patch is still relevant and fixes an actual bug in Thunderbird: unlike other mail.identity.default prefs, the one for encryptionpolicy is overriden due to buggy code so it is ineffective. So IMHO #1674995 is still a defect and my patch should be applied to fix it. Your/my proposal below is an enhancement and should be moved to a new bug. Or do you see it differently?

If the recently added "automatically enable encryption when possible" is off

I am still on Thunderbird 102 and wasn't familiar with this new option, but I just had a look at version 115 where it is present. I also noticed that the account settings for End-to-End Encryption → Default settings for sending emails now doesn't have the Disable/Enable encryption for new messages radio buttons any more, so there seems to be no way for users to easily (i.e. through the settings GUI) change an account's encryptionpolicy pref.

This is a serious regression: users cannot enforce a safe policy so they risk sending sensitive emails in plaintext by mistake. "Opportunistic" encryption is not a safe-guard and no alternative to a strict policy, it's somewhat effective at increasing the volume of encrypted messages which is a jab against dragnet surveillance, and for individual users with no special needs for privacy it can maybe prevent some unnecessary leaks of private information. But for users that strictly need encryption (think whistleblowers, political dissidents, journalists communicating with their sources) sending plaintext by mistake can be life-threatening, so they need to be able to select a strict encryption policy.

Do you agree?

we could enable two alternative choices:

• Initially enable encryption when composing an email, sending unencrypted messages requires a manual action to disable encryption
• The Settings per account and identity control whether an email will be initially set to use or not use encryption

My interpretation (please correct me I am wrong!) is that your proposal have four situations:

• None of the above is enabled (so something like Initially disable encryption...): equivalent to all accounts' encryptionpolicy prefs being set to 0
• Automatically enable encryption when possible enabled: if we have keys for all recipients, enable encryption (I am unsure how it interacts with the account's encryptionpolicy but my hope is that it only overrides if it is 0 and does nothing if it is 2)
• Initially enable encryption when composing an email [...] enabled: equivalent to all accounts' encryptionpolicy prefs being set to 2
• The Settings per account and identity control [...] enabled: this is equivalent to the "old" behavior (e.g. what I have in Thunderbird 102), i.e. each account's encryptionpolicy pref is set according to its End-to-End Encryption → Default settings for sending emails where the Disable/Enable encryption for new messages radio buttons have been reintroduced (or equivalent). I guess these radio buttons are made insensitive ("greyed out") when this option is not enabled.

I think this is unnecessarily confusing for users, e.g. options are split between global and per-account settings, so after a user has set up encryption in an accounts End-to-End Encryption page they might now have to go to the global settings to get the configuration they want. I also think the implementation will be more complex: when mail.e2ee.auto_enable was introduced it had to be added into the logic checking for encryptionpolicy, increasing the complexity, and I am pretty sure it will be the same with this new pref, further increasing complexity.

Why do we need separate, global prefs at all? Wouldn't it be nice and simple if encryptionpolicy was the only pref that needs checking? Since you suggested supporting per-account settings (which is very nice!) I think all of the above belongs there instead, i.e. in each account's End-to-End Encryption → Default settings for sending emails section we have these radio buttons:

• Disable encryption for new messages: sets mail.identity.idX.encryptionpolicy to 0
• Automatically enable encryption when possible (default): sets mail.identity.idX.encryptionpolicy to 1, which I can see in the code used to mean "IfPossible", so someone evidently already thought about this part of my proposal. So let's deprecate the global mail.e2ee.auto_enable pref currently used for this feature.
• Enable encryption for new messages: sets mail.identity.idX.encryptionpolicy to 2

And in addition to this you apply my patch to make mail.identity.default.encryptionpolicy do what it is supposed to, and set it to 1 so new accounts defaults to Automatically enable encryption when possible just as it does in current Thunderbird. In Tails we would change this default to 2 to meet our users stricter security needs.

What do you think of this proposal?

Comment 13

[anonym]

(In reply to anonym from comment #12)

I am still on Thunderbird 102 and wasn't familiar with this new option, but I just had a look at version 115 where it is present. I also noticed that the account settings for End-to-End Encryption → Default settings for sending emails now doesn't have the Disable/Enable encryption for new messages radio buttons any more, so there seems to be no way for users to easily (i.e. through the settings GUI) change an account's encryptionpolicy pref.

This is a serious regression: users cannot enforce a safe policy so they risk sending sensitive emails in plaintext by mistake. "Opportunistic" encryption is not a safe-guard and no alternative to a strict policy, it's somewhat effective at increasing the volume of encrypted messages which is a jab against dragnet surveillance, and for individual users with no special needs for privacy it can maybe prevent some unnecessary leaks of private information. But for users that strictly need encryption (think whistleblowers, political dissidents, journalists communicating with their sources) sending plaintext by mistake can be life-threatening, so they need to be able to select a strict encryption policy.

Do you agree?

I some how got this wrong: the Disable/Enable encryption for new messages radio buttons are present in version 115.1 that I just upgraded to, so all the quoted bits above should be disregarded (I tried to edit my comment, but apparently bugzilla doesn't support that). Sorry for the confusion!

Comment 14

[Kai Engert (:KaiE:)]

Correct. If the new automatic mode is disabled in global prefs, you can still configure the default per account/identity.

Comment 15

[anonym]

(In reply to Kai Engert (:KaiE:) from comment #14)

Correct. If the new automatic mode is disabled in global prefs, you can still configure the default per account/identity.

Note that the most of my comment 12 still is relevant, so I repost the parts that still are valid:

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

You're asking for a configuration mechanism to request:
"Enable encryption for all messages in composer by default. Never send out an unencrypted message, unless the user has explicitly demanded to disable encryption."

Not really, we already have this with the per-identity pref mail.identity.idX.encryptionpolicy set to 2. For new accounts/identities it defaults to 0 (plaintext), and what I want is to be able to change that default to 2, which is what my patch makes possible.

I think it would be good to have that as a global pref, rather than trying to use other prefs to trick thunderbird into the behavior.

I don't get the "trick" part. There are many mail.identity.default prefs that sets defaults for some of the prefs of new accounts/identities, so it seems like a commonly used convention in Thunderbird.

My patch is still relevant and fixes an actual bug in Thunderbird: unlike other mail.identity.default prefs, the one for encryptionpolicy is overriden due to buggy code so it is ineffective. So IMHO #1674995 is still a defect and my patch should be applied to fix it. Your/my proposal below is an enhancement and should be moved to a new bug. Or do you see it differently?

If the recently added "automatically enable encryption when possible" is off
we could enable two alternative choices:

• Initially enable encryption when composing an email, sending unencrypted messages requires a manual action to disable encryption
• The Settings per account and identity control whether an email will be initially set to use or not use encryption

My interpretation (please correct me I am wrong!) is that your proposal have four situations:

• None of the above is enabled (so something like Initially disable encryption...): equivalent to all accounts' encryptionpolicy prefs being set to 0
• Automatically enable encryption when possible enabled: if we have keys for all recipients, enable encryption (I am unsure how it interacts with the account's encryptionpolicy but my hope is that it only overrides if it is 0 and does nothing if it is 2)
• Initially enable encryption when composing an email [...] enabled: equivalent to all accounts' encryptionpolicy prefs being set to 2
• The Settings per account and identity control [...] enabled: this is equivalent to the "old" behavior (e.g. what I have in Thunderbird 102), i.e. each account's encryptionpolicy pref is set according to its End-to-End Encryption → Default settings for sending emails where the Disable/Enable encryption for new messages radio buttons have been reintroduced (or equivalent). I guess these radio buttons are made insensitive ("greyed out") when this option is not enabled.

I think this is unnecessarily confusing for users, e.g. options are split between global and per-account settings, so after a user has set up encryption in an accounts End-to-End Encryption page they might now have to go to the global settings to get the configuration they want. I also think the implementation will be more complex: when mail.e2ee.auto_enable was introduced it had to be added into the logic checking for encryptionpolicy, increasing the complexity, and I am pretty sure it will be the same with this new pref, further increasing complexity.

Why do we need separate, global prefs at all? Wouldn't it be nice and simple if encryptionpolicy was the only pref that needs checking? Since you suggested supporting per-account settings (which is very nice!) I think all of the above belongs there instead, i.e. in each account's End-to-End Encryption → Default settings for sending emails section we have these radio buttons:

• Disable encryption for new messages: sets mail.identity.idX.encryptionpolicy to 0
• Automatically enable encryption when possible (default): sets mail.identity.idX.encryptionpolicy to 1, which I can see in the code used to mean "IfPossible", so someone evidently already thought about this part of my proposal. So let's deprecate the global mail.e2ee.auto_enable pref currently used for this feature.
• Enable encryption for new messages: sets mail.identity.idX.encryptionpolicy to 2

And in addition to this you apply my patch to make mail.identity.default.encryptionpolicy do what it is supposed to, and set it to 1 so new accounts defaults to Automatically enable encryption when possible just as it does in current Thunderbird. In Tails we would change this default to 2 to meet our users stricter security needs.

What do you think of this proposal?