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

feat(iroh-relay)!: add a QUIC server for QUIC address discovery to the iroh relay. #2965

Merged
merged 19 commits into from
Dec 2, 2024

Conversation

ramfox
Copy link
Contributor

@ramfox ramfox commented Nov 25, 2024

Description

This PR adds a QUIC endpoint to the relay server that can do QUIC address discovery. It also contains structs/functions for properly doing the Client side interaction for this process.

Also, this adjust the RelayNode to include configuration on how to speak to the QUIC endpoint on the relay server.

QUIC is disabled by default and requires a TlsConfig to be configured in order to work.

closes #2964

Breaking Changes

  • iroh_base::relay_map::RelayNode now has field quic that takes a Option<iroh_base::relay_map::QuicConfig>
  • iroh::test_utils::run_relay_server_with(stun: Option<StunConfig>) => iroh::test_utils::run_relay_server_with(stun: Option<StunConfig>, quic: bool)
    • when quic is true, it will start a quic server for QUIC address discovery, that has self signed tls certs for testing.
  • iroh_relay::server::ServerConfig has field quic that takes a Option<iroh_relay::server::QuicConfig>
  • iroh_relay::server::TlsConfig.quic_bind_addr is a new field that takes a SocketAddr
  • iroh_relay::server::TlsConfig.server_config is a new field that takes a rustls::ServerConfig
  • field config has been removed from variant iroh_relay::server::CertConfig::LetsEncrypt
  • variant iroh_relay::server::CertConfig::LetsEncrypt has a new field state that takes a tokio_rustls_acme::AcmeState<EC, EA>
  • variant iroh_relay::server::CertConfig::Manual no longer has field private_key

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

Copy link

github-actions bot commented Nov 25, 2024

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2965/docs/iroh/

Last updated: 2024-12-02T04:43:46Z

Copy link

github-actions bot commented Nov 25, 2024

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: d806b5b

@ramfox ramfox force-pushed the ramfox/relay-qad branch 2 times, most recently from ad6423e to ec4ddb3 Compare November 25, 2024 22:11
@ramfox ramfox changed the title feat(iroh-relay): add a QUIC server for QUIC address discovery to the iroh relay. feat(iroh-relay)!: add a QUIC server for QUIC address discovery to the iroh relay. Nov 25, 2024
@ramfox ramfox self-assigned this Nov 25, 2024
@ramfox ramfox added this to the v0.29.0 milestone Nov 25, 2024
Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

Don't let the extensive commenting put you off, it looks good!

The config plumbing is just painful.

example.config.toml Show resolved Hide resolved
example.config.toml Show resolved Hide resolved
iroh-net/src/defaults.rs Outdated Show resolved Hide resolved
iroh-net/src/test_utils.rs Outdated Show resolved Hide resolved
iroh-net/src/test_utils.rs Outdated Show resolved Hide resolved
iroh-relay/src/quic.rs Outdated Show resolved Hide resolved
iroh-relay/src/quic.rs Show resolved Hide resolved
iroh-relay/src/quic.rs Show resolved Hide resolved
iroh-relay/src/quic.rs Outdated Show resolved Hide resolved
iroh-relay/src/quic.rs Outdated Show resolved Hide resolved
@ramfox
Copy link
Contributor Author

ramfox commented Nov 28, 2024

@flub can you take a look at the config options for the relay server again.

I removed the option to generate self-signed certs automatically and added info to the readme about how to pass those in.

However, the --dev flag removes all tls config in the code and I wanted to keep the "simple" dev mode available for those who don't care about QUIC addr disc. SO there are now 2 dev-like flags, --dev that runs the relay server locally over http with QAD disabled and --dev-quic that expects tls to be configured for QAD, but still runs the server over http.

Is this the worst? I tried to keep it as simple as possible 😂 but could use another set of eyes.

@ramfox
Copy link
Contributor Author

ramfox commented Nov 28, 2024

actually, @flub

I'm wondering if instead, enabled_quic_addr_discovery should actually default to false

So if you want to run QAD locally you would run:
cargo run --bin iroh-relay -- --config-path=PATH --dev with a config file that does:

enable_quic_addr_discovery = true

[tls]
#tls config goes here

That feels cleaner? thoughts?

@flub
Copy link
Contributor

flub commented Nov 28, 2024

That feels cleaner? thoughts?

Hum, it is awkward that TLS certs are basically becoming mandatory even for a local dev relay server. But it is inevitable and we'll only grow more dependence over a QUIC transport over time.

But, the good news is that we don't have to get this right from the start. The level of stability for --dev really doesn't matter that much. And as you say, right now qad is just an optional extra rather than an integral part.

So I'm fine with both of these options. And yes, the latter one, disable qad by default, is the simpler one so why not.

Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

Looks great! Don't think there's anything blocking but still a few suggestions that are hopefully useful.

iroh-net/src/defaults.rs Outdated Show resolved Hide resolved
iroh-relay/README.md Outdated Show resolved Hide resolved
iroh-relay/src/client.rs Outdated Show resolved Hide resolved
iroh-relay/src/client.rs Outdated Show resolved Hide resolved
iroh-relay/src/main.rs Outdated Show resolved Hide resolved
iroh-relay/src/quic.rs Outdated Show resolved Hide resolved
iroh-relay/src/quic.rs Show resolved Hide resolved
iroh-relay/src/quic.rs Outdated Show resolved Hide resolved
iroh-relay/src/server.rs Show resolved Hide resolved
iroh-relay/src/server.rs Outdated Show resolved Hide resolved
@ramfox ramfox added this pull request to the merge queue Dec 2, 2024
Merged via the queue into main with commit b2cb0ca Dec 2, 2024
25 of 26 checks passed
@dignifiedquire dignifiedquire deleted the ramfox/relay-qad branch December 2, 2024 09:08
@ramfox ramfox restored the ramfox/relay-qad branch January 31, 2025 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

feat(iroh-relay): add QUIC endpoint for QUIC Address Discovery
3 participants