Implementing XFCC header by myidpt · Pull Request #1087 · envoyproxy/envoy

myidpt · 2017-06-12T07:23:41Z

This implements XFCC as discussed in issue #794.

Fixes #794

htuch · 2017-06-12T21:49:57Z

test/test_common/environment.cc

+  return temporaryFileSubstitute(path, ParamMap(), port_map, version);
+}
+
+std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,


I don't want to pick on this change, as the problem was building up before, but we've reached the point where my initial hack in temporaryFileSubstitute to do some simple regex in a Jinja-style is now not scaling all that well. We can iterate over the same config files probably O(10-100) times now, which doesn't seem great. I don't think you need to fix it, but maybe put in a TODO(htuch): Switch to a single pass variable substitution..

Yes I agree. So I will put a TODO for you, and I'll keep this change because I need to make this json file configurable in my integration tests.

Sure, a TODO and filing an issue is the right thing here.

Can you add this TODO please and file a related issue @myidpt ?

Done. I put the TODO in the .h file but I can move it to here.

lizan · 2017-06-13T00:54:52Z

source/common/http/conn_manager_utility.cc

+
+
+  if (connection.ssl() || config.forwardClientCert() == Http::ForwardClientCertType::AlwaysForwardOnly) {
+    std::string clientCertDetails = "";


nit no need = ""

lizan · 2017-06-13T00:55:47Z

source/common/http/conn_manager_utility.cc

+        request_headers.removeForwardedClientCert();
+        break;
+      case Http::ForwardClientCertType::SanitizeSet:
+        request_headers.removeForwardedClientCert();


not needed, insert will replace existing one.

lizan · 2017-06-13T00:56:48Z

source/common/http/conn_manager_utility.cc

+        break;
+      case Http::ForwardClientCertType::AppendForward:
+        // Get the Cert info.
+        if (!request_headers.ForwardedClientCert()->value().empty()) {


request_headers.ForwardedClientCert() might be nullptr

htuch

Looks like this is coming together nicely.

htuch · 2017-06-18T18:41:17Z

source/common/http/conn_manager_impl.h

+
+  /**
+   * @return ClientCertDetailsType the details of the client cert to forward to the next hop.
+   */


The name of this method is strange. From its name, it seems to imply it is mutating (i.e. it sets something), but it takes no arguments. Can you explain how this is supposed to be used more clearly? Also, it returns a list of ClientCertDetailsType, which is also a bit confusing, since this is basically just a Subject/SAN enum, why a list of this?

Yes, the naming is simply following the routine to be the same name of the field in config. This function is directly returning the set_client_cert_details field in the configuration, that indicates which information from the current client cert should be set into the x-forwarded-client-cert header. It can be a list because there are cases you want to set both subject and SAN into the header, and in the future more information may be needed as well.

htuch · 2017-06-18T18:44:27Z

source/common/http/conn_manager_utility.cc

+          // TODO. Get Cert Subject.
+        } else if (detail == Http::ClientCertDetailsType::SAN) {
+          // Currently, we only support a single SAN field.
+          clientCertDetails += ";SAN=" + connection.ssl()->uriSanPeerCertificate();


We need to document this header format and the configuration of this features in https://github.com/lyft/envoy/tree/master/docs somewhere.

htuch · 2017-06-18T18:49:53Z

source/common/ssl/connection_impl.cc

 void ConnectionImpl::onConnected() { ASSERT(!handshake_complete_); }

+bool ConnectionImpl::isMtls() {
+  if (SSL_get_peer_certificate(ssl_.get())) {


Is it an invariant that if we've received a client cert that this cert has been appropriately verified and trusted?

That's a good point. I think this will just return whether the client cert is present, regardless of whether the client cert is valid.
And I don't think this function should depend on whether the client cert is valid, just like the other function sha256PeerCertificateDigest().

The implementation here is wrong in case of the connection is a client connection. We could call this peerCertificatePresented instead of isMtls, or check both certificate.

Also note the X509 pointer returned by SSL_get_peer_certificate need to be freed. Use bssl::UniquePtr.

That makes sense. Done.

htuch · 2017-06-18T18:51:08Z

source/server/config/network/http_connection_manager.cc


+  if (config.hasObject("forward_client_cert")) {
+    // TODO: Check mTLS.
+    const std::string forward_client_cert = config.getString("forward_client_cert");


Can you write this as const std::string forward_client_cert = config.getString("forward_client_cert", Http::ForwardClientCertType::Sanitize); to simplify default case handling?

htuch · 2017-06-18T18:52:00Z

source/server/config/network/http_connection_manager.cc

+    forward_client_cert_ = Http::ForwardClientCertType::Sanitize;
+  }
+
+  if (config.hasObject("set_client_cert_details")) {


BTW, can you contribute the new config stuff to https://github.com/lyft/envoy-api as well? Thanks.

Sure. I will do that.

htuch · 2017-06-18T18:54:41Z

test/integration/xfcc_integration_test.cc

@@ -0,0 +1,243 @@
+#include "xfcc_integration_test.h"


It seems overkill to create an entire new test config file for every feature we want to integrate. I think this is a general issue with integration tests today - there's no easy way to templatize the creation of the configs to turns on/off individual features while retaining a common base. This results in a lot of boiler plate to maintain. @mattklein123 for comment.

I haven't looked through this change yet, but, it seems unnecessary for a whole new config. Can it just be part of the SSL integration test? In general, the fact there are multiple different integration test configs is really arbitrary. They could all be in a single config which just has more listeners and more cluster definitions. I'm certainly happy to figure out a way to simplify things there, but nothing comes to mind quickly, and that is orthogonal to this change. In this case I would probably just move the integration tests into the SSL one even if new listeners and upstreams need to be added.

I originally kept it separately because the xfcc header can also be forwarded without ssl (the "always_forward_only" option). So if you take a look at the config, I have two listeners, one with ssl_context and the other not. But if it's overkill, I can merge the xfcc tests with ssl, because they are very related.

I don't have an opinion on this. I agree there is a lot of boilerplate but at the same time it's not that bad. Whatever you both want is fine with me.

I've created #1162 to track, let's stick with what is there for now.

htuch · 2017-06-18T18:55:34Z

test/test_common/environment.cc

+  return temporaryFileSubstitute(path, ParamMap(), port_map, version);
+}
+
+std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,


Sure, a TODO and filing an issue is the right thing here.

htuch · 2017-06-22T01:27:29Z

docs/configuration/http_conn_man/headers.rst

+---------------
+
+*x-forwarded-clinet-cert* (XFCC) is a proxy header which indicates certificate information of part
+or all of the clients that a request has flowed through on its way from the client to the server.


of the clients or proxies

htuch · 2017-06-22T01:28:40Z

docs/configuration/http_conn_man/headers.rst

+
+The following keys are supported:
+
+1. ``BY`` The Subject Alternative Name (SAN) (Common Name if SAN is not available) of the current


htuch · 2017-06-22T01:29:09Z

docs/configuration/http_conn_man/headers.rst

+   proxy's certificate.
+2. ``Hash`` The SHA 256 diguest of the current client certificate.
+3. ``SAN`` The SAN field (URI type) of the current client certificate. There can be multiple SANs.
+4. ``Subject`` The Subject field of the current Client certificate. The value is quoted.


s/Client certificate/client certificate/

htuch · 2017-06-22T01:29:42Z

docs/configuration/http_conn_man/headers.rst


 .. _config_http_conn_man_headers_x-forwarded-for:

+x-forwarded-client-cert


Thanks for adding this.

No problem :)

htuch · 2017-06-22T01:30:07Z

docs/configuration/http_conn_man/headers.rst

+or all of the clients that a request has flowed through on its way from the client to the server.
+A proxy may choose to sanitize/append/forward the XFCC header before proxying the reqeust.
+
+The XFCC header value is a comma (",") separated string. Each substring is an XFCC element, which


Can any of the key/values contains = or ,? Is there a need to consider escaping?

Yes, the value should be double-quoted. Like the "Subject" field. Updated the doc. Thanks for the catch.

lizan · 2017-06-22T08:38:02Z

source/common/http/conn_manager_utility.cc

+
+  if ((connection.ssl() && connection.ssl()->isMtls()) ||
+      config.forwardClientCert() == Http::ForwardClientCertType::AlwaysForwardOnly) {
+    std::string clientCertDetails;


Extract this part to a function and call it from inside switch below, so you don't always build clientCertDetails even if it is not needed below.

Quick related drive by comment (not full review): Can we pull all of this code out of mutateRequestHeaders() into a new function that can be self contained tested? This function is getting too big as it is.

lizan · 2017-06-22T08:42:11Z

source/common/http/conn_manager_utility.cc

+        clientCertDetails += "By=" + connection.ssl()->uriSanLocalCertificate();
+      }
+      if (!connection.ssl()->sha256PeerCertificateDigest().empty()) {
+        clientCertDetails += ";Hash=" + connection.ssl()->sha256PeerCertificateDigest();


If uriSanLocalCertificate() is empty, then you don't need ; here. Same applies to below.

lizan · 2017-06-22T08:43:08Z

source/common/http/conn_manager_utility.cc

+        clientCertDetails += ";Hash=" + connection.ssl()->sha256PeerCertificateDigest();
+      }
+      for (auto detail : config.setCurrentClientCertDetails()) {
+        if (detail == Http::ClientCertDetailsType::Subject) {


nit: switch?

lizan · 2017-06-22T09:02:27Z

source/common/ssl/connection_impl.cc

 void ConnectionImpl::onConnected() { ASSERT(!handshake_complete_); }

+bool ConnectionImpl::isMtls() {
+  if (SSL_get_peer_certificate(ssl_.get())) {


The implementation here is wrong in case of the connection is a client connection. We could call this peerCertificatePresented instead of isMtls, or check both certificate.

Also note the X509 pointer returned by SSL_get_peer_certificate need to be freed. Use bssl::UniquePtr.

lizan · 2017-06-22T09:04:44Z

source/server/config/network/http_connection_manager.cc

+  }
+
+  if (config.hasObject("set_current_client_cert_details")) {
+    for (const std::string detail : config.getStringArray("set_current_client_cert_details")) {


nit: const std::string&

lizan · 2017-06-22T09:05:38Z

source/common/http/conn_manager_impl.h

+  /**
+   * @return list of ClientCertDetailsType the configuration of the current client cert's details to
+   * be forwarded.
+   */


nit: const std::list<Http::ClientCertDetailsType>&

lizan · 2017-06-24T00:40:37Z

include/envoy/ssl/connection.h

@@ -17,7 +17,7 @@ class Connection {
  /**
   * @return whether the connection is Mutual TLS.


fix comment

mattklein123 · 2017-06-27T22:26:38Z

@myidpt is this ready for wider review? I will take a look tomorrow.

myidpt · 2017-06-27T22:43:12Z

@mattklein123 Yes, it is ready.

mattklein123

This is really excellent work, thanks. Some small nits. @htuch did you want to take another pass? I think this is pretty much ready to go.

mattklein123 · 2017-06-28T14:49:27Z

source/common/json/config_schemas.cc

      "use_remote_address" : {"type" : "boolean"},
+      "forward_client_cert" : {
+          "type" : "string",
+          "enum" : ["forward_only", "append_forward", "sanitize", "sanitize_set", "always_forward_only", "always_append_forward"]


nit: please break at ~100 cols

mattklein123 · 2017-06-28T14:50:54Z

source/common/http/conn_manager_utility.cc

    Tracing::HttpTracerUtility::mutateHeaders(request_headers, runtime);
  }
+
+  if ((connection.ssl() && connection.ssl()->peerCertificatePresented()) ||


nit: I still think it might be better to have this code be a separate routine, even if private. It's a lot to look through and the mutateRequestHeaders function is already huge. (If it was a separate routine I think it would probably be less nested because you could do early return). I could go either way on this but thought I would mention.

Separated to a private function.

mattklein123 · 2017-06-28T14:54:49Z

source/server/config/network/http_connection_manager.h

  Http::ConnectionManagerTracingStats tracing_stats_;
  bool use_remote_address_{};
+  Http::ForwardClientCertType forward_client_cert_;
+  std::list<Http::ClientCertDetailsType> set_current_client_cert_details_;


nit: I would make this std::vector for slight perf boost

mattklein123 · 2017-06-28T14:56:57Z

test/integration/xfcc_integration_test.cc

+                                           client_san_);
+}
+
+/*


Oh sorry, forgot to remove it :P

mattklein123 · 2017-06-28T14:57:26Z

test/integration/xfcc_integration_test.h

+#include "gtest/gtest.h"
+
+namespace Envoy {
+using testing::NiceMock;


nit: please moving using statements outside of Envoy namespace (elsewhere also)

mattklein123

Few remaining tiny nits. LGTM pending comments from anyone else.

mattklein123 · 2017-06-28T23:21:46Z

source/common/http/conn_manager_utility.h

  //       provides behavior that most consumers would expect.
  static std::atomic<uint64_t> next_stream_id_;
+
+  static void mutateXfccRequestHeader(Http::HeaderMap& request_headers,


nit: function definitions go above variable definitions

mattklein123 · 2017-06-28T23:25:14Z

source/common/http/conn_manager_utility.cc

+    request_headers.removeForwardedClientCert();
+    return;
+  }
+  std::string clientCertDetails;


nit: This function is super dense and a bit hard to dive into. Can we get some high level comments before line 140, 145, and 178 on what is about to happen? Would help for others coming along.

mattklein123 · 2017-06-28T23:26:20Z

test/integration/ssl_integration_test.cc

 using testing::Return;

+namespace Envoy {
+


super small nit: del new line between namespace decls when nothing in between (same in other files)

lizan · 2017-06-28T22:34:23Z

include/envoy/ssl/connection.h

 public:
  virtual ~Connection() {}

+  /**


the comment should be fixed

@myidpt the diff is still showing old comment.

lizan · 2017-06-28T23:39:45Z

source/common/http/conn_manager_utility.cc

+    request_headers.removeForwardedClientCert();
+    return;
+  }
+  std::string clientCertDetails;


Since clientCertDetails is only used when SanitizeSet or AppendForward set, process other cases before building it. Also, append directly to header->value() might be more efficient.

Reorganized the code. I don't know what you meant by append directly to header->value(), that seems exactly what I'm doing.

I meant you can directly use append(), since you're only doing += for string, i.e. something similar to here:
https://github.com/lyft/envoy/blob/master/source/common/grpc/common.cc#L137

Instead of even building a std::string. That will avoid to copy that when you append at the end. It is OK to optimize that based on perf analysis.

I see. I think that can be done as part of the code optimization (I added a TODO here). We may want to have an inline function to do that.

htuch

Looks great. Final round of minor comments.

htuch · 2017-06-29T01:48:01Z

include/envoy/ssl/connection.h

+  virtual std::string uriSanLocalCertificate() PURE;
+
  /**
   * @return the SHA256 digest of the peer certificate. Returns "" if there is no peer certificate


What is a "server side connection"?

I think the original author means TLS instead of mTLS. I will update it.

htuch · 2017-06-29T01:50:11Z

test/test_common/environment.cc

+  return temporaryFileSubstitute(path, ParamMap(), port_map, version);
+}
+
+std::string TestEnvironment::temporaryFileSubstitute(const std::string& path,


Can you add this TODO please and file a related issue @myidpt ?

htuch · 2017-06-29T01:54:21Z

docs/configuration/http_conn_man/headers.rst

+
+Each XFCC element is a semicolon ";" separated string. Each substring is a key-value pair, grouped
+together by an equals ("=") sign. The keys are case-insensitive, the values are case-sensitive. If
+",", ";" or "=" appear in a value, the value should be double-quoted.


What if " appears inside the double-quoted string? Is there a need to escape this? If it can't appear, this should probably also be made explicit here.

htuch · 2017-06-29T01:58:35Z

source/common/http/conn_manager_utility.cc

+    for (auto detail : config.setCurrentClientCertDetails()) {
+      switch (detail) {
+      case Http::ClientCertDetailsType::Subject:
+        if (!clientCertDetails.empty()) {


This (and below) can be lifted above the switch.

htuch · 2017-06-29T02:00:19Z

source/common/http/conn_manager_utility.cc

+    }
+    if (!connection.ssl()->sha256PeerCertificateDigest().empty()) {
+      if (!clientCertDetails.empty()) {
+        clientCertDetails += ";";


Why not just build a list of clientCertDetails components and then do a https://github.com/lyft/envoy/blob/master/source/common/common/utility.h#L140 with ";"? A join is what's happening logically here.

Yes, you are right.

htuch · 2017-06-29T02:00:49Z

source/common/http/conn_manager_utility.cc

+      }
+      clientCertDetails += "Hash=" + connection.ssl()->sha256PeerCertificateDigest();
+    }
+    for (auto detail : config.setCurrentClientCertDetails()) {


Nit: const auto& detail

htuch · 2017-06-29T02:02:40Z

source/common/json/config_schemas.cc

+            "sanitize",
+            "sanitize_set",
+            "always_forward_only",
+            "always_append_forward"


This doesn't appear to be referenced anywhere in docs or code?

Good call. It's not a valid option. I will remove it.

htuch · 2017-06-29T02:03:26Z

source/common/ssl/connection_impl.cc


+bool ConnectionImpl::peerCertificatePresented() {
+  bssl::UniquePtr<X509> cert(SSL_get_peer_certificate(ssl_.get()));
+  if (cert) {


You can't do something like return cert and get an implicit cast?

Yes I think that's better.

mattklein123 · 2017-06-29T14:47:52Z

source/common/http/conn_manager_utility.cc

+  } else if (config.forwardClientCert() == Http::ForwardClientCertType::SanitizeSet) {
+    request_headers.insertForwardedClientCert().value(client_cert_details_str);
+  }
+  // It should never reach here.


Then crash with NOT_REACHED

mattklein123 · 2017-06-29T14:49:12Z

source/common/http/conn_manager_utility.cc

+  }
+
+  // TODO(myidpt): Handle the special characters in By and SAN fields.
+  std::vector<std::string> client_cert_details;


FYI this is going to be substantially slower than what you had before. This is related to what @lizan mentioned with doing direct append into the header value. In the interest of getting this out and not doing premature optimization, let's leave this simpler code, but please add a TODO that we should look at perf in this case. (Basically anything that is done on every request is something that we need to look at perf for).

Yeah, the join will be slower. My main aim in suggesting this was to avoid the repeated boilerplate pattern in the code. I think it could also have been made an inline function or something like that as well.

OK. I will leave a TODO here.

Just curious, do we have the performance benchmarks?

Nothing official currently. Official stuff is being worked on in https://github.com/lyft/envoy-perf

mattklein123 · 2017-06-29T14:50:35Z

source/common/ssl/connection_impl.cc


+bool ConnectionImpl::peerCertificatePresented() {
+  bssl::UniquePtr<X509> cert(SSL_get_peer_certificate(ssl_.get()));
+  return cert.get();


nit: cert != nullptr

htuch

LGTM modulo @mattklein123's remaining comments.

mattklein123 · 2017-06-29T18:47:10Z

@htuch @alyssawilk in the interest of moving this along and unblocking people, can we merge this and the cleanup/refactor for SSL utility code actually be done in #1183 ?

alyssawilk · 2017-06-29T19:10:35Z

Fine by me

…

On Thu, Jun 29, 2017 at 2:47 PM, Matt Klein ***@***.***> wrote: @htuch <https://github.com/htuch> @alyssawilk <https://github.com/alyssawilk> in the interest of moving this along and unblocking people, can we merge this and the cleanup/refactor for SSL utility code actually be done in #1183 <#1183> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1087 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ARYFvbOqG7UB1cG9lSm9ixxQgr1gZLgkks5sI_E0gaJpZM4N2vPv> .

timperrett · 2017-06-29T20:20:35Z

@myidpt et al, thanks for the work on this - will merge it into our fork and test it out!

myidpt · 2017-06-29T20:57:13Z

Thank you folks for the review :)

This PR adds a new set of `onEngineRunning` callbacks to the platform (Swift/Kotlin) interfaces. The intent behind these callbacks is to allow platform consumers to observe when Envoy Mobile has finished setting itself up internally. When `build()` is called from an `EnvoyEngineBuilder`, internal library setup is not complete when the allocation returns. Instead, Envoy actually does additional setup before being called back with a "post init" step from the core Envoy layer indicating that it's ready to send requests. In practice, this doesn't have any direct effect on consumers since any requests sent into the library during this timeframe are queued for sending automatically once setup is complete. Based on my localized runs of the library, I noticed 800-1000ms between the start of allocation and the time at which Envoy Mobile has completed its setup. There is room for improvement here which will be looked to in the future. Changes: - Add the new interface to platform layers - Implement the core wiring for this callback and pipe that all the way through JNI / Objective-C up to Kotlin/Swift - Update sample apps to utilize this functionality - Add core unit test to validate the core callback completes Signed-off-by: Michael Rebello <me@michaelrebello.com> Co-authored-by: Alan Chiu <achiu@lyft.com> Signed-off-by: JP Simard <jp@jpsim.com>

…ns (#1087) **Description** This commit corrects the missing unmarshal support for token embeddings which otherwise crash with 502. To ensure no other bugs, it backfills embeddings recordings for all main scenarios. To simplify maintenance, this reduces work in test setup when the proxy response is exactly the same as upstream. **Related Issues/PRs (if applicable)** Backfills tests for #1085 Signed-off-by: Adrian Cole <adrian@tetrate.io>

Oliver Liu and others added 10 commits June 3, 2017 06:50

Initial change to support xfcc.

0e00115

Merge branch 'xfcc' of https://github.com/myidpt/envoy into xfcc

32e8e2e

Add SAN support.

dcdad7f

Half done integration tests.

4a1f02b

Started integration test.

2bf7270

Finish integration test.

6f83512

Improve integ tests.

4cab783

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

7fb17e0

Add integration tests for SSL connections.

65a61d1

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

3b78437

htuch reviewed Jun 12, 2017

View reviewed changes

lizan reviewed Jun 13, 2017

View reviewed changes

Oliver Liu added 2 commits June 17, 2017 06:51

Complete SSL integration test. Fix format.

cf6e403

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

787a92f

htuch reviewed Jun 18, 2017

View reviewed changes

Oliver Liu added 6 commits June 19, 2017 07:39

Small naming fixes.

33a92b3

Update doc.

81dc115

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

b4e50b8

Small fixes.

aa7886b

Support Subject.

319aac9

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

59e659e

htuch reviewed Jun 22, 2017

View reviewed changes

myidpt changed the title ~~WIP: Implementing XFCC header~~ Implementing XFCC header Jun 22, 2017

Oliver Liu added 3 commits June 22, 2017 06:57

Add setting Subject field into XFCC.

19e22f5

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

9a60284

Small fixes.

ebf3108

lizan reviewed Jun 22, 2017

View reviewed changes

Oliver Liu added 2 commits June 23, 2017 21:27

Small fixes.

b560764

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

fd8dc74

lizan reviewed Jun 24, 2017

View reviewed changes

mattklein123 reviewed Jun 28, 2017

View reviewed changes

Oliver Liu added 2 commits June 28, 2017 22:53

Small fixes according to feedback.

89fe10f

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

9e67435

mattklein123 reviewed Jun 28, 2017

View reviewed changes

lizan reviewed Jun 28, 2017

View reviewed changes

htuch reviewed Jun 29, 2017

View reviewed changes

Oliver Liu added 2 commits June 29, 2017 07:23

Fix according to comments.

82e1c8b

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

388f0c0

mattklein123 reviewed Jun 29, 2017

View reviewed changes

htuch mentioned this pull request Jun 29, 2017

Adding a test of TCP with TLS termination (test-only change) #1183

Merged

htuch previously approved these changes Jun 29, 2017

View reviewed changes

Oliver Liu added 2 commits June 29, 2017 18:07

Small fixes.

ae6348e

Merge branch 'master' of https://github.com/lyft/envoy into xfcc

48e64c0

myidpt dismissed htuch’s stale review via 48e64c0 June 29, 2017 18:11

Small fixes.

0fe4d40

mattklein123 approved these changes Jun 29, 2017

View reviewed changes

mattklein123 merged commit b7c5077 into envoyproxy:master Jun 29, 2017

myidpt deleted the xfcc branch June 29, 2017 20:55

eoftedal mentioned this pull request Jan 16, 2018

Support XFCC and similar headers in calls from proxy to app pod istio/old_issues_repo#165

Closed



		if (connection.ssl() \|\| config.forwardClientCert() == Http::ForwardClientCertType::AlwaysForwardOnly) {
		std::string clientCertDetails = "";


		The following keys are supported:

		1. ``BY`` The Subject Alternative Name (SAN) (Common Name if SAN is not available) of the current


		.. _config_http_conn_man_headers_x-forwarded-for:

		x-forwarded-client-cert

		@@ -17,7 +17,7 @@ class Connection {
		/**
		* @return whether the connection is Mutual TLS.

Conversation

myidpt commented Jun 12, 2017 • edited by mattklein123 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myidpt Jun 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

myidpt Jun 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

myidpt commented Jun 12, 2017 •

edited by mattklein123

Loading

myidpt Jun 19, 2017 •

edited

Loading

myidpt Jun 19, 2017 •

edited

Loading