Skip to content

Add opt-in TLS 1.3 ticket flushing during the handshake - #3646

Draft
justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:tls13-flush-tickets-mode
Draft

justsmth wants to merge 1 commit into
aws:mainfrom
justsmth:tls13-flush-tickets-mode

Conversation

@justsmth

Copy link
Copy Markdown
Contributor

Related

  • Addresses: P526146963

Context and motivation

Client-speaks-first protocols can stall on Nagle plus delayed ACK while tickets wait for a server write.

Description of changes

Add opt-in SSL_MODE_FLUSH_TLS13_TICKETS to flush tickets before handshake completion, including normal write backpressure and failures.

Testing

Debug build, targeted/full SSL unit and runner suites, and documentation generation passed.

Review considerations

Adds a public mode macro, no symbols. Defaults unchanged; outside the FIPS boundary.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread ssl/ssl_version_test.cc Outdated
const bool tls13 = !is_dtls() && version() == TLS1_3_VERSION;
EXPECT_EQ(!tls13, g_last_session != nullptr);
EXPECT_EQ(tls13 && flush, BIO_pending(SSL_get_rbio(client_.get())) != 0);
char buf;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'buf' is not initialized [cppcoreguidelines-init-variables]

Suggested change
char buf;
char buf = 0;

Comment thread ssl/ssl_version_test.cc Outdated
ASSERT_TRUE(Connect());
EXPECT_FALSE(SSL_in_init(server_.get()));
EXPECT_EQ(0u, BIO_pending(SSL_get_rbio(client_.get())));
char buf;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'buf' is not initialized [cppcoreguidelines-init-variables]

Suggested change
char buf;
char buf = 0;

Comment thread ssl/ssl_version_test.cc Outdated
continue;
}

char buf;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: variable 'buf' is not initialized [cppcoreguidelines-init-variables]

Suggested change
char buf;
char buf = 0;

@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security Review — View Report

Please review before merging.

@justsmth
justsmth force-pushed the tls13-flush-tickets-mode branch from 1bd8c0e to 3d46006 Compare October 12, 2026 18:06
@codecov-commenter

codecov-commenter commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.73%. Comparing base (c3661cc) to head (cc9979e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3646      +/-   ##
==========================================
+ Coverage   78.47%   78.73%   +0.26%     
==========================================
  Files         705      705              
  Lines      130687   130792     +105     
  Branches    17913    17932      +19     
==========================================
+ Hits       102557   102983     +426     
+ Misses      27152    26832     -320     
+ Partials      978      977       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth
justsmth force-pushed the tls13-flush-tickets-mode branch from 3d46006 to cc9979e Compare October 12, 2026 20:52

This branch has not been deployed

No deployments
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