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

Cannot access peer certificates with example's TlsStream #198

Open
moshec2 opened this issue Mar 22, 2023 · 7 comments
Open

Cannot access peer certificates with example's TlsStream #198

moshec2 opened this issue Mar 22, 2023 · 7 comments

Comments

@moshec2
Copy link

moshec2 commented Mar 22, 2023

In the example's TlsStream, you cannot access the peer certificates from tokio_rustls's TlsStream in make_service_fn, even if you try to expose the state property, it's basically always Handshaking. Is there some way to change the example so you could wait until the handshaking is over so you can access this?

@moshec2
Copy link
Author

moshec2 commented Mar 26, 2023

I will add some info on what I tried. One of the things I tried is exposing the state object in the hopes that inside make_service_fn it would be Streaming and I could get the peer certificates from the server connection, but sadly it's always Handshaking (which makes sense since it only does the handshake on a read/write):
https://gist.github.com/moshec2/8b1c99396f61e4b132412468fdb5e2c2#file-server-rs-L103
https://gist.github.com/moshec2/8b1c99396f61e4b132412468fdb5e2c2#file-server-rs-L68
Then I tried forcing the handshake to occur before and make the acceptor return the straight up tokio_rustls::server::TlsStream object, by creating the tokio_rustls::Accept inside the custom acceptor and polling it immediately:

impl Accept for TlsAcceptor {
    // type Conn = TlsStream;
    type Conn = tokio_rustls::server::TlsStream<AddrStream>;
    type Error = io::Error;

    fn poll_accept(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Self::Conn, Self::Error>>> {
        let pin = self.get_mut();
        match ready!(Pin::new(&mut pin.incoming).poll_accept(cx)) {
            Some(Ok(sock)) => {
                let mut accept = tokio_rustls::TlsAcceptor::from(pin.config.clone()).accept(sock);

                match ready!(Pin::new(&mut accept).poll(cx)) {
                    Ok(stream) => Poll::Ready(Some(Ok(stream))),
                    Err(e) => Poll::Ready(Some(Err(e))),
                }

                // Poll::Ready(Some(Ok(TlsStream::new(sock, pin.config.clone()))))
            }
            Some(Err(e)) => Poll::Ready(Some(Err(e))),
            None => Poll::Ready(None),
        }
    }
}

Which reads like it would work, and after changing my make_service_fn accordingly this compiles but completely breaks the HTTPS server, the first connection ends abruptly during the handshake and then the server hangs indefinitely and I couldn't figure out what happened.

@moshec2
Copy link
Author

moshec2 commented Apr 3, 2023

I've also been struggling with this for a few days and decided to go this route for now (use a different crate and replace last with first).

I've ended up using the tls-listener crate and the code from their example.

@shaug
Copy link

shaug commented Jun 13, 2023

I've been struggling with this too. Now that the TlsAcceptor and TlsStream have been promoted to the project proper, rather than just the examples, it would be nice to both have a way the inner state of the TlsStream, but have an example of how to make the ServerConnection (and thus the peer certificates) available to the request.

If someone can describe how to make the connection available for the request, I'd be happy to put together a PR for the above.

Note: @moshec2's tis-listener solution in the previous comment worked quite well for me.

@djc
Copy link
Member

djc commented Jun 13, 2023

@shaug does #207 do what you need?

@moshec2
Copy link
Author

moshec2 commented Jun 13, 2023

I will add some info on what I tried. One of the things I tried is exposing the state object in the hopes that inside make_service_fn it would be Streaming and I could get the peer certificates from the server connection, but sadly it's always Handshaking (which makes sense since it only does the handshake on a read/write): https://gist.github.com/moshec2/8b1c99396f61e4b132412468fdb5e2c2#file-server-rs-L103 https://gist.github.com/moshec2/8b1c99396f61e4b132412468fdb5e2c2#file-server-rs-L68 Then I tried forcing the handshake to occur before and make the acceptor return the straight up tokio_rustls::server::TlsStream object, by creating the tokio_rustls::Accept inside the custom acceptor and polling it immediately:

impl Accept for TlsAcceptor {
    // type Conn = TlsStream;
    type Conn = tokio_rustls::server::TlsStream<AddrStream>;
    type Error = io::Error;

    fn poll_accept(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Self::Conn, Self::Error>>> {
        let pin = self.get_mut();
        match ready!(Pin::new(&mut pin.incoming).poll_accept(cx)) {
            Some(Ok(sock)) => {
                let mut accept = tokio_rustls::TlsAcceptor::from(pin.config.clone()).accept(sock);

                match ready!(Pin::new(&mut accept).poll(cx)) {
                    Ok(stream) => Poll::Ready(Some(Ok(stream))),
                    Err(e) => Poll::Ready(Some(Err(e))),
                }

                // Poll::Ready(Some(Ok(TlsStream::new(sock, pin.config.clone()))))
            }
            Some(Err(e)) => Poll::Ready(Some(Err(e))),
            None => Poll::Ready(None),
        }
    }
}

Which reads like it would work, and after changing my make_service_fn accordingly this compiles but completely breaks the HTTPS server, the first connection ends abruptly during the handshake and then the server hangs indefinitely and I couldn't figure out what happened.

By the way, from what I understood, to make this work you need to save the accept future you get from the acceptor on the struct and keep polling it, in my code it polls it once and that's it reaches a weird invalid state. I did not finish implementing this though since I moved to using tls-listener as mentioned.

@shaug
Copy link

shaug commented Jun 13, 2023

@shaug does #207 do what you need?

@djc That is necessary, but not sufficient. I had that exact change in my local setup, but as @moshec2 describes, attempting to access that in the make_subscriber_fn callback always returns None, because the internal state always appears to be Handshaking at that point.

    let service = make_service_fn(move |conn: &TlsStream| {
        let peer_cert = match conn.get_ref() {
            // This branch is never called, because `.get_ref()` always returns `None`
            Some((_, session)) => session
                .peer_certificates()
                .and_then(|certs| certs.first())
                .cloned(),
            None => None,
        };

You can't carry the TlsStream reference across the async boundary into the service_fn function, and it's not even clear that it would work (i.e., if it would be in a Streamingstate at that point) if that were possible.

As @moshec2 points out, there appears to be additional work needed for this acceptor implementation to have a usable ServerConnection instance to make available to the request. It is unclear to me exactly what that work looks like, and at this point, (also as moshec2 points out) the tis-listener-based implementation works fine. I'd prefer not to have to introduce an additional library, since I'm using hyper-rustls for other parts of my project, but I'm not sure I know how to make the current implementation provide the information I need for my request processing.

@djc
Copy link
Member

djc commented Jun 14, 2023

Unfortunately with hyper's traits it's pretty hard to set this up in a clean way. If someone wants to copy over a bunch of tls-listener code into hyper-rustls I'm open to reviewing it, but I won't have time to work on this myself.

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

No branches or pull requests

3 participants