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

More epochs per connection (uint32 or uint64) #250

Closed
wants to merge 1 commit into from
Closed

More epochs per connection (uint32 or uint64) #250

wants to merge 1 commit into from

Conversation

emanjon
Copy link
Contributor

@emanjon emanjon commented Jul 14, 2021

This PR addresses #249.

Larger epoch (e.g. 32-bit) seems quite straightforward.

  • It changes the DTLSPlaintext structure which is not sent on the wire.

  • It changes the RecordNumber structure used in the ACK message. This should likely contain the full epoch

  • It changes the 64-bit "record_sequence_number". This PR suggest the 64 low order bits of the RecordNumber.
    I think it could also be set to 0x0000 || sequence_number. I don't think epoch is needed here as the keys are
    different.

Note that a 32-bit epoch still limits the number of packets in a AES-GCM connection compared to what is allowed in DTLS 1.2.
2^56.6 compared to 2^64. 2^56.5 is likely enough for all use cases but epoch could also be made even larger 2^48 or 2^64.

This PR addresses #249.

Larger epoch (e.g. 32-bit) seems quite straightforward.

- It changes the DTLSPlaintext structure which is not sent on the wire.

- It changes the RecordNumber structure used in the ACK message. This should likely contain the full epoch

- It changes the 64-bit "record_sequence_number". This PR suggest the 64 low order bits of the RecordNumber.
I think it could also be set to 0x0000 || sequence_number. I don't think epoch is needed here as the keys are
different.

Note that a 32-bit epoch still limits the number of packets in a AES-GCM connection compared to what is allowed in DTLS 1.2.
2^56.6 compared to 2^64. 2^56.5 is likely enough for all use cases but epoch could also be made even larger 2^48 or 2^64.
@@ -324,7 +324,7 @@ meaning of the fields is unchanged from previous TLS / DTLS versions.
struct {
ContentType type;
ProtocolVersion legacy_record_version;
uint16 epoch = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we actually change the DTLSPlaintext format like this (recalling that it is used for the initial ClientHello)?

I would mostly have expected to leave DTLSPlaintext unchanged since you are only going to be using it with epoch values less than four.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we actually change the DTLSPlaintext format like this (recalling that it is used for the initial ClientHello)?

@kaduk can you elaborate on the problem here? I'm not seeing the issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just keep this as uint16 and just call it epoch % 2^16?

Copy link
Contributor

Choose a reason for hiding this comment

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

That seems like the simplest solution here, yeah. We can just track epoch as a uint64 internally, but only take the LSB in the plaintext here.

Copy link
Contributor

Choose a reason for hiding this comment

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

(@ekr reminded me offline that this is a shared structure with DTLS 1.2, so not changing it is best.)

Copy link
Contributor

Choose a reason for hiding this comment

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

(@ekr reminded me offline that this is a shared structure with DTLS 1.2, so not changing it is best.)

Right, that is the problem I was thinking of.

I think epoch % 2^16 should work, especially since we don't actually use this structure with large epochs (we've moved to DTLSCiphertext by then).

@chris-wood
Copy link
Contributor

Based on IETF 112, closing in favor of #257.

@chris-wood chris-wood closed this Nov 9, 2021
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.

4 participants