This adds an automatic KeyUpdate to TLS 1.3, so that we don't ever
have to worry about running cipher suites out. This fails if the number of
updates gets too high, but that's an implausibly large number of records.
Details
Details
Diff Detail
Diff Detail
- Repository
- rNSS nss
- Branch
- NSS_TLS13_DRAFT19_BRANCH
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 373 Build 441: arc lint + arc unit
Event Timeline
Comment Actions
Why are you leaving any margin at all on write? Is that to avoid the other side asking for update too early? But if so, why is that really important except for testing?
| gtests/ssl_gtest/ssl_ciphersuite_unittest.cc | ||
|---|---|---|
| 259 | This comment is pretty unclear.... | |
| 275 | Why is this needed? | |
| lib/ssl/tls13con.c | ||
| 723 | Hmm.... This was in the other patch. | |
Comment Actions
In preparation for DTLS. If we trigger an update, it will take a while to be acknowledged. The margin is arbitrary though. We only need a few RTTs worth in practice, but I don't know what that number would be, so 1/8 is what I ended up with. I suppose 1/1024 would work just as well.
| gtests/ssl_gtest/ssl_ciphersuite_unittest.cc | ||
|---|---|---|
| 275 | It's the same code. I was going to change this, but ended up changing it back (the two lines here are equivalent). | |
| lib/ssl/sslsecur.c | ||
| 781 | Bug 1413368 | |
| lib/ssl/tls13con.c | ||
| 723 | Odd. It doesn't show there. | |
Comment Actions
I'm not sure I follow your argument about margin. You start with the new key immediately. is your point you might need to retransmit the KeyUpdate and you need some headroom for that? You could just retransmit the packet, though.
| lib/ssl/tls13con.c | ||
|---|---|---|
| 723 | Maybe I missed it. | |
Comment Actions
In DTLS I think that we need to wait for an ACK before we apply the update (see the comment on D24 for an explanation there).
The idea that I have here is that ideally the writing peer sends a KeyUpdate for its changes. Requesting an update from the reading peer causes two updates to happen, and it would be ideal to avoid that. For that to work, the writing peer needs to have a trigger that is earlier than the reading peer.
The reading peer needs to request an update, which I will assume will take 1.5 round trips at best, but if we allow for loss that blows out to 2 round trips with loss recovery (3RTO is the pessimistic value we use in QUIC). That means (6RTO*packet_send_rate) packets sent before you can switch. We don't measure anything that would let us calculate that so we'd have to guess. I picked 1/8 of the total space, which is definitely too conservative, but I didn't want to get into the calculations about worst case usage, send rates and all that. I can find a smaller value if you want me to spend the time to calculate something conservative.
Giving the writing peer 1.5 times that margin (one extra round trip effectively) means that they get a decent chance to complete their update before the reader decides to request one.
| gtests/ssl_gtest/ssl_ciphersuite_unittest.cc | ||
|---|---|---|
| 259 | I hope that the new one is clearer. | |
Comment Actions
All good, just waiting on the other, which I'm tweaking now to enable support in DTLS. (I'll implement the current draft, even though I don't like it.)