Privacy enhancement: Create option for local time disclosure via Message-ID header

RESOLVED FIXED in Thunderbird 45.0

Status

defect
RESOLVED FIXED
6 years ago
8 months ago

People

(Reporter: jacob, Assigned: mozbugs)

Tracking

unspecified
Thunderbird 45.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 5 obsolete attachments)

Reporter

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.110 Safari/537.36

Steps to reproduce:

Please see #776397 for details on previous work on this topic. We wanted to break the older bug into two different bugs - this should help with some confusion and to ensure that the discussion stays focused.

For this bug, we wish to add an option that allows a user to change the construction of the Message-ID header in outgoing email. This header discloses the local clock/timing information and when Thunderbird is composed with The Tor Network ala TorBirdy, we find that this is one of two major issues leaking information about a user's computer. Please see bug #902573 for more details on the related Date header information disclosure issue.

We think that it is reasonable to offer two possible states - there may be other states:

  Keep the Message-ID header as it is now (eg: leak timing information to the network)
  Randomize the time related Message-ID header construction bytes (eg: don't leak clock in every message)
  Remove the Message-ID header entirely (and let SMTP or NNTP sort it out - eg: what already happens without a valid hostname)

Keeping the Message-ID header would be the default state. It requires little discussion, we think.

Randomizing the bits in the Message-ID header that are time dependent requires some discussion. Generally, we support a design where we replace the clock with random data taken from the system entropy pool.

Removing the Message-ID entirely requires more discussion - what happens in this case?

We'll attach a patch that allows for configuration of the Message-ID header state shortly.
Reporter

Updated

6 years ago
Summary: ivacy enhancement: Create option for local time disclosure via Message-ID header → Privacy enhancement: Create option for local time disclosure via Message-ID header
Assignee: nobody → sukhbir.in
Blocks: 776397
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Comment on attachment 787176 [details] [diff] [review]
Patch for preventing local timestamp disclosure via message-ID header

Just to re-summarise my comments on the other bug for easy reference here:

Setting a preference to set a value in an outgoing message isn't acceptable. This needs to be a proper extension hook with an explicit call to get the value.

The issue is that you might get (as in the case of Lightning), emails being sent in the background or simply just being saved to folders, and you need to catch all of those. An explicit call makes that easy, listening to various notifications and hoping you've got the right ones when you set the preference doesn't.
Attachment #787176 - Flags: feedback-
Assignee

Comment 3

6 years ago
How would you like this patch to be designed? The reason I ask is so that we can work on it accordingly, based on what you think is an acceptable change to the Message-ID header. 

Just to reiterate: "Generally, we support a design where we replace the clock with random data taken from the system entropy pool." Will such a patch that changes Thunderbird's message-ID format be acceptable or should we work on a TorBirdy-specific solution instead?
I have already explained how I would like it designed in comment 2.
sukhbir, are you still interested in working on this?
Component: Untriaged → Composition
Flags: needinfo?(sukhbir.in)
Product: Thunderbird → MailNews Core
I'm dont' think comment 2 applies to this patch. It changes msg_generate_message_id which is a direct change, not listening to a notification.
Posted patch bug902580.diff (obsolete) — Splinter Review
In this new patch, when the "privacy.resistFingerprinting" pref is enabled, the two non-host parts of Message-ID are set to "high entropy" randomized hex strings. No local date-time is leaked. For example:

Message-ID: <B8AC3FC5.72ECB4AC@gmail.com>

Also, for extra credit, the MIME boundaries also get high-entropy random hex strings.
Attachment #787176 - Attachment is obsolete: true
Attachment #8669372 - Flags: review?(standard8)
Comment on attachment 8669372 [details] [diff] [review]
bug902580.diff

Sorry, I'm no longer active on the Thunderbird project. Passing to someone who is.
Attachment #8669372 - Flags: review?(standard8) → review?(Pidgeot18)
Attachment #8669372 - Flags: review?(mkmelin+mozilla)
I don't know if this actually needs to be behind a pref.

xref bug 765897.
Posted patch bug902580.diff (obsolete) — Splinter Review
Here is a new version of the patch that uses securely random hexadecimal strings by default. No pref is involved.
Attachment #8669372 - Attachment is obsolete: true
Attachment #8669372 - Flags: review?(mkmelin+mozilla)
Attachment #8669372 - Flags: review?(Pidgeot18)
Attachment #8682974 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8682974 [details] [diff] [review]
bug902580.diff

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

Some notes on style:
In C++ use code, braces go on their own line, and we use Foo *bar; instead of Foo* bar;

::: mailnews/compose/src/nsMsgCompUtils.cpp
@@ +516,5 @@
>  static void
>  GenerateGlobalRandomBytes(unsigned char *buf, int32_t len)
>  {
> +  nsCOMPtr<nsIRandomGenerator> randomGenerator(do_GetService("@mozilla.org/security/random-generator;1"));
> +  if (randomGenerator) {

Rather than making this an if statement if it exists, assert the existence of the object using MOZ_ASSERT.

@@ +518,5 @@
>  {
> +  nsCOMPtr<nsIRandomGenerator> randomGenerator(do_GetService("@mozilla.org/security/random-generator;1"));
> +  if (randomGenerator) {
> +    uint8_t* tempBuffer;
> +    randomGenerator->GenerateRandomBytes(len, (uint8_t**) &tempBuffer);

I'm nervous about this code since it turns randomness from a LCPRG seeded with a weak seed but is guaranteed to always work to something that isn't.

@@ +926,2 @@
>    return PR_smprintf("<%lX.%lX@%s>",
> +           (unsigned long) salt1, (unsigned long) salt2, host);

It's more efficient to generate a single 64-bit integer rather than two 32-bit integers.

In any case, I think the message-ID needs to be longer than just 16 bytes in the local part. UUID generation does seem to be a popular way of generating the localpart.

@@ +1423,5 @@
> +        for (int32_t i = 0; i < 8; i++) {
> +          char* hex = PR_smprintf("%02x", filePrefix[i]);
> +          filename.Append(hex);
> +          PR_smprintf_free(hex);
> +        }

So the problem here is that you're changing code that produces an alphabetical filename to one that produces a hexadecimal filename, and I don't think I want that change.
Attachment #8682974 - Flags: review?(mkmelin+mozilla) → feedback+
Posted patch bug902580.diff (obsolete) — Splinter Review
Hi Joshua -- thanks for the review. I've made revisions to the patch to try to address all of your comments. Now if the high-entropy random number generator fails, we fall back to the low-entropy PRNG.
Attachment #8682974 - Attachment is obsolete: true
Attachment #8685096 - Flags: review?(Pidgeot18)
Comment on attachment 8685096 [details] [diff] [review]
bug902580.diff

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

::: mailnews/compose/src/nsMsgCompUtils.cpp
@@ +934,5 @@
>    /* If we couldn't find a valid host name to use, we can't generate a
>       valid message ID, so bail, and let NNTP and SMTP generate them. */
>      return 0;
>  
> +  // Generate 128-bit UUID for the local part (using the high-entropy GenerateGlobalRandomBytes).

There is an nsIUUIDGenerator that you can use to generate the string. See for example <https://dxr.mozilla.org/comm-central/source/mozilla/dom/datastore/DataStoreService.cpp#1418>. (Although you can probably just say "return id.ToString()" instead of using ToProvidedString).
Attachment #8685096 - Flags: review?(Pidgeot18) → feedback+
Posted patch bug902580.diff (obsolete) — Splinter Review
(In reply to Joshua Cranmer [:jcranmer] from comment #13)
> Comment on attachment 8685096 [details] [diff] [review]
> bug902580.diff
> 
> Review of attachment 8685096 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mailnews/compose/src/nsMsgCompUtils.cpp
> @@ +934,5 @@
> >    /* If we couldn't find a valid host name to use, we can't generate a
> >       valid message ID, so bail, and let NNTP and SMTP generate them. */
> >      return 0;
> >  
> > +  // Generate 128-bit UUID for the local part (using the high-entropy GenerateGlobalRandomBytes).
> 
> There is an nsIUUIDGenerator that you can use to generate the string. See
> for example
> <https://dxr.mozilla.org/comm-central/source/mozilla/dom/datastore/
> DataStoreService.cpp#1418>. (Although you can probably just say "return
> id.ToString()" instead of using ToProvidedString).

Thanks again for the review. Here's a new version that uses the nsID to generate the UUID string. I'm still using GenerateGlobalRandomBytes because nsUUIDGenerator is not guaranteed to be high-entropy.
Attachment #8685096 - Attachment is obsolete: true
Attachment #8687635 - Flags: review?(Pidgeot18)
No longer blocks: 902573
Comment on attachment 8687635 [details] [diff] [review]
bug902580.diff

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

::: mailnews/compose/src/nsMsgCompUtils.cpp
@@ +520,5 @@
> +  // Attempt to generate bytes from system entropy-based RNG.
> +  nsCOMPtr<nsIRandomGenerator> randomGenerator(do_GetService("@mozilla.org/security/random-generator;1"));
> +  MOZ_ASSERT(randomGenerator, "nsIRandomGenerator service not retrievable");
> +  uint8_t *tempBuffer;
> +  nsresult rv = randomGenerator->GenerateRandomBytes(len, (uint8_t**) &tempBuffer);

You shouldn't need a cast here.
Attachment #8687635 - Flags: review?(Pidgeot18) → review+
(In reply to Joshua Cranmer [:jcranmer] from comment #15)
> Comment on attachment 8687635 [details] [diff] [review]
> bug902580.diff
> 
> Review of attachment 8687635 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mailnews/compose/src/nsMsgCompUtils.cpp
> @@ +520,5 @@
> > +  // Attempt to generate bytes from system entropy-based RNG.
> > +  nsCOMPtr<nsIRandomGenerator> randomGenerator(do_GetService("@mozilla.org/security/random-generator;1"));
> > +  MOZ_ASSERT(randomGenerator, "nsIRandomGenerator service not retrievable");
> > +  uint8_t *tempBuffer;
> > +  nsresult rv = randomGenerator->GenerateRandomBytes(len, (uint8_t**) &tempBuffer);
> 
> You shouldn't need a cast here.

Thank you for the review. Here is the revised patch, with the cast removed.
Attachment #8687635 - Attachment is obsolete: true

Comment 17

4 years ago
https://hg.mozilla.org/comm-central/rev/a8573d4c67292962f9dd9b8f51496e9f62bbedb7
Bug 902580 - Use securely random Message-ID to avoid fingerprinting. r=jcranmer

Updated

4 years ago
Keywords: checkin-needed
Target Milestone: --- → Thunderbird 45.0

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Blocks: 765897

Comment 18

3 years ago
Is there a way for users (like me) who do not require or want a securely random Message-ID value to continue using the previous Message-ID format based on the clock?
No. What would be the use case for something like that? 
Note also that what we were using was too short (bug 765897)

Updated

8 months ago
Flags: needinfo?(mozbugs)
You need to log in before you can comment on or make changes to this bug.