Unable to use TLS termination over Nginx

[Voxelot]

Bug Report

The Tonic generated gRPC client has protocol errors when communicating with Tonic server over Nginx with TLS termination.

Version

├── tonic v0.2.1
│   ├── hyper v0.13.5
│   │   ├── h2 v0.2.4

Platform

Linux f20019cca92f 5.4.0-31-generic #35-Ubuntu SMP Thu May 7 20:20:34 UTC 2020 x86_64 GNU/Linux

Description

When attempting to use our Tonic generated client we encounter the following error when using TLS termination with Nginx.

http2 error: protocol error: frame with invalid size

Other gRPC clients in Typescript and Go are able to communicate with the Tonic server without any problems. Only the Tonic based rust client has this issue.

[LucioFranco]

@seanmonstar looks like something to do with h2?

[seanmonstar]

To clarify, the error occurs in the tonic client? How do you configure it?

[Voxelot]

Currently without the .tls_config builder option. I was hoping that by enabling the tls and tls-roots features it would be able to detect the certificate and use TLS by inferring the scheme and domain from the URI...

I was able to reproduce this issue using Tonic examples using the following steps:

1. Setup the nginx conf file in examples/data/nginx.conf

user  nginx;
worker_processes  1;

error_log  /var/log/nginx/error.log warn;
pid        /var/run/nginx.pid;

events {
    worker_connections  1024;
}

http {
    include       /etc/nginx/mime.types;
    default_type  application/octet-stream;

    log_format  main  '$remote_addr - $remote_user [$time_local] "$request" '
                      '$status $body_bytes_sent "$http_referer" '
                      '"$http_user_agent" "$http_x_forwarded_for"';

    access_log  /var/log/nginx/access.log  main;

    server {
        listen 443 ssl http2;

        ssl_certificate     /tls/server.pem;
        ssl_certificate_key /tls/server.key;

        location / {
            # Replace localhost:50051 with the address and port of your gRPC server
            # The 'grpc://' prefix is optional; unencrypted gRPC is the default
            grpc_pass grpc://[::1]:50051;
        }
    }
}

2. Start the nginx server with the following docker command from the tonic repo root

docker run --name grpc-nginx  --network="host" -v $(pwd)/examples/data/nginx.conf:/etc/nginx/nginx.conf:ro -v "$(pwd)/examples/data/tls:/tls:ro"  nginx

3. Start the hello-world server since nginx is performing the TLS encryption

cargo run --bin helloworld-server

4. Modify the connection string in the tls client.rs example in examples/src/tls/client.rs to the following:

let channel = Channel::from_static("https://localhost:443")

This will return an unimplemented error, which seems to be just because the hello world server is implemented differently than the tls server. However it seems to work over Nginx when the tls_config is manually specified.

5. Remove the .tls_config(tls) from the client builder and you'll see the frame size error.

It's worth noting that when you remove tls_config from the client builder and point directly to the tls server you get a broken pipe error instead of invalid frame error.

[Voxelot]

Ended up fixing the problem by stuffing in a default ClientTlsConfig object into the connection when the URL scheme has https in it.

    async fn connect_internal(url: &Url) -> Result<Channel, Error> {
        let conn = tonic::transport::Endpoint::new(url.to_string())?;
        if url.scheme().eq("https") {
            conn.tls_config(ClientTlsConfig::new())
        } else {
            conn
        }
        .connect()
        .await
    }

1. frame size invalid was not a very helpful error
2. Is there a way for tonic to auto-upgrade to TLS for scenarios like this?

[seanmonstar]

I've filed hyperium/h2#470 about the frame size error.

[Voxelot]

Thanks @seanmonstar!

@LucioFranco what do you think about an improvement to tonic where TLS is assumed based on the URI scheme? The above snippet I posted for my workaround feels kind of silly. It seems like I shouldn't have to pass in a dummy config object if tonic is already smart enough to auto-configure that once it's passed in. This is just ergonomics though.

[LucioFranco]

That could work this is what I believe the other connectors do as well.

[LucioFranco]

Closing in-favor of #418