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

Support for custom TLS ServerName or VerifyPeerCertificate function #2779

Open
mmailhos opened this issue Nov 19, 2022 · 2 comments
Open

Support for custom TLS ServerName or VerifyPeerCertificate function #2779

mmailhos opened this issue Nov 19, 2022 · 2 comments
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature new-http issues that would require (or benefit from) a new HTTP API

Comments

@mmailhos
Copy link

mmailhos commented Nov 19, 2022

Feature Description

Problem

The k6 client targets internal services which don't reply with the same x509 CN/SAN domain as the one passed in the HTTP Request. This internal domain can not be resolved by the k6 client, who has to use the load balancer address.
This leads to a domain mismatch between the request and the response, and a failing TLS peer verification.

Screen Shot 2022-11-28 at 11 14 35 am

I have seen TLSAuth feature but it seems to be used to assign the right certificate to the connection, I don't think it would help in this case as the certificate CN would still mismatch. Besides, as documented in the code, it is going to be deprecated in the next release or so.

Suggested Solution (optional)

Solution 1: Override ServerName

The first obvious, and most straightforward solution is to override tls.Config.ServerName in the request.

We can potentially do that by providing a new k6 Option to override ServerName for specific TLSAuth.Domain.

I would imagine the user interface such as:

export const options = {
  tlsServerName: { 
       "foo.us-east-1.aws.com": "foo.internal",
    },
};

Solution 2: Let the user pass a custom TLS Verifier via an xk6 module

In short, a custom tls.Config{} can be passed.

For example, this lets users write their own tls.Config.VerifyPeerCertificate() function that best matches their infra setup.

I can imagine other scenarios than this one where users would need to override more parameters of tls.Config.
The domain can be used to properly map the module to the right http.Transport connection.

While surely a little bit more complicated, it is also the most flexible solution.

Already existing or connected issues / PRs (optional)

No response

@na-- na-- added evaluation needed proposal needs to be validated or tested before fully implementing it in k6 new-http issues that would require (or benefit from) a new HTTP API labels Nov 21, 2022
@na--
Copy link
Member

na-- commented Nov 21, 2022

Hmm, from what I know, isn't it the usual practice that the HTTPS connections terminate on the load balancer? The communication between it and the service can then be handled separately, often done over plaintext HTTP, if the service and load balancer are in the same network 🤔

In any case, the tlsAuth option is not going to be deprecated, but it is meant for client-side certificates, so it's not suitable for solving this issue in its current form.

Adding something like a new global k6 option tlsServerName as you suggested is probably not an option at this point in time either. Our configuration is already quite complex (#883) and global options like this are quite inflexible and often problematic. It is much more likely that we'd refactor the k6/http code so it's a bit more flexible and modify-able from xk6 modules, i.e. your proposed solution 2. Something like this is already the goal, eventually, see #2293 (comment). As for a purely JavaScript solution, #2461 would be my preferred way to avoid new global k6 options.

However, there might be an easier way to implement this that won't introduce a new k6 option or wait for a completely new HTTP client API. I think we can just expand the already existing hosts option in a very natural way. It currently expects a single ip[:port] value for every host override, though we already want to support multiple values as well (#2246). And I don't see why we can't essentially support the equivalent of CNAME records in hosts, i.e. one domain to substitute for another one 🤔 Then, you should be able to do something like this:

export const options = {
  hosts: { 
       "foo.internal": "foo.us-east-1.aws.com",
    },
};

export default function() {
  http.get("https://foo.internal");
}

There would still be the matter of trusting self-signed foo.internal certificates, though that is a somewhat separate issue. When the domains match, the CA cert can be added to the local OS/container trust store or something like that. A separate option/API for adding trusted CAs is potentially also on the table, though I'd prefer this to be a separate JS Dialer object which can be configured and passed to a Client, or something else that is similar to how Go does it and probably done as a part of #2461 🤔

For now, until any of these things are actually done, you (and anyone else that stumbles on this issue) can use the insecureSkipTLSVerify option. This will cause k6 to still use TLS, but to not verify the TLS connection it makes, which is generally not a problem for running load tests. It's not ideal, but it's the only workaround for your current setup where the publicly advertised TLS certificate is a self-signed one for the private domain.

This setup still seems quite unusual to me, so if anyone else has the same or a similar problem, please chime in this issue and explain your use case 🙏 This will help us to understand what the priority of this is and what the various use cases we might need to cover are.

@mmailhos
Copy link
Author

mmailhos commented Nov 28, 2022

Hmm, from what I know, isn't it the usual practice that the HTTPS connections terminate on the load balancer? The communication between it and the service can then be handled separately, often done over plaintext HTTP, if the service and load balancer are in the same network 🤔

Actually, (sorry poor wording), the Load Balancer terminates TLS. The certificate CN just mismatch the requested Host.

Adding something like a new global k6 option tlsServerName as you suggested is probably not an option at this point in time either. Our configuration is already quite complex (#883) and global options like this are quite inflexible and often problematic.

Got it, thanks for all the details 👍!

It is much more likely that we'd refactor the k6/http code so it's a bit more flexible and modify-able from xk6 modules, i.e. your proposed solution 2. Something like this is already the goal, eventually, see #2293 (comment). As for a purely JavaScript solution, #2461 would be my preferred way to avoid new global k6 options.

That makes sense. I personally like the xk6 module option as it abstracts away low-level complexity to the developer and prevents some misconfiguration. It helps to keep the load test script to a minimum. I'd also imagine that any Go logic would run faster than if written in JS, especially if there is some kind of crypto operation or extra network call involved (eg OCSP support).
Eventually, the JS options can override any module's behavior, but, in my use case, I'd rather not expose that directly.

However, there might be an easier way to implement this that won't introduce a new k6 option or wait for a completely new HTTP client API. I think we can just expand the already existing hosts option in a very natural way. It currently expects a single ip[:port] value for every host override, though we already want to support multiple values as well (#2246). And I don't see why we can't essentially support the equivalent of CNAME records in hosts, i.e. one domain to substitute for another one 🤔

In my case, foo.internal is not resolvable by the client so I can't initiate the connection with foo.internal.
Only the Load Balancer DNS can be resolved. Since all our services are using the same stack, they all implement their own TLS checker (which is a missing component when initiating connections from k6).

This setup still seems quite unusual to me, so if anyone else has the same or a similar problem, please chime in this issue and explain your use case 🙏 This will help us to understand what the priority of this is and what the various use cases we might need to cover are.

Yes, it's a bit unusual 😅 . All things considered, I'll be looking at updating the Load Balancer certificate to include Subject Alternative Names, which should fix the hostname mismatch and make the infra setup a bit more standard 👍 I will also be using TLS Skip Verify for now. Thanks a lot for your help and all the details again, that should solve my issue!

Last 2cents, I think having the ability to customize the TLS Config and the HTTP Client or Transport can be helpful to developers who might have to deal with non-standard infrastructures and proxies. The module approach looks best as it lets users add their own functions (say tlsConfig.GetCertificate or VerifyPeerCertificate), which can't be passed via Options. It would also solve the issue of trusting new CA certificates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evaluation needed proposal needs to be validated or tested before fully implementing it in k6 feature new-http issues that would require (or benefit from) a new HTTP API
Projects
None yet
Development

No branches or pull requests

2 participants