Skip to content

[TLS 1.3] Callback for deterministic timestamps#2975

Closed
reneme wants to merge 1 commit intodev/tls-13from
tls13/timestamp_callback
Closed

[TLS 1.3] Callback for deterministic timestamps#2975
reneme wants to merge 1 commit intodev/tls-13from
tls13/timestamp_callback

Conversation

@reneme
Copy link
Copy Markdown
Collaborator

@reneme reneme commented May 17, 2022

The new TLS::Callbacks::tls_current_timestamp() provides a tap to influence the timestamps required in the TLS stack. This is particularly useful when deterministic timestamps are required for testing purposes.

Examples are the BoGo -resumption-delay shim flag and also the 0-RTT test in RFC 8448. Both depend on this callback to hit timestamps with millisecond precision and no grace period. Relevant tests that depend on this machinery are coming with the TLS 1.3 session resumption but the test facilities in the BoGo shim and RFC 8448 test case are already introduced here.

In its default implementation the callback returns std::chrono::system_clock::now().

Copy link
Copy Markdown
Owner

@randombit randombit left a comment

Choose a reason for hiding this comment

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

One minor comment but otherwise lgtm

return std::chrono::duration_cast<std::chrono::seconds>(
std::chrono::system_clock::now() - m_start_time);
}
std::chrono::seconds age() const;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think it would be better to pass the Callback& to age rather than have it be a member variable.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Or perhaps instead of having an age function at all, just expose start_time and in the code that needs the age, have it invoke the Callback& directly, eg auto age = cb.tls_current_timestamp() - record.start_time()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In fact this method doesn't seem to be used at all anymore in the TLS 1.2 code base.

@randombit
Copy link
Copy Markdown
Owner

Can we merge this directly to master instead of into the 1.3 branch?

@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented May 17, 2022

Can we merge this directly to master instead of into the 1.3 branch?

Sure. I'll need to disentangle the RFC 8448 test though. Will look into it tomorrow.

@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented May 18, 2022

I opened another PR (#2976) targeting master. Obviously this doesn't contain the TLS 1.3 specific adaptions. I'll integrate those changes into #2922 and rebase once this one merged. Closing...

@reneme reneme closed this May 18, 2022
@randombit randombit deleted the tls13/timestamp_callback branch January 1, 2023 14:50
@randombit
Copy link
Copy Markdown
Owner

@reneme I was deleting old branches of closed PRs but possibly some of the logic here is still needed?

@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented Jan 1, 2023

Not that I'm aware of. I think the important bits found their way into master already.

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.

2 participants