Skip to content

Initial interface #1

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

Merged
merged 4 commits into from
Jan 25, 2025
Merged

Initial interface #1

merged 4 commits into from
Jan 25, 2025

Conversation

badeend
Copy link
Collaborator

@badeend badeend commented Jan 23, 2025

Aside from the boilerplate, this adds types.wit.

This should be the bare minimum interface to set up client TLS connections within a WASI guest.

Compared to the PR over at the wasi-sockets repo, this PR:

  • has merged the client-connection::constructor and client-connection::connect methods into the client-handshake::constructor, and
  • the client-connection is now returned asynchronously by the client-handshake::finish method.

This decreases the API surface area and removes a chance of user-error thanks to the introduced typestate.

The general process now looks like:

  • Call client-handshake::constructor
  • Configure handshake to your liking. In this PR there's nothing to configure yet.
  • Call and await client-handshake::finish
  • Use returned IO streams and client-connection as usual.

Next steps, for future PRs:

  • Add documentation
  • Add proper error handling

Copy link
Collaborator

@dicej dicej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

subscribe: func() -> pollable;

@unstable(feature = tls)
get: func() -> option<result<result<tuple<client-connection, input-stream, output-stream>>>>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine we'll want to define a variant error type with cases covering at least the most common failure cases (e.g. expired certificate, hostname mismatch, etc.) and use it here, eventually.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, definitely. 👍

I wasn't in the mood to do a full inventory of all the ways a TLS connection may fail just yet, so I lazily wrote it off as a problem for future me 😅

}

@unstable(feature = tls)
resource client-connection {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing we will need eventually for dotnet is the server-identity: func() -> option<public-identity>;. After accepting the connection dotnet SSL client runs through a second round of server certificate validation.

@jsturtevant
Copy link
Collaborator

Another thing that we are using the .net implementation is the make-pipe: func() -> tuple<input-stream, output-stream>;

Should that live here or wasi:io/streams? This will go away as we look towards wasi 0.3?

Copy link
Collaborator

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@badeend
Copy link
Collaborator Author

badeend commented Jan 25, 2025

@jsturtevant Replying to your server-identity & make-pipe dependency simultaneously: for the purposes of this PR, they seem to be purely additive to the initial interface, so I'm going to merge this one. I'd be glad to continue discussing the needs of .NET outside of this PR. Hit me up on Zulip, a video call, a Github issue, or however you want.

Should that live here or wasi:io/streams?

Ideally we shouldn't need this at all. Second choice: add it to wasi-io. But lets discuss further.

This will go away as we look towards wasi 0.3?

Yes, this is v0.2-only.

@badeend badeend merged commit fc33a59 into WebAssembly:main Jan 25, 2025
1 check failed
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 this pull request may close these issues.

3 participants