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

gRPC TLS is broken for all exporters #1227

Closed
jpkrohling opened this issue Jun 29, 2020 · 2 comments
Closed

gRPC TLS is broken for all exporters #1227

jpkrohling opened this issue Jun 29, 2020 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@jpkrohling
Copy link
Member

jpkrohling commented Jun 29, 2020

Describe the bug
When specifying the TLS options for both the client and server, the gRPC server is correctly set with those options, but the cmux matcher doesn't get a chance of getting a peek into the content, as the TLS handshake happens before these headers can be evaluated. As such, cmux will fallback to HTTP, which will cause clients to fail with:

transport: authentication handshake failed: tls: first record does not look like a TLS handshake

Note that this message is swallowed and the end-user will just not get any feedback (see #1192). One easy way to verify this is to setup two pipelines, with the first sending the spans to the second, and both having the logging exporter. This way, two log entries are expected, but only one will appear (see config below).

Changing the cmux matcher to match on cmux.TLS() makes it work, as the gateway's HTTP port isn't ever configured to use TLS, even when the TLS options are specified.

Steps to reproduce

  1. generate a set of certs (see below)
  2. use the configuration file below
  3. generate a span

To generate the certs, start with a /tmp/cert/csr.json as follows:

{
    "hosts": [
        "guarana",
        "localhost",
        "127.0.0.1"
    ],
    "key": {
        "algo": "rsa",
        "size": 2048
    },
    "names": [
        {
            "C":  "DE",
            "L":  "Berlin",
            "O":  "kroehling.de",
            "OU": "WWW",
            "ST": "Berlin"
        }
    ]
}

Then:

  1. cd /tmp/cert
  2. cfssl genkey -initca csr.json | cfssljson -bare ca
  3. cfssl gencert -ca ca.pem -ca-key ca-key.pem -hostname=guarana,localhost,127.0.0.1 csr.json | cfssljson -bare self-signed

What did you expect to see?

2020-06-29T17:12:46.917+0200	INFO	loggingexporter/logging_exporter.go:102	TraceExporter	{"#spans": 2}
2020-06-29T17:12:47.917+0200	INFO	loggingexporter/logging_exporter.go:102	TraceExporter	{"#spans": 2}

What did you see instead?

2020-06-29T17:13:53.741+0200	INFO	loggingexporter/logging_exporter.go:102	TraceExporter	{"#spans": 2}

What version did you use?
Version: master at 12b3d9ccb8d20f072a25737c4b745512d60e2ee0

What config did you use?
Config:

receivers:
  otlp:
    endpoint: "localhost:55680"
    tls_credentials:
      cert_file: /tmp/cert/self-signed.pem
      key_file: /tmp/cert/self-signed-key.pem
  jaeger:
    protocols:
      thrift_compact:

processors:
  batch:
    timeout: 1s

exporters:
  otlp:
    endpoint: "localhost:55680"
    ca_file: /tmp/cert/ca.pem
  jaeger:
    endpoint: "localhost:14250"
    insecure: true
  logging:

service:
  pipelines:
    traces/custom-1:
      receivers: [jaeger]
      processors: [batch]
      exporters: [logging, otlp]
    traces/custom-2:
      receivers: [otlp]
      processors: [batch]
      exporters: [logging, jaeger]

Environment
OS: Fedora 32 latest updates applied
Compiler(if manually compiled): go1.14.3

Additional context
The following diff would make the configuration work, but not sure it's a proper solution:

diff --git a/receiver/otlpreceiver/otlp.go b/receiver/otlpreceiver/otlp.go
index d6174430..ccc3d162 100644
--- a/receiver/otlpreceiver/otlp.go
+++ b/receiver/otlpreceiver/otlp.go
@@ -246,9 +246,7 @@ func (r *Receiver) startServer(host component.Host) error {
 
                // Start the gRPC and HTTP/JSON (grpc-gateway) servers on the same port.
                m := cmux.New(r.ln)
-               grpcL := m.MatchWithWriters(
-                       cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"),
-                       cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"))
+               grpcL := m.Match(cmux.TLS())
 
                httpL := m.Match(cmux.Any())
                go func() {
@jpkrohling jpkrohling added the bug Something isn't working label Jun 29, 2020
@tigrannajaryan
Copy link
Member

@bogdandrutu assigning this too you since you are already working on this area.

@bogdandrutu
Copy link
Member

This is fixed in #1223. I could not find a solution to reuse the same port and support custom TLS credentials for gRPC and http. But now it should work

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this issue Nov 11, 2021
* Add a Shutdown method to api TraceProvider

- sdktraceprovider shutdown span processors
- In examples, replace processosr shutdown with
  traceprovider's shutdown

Signed-off-by: Hui Kang <[email protected]>

* remove shutdown in the api provider interface

* Add context in parameter and return error

* handle error in shutdown

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <[email protected]>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this issue Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants