unbuffered: B: CapacityBuffer for output_tls.capacity()

[pinkforest]

Checklist

• I've searched the issue tracker for similar requests

TL;DR - This might be long so maybe I can / should write RFC given 🕳️ 🐰 ?

Is your feature request related to a problem? Please describe.
TL;DR Many growable and fixed size - e.g. heapless Vec - buffers need to be explicitly pre-filled to some / full capacity.

In my test impl - I need to fill to some predicted initial capacity because unbuffered API behind the scenes uses len() on output_tls - the same holds true for heapless::vec::Vec which needs memset before copying data in.

On my own use-case I use BytesMut for io_uring where the difference mainly is around owned / shared ownership esp in tokio-uring between kernel / user.

I've ended up implementing the unbuffered rustls io_uring shim for fun in:

• imap4spec/clients/yaws-client-imap4/src/tls/rustls_io_uring.rs

It would be arguably true and correct to use len() for non-mutable slice form to determine it's true capacity.

And same can be said for upholding the "I'm not going to allocate here" -

Nonetheless rustls output_tls is mutable slice on which using len() for doing a capacity check may confuse users given they think they are passing a mutable thing to it which has .capacity().

This leads to problems where it was coaxed into it's wanted form as mutable slice which has no capacity() and where as the underlying maybe-growable type has it.

Not only that but many types coax into &[u8] implicitly without explicit into() e.g. my io_client.buf_out supplied into encrypt() as ref happily passes as &mut [u8] without any coaxing - same happens with other buffer-like types, e.g. Vec

Another potential footgun is where underlying buffer that gets coaxed into slice has other data in there one may end up sending the data behind the encoded data given it doesn't zero the rest of the mutable slice that was passed to it - or re-size it given there is no discard_out similar to input buffer handling and which may go unhandled when sending the whole buffer out when re-using.

For the growable types - both footgun and - a nice feature - whether in heap or in stack - they can grow to OOM / stack overflow - but they also typically provide capacity()

This capacity() can also be in some types where everything is pre-allocated or where the allocation is finely controlled e.g. heapless's Vec among others.

Real problem with existing code is behind the API where it uses len() internally to check the supposed capacity of the underlying slice.

tokio-uring has IoBuf which implements it's specific IoBuf trait for foreign types over &'static [u8] + IoBufMut for:

• impl IoBuf for Vec<u8>
• impl IoBuf for Bytes - with cfg(feature = "bytes")
• impl IoBuf for BytesMut - with cfg(feature = "bytes")

It also has owned Slice because io_uring needs the ownership.

This leads to a minor friction with allocating in the middle for the unbuffered API when handling the ConnectionState

This may end up to situations where one may need to fill up megabytes+ of data leading to zeroization and then encoding in-place for WriteTraffic - though application protocol would be to blame for doing that - true that as well.

I currently need to do special handling in the middle for these follow-up handlers:

• EncodeTlsData - pub fn encode(&mut self, outgoing_tls: &mut [u8]) -> Result<usize, EncodeError>
• WriteTraffic - encrypt and queue_close_notify

Another problem is that many heapers / growables buffers coax readily into &[u8] and leads to confusion when there is error about buffer not having capacity - but it's minor friction to go over and read what the underlying code did and discover it does len() and not capacity() - which is probably mostly problem of $[u8] type not having capacity() / the nature of it.

Describe the solution you'd like
It would be nice if Rustls creates it's own trait for some kind of Rustls relevant Buffer type which has capacity that can be passed as generic to these implementations.

This trait could be implemented for several known types and other crates could have foreign implementations which require rustls where special buffer handling is of essence, e.g. in io_uring case.

Most ideally this trait could be "isolated" within a separate crate to let other implementers to follow about it without necessary pulling the whole rustls for use-cases where given implementer crate only implements underlying buffer but not other parts of rustls.

I would be more than happy to send a PR if you would be amenable to something like this or something else which potentially could maybe reduce the need of zero-initialization and ensuring the output is sized correctly for sending out.

If this is not feasible then I could send a PR to document this perhaps to ensure nobody else steps into this little footgun I did.

There are also considerations around if something like MaybeUnunit could be used but allowing the use of capacity() could be safer/r somewhat w/o requiring unsafe ?

Related side note - The internal implementation in unbuffered also assumes Vec<u8> internally quite bit and it would be nice if it would be T: SomeBufType instead to reduce potentially use of allocations vs e.g. using heapless::vec::Vec

Describe alternatives you've considered
My work/around currently is either to allocate temporary zeroed buffer that gets overwritten or temporary types.

I thought about implementing some intermediary type but that would require worse footguns than zeroing expected encrypted.

Additional context
Add any other context or screenshots about the feature request here.

Saw these related:

• Fallible allocation #886

[cpu]

👋 Thanks for filing this detailed issue!

I think in general since the unbuffered API surface is so new it hasn't seen a lot of downstream usage and feedback on the ergonomics and remaining rough edges is very helpful.

It would be nice if Rustls creates it's own trait for some kind of Rustls relevant Buffer type which has capacity that can be passed as generic to these implementations.

This seems reasonable to me, but I suspect I have less opinions on this area than the other maintainers.

Most ideally this trait could be "isolated" within a separate crate to let other implementers to follow about it without necessary pulling the whole rustls for use-cases where given implementer crate only implements underlying buffer but not other parts of rustls.

I'm not sure I understand what value there would be in only implementing the buffer without intent to integrate with Rustls as a larger goal but in either case, perhaps this could be something that's a fit for the pki-types crate? It seems conceivable that it would be useful in other Rustls crates as well.

I would be more than happy to send a PR if you would be amenable to something like this or something else which potentially could maybe reduce the need of zero-initialization and ensuring the output is sized correctly for sending out.

I think before going too far down that road it would make sense to check that djc and ctz are on board with the general premise + proposed solution. If they are, I'd be happy to try and review. I've had to pause my own work on reducing the bifurcation of the buffered/unbuffered APIs to focus on other areas but I think I could find the time to support this.

If this is not feasible then I could send a PR to document this perhaps to ensure nobody else steps into this little footgun I did.

That sounds worthwhile, thank you!

[djc]

I feel like we should rely on the std BorrowedBuf and co here? Those are still unstable but I don't love the idea of reinventing that stuff ahead of them becoming stable.

[pinkforest]

That would require std though ? unbuffered typically operates in no_std context at least for /some of/ my use-cases.

[ctz]

BorrowedBuf is in core::io

[pinkforest]

Ah thanks for pointing that out - my bad

It doesn't on quick glance work for io_uring case though as it requires owned buffers due to kernel/userspace sharing ?

EDIT: I might have most definitely misunderstood how to use BorrowedCursor as owned - I need to check that

[pinkforest]

Would you be okay with experimental support via BorrowedBuf via PR if I hook it up ?

e.g. encode could be encode_borrowed() so it doesn't disturb the existing API and is over cfg-feature-gate e.g. borrowed_buf for unbuffered ?

[djc]

Sounds okay to me!

[pinkforest]

Sweet, PR on your way soon(tm)

[pinkforest]

Also if there is appetite to check feature use vs used rustc version given the version-dependant feature -

We could either check nightly in build.rs or just add fallible feature if someone tries to use the feature at stable that doesn't have the said feature.

e.g. detecting nightly w/ rustc_version crate and relaying appropriate cfg-gating

    if rustc_version::version_meta()
        .expect("failed to detect rustc version")
        .channel
        == rustc_version::Channel::Nightly
    {
        println!("cargo:rustc-cfg=nightly");
    }

With dalek we also have AVX2 gated for stable rust as below and nightly for AVX512
e.g. when the feature stabilises this could be used to guard it's use - if needed / wanted ?

    let rustc_version = rustc_version::version().expect("failed to detect rustc version");
    if rustc_version.major == 1 && rustc_version.minor <= 64 {
        // Old versions of Rust complain when you have an `unsafe fn` and you use `unsafe {}` inside,
        // so for those we want to apply the `#[allow(unused_unsafe)]` attribute to get rid of that warning.
        println!("cargo:rustc-cfg=allow_unused_unsafe");
    }

EDIT: Looks like there is already nightly check .. and for read-buf 🤦‍♀️ can just extend that 😅