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

Connection allow get/set context data to control ClientHelloCallback #2437

Open
taikulawo opened this issue Jan 6, 2025 · 9 comments · May be fixed by #2448
Open

Connection allow get/set context data to control ClientHelloCallback #2437

taikulawo opened this issue Jan 6, 2025 · 9 comments · May be fixed by #2448
Labels
enhancement New feature or request size/medium

Comments

@taikulawo
Copy link

taikulawo commented Jan 6, 2025

Problem:

We have Rust-based NGINX, so many domain can listen on same SocketAddr, we want to get some args not only ClientHello to decide which Config to use.

# different domain listen on same port
server {
    server_name a.com;
    listen 8443 quic reuseport;
    ssl_certificate     certs/example.com.crt;
    ssl_certificate_key certs/example.com.key;
    location / {
        return 200;
    }
}
server {
    server_name b.com;
    listen 8443 quic;
    ssl_certificate     certs/example.com.crt;
    ssl_certificate_key certs/example.com.key;
    location / {
        return 200;
    }
}
impl ClientHelloCallback for ClientCallback {
    fn on_client_hello(
        &self,
        connection: &mut QuicConnection,
    ) -> Result<Option<Pin<Box<dyn ConnectionFuture>>>, S2nError> {
        let host = connection.server_name();
        // get context.
        let ctx = connection.???????;
        match lookup_server_block(&ctx, host, &ctx.servers) {
            Ok(block) => {
                // after lookup, get config from v
                let config = self.cache.get(&block).unwrap();
                // store block in Context
                ctx.server_block = v;
                // set Connection config
                connection.set_config(config.clone()).unwrap();
        
                Ok(None)
            }
            Err(err) => {
                return Err(S2nError::application(Error::e_cause_by(
                    ErrorType::Other,
                    "no server name found",
                    err,
                )))
            }
        }
    }
}

Solution:

Connection add API set_raw_ctx/get_raw_ctx

#[derive(Clone)]
struct Context{
    block: Block,
    servers: Servers,
};
struct ClientCallback{
    ctx: Context,
    cache: HashMap<String, Block>,
}
impl ClientHelloCallback for ClientCallback {
    fn on_client_hello(
        &self,
        connection: &mut QuicConnection,
    ) -> Result<Option<Pin<Box<dyn ConnectionFuture>>>, S2nError> {
        let host = connection.server_name();
        let ctx = Box::into_raw(Box::new(self.ctx.clone()));
        connection.set_raw_ctx(ctx);
        // ctx are per connection level.
        let ctx = connection.get_raw_ctx();
        match lookup_server_block(&ctx, host, &ctx.servers) {
            Ok(block) => {
                // after lookup, get config from v
                let config = self.cache.get(&block).unwrap();
                // store block in Context
                ctx.server_block = v;
                // set Connection config
                connection.set_config(config.clone()).unwrap();
        
                Ok(None)
            }
            Err(err) => {
                return Err(S2nError::application(Error::e_cause_by(
                    ErrorType::Other,
                    "no server name found",
                    err,
                )))
            }
        }
    }
}
let cb = ClientCallback {};
let tls = s2n_quic::provider::tls::s2n_tls::Server::builder()
            .with_client_hello_handler(cb)
            .unwrap()
            .build()
            .unwrap();
let io = IoBuilder::default()
    .with_rx_socket(socket)
    .unwrap()
    .build()
    .unwrap();
let post_callback = Callback {}
let mut server = Server::builder()
    .with_tls(tls)
    .unwrap()
    .with_io(io)
    .unwrap()
    .start()
    .unwrap();
while let Some(mut connection) = server.accept().await {
    let inbound = self.clone();

    // So we can get ctx which bind to connection.
    let ctx:&Context = connection.get_ctx();
    tokio::task::spawn_local(async move {
        loop {
            match connection.accept_bidirectional_stream().await {
                Ok(Some(s)) => {
                    // do our job...
                }
                Ok(None) => {
                    break;
                }
                Err(err) => {
                    log::debug!("accept quic stream from connection error {err}");
                    break;
                }
            }
        }
    });
}
  • Does this change what s2n-quic sends over the wire?
    No
  • Does this change any public APIs?
    No

Requirements / Acceptance Criteria:

Out of scope:

@taikulawo taikulawo changed the title Connection allow get/set context data to contrl ClientHelloCallback Connection allow get/set context data to control ClientHelloCallback Jan 6, 2025
@taikulawo
Copy link
Author

related quinn-rs/quinn#2024

@taikulawo
Copy link
Author

rustls use Acceptor, so we can decide everything before continue to something. https://docs.rs/tokio-rustls/latest/tokio_rustls/struct.LazyConfigAcceptor.html

But openssl use callback style, which require void *p data support, like SSL_set_ex_data

@jouho
Copy link
Contributor

jouho commented Jan 8, 2025

Hello @taikulawo, we do have an existing API to set/get the application context on connection through s2n-tls:

setting application context: https://docs.rs/s2n-tls/0.3.9/s2n_tls/connection/struct.Connection.html#method.set_application_context
getting application context: https://docs.rs/s2n-tls/0.3.9/s2n_tls/connection/struct.Connection.html#method.application_context

You may also find other connection level functions here: https://github.com/aws/s2n-tls/blob/61c2a660cf4363d34ee84b503aafd00d8451fc08/bindings/rust/extended/s2n-tls/src/connection.rs#L100

Hope this helps, and let us know if you have any further questions!

@taikulawo
Copy link
Author

How I get context back in s2n_quic::connection::Connection?

let tls = s2n_quic::provider::tls::s2n_tls::Server::builder()
    .with_client_hello_handler(callback)
    .unwrap()
    .build()
    .unwrap();
let io = IoBuilder::default()
    .with_rx_socket(socket)
    .unwrap()
    .build()
    .unwrap();
let mut server = Server::builder()
    .with_tls(tls)
    .unwrap()
    .with_io(io)
    .unwrap()
    .start()
    .unwrap();
while let Some(connection) = server.accept().await {
    // quic connection
    // NO application_context here
    // ?????
    connection.application_context();
}

impl ClientHelloCallback for Bar {
    fn on_client_hello(
        &self,
        connection: &mut TlsConnection,
    ) -> Result<Option<Pin<Box<dyn ConnectionFuture>>>, S2nError> {
        struct Foo {};
        // tls connection
        connection.set_application_context(Foo{});
        todo!();
    }
}

@camshaft camshaft added enhancement New feature or request size/medium labels Jan 9, 2025
@camshaft
Copy link
Contributor

camshaft commented Jan 9, 2025

That functionality isn't currently supported, unfortunately. The ClientHelloCallback is defined in the s2n-tls crate, which doesn't have any knowledge of any s2n-quic types/traits.

That being said, I think it should be possible to support. We would need to provide a callback on the TLS builder that transfers the TLS context from the TLS connection to the QUIC connection. This is needed because s2n-quic discards the TLS session as soon as the handshake completes and won't exist by the time the application goes to read the application context. Additionally, the connection API will need a way to get at the transferred context.

We'd be open to accepting a PR if you have time to get one together. Thanks!

@taikulawo
Copy link
Author

taikulawo commented Jan 10, 2025

I see ClientHelloCallback is been unstable feature a long time, Do we have plan to stabilize it?

@jouho
Copy link
Contributor

jouho commented Jan 10, 2025

We could consider stabilizing it. Is this a blocker for your implementation?

@taikulawo
Copy link
Author

taikulawo commented Jan 13, 2025

I could help. but we should address issue one by one.

@taikulawo taikulawo linked a pull request Jan 15, 2025 that will close this issue
@taikulawo
Copy link
Author

taikulawo commented Jan 15, 2025

I have open a prototype PR to discuss #2448

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request size/medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants