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

TLS and Connection information through headers #280

Closed
hkolk opened this issue May 1, 2017 · 5 comments
Closed

TLS and Connection information through headers #280

hkolk opened this issue May 1, 2017 · 5 comments
Milestone

Comments

@hkolk
Copy link
Contributor

hkolk commented May 1, 2017

In our setup, we are terminating TLS traffic on Fabio. This means that Fabio is the main/only place where we know which protocol was used.

My suggestion is to add Info headers to the backend request, to facilitate logging within the application.

Current concept:

Fabio-Tls-Version: TLS12
Fabio-Tls-Cipher: 0xcca9
Fabio-Http-Protocol: h2

Because these are not really universal, I prefer to prefix them with Fabio-. The ID used for the cipher is from the IANA list:
https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml
https://golang.org/src/crypto/tls/cipher_suites.go#L368

For enabling/disabling these headers, I propose the proxy.header.info.enabled as a boolean, defaulting to true.

Later on, we can add additional informational headers, such as what Cloudflare has done with the CF-0RTT-Unique header for TLS1.3

I have the commits for this on my cloned branch. I'll create a pull request in a couple of days based on the input from this issue

@magiconair
Copy link
Contributor

magiconair commented May 1, 2017

Since these are not fabio but protocol specific headers I'd prefer to drop the Fabio- prefix. Just use Tls-Version, Tls-Cipher and Http-Proto since the X-Forwarded-Proto also doesn't spell out Protocol. In general, I prefer the header names to be configurable to provide mechanisms for avoiding conflicts with existing setups. Disabling the header could then be achieved by setting them to empty values. However, then three parameters would have to be configured to fully disable them.

I'm reluctant for a general info.disabled flag since info is too general. All the X-Forwarded-* and X-Real* headers also contain information. What qualifies a header as an info header?

Opinions welcome.

@magiconair
Copy link
Contributor

@hkolk What if we add the fields to the Forwarded header instead of introducing new headers? It is already provided and contains information about the connection. We can just add some fields, e.g. tlsver, tlscipher and httpproto?

magiconair added a commit that referenced this issue Jun 6, 2017
This patch adds an 'httpproto' field which contains the lower case
version of the HTTP protocol to the Forwarded header of all upstream
requests.

When fabio terminates the TLS connection two additional fields 'tlsver'
and 'tlscipher' are added to the Forwarded field. 'tlsver' contains the
TLS version as 'tlscipher' the chosen cipher suite. The cipher suite is
formatted as a hex number from
https://golang.org/pkg/crypto/tls/#pkg-constants.

Fixes #280
@magiconair
Copy link
Contributor

@hkolk I've pushed a change which adds the fields to the Forwarded header as described. Could you check whether this works for you? If possible, I'd like to merge this to next release this week.

@magiconair
Copy link
Contributor

Example:

Forwarded: for=1.2.3.4; proto=https; httpproto=http/1.1; tlsver=tls10; tlscipher=0xc023

magiconair added a commit that referenced this issue Jun 7, 2017
This patch adds an 'httpproto' field which contains the lower case
version of the HTTP protocol to the Forwarded header of all upstream
requests.

When fabio terminates the TLS connection two additional fields 'tlsver'
and 'tlscipher' are added to the Forwarded field. 'tlsver' contains the
TLS version as 'tlscipher' the chosen cipher suite. The cipher suite is
formatted as a hex number from
https://golang.org/pkg/crypto/tls/#pkg-constants.

Fixes #280
@magiconair magiconair added this to the 1.5.0 milestone Oct 10, 2017
@taemon1337
Copy link

Don't forget the TLS client authentication information like Common Name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants