Closed Bug 1720601 Opened 3 years ago Closed 2 years ago

Only use TLS session tickets once

Categories

(Core :: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox107 --- fixed

People

(Reporter: mt, Assigned: kershaw)

References

Details

(Whiteboard: [necko-triaged])

Attachments

(1 file)

TLS session tickets are currently reused. As tickets include information that is sent without protection, the value of the ticket can be used to link connections for the same user by passive network observers. This is a privacy problem we can avoid.

This might reduce the amount we are able to use resumption. We might be able to manage that by implementing TLS ticket requests, but that will require NSS work, as well as some decisions about how many tickets we request. Ticket requests is not widely implemented right now, so it might not be something we can rely on.

We should discuss whether we are willing to reduce resumption rates to get the small privacy gain without ticket requests. I would say that the advantages are worth looking into, particularly since some servers will try to reject tickets if they are reused.

[1] https://searchfox.org/mozilla-central/rev/42ae3bea104c37a9986c6f18d17bd9ddb387129c/netwerk/base/SSLTokensCache.cpp#270

See Also: → 1720602
Severity: -- → S3
Priority: -- → P2
Whiteboard: [necko-triaged]
Whiteboard: [necko-triaged] → [necko-triaged][necko-priority-queue]
Assignee: nobody → kershaw

Good to see movement on this Kershaw. This is a real privacy advantage.

It might regress performance though. I wonder if it would be worth increasing our ticket quota before we ship this change, so that we don't run out so easily. Two tickets might be enough. RFC 9149 is the spec. Dennis might be able to give us a sense of how hard this is to implement and how it might fit into schedules (my sense is that it is pretty trivial, but I have no insight into what other work is ongoing).

You can also probably implement this on your own using the extension API, but I'd rather see it done in NSS proper.

Flags: needinfo?(djackson)

Implementing RFC 9149 is straightforward, but I'm not sure how much impact it will have at present (limited server-side support) so I don't think there's any need to block on it. Increasing our ticket quota seems fairly critical though.

Flags: needinfo?(djackson)

I'll let Kershaw weigh in on the ticket count thing. I'd like to make sure that we don't drop the ball on this one.

Kershaw, how feasible would it be to increase the number of tickets we retain? I'm thinking that we probably want configuration for that. If we do that, then implementing RFC 9149 makes a lot of sense.

Note that I understand Google sends two tickets on new connections so that we should have at least some servers keeping us topped up, even if we don't do RFC 9149.

Flags: needinfo?(kershaw)

(In reply to Martin Thomson [:mt:] from comment #4)

I'll let Kershaw weigh in on the ticket count thing. I'd like to make sure that we don't drop the ball on this one.

Kershaw, how feasible would it be to increase the number of tickets we retain? I'm thinking that we probably want configuration for that. If we do that, then implementing RFC 9149 makes a lot of sense.

Sorry for the late reply.
I think it shouldn't be too difficult to support this. I'll try to create a patch soon.

Flags: needinfo?(kershaw)
  1. Allow to store more than one token per key.
  2. Allow to use the token only once. The token will be removed after reading it.
  3. Add a gtest.
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/97eccf466bf3
Allow token cache to store more than one token per key, r=necko-reviewers,dragana

I am investigating the test failure.

Flags: needinfo?(kershaw)
Whiteboard: [necko-triaged][necko-priority-queue] → [necko-triaged]

Well, I have no idea why my patch caused this test failure.
The test fails at this line, which implies that the storage usage is changed. However, the patch in this bug should only affect whether TLS session resumption is performed or not.
Another interesting thing is that this test failure only happens when running with other tests together. If only run this test locally, this test is passed.
Here is the steps to reproduce this locally:

  1. Apply this patch.
  2. Run this command: ./mach mochitest dom/cache/test/mochitest/

Hi Jan,
Do you probably have an idea why the storage usage can be affected by TLS session resumption?
Thanks.

Flags: needinfo?(jvarga)

I'd like to land my code behind a pref and fix the test failure later, since I believe my patch should have nothing to do with the test failure.

Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7dfc84c3338c
Allow token cache to store more than one token per key, r=necko-reviewers,dragana
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch

(In reply to Kershaw Chang [:kershaw] from comment #10)

Well, I have no idea why my patch caused this test failure.
The test fails at this line, which implies that the storage usage is changed. However, the patch in this bug should only affect whether TLS session resumption is performed or not.
Another interesting thing is that this test failure only happens when running with other tests together. If only run this test locally, this test is passed.
Here is the steps to reproduce this locally:

  1. Apply this patch.
  2. Run this command: ./mach mochitest dom/cache/test/mochitest/

Hi Jan,
Do you probably have an idea why the storage usage can be affected by TLS session resumption?
Thanks.

This test failure seems to be fixed.

Flags: needinfo?(jvarga)
Blocks: 1800751
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: