Client certificate validation (mTLS) design proposal

[uablrek]

Implementation proposal for #1090

See also PR #1226

[davecheney]

Thank you for breaking this out. My overall feedback is this proposal is missing the mark. What I believe you want is to be able to use XFCC to secure, or authenticate (its' not clear to me), communication between a remote user and a backend in a k8s cluster.

If this is the problem statement I suggest starting from that as a proposal.

[davecheney]

You lost me here. The previous section talked about XFCC being between the end user and and backend application server. Now this is talking about a change to the tls settings for the whole virtual host. Is this a problem with the problem statement? Is this a security issue? If so what is the threat model? Is this an authentication issue? It's not clear to me at the moment.

[uablrek]

I think you may be right. I will remove the XFCC header from the proposal since it may be confusing. It can be handled separately later on in another issue.

[stevesloka]

Hey @uablrek, just taking a look here as well.

I'm curious about @davecheney's comment earlier about if this is a vhost setting or a per route setting. Could I have each route in my cluster use a different cert based upon the upstream it's calling?

[uablrek]

@stevesloka I asked the very same question in the Envoy slack 😄 The ansver is; No client certificate validation is per vhost, not for routes. The reason is that the route is in the data, but the client validation is performed before data is decrypted.

[uablrek]

For the XFCC header on the other hand it seems possible to forward different certificate details to different routes. But as @davecheney commented it is much better to add XFCC support as a different item, so I do not mention it in the design document any more.

[stevesloka]

Ok so to clarify, is this looking to mTLS a public client calling into Envoy with a backend service? Or to secure communication from Envoy to a backend service inside the cluster?

[uablrek]

This is not about cluster internal Envoy-backend communication. It is mTLS used to validate an external client accessing the service through contour/envoy from outside the cluster.

[davecheney]

I'd like to see an issue for something like XFCC should be supported by Contour. If that's the actual goal of this proposal then lets agree on that first before getting side tracked into implementation issues.

[uablrek]

Please see the comment above

[uablrek]

To clearify; the XFCC header is not necessary for mTLS. It is a way of informing the backend that client validation has been conducted by the proxy with a certain certificate.

So, XFCC is informational data to the backend, it has nothing to do with security.

[uablrek]

More about XFCC; Different client certificates may be used, e.g an "admin" certificate or "user", having different rights. Then it is not sufficient for the server that the client is validated as any client, the server must know which client that is validated. Then the XFCC is used to forward the necessary client certificate details.

Not all applications needs client certificate details, only those who uses different types of client certificates, and the way of forwarding client certificate details is not standard and varies between proxies which of course makes it hard for applications

[uablrek]

@davecheney Can you please re-check the document?

[davecheney]

I’m sorry I ran out of time this week to reply. I’ll try to review on Monday.

…

On 11 Jul 2019, at 18:12, Lars Ekman ***@***.***> wrote: @davecheney Can you please re-check the document? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[stevesloka]

It would be great if you could add a top-level summary of the feature. Just a quick explanation of what you're looking to accomplish and some use-cases.

I'm also curious about the placement of the new ClientValidation struct (See comment).

[stevesloka]

Can only one type be specified? Envoy allows multiple, but I'm not sure of the use-cases that would drive picking multiple.

When both: verify_certificate_hash and verify_certificate_spki are specified, a hash matching value from either of the lists will result in the certificate being accepted.

[uablrek]

Me neither to be honest. I rely on that Envoy knows what they are doing and try to allow all Envoy configurations for mTLS also in contour. That is also a reason for the non-goal;

Repeat configuration details described in Envoy documentation (make references)

I will pass the question on to a security expert, but they tend to be busy...

[tsaarni]

I think using multiple types makes most sense in combination of a trusted_ca and verify_subject_alt_name. In this case the administrator wants to uniquely authenticate a particular client(s) before allowing them to establish TLS connection. For example a client with identity "CN=Joe" issued by certain trusted CA (or a sub-CAs of the trusted CA). This is by far the most common use case for client certificates in general. However, like discussed in PR comments, in some cases the specific client identity might not matter and one would then specify trusted_ca alone.

Combination of verify_certificate_spki and verify_certificate_hash does not make sense to me, since it is likely that administrator picks just one way to pin a specific certificate: either administrator wants to identify client(s) by hash of public key or hash of the complete certificate file, not both at the same time. Since this is an act of whitelisting a specific client certificate, it would be bit redundant or at least confusing, to use trusted_ca in combination with either of these two methods. When a client identified by a specific cert is explicitly marked as allowed to establish a TLS connection, validating that it is also issued by a trusted CA should not generally matter (though I have not tested if this combination is supported by Envoy). Obviously Envoy will still require the client to demonstrate proof-of-possession of the private key associated to the client certificate, in addition to validating the hash of the public part.

In theory one could imagine a use case where some clients are allowed to establish a connection because they are issued by trusted_ca and others because they are specifically pinned/whitelisted by their verify_certificate_spki and verify_certificate_hash fingerprint but I don't think this is very common scenario.

[uablrek]

@tsaarni I have ignored verify_subject_alt_name since I did not understand it's use. From your comment I see it's important. Can the configuration look something like this?;

      clientValidation:
        secretName: clientsecret
        subjectAltNames:
          - Joe

The list of subjectAltNames would be passed to Envoy in the verify_subject_alt_name field.

[uablrek]

Envoy doc; https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/auth/cert.proto.html?highlight=require_client_certificate#auth-certificatevalidationcontext

[tsaarni]

Correction to my comment: My example "CN=Joe" (X509 Common Name field) was for subjectName and subjectAltName would rather be something like email "joe@example.com", instead of just "joe". Obvious from documentation, Envoy only supports validation of subjectAltName field.

One complication is that X509 subjectAltName is not just free text: there is type involved (see list here and definition of GeneralName in RFC5280. However Envoy documentation does not seem to specify what format it supports?

I have small doubt that maybe it just supports URIs since this feature might be added to Envoy due to Istio, which uses subjectAltName in the SPIFFE certs and it leaves subjectName empty. I might try to find this from envoy code.

[uablrek]

Ok thanks for the update. I will change from "Joe" to an URI, "service.example.com".

[tsaarni]

Just out of curiosity I checked what types of subjectAltName's Envoy supports as values of verify_subject_alt_name. The code is here. The value can be DNS name, URI or IPv4/6 address.

[uablrek]

@stevesloka

I have improved the "Background" chapter. Please notify me if you think it should be more.

We want to use contour but a stopping issue is lack of support for mTLS. A proposal is to use istio as it also uses Envoy and have mTLS support, but I would not like to go that way. I do not know the use cases in our product that require mTLS, just that they do.

[uablrek]

@davecheney @stevesloka Can you please take another look at this after the updates?

If the proposal is acceptable I can continue with the implementation.

[tsaarni]

Here are some common example scenarios for protecting REST APIs:

• Elasticsearch did not support authentication, until very recently. The common pattern was to put reverse proxy in front of Elasticsearch to handle TLS and authentication
• Prometheus does not support either TLS or authentication. Common patter is to put reverse proxy in front.

[uablrek]

Thanks!

[stevesloka]

I'd like to challenge this statement: "I can't give a real-life example but it is easy to imagine cases".

You're looking to add this feature, so there's a need to have this, right? We should be adding real-life use-cases to back up the proposal, without those, then I question why we are adding this feature.

[uablrek]

I have updated the examples with the ones in the comment above #1233 (comment)

[stevesloka]

What is the same? How communication is secured? How secrets are named?

[uablrek]

They are named, used and created in the same way.

I removed the reference and copied the chapter from tls-backend-verification.md instead to make it clear.

[uablrek]

Communication is secured with TLS (encryption), same as always. mTLS is just a way to limit the access to the server to validated clients. You may see it as an "add-on" to normal TLS, not a replacement.

[uablrek]

@stevesloka @davecheney Can you please have another look at this?

I have added use-cases as requested.

Please note that mTLS is not something I have invented myself and try to impose on contour. mTLS is a well established standard for client authentication over TLS, and it has support in envoy. I just would like to make it available for contour users since we need client authentication for some REST interfaces.

[uablrek]

Here is another use-case, client validation using mTLS for helm-charts with an example fow to configure the nginx ingress controller for mTLS; https://hub.helm.sh/charts/drgrove/mtls

[tsaarni]

I would like to continue with this PR (which we agreed with @uablrek). The design doc now only contains proposal for validating external client certificate with trusted CA certificate, all other proposals were removed.

I'd be grateful for any feedback!

[youngnick]

I think this document is ready now. It's well scoped, has a clear plan, and is implementable pretty much straight away.

I'd like to see the commits squashed before merging though.

[youngnick]

Oops, I also missed that CI hasn't passed, looks like there's a misspelling in the document. @tsaarni, could you run make check in your local repo and fix any errors? Once the CI is green and @davecheney has given the 👍 , then I'll squash and merge for you.

[davecheney]

Thank you for working on this. I think this is very close. In addition to some formatting nits, I would like to see any reference to mTLS removed -- that word is cursed, different people use it to mean different things so avoiding it completely will make the document more precise.

Other than that this looks pretty straight forward.

[davecheney]

s/(mutual TLS authentication)//

[tsaarni]

I have now removed references to mutual TLS authentication and mTLS.

[davecheney]

Do not wrap lines. One line per sentence please. This makes it easier to comment a specific line, and reduces the diff size for comments.

[tsaarni]

I have now reformatted the document accordingly.

[davecheney]

Please remove all the words about mutual TLS authentication, as I understand it this proposal is only about client side certificate authentication. If you want to add mutual TLS as a non goal, please do so in the previous section.

[tsaarni]

I have now removed references to mutual TLS authentication and mTLS.

[davecheney]

What is the justification for a new key, do you think we'll have to add additional keys under clientValidation in the future?

[tsaarni]

I wish that in future we could add verification of subject alternative name of the client (option 2. in the list above).

[jpeach]

We should use UpstreamValidation (with a new name) to describe both upstream and downstream TLS validation policy.

[davecheney]

remove newline

[tsaarni]

Done!

[davecheney]

remove newline

[tsaarni]

Done!

[davecheney]

remove newline

[tsaarni]

Done!

[davecheney]

remove newline

[tsaarni]

Done!

[tsaarni]

Thanks @davecheney and @youngnick for the review! I have now corrected the document accordingly. If there is further comments, I'll gladly do updates!

[tsaarni]

I wonder what your thinking is about this feature currently: Would it be something that could be accepted? Is there some changes or investigations that I could do?

[jpeach]

On 23 Jan 2020, at 3:33 am, Tero Saarni ***@***.***> wrote: I wonder what your thinking is about this feature currently: Would it be something that could be accepted? Is there some changes or investigations that I could do?

This is approved, so I'll do a final review pass and land it in the next couple of days.

[jpeach]

LGTM, with the proviso that in the final implementation, the upstream and downstream validation fields should use the same struct (i.e. the one currently named "UpstreamValidation").