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

Fix QUIC_TLS_SECRETS on Server and Client. #3539

Merged
merged 11 commits into from
Apr 5, 2023

Conversation

anrossi
Copy link
Contributor

@anrossi anrossi commented Mar 27, 2023

Description

Fixes #2344
Specifically,

  • Copies the ClientRandom on server side.
  • Copies the ClientEarlyTrafficSecret on server side.
  • Copies the ClientRandom on client side when 0-RTT is used.

Testing

  • Manual testing with quicinteropserver to ensure the ClientRandom is logged.
  • New automated tests.

Documentation

N/A

@anrossi anrossi requested a review from a team as a code owner March 27, 2023 22:50
@anrossi anrossi added the Bug: Core A code bug in the Core MsQuic code label Mar 27, 2023
src/core/crypto.c Outdated Show resolved Hide resolved
@nibanks
Copy link
Member

nibanks commented Mar 27, 2023

We should write a test that verifies the client and server secrets & random match.

@anrossi
Copy link
Contributor Author

anrossi commented Mar 27, 2023

We should write a test that verifies the client and server secrets & random match.

Gonna need to think about this a bit. It would involve recreating some TLS logic, and what exactly what it be compared against?
The test hooks might not exist for this yet.

@nibanks
Copy link
Member

nibanks commented Mar 27, 2023

We should write a test that verifies the client and server secrets & random match.

Gonna need to think about this a bit. It would involve recreating some TLS logic, and what exactly what it be compared against? The test hooks might not exist for this yet.

Just take a simple handshake test, update both sides to grab the secrets. After the handshake completes, compare the client and server secrets.

src/core/crypto.c Outdated Show resolved Hide resolved
@anrossi anrossi changed the title Fix QUIC_TLS_SECRETS on Server. Fix QUIC_TLS_SECRETS on Server and Client. Mar 29, 2023
nibanks
nibanks previously approved these changes Mar 29, 2023
src/core/crypto.c Outdated Show resolved Hide resolved
src/core/crypto_tls.c Outdated Show resolved Hide resolved
@anrossi anrossi enabled auto-merge (squash) April 4, 2023 20:54
src/core/crypto_tls.c Outdated Show resolved Hide resolved
nibanks
nibanks previously approved these changes Apr 4, 2023
nibanks
nibanks previously approved these changes Apr 4, 2023
@anrossi anrossi merged commit 43de82a into main Apr 5, 2023
@anrossi anrossi deleted the anrossi/fix-tls-secrets-server branch April 5, 2023 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Core A code bug in the Core MsQuic code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QUIC_TLS_SECRETS Not Fully Populated on Server
2 participants