Skip to content

Conversation

pallas
Copy link

@pallas pallas commented Sep 15, 2025

I verified that this works in a dependent C program using rustls-ffi. Open to feedback, but please note that I am not familiar with idiomatic rust, having never written in the language before.

Add rustls_connection_ktls_secrets, which consumes rustls_connection to generate tx and rx secrets for kTLS. Secrets are borrowed and passed to a callback.

static rustls_io_result
ktls_secrets_callback(void *userdata,
    const uint8_t *rx_buf, size_t rx_n,
    const uint8_t *tx_buf, size_t tx_n)
{
    int result;
    int fd = *(int*)userdata;

    result = setsockopt(fd, SOL_TCP, TCP_ULP, "tls", 4);
    if (result < 0) return result;

    result = setsockopt(fd, SOL_TLS, TLS_RX, rx_buf, rx_n);
    if (result < 0) return result;

    result = setsockopt(fd, SOL_TLS, TLS_TX, tx_buf, tx_n);
    if (result < 0) return result;

    return 0;
}
int fd /* = ... */;
struct rustls_connection * connection /* = ... */;

while (rustls_connection_is_handshaking(connection)) {
    /* process reads and writes */
}

while (rustls_connection_wants_write(connection)) {
    /* flush outbound packets */
}

rustls_result result = rustls_connection_ktls_secrets(connection
    ktls_secrets_callback, &fd);

In order to use rustls_connection_ktls_secrets, secret extraction must be enabled via rustls_*_config_builder_set_enable_secret_extraction.

@pallas pallas force-pushed the implement-ktls branch 5 times, most recently from fc580fb to f05e9d6 Compare September 15, 2025 20:33
@pallas
Copy link
Author

pallas commented Sep 15, 2025

I'm really sorry for all the noise.

@cpu
Copy link
Member

cpu commented Sep 15, 2025

I'm really sorry for all the noise.

No worries, I won't personally have a chance to peek at this PR for a bit so churn on that front is no biggie. However, since you haven't contributed to the repo before the CI runs have to be approved manually, and that's likely a drag on your end.

If you want an easier way to iterate without updating this PR you could rename your branch with a ci/ prefix in the name and push it to your fork (after enabling actions for your forked repo). Those pushes will run without needing approval or pinging this PR. When stuff is passing you can rename back to implement-ktls without the prefix and force push to update this PR and someone just has to approve it that one last time.

@pallas
Copy link
Author

pallas commented Sep 15, 2025

Will do. Just one question though: Do you know how to gate importing ktls in Cargo.toml on being on a platform that can use it, i.e. unix but not MacOS? I'm trying to figure it out, but would love a pointer.

@pallas
Copy link
Author

pallas commented Sep 15, 2025

@cpu
Copy link
Member

cpu commented Sep 15, 2025

Do you know how to gate importing ktls in Cargo.toml on being on a platform that can use it, i.e. unix but not MacOS? I'm trying to figure it out, but would love a pointer.

Hmmm. I think you have the syntax right for adding a platform-conditional dependency in Cargo.toml. I think what's happening is that your CMake.options diff is setting the ktls option to default on, which means the MacOS/Windows builds in CI done through cmake are appending ktls to their cargo --features. That means the rustls_connection_ktls_secrets() fn you have in connection.rs is trying to build, but since the Cargo.toml config didn't add the dep for that platform, it finds the symbols from the ktls:: crate unresolvable.

I think you want to deactivate to activate the ktls cargo flag by default through cmake (generally since it's not a great default IMO, and especially for unsupported platforms)

Add `rustls_connection_ktls_secrets`, which consumes `rustls_connection` to
generate tx and rx secrets for kTLS.  Secrets are borrowed and passed to a
callback.

```c
static rustls_io_result
ktls_secrets_callback(void *userdata,
    const uint8_t *rx_buf, size_t rx_n,
    const uint8_t *tx_buf, size_t tx_n)
{
    int result;
    int fd = *(int*)userdata;

    result = setsockopt(fd, SOL_TCP, TCP_ULP, "tls", 4);
    if (result < 0) return result;

    result = setsockopt(fd, SOL_TLS, TLS_RX, rx_buf, rx_n);
    if (result < 0) return result;

    result = setsockopt(fd, SOL_TLS, TLS_TX, tx_buf, tx_n);
    if (result < 0) return result;

    return 0;
}
```

```c
int fd /* = ... */;
struct rustls_connection * connection /* = ... */;

while (rustls_connection_is_handshaking(connection)) {
    /* process reads and writes */
}

while (rustls_connection_wants_write(connection)) {
    /* flush outbound packets */
}

rustls_result result = rustls_connection_ktls_secrets(connection
    ktls_secrets_callback, &fd);
```

In order to use `rustls_connection_ktls_secrets`, secret extraction must be
enabled via `rustls_*_config_builder_set_enable_secret_extraction`.
@pallas
Copy link
Author

pallas commented Sep 15, 2025

Ok, I turned off the feature by default and it seems to be gated correctly for non-Linux.

@cpu
Copy link
Member

cpu commented Sep 16, 2025

Thanks for the PR and the continued work on the CI front. I think I'd prefer to see the changes supporting secret extraction separated into its own PR if you're amenable to that. I think it'll be easier to review and less controversial and we can probably land that pretty quickly.

Happy for other maintainers to weigh in, but personally on the kTLS front I'm not super enthusiastic about exposing rustls/ktls through this crate. I think there are a few reasons it feels unappealing:

  • It means a new cargo feature, which is generally pretty painful (and something we're moving away from upstream in rustls)
  • I think we might want to be careful to not become a catch-all FFI API for a variety of rustls ecosystem crates if possible. We've not been 100% pure about that in the past, for example exposing parts of rustls-platform-verifier, but in that case I think the value-add is much clearer.
  • kTLS is Linux specific, and so far, we've managed to make this crate more or less platform agnostic.
  • There's no end-to-end test coverage in-repo, so it's tricky to assure we won't break it going forward.
  • It pushes up our MSRV (though we could probably say MSRV doesn't apply to that feature).
  • It feels like the rustls kTLS crate is still very early in its design/lifecycle (e.g. see the significant rewrite being proposed in Reimplement library on top of rustls::kernel API (continuation of #61) ktls#62).
  • The number of active contributors/reviewers for rustls/ktls is even lower than this repo.

My thinking is that this crate should retain the necessary bits to enable kTLS (like Rustls does upstream), but the FFI for kTLS should be handled in a separate crate (like how kTLS is in rustls/ktls).

WDYT?

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.

2 participants