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

Expand "Binding Object" section #2

Closed
Ethan-Arrowood opened this issue Aug 18, 2023 · 6 comments · Fixed by #6
Closed

Expand "Binding Object" section #2

Ethan-Arrowood opened this issue Aug 18, 2023 · 6 comments · Fixed by #6

Comments

@Ethan-Arrowood
Copy link

Can we add some more information to the Binding Object section? I don't fully understand what is meant by it.

@dom96
Copy link
Collaborator

dom96 commented Aug 21, 2023

The reason I kept it short is because it wasn't clear to us whether we want it part of this spec or not. So I'd like to discuss it here before expanding that section.

At Cloudflare we like bindings (some examples of them here: https://developers.cloudflare.com/workers/configuration/bindings/) because they offer capability-based security, they also enable easier setup, observability and testability. A binding in the context of sockets would represent one of the following:

  • another worker - e.g. env.myWorker.connect() would create a socket which connects to that worker.
  • a pre-configured socket - e.g. env.myTlsConnection.connect("google.com:443") with the myTlsConnection binding already configuring TLS certs/keys

The bindings are configured outside the JS code (inside the worker.toml file).

CC @kentonv

@dom96
Copy link
Collaborator

dom96 commented Aug 21, 2023

Perhaps the spec could, at a minimum, define the concept of a "binding" as:

  • an object which tracks some socket configuration; and
  • provides a connect method that uses that configuration

The runtimes wouldn't necessarily need to support this at all. But libraries which offer support for this socket spec would need to offer the ability to pass in a "binding" object, which when specified is used for the connect call. As an example:

import { Client } from 'pg'
const client = new Client({
  binding: env.myTlsConnection
})
await client.connect() // calls env.myTlsConnection.connect() instead of the `connect` from the `sockets` module

@Ethan-Arrowood
Copy link
Author

Very interesting. I have mixed feeling at the moment.

Is the main purposes such that any runtime could use this as a way to make the API more extensible?

Instead, I feel like we should consider adding additional options to the connect method itself instead. Like it would make more sense to me to do:

connect('google.com:445', { tls: { cert: 'x', key: 'y', ... }});

Otherwise I fear that this api would lose its interoperability quite quickly if every runtime came up with its own TLS interface

@kentonv
Copy link

kentonv commented Aug 21, 2023

So, this has been the subject of quite a lot of debate going back years. We (Cloudflare) are all-in on bindings as a core design feature of Cloudflare Workers, but we've not yet fully explained publicly all the reasons we believe this is the best design. I'm actually working on a blog post about this. My hope is that we can convince other vendors to take this road too, because I really believe it'd be a good thing for the industry, but admittedly I don't really expect it'll be so easy to convince everyone. As such, this may be "too controversial" to standardize at this time.

The blog post I'm working on is too long to fully rehash in the context of this github thread, but let me try to highlight a couple key arguments that apply to sockets and mTLS in particular...

I feel like we should consider adding additional options to the connect method itself instead.

One drawback of the options approach is that it requires revealing the secret key to the application code. With the binding approach, the key is never revealed to the app, so it's impossible for buggy code to leak the key.

Another advantage of bindings is more abstract: There are many different ways that you might want to secure a network transport, with TLS being just one of them. It would be nice to keep these logistical details out of the application code, so that the transport mechanism can be changed without changing the code.

For example, maybe you have an application that initially uses mTLS to connect to an external service. But, at some point, that service itself is moved onto Workers, and now you have a direct service binding from Worker to Worker. There's no longer any need for mTLS authentication as Cloudflare itself can just tell you which Worker is running at the other end of the transport. Or, maybe you switch to using Argo Tunnel, a Cloudflare feature which creates a VPN-like tunnel into a private network.

The application doesn't need to know the difference between these -- in all cases, it's just a binding with a connect() method. The binding can be swapped out as a matter of configuration.

You might argue instead that perhaps the system could implicitly apply transport security settings based on the hostname you are trying to connect to. So you could configure that connections to my-private-backend.example.com will implicitly use some client certificate and key, or always go to some particular other Worker. This seems to achieve all the benefits described above, but it creates a new problem: SSRF. If the client ever tries to connect to a hostname that is provided by an untrusted user, an attacker could specify my-private-backend.example.com as the host and trick the application into exercising its credentials to invoke that service. Bindings are inherently SSRF-proof because the application must explicitly specify, in code, that it is using the privileged binding.

@Ethan-Arrowood
Copy link
Author

Great points @kentonv and I'm very excited to learn more whenever you get to publishing that full post!

I think we could keep this, but we just need to iterate a little bit. I think maybe if it was earlier in the spec it would make sense.

As I am interpreting the spec currently, an implementation would consist of a global connect method and thats sorta it. If we adjust the spec to dictate that connect is a method that can (or maybe should) exist on some type of higher-level interface, then it would be so much clearer from the beginning how a runtime should reasonably implement this api

@Ethan-Arrowood
Copy link
Author

And maybe it could even specify the responsibility of this higher level interface (like tls options).

The spec could still specify that the method could exist standalone, but that runtimes may choose to include the method as apart of other interfaces.

And better yet it could take on some level of OOP where other interfaces could extend or implement the Socket API Interface that contains the definition for connect.

I'm just throwing some ideas around; i think theres an improvement to be made here, just not entirely sure what it should be yet.

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

Successfully merging a pull request may close this issue.

3 participants