Skip to content

aqc: initial implementation #214

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 97 commits into from
May 28, 2025
Merged

aqc: initial implementation #214

merged 97 commits into from
May 28, 2025

Conversation

benz-spideroak
Copy link
Contributor

@benz-spideroak benz-spideroak commented Apr 21, 2025 •
edited by gknopf-aranya
Loading

Implements AQC networking code.

  • Binds QUIC server to a network address
  • Binds QUIC client to 0.0.0.0:0 (this should be configurable in the future)
  • Fetches the AQC PSK from the daemon via the encrypted tarpc IPC
  • Integrates with rustls w/ PSK support based on this PR (add TLS 1.3 PSK support rustls/rustls#2424)
  • Initializes the QUIC server with a PSK keystore
  • Adds AQC PSKs to the key store when AQC channels are created or received.
  • Creates bidirectional and unidirectional AQC channels.
  • Supports creating new QUIC streams on existing AQC channels.
  • Supports sending/receiving data via AQC QUIC data streams.
  • Adds AQC example to Rust example application.

TODO:

  • Removal of test certs once rustls is updated to no longer require the server to be initialized with a cert when using PSKs.
  • Deletion of bidirectional/unidirectional channels (including deletion of corresponding PSKs)
  • Make QUIC client address configurable
  • More robust testing (e.g. multiple peers creating/deleting channels and sending/receiving in parallel)

@benz-spideroak benz-spideroak marked this pull request as draft April 21, 2025 14:40
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed Apr 21, 2025
View reviewed changes
elagergren-spideroak
elagergren-spideroak reviewed May 27, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed May 28, 2025
View reviewed changes
ssweetney-spok
ssweetney-spok reviewed May 28, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed May 28, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed May 28, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed May 28, 2025
View reviewed changes
gknopf-aranya
gknopf-aranya reviewed May 28, 2025
View reviewed changes
@jdygert-spok
Copy link
Contributor

jdygert-spok commented May 28, 2025 •
edited
Loading

Summary of unresolved discussions and a few more TODOs.

IMO these can be handled after merging this PR. Let me know if you disagree or I missed something.

  • Iron out closing channels and streams (finish / reset / send error code etc)
  • Remove PSK when channel closed
  • Notify peer when channel closed
  • Multiple CTRL psks
  • Use published forks
  • Nonblocking try_receive_channel
  • Don't use channel for identity, store on connection or similar.
  • Stress test opening channels and sending data
  • Allow configuring client addr
  • AQC_VERSION
  • make try_receive* APIs more consistent (Closed vs ConnectionClosed vs None, etc)
  • crypto frames for ctrl exchange

@djohnson-spideroak
Copy link
Contributor

Summary of unresolved discussions and a few more TODOs.

IMO these can be handled after merging this PR. Let me know if you disagree or I missed something.

  • Iron out closing channels and streams (finish / reset / send error code etc)
  • Remove PSK when channel closed
  • Notify peer when channel closed
  • Multiple CTRL psks
  • Use published forks
  • Nonblocking try_receive_channel
  • Don't use channel for identity, store on connection or similar.
  • Stress test opening channels and sending data
  • Allow configuring client addr
  • AQC_VERSION
  • make try_receive* APIs more consistent (Closed vs ConnectionClosed vs None, etc)
  • crypto frames for ctrl exchange

I think so, lets get this reviewed. In order of importance once we merge, we need to get the protocol versioned properly & any API changes, the crypto changes, then additional config, etc.

And of course, we will need the forks for the release.

djohnson-spideroak
djohnson-spideroak approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@djohnson-spideroak djohnson-spideroak left a comment

Choose a reason for hiding this comment

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

LGTM, I want at least 3 approvals for this.

@elagergren-spideroak elagergren-spideroak changed the title 168: aqc net code aqc: initial implementation May 28, 2025
@djohnson-spideroak djohnson-spideroak merged commit 916af86 into main May 28, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdygert-spok jdygert-spok jdygert-spok left review comments

@djohnson-spideroak djohnson-spideroak djohnson-spideroak approved these changes

@gknopf-aranya gknopf-aranya gknopf-aranya approved these changes

@ssweetney-spok ssweetney-spok ssweetney-spok approved these changes

@mooreoak mooreoak Awaiting requested review from mooreoak

@ycarmy ycarmy Awaiting requested review from ycarmy

@nikki-aranya nikki-aranya Awaiting requested review from nikki-aranya

@elagergren-spideroak elagergren-spideroak Awaiting requested review from elagergren-spideroak elagergren-spideroak is a code owner

@chip-so chip-so Awaiting requested review from chip-so

+1 more reviewer

@gknopf-spideroak gknopf-spideroak gknopf-spideroak left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@jdygert-spok jdygert-spok

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@benz-spideroak @nikki-aranya @gknopf-aranya @jdygert-spok @djohnson-spideroak @elagergren-spideroak @gknopf-spideroak @chip-so @ssweetney-spok