Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extended epoch like tls. Fixed #249 #257

Merged
merged 10 commits into from Feb 25, 2022
Merged

Conversation

ekr
Copy link
Collaborator

@ekr ekr commented Nov 8, 2021

Yet another attempt to fix #249.

This builds on Chris's PR but just omits the epoch entirely from the AEAD nonce calculation. This is more consistent with TLS and doesn't require special case reasoning about why we only need to build in the bottom 16 bits. This relies entirely on the keys being different. Needs analysis.

@ekr ekr requested a review from chris-wood November 8, 2021 19:16
@kaduk
Copy link
Contributor

kaduk commented Nov 8, 2021

This seems promising.

Copy link
Contributor

@chris-wood chris-wood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks -- this LGTM. We'll need to do a brief consensus call given the type of change.

Copy link
Contributor

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many implementations of DTLS use a 16-bit integer to hold epoch. Sometimes, that leaks into places that mean that it is hard to change.

Those of us with this problem can probably pull the same trick as here and only expose the low 16 bits, but would it be possible to implement this without allowing key updates that take epoch to 2^16?

of the connection epoch, which is an 8 octet counter incremented on every
KeyUpdate. See {{seq-and-epoch}} for details. The sequence number is set to
be the low order 48 bits of the 64 bit sequence number. Plaintext records
MUST NOT be sent with sequence numbers that would exceedd 2^48-1, so the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
MUST NOT be sent with sequence numbers that would exceedd 2^48-1, so the
MUST NOT be sent with sequence numbers that would exceed 2^48-1, so the

@@ -2151,6 +2164,14 @@ Client Server
~~~
{: #dtls-key-update title="Example DTLS Key Update"}

Implementations MUST NOT send KeyUpdates that would result
in epochs of greater than or equal to 2^{48}-1. This limits
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat arbitrary. If the epoch can have 64 bit values, then why not allow for the full range of values to be used? Is there an analysis that suggests that rekeying too often leads to this problem?

Also, I would have said >2^48-1 OR >= 2^48, but not this. I can almost guarantee a test failure in some compliance test arising from that choice. The test will be pointless too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With a key space of 2^{128}, 2^{64} rekeys has a significant chance of key repetition.

The IV randomization is intended to address this, but I prefer to be cautious, especially as there is no realistic scenario in which > 24^{48} rekeys will be needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to change this to >= 2^{48}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we accept the premise that the nonce is effectively part of the key space, then AES-128 is still probably OK with that many rekeys. AES-256 and ChaCha20Poly1305 have no such problem. I'm just concerned that this is arbitrary and unexplained.

@chris-wood
Copy link
Contributor

@ekr this LGTM, but there's a conflict. Can you resolve prior to merging?

@martinthomson, are you OK with the new rationale, and text that allows it to be relaxed in the future?

Copy link
Contributor

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems OK.

draft-ietf-tls-dtls13.md Outdated Show resolved Hide resolved
Comment on lines +2170 to +2172
enforce this rule. If a sending implementation receives a KeyUpdate
with request_update set to "update_requested", it MUST NOT send
its own KeyUpdate if that would cause it to exceed these limits.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What can it do instead?

Co-authored-by: Martin Thomson <mt@lowentropy.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DTLS 1.3 limits the number of packets that can be encrypted with AES-GCM to 2^40.5
4 participants