feat(runtime/tls): Add support for client certificates to connectTls

[erik]

Allows specifying a client certificate for mutual TLS in calls to Deno.connectTls().

await Deno.connectTls({
  hostname: "example.com",
  port: 443,
  certChain: "-----BEGIN CERTIFICATE-----\n...",
  privateKey: "-----BEGIN PRIVATE KEY-----\n...",
});

Some implementation notes:

• The existing ConnectTlsOptions parameter certFile is confusing when combined with client certificate param certChain. Should we pull them into a separate object? Prefix the names with client? Open to suggestions.
• The certificate chain and private key are passed in as a string rather than a file path for flexibility (as discussed in Allow Setting SSL Certs via string instead of just File Path #5810). This is inconsistent with the behavior of the existing certFile param.
• I couldn't think of a great way to unit test that the certificates are actually set properly on the underlying connection, since Deno.Conn has no way of exposing the peer's client certificate from the other end of the connection.

Partially implements #6170

[CLAassistant]

All committers have signed the CLA.

[bnoordhuis]

Thanks for the PR. The changes LGTM in themselves but another maintainer should make the call on the API.

I personally think the file-based API is kind of clunky but it's precedent. It also has going for it that key data doesn't go through JS land.

[lucacasonato]

We have already started moving away from the file based APIs for new unstable APIs like Deno.createHttpClient(), so I think there is at least some precedent here.

[bartlomieju]

We have already started moving away from the file based APIs for new unstable APIs like Deno.createHttpClient(), so I think there is at least some precedent here.

Agree with Luca. Additionally Deno.connectTls is a stable API, so we need to be extra careful when changing the interface (that means reading cert from file should still be supported). We should follow the same convention as in Deno.createHttpClient() for the field names.

[lucacasonato]

that means reading cert from file should still be supported

Why? We don't support it for Deno.createHttpClient(). I think we should deprecate reading certificates from files for 2.0.

[bartlomieju]

that means reading cert from file should still be supported

Why? We don't support it for Deno.createHttpClient(). I think we should deprecate reading certificates from files for 2.0.

Yes, for 2.0, but for now it should still be supported.

[bartlomieju]

We have already started moving away from the file based APIs for new unstable APIs like Deno.createHttpClient(), so I think there is at least some precedent here.

Agree with Luca. Additionally Deno.connectTls is a stable API, so we need to be extra careful when changing the interface (that means reading cert from file should still be supported). We should follow the same convention as in Deno.createHttpClient() for the field names.

Disregard this comment, I misunderstood what this PR does.

Implementation looks good to me, but let's put it in the unstable namespace for now.

[erik]

Thanks for the quick review all!

@bartlomieju updated, let me know if the refactor looks right

[erik]

I decided not to use Deno for the project this was relevant to, so I'll be closing out the PR

If someone else decides to pick this up later, the implementation works fine as is, but integration tests aren't working on GitHub CI for reasons I couldn't figure out. They pass reliably locally. Probably something simple

[lucacasonato]

@erik No worries.This PR is 99% there, just need to get the tests working in CI. I will pick this up for the 1.9 release. Thanks for the effort!

[justinmchase]

I will pick this up for the 1.9 release.

I'm currently using 1.11.5 and it seems as if this isn't yet available, is there an update on this? I'm trying to use the kubernetes api which in some cases require a client certificate for authentication.

[justinmchase]

Related To #10516

[cryptographix]

@lucacasonato any update on this change? Just spent a couple of hours writing a node mTLS proxy server to sit behind deno, but would prefer to do everything with deno. Can I help with getting this PR into the next release?

[ry]

@seanwykes I've rebased this patch and opened a new PR in #11598 - we'll try to get it in for 1.13.0 (which comes out next week)