Envoy: Remove curl as an Envoy dependency

[htuch]

Currently we depend on libcurl for URL fetching in AWS extension common utils https://github.com/envoyproxy/envoy/blob/89d6c6c6fa202aa7a01e85d2d887b56c8a3268ad/source/extensions/common/aws/utility.cc#L97 and I think OpenCensus (@g-easy can you confirm?)

The use of curl is largely redundant, since Envoy itself can do HTTP fetch. In addition, Curl does not have a compatible threading and observability model with Envoy. The recent disclosures of CVE-2020-8169 and CVE-2020-8177 provide an example of why we should eliminate this from our trusted compute base.

Opening this tracking ticket to discuss further whether we can remove this dep.

[mattklein123]

+1 for removal. I was against adding in the first place.

[moderation]

+1 for removal

[g-easy]

Yes, OpenCensus uses curl in its zipkin exporter (to push traces to zipkin).

[kyessenov]

Having an indirect networking API instead of curl in libraries would also help compiling them to Wasm binaries for sandboxing. The API would be implemented via Wasm ABI.

[htuch]

@g-easy would it be possible to modify the Zipkin exporter to work with some more generic HTTP interface that Envoy can supply?

[moderation]

As per https://github.com/envoyproxy/envoy/issues/9958 at some point OpenCensus should be replaced by the new OpenTracing + OpenCensus project called OpenTelemetry. One would hope that OpenTelemetry doesn't require curl

[dio]

For opencensus, since the "issue" is with the Zipkin span exporter implementation (opencensus_exporter_zipkin). I think we could own Envoy-specific implementation of that in our tree (I can try to own it if it is desired)? At minimum:

class ZipkinSpanExporterHandler : public ::opencensus::trace::exporter::SpanExporter::Handler,
                                  public Http::AsyncClient::Callbacks {
  ...
};

Which basically "retrieves" the spans stored by opencensus-cpp worker thread implementation and send that to Envoy's thread which has access to a dispatcher (I think server's dispatcher is OK to use here). As a "bonus", we also need to serialize the opencensus::trace::exporter::SpanData to Zipkin's v2 via ProtobufWkt::Struct (instead of rapidjson employed by the current opencensus-cpp Zipkin span exporter implementation).

However, this will "force" the configuration to require a defined collector cluster to let Envoy's async-client to work (I think this is also relevant to the approach for the "aws" utility). Even the dynamic forward proxy requires a cluster definition. I wonder if we can/are allowed to somehow construct a cluster without defining in the config, but build it on the fly when it is required (https://github.com/envoyproxy/envoy/pull/3479 cc. @ramaraochavali).

@moderation, when we are needed to "upgrade" to OpenTelemetry, if the approach is the same (i.e. data is gathered in worker thread managed by the "tracer" library) and we want to support Zipkin exporter, we still need to provide an Envoy's async-client-friendly implementation for that.

[g-easy]

One would hope that OpenTelemetry doesn't require curl

This is tracked in https://github.com/open-telemetry/opentelemetry-cpp/issues/6

[g-easy]

https://github.com/envoyproxy/envoy/issues/11816#issuecomment-653838495 sounds pretty great to me.

Note that opencensus's existing background thread calls into Handler::Export() periodically.

[dio]

@mattklein123 what is the best way of resolving URI for async client calls without explicitly defining a cluster for it? Is https://github.com/envoyproxy/envoy/pull/3479 the way to do it? Need your advice on this. Thank you!

[mattklein123]

@dio yeah we would probably need to add new functionality to AsyncClient to optionally expose the dynamic forward proxy functionality.

[wrowe]

PR13063 is another example, which picks up the fix of

CVE-2020-8231: libcurl: wrong connect-only connection

As Envoy does NOT use the affected CURLOPT_CONNECT_ONLY toggle,
this change simply satisfies overly-simplistic audits.

[htuch]

As part of my EnvoyCon talk, I looked at all CVEs for our dependencies since 2018. Curl is by far the worst here, with 14 CVEs and 23 releases since 2018. Further evidence that we really want to make some progress here.