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

Using custom TlsConnector #2

Closed
pbor opened this issue Nov 23, 2019 · 7 comments
Closed

Using custom TlsConnector #2

pbor opened this issue Nov 23, 2019 · 7 comments

Comments

@pbor
Copy link

pbor commented Nov 23, 2019

Disclaimer: I am not convinced this is a problem with async-tungstenite itself, rather than just being the consequence of the in-flux state of the async ecosystem, but I figured I might as well file it at the "entry point" of my problem, since I could not think of a better alternative :-)

I am trying to connect to a websocket server with a self-signed certificate.

Digging into things a little bit, I figured that I need to use the client api instead of the high level connect-async, and use my own TlsConnector, see for instance (snapview/tungstenite-rs#84).
Since this is the async version of tungstenite, I figured I would need an async TLS connector.
I also saw that tungstenite uses native-tls.

I found that https://github.com/dbcfd/tls-async is an async version of native-tls, but it does not seem active and currently does not compile.

On the other hand, there is https://github.com/async-rs/async-tls but that uses rust-tls, so I do not think I can use that with async-tungstenite,

Any suggestion?

@sdroege
Copy link
Owner

sdroege commented Nov 23, 2019

On the other hand, there is https://github.com/async-rs/async-tls but that uses rust-tls, so I do not think I can use that with async-tungstenite,

async-tungstenite uses async-tls if the "tls" feature is enabled. It's then used inside the connect_async() function.

Check the implementation of the connect_async() function to see how to use your own TlsConnector.

Does that help?

@pbor
Copy link
Author

pbor commented Nov 23, 2019

Thanks for the quick reply!

Oh ok, so async-tungstenite use async-tls which in turn uses rusttls, instead tungstenite uses native-tls and does nor currently support rusttls. That is kind of confusing, and should maybe be called out explicitly in the docs.

Anyway, thanks for clarifying that, I guess my next step is to look into how to pass a rusttls ClientConfig to the async_tls::TlsConnector so that I can accept self signed certs.

(The fact that we end up using rusttls instead of native-tls, is kind of annoying, since I fear I will have a much harder time getting this past appsec review, but right now I can focus on my prototype)

@pbor
Copy link
Author

pbor commented Nov 23, 2019

Code is pretty terrible right now, since I just hammered till it worked, but this seems to work, in case it is useful to anyone else:


// We use our own connection code, because the high level `connect_async`
// function of async-tungstenite does not give us control over the
// TLS connector, and we need to accept self-signed certificates.
mod ws {
    use async_std::net::TcpStream;
    use async_tls::{client::TlsStream, TlsConnector};
    use async_tungstenite::{
        client_async,
        stream::Stream as StreamSwitcher,
        tungstenite::{
            client::url_mode, handshake::client::Request, handshake::client::Response,
            stream::Mode, Error,
        },
        WebSocketStream,
    };
    use futures::io::{AsyncRead, AsyncWrite};
    use rustls;
    use std::sync::Arc;
    use webpki;

    /// A stream that might be protected with TLS.
    pub type MaybeTlsStream<S> = StreamSwitcher<S, TlsStream<S>>;

    pub type AutoStream<S> = MaybeTlsStream<S>;

    pub async fn connect_async(
        request: Request<'_>,
    ) -> Result<(WebSocketStream<AutoStream<TcpStream>>, Response), Error> {
        let domain = request.url.host_str().expect("Host name missing");
        let port = request.url.port_or_known_default().expect("Port missing");

        let try_socket = TcpStream::connect((domain, port)).await;
        let socket = try_socket.map_err(Error::Io)?;

        let mode = url_mode(&request.url)?;
        let stream = wrap_stream(socket, domain, mode).await?;

        client_async(request, stream).await
    }

    async fn wrap_stream<S>(socket: S, domain: &str, mode: Mode) -> Result<AutoStream<S>, Error>
    where
        S: 'static + AsyncRead + AsyncWrite + Send + Unpin,
    {
        match mode {
            Mode::Plain => Ok(StreamSwitcher::Plain(socket)),
            Mode::Tls => {
                let connector = tls_connector();
                let connected = connector.connect(domain, socket)?.await;
                match connected {
                    Err(e) => Err(Error::Io(e)),
                    Ok(s) => Ok(StreamSwitcher::Tls(s)),
                }
            }
        }
    }

    fn tls_connector() -> TlsConnector {
        // FIXME: this is expensive and we should have one config per process
        let mut config = rustls::ClientConfig::new();
        config
            .root_store
            .add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS);
        config
            .dangerous()
            .set_certificate_verifier(Arc::new(NoCertificateVerification {}));

        let config = Arc::new(config);
        config.into()
    }

    // FIXME! we do not want to blindly accept invalid certs,
    // we want to ask the user to trust the cert and add it

    struct NoCertificateVerification {}

    impl rustls::ServerCertVerifier for NoCertificateVerification {
        fn verify_server_cert(
            &self,
            _roots: &rustls::RootCertStore,
            _presented_certs: &[rustls::Certificate],
            _dns_name: webpki::DNSNameRef<'_>,
            _ocsp: &[u8],
        ) -> Result<rustls::ServerCertVerified, rustls::TLSError> {
            Ok(rustls::ServerCertVerified::assertion())
        }
    }
}

@sdroege
Copy link
Owner

sdroege commented Nov 23, 2019 via email

@Darkspirit
Copy link

Yeah, an option to use a custom client config would indeed be useful. Thanks!

Proliferation of native-tls is not desirable. Having most code in safe Rust is an achievement. If needed, https://github.com/ctz/rustls-native-certs could be used instead of webpki-roots.

@sdroege
Copy link
Owner

sdroege commented Nov 23, 2019

Proliferation of native-tls is not desirable

While I generally agree with you that it would be nice to have a mature and battle-proven TLS stack in a memory-safe language, that's not where we are yet :)

Also having each application ship their own TLS stack is also not optimal, even more so if it's a rather immature and new one. And there can be many non-technical reasons for preferring the platform's TLS stack too.

@sdroege
Copy link
Owner

sdroege commented Dec 12, 2019

That's done now, and you can also optionally build with native-tls instead of async-tls. You'd have to compile with e.g. --no-default-features --features=connect,native-tls,async_std_runtime

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