make rustls_slice_slice_bytes_len function unsafe

[kitcatier]

https://github.com/rustls/rustls-ffi/blob/55df12216f0004af12c5967725a0b46d9c4bd466/src/rslice.rs#L80-L87 Hello, It is not a good choice to mark the entire function body as unsafe, which will make the caller ignore the safety requirements that the function parameters must guarantee, the developer who calls the start function may not notice this safety requirement. The unsafe function called needs to ensure that the parameter must be:

• The pointer must be properly aligned.
• It must be “dereferenceable” in the sense defined in the module documentation.
• The pointer must point to an initialized instance of T.
• You must enforce Rust’s aliasing rules, since the returned lifetime 'a is arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. In particular, while this reference exists, the memory the pointer points to must not get mutated (except inside UnsafeCell).

https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref

Marking them unsafe also means that callers must make sure they know what they're doing.

[djc]

If we're going to make it, we should probably document the guarantees you're listing here in the code?

[jsha]

Hi @kitcatier! Welcome to the rustls-ffi project, and thanks for the pull request.

All of the code in rustls-ffi is meant to be called by non-Rust code, and so uses raw pointers. So far we haven't chosen to mark the functions as unsafe for a couple of reasons:

• In addition to making the function itself unsafe, it acts like putting the whole function body in a big unsafe block. We'd prefer not to do that. Inside our functions we want to explicitly mark unsafe operations with a minimal unsafe block. Looking at docs, it appears we do have the option to enable a special lint for that if we wanted to.
  • It would make calling these functions from unittests somewhat more annoying.

We arrange for (non-Rust) callers of these function to uphold the safety guarantees by asking them to uphold a few properties:

• We only hand out pointers to our structs that follow the rules above (aligned, initialized, etc).
• We document the lifetime requirements of those pointers.
• We provide *const pointers for things that may have aliases.
• We expect callers to never cast between types or cast away const.
• We provide *mut pointers for things that may be mutated. This is actually an area we should tighten up. The rules for what you can do with a *mut pointer are very slightly looser than the rules for what you can do with an &mut reference. For instance, the rules are triggered on dereference of raw pointers, while the rules are triggered on mere existence for references. But because we almost always have to convert a *mut pointer to an &mut reference to call methods, we have to ask our callers to uphold the more stringent &mut rules.

So, my overall summary is: you make a good point, but the same arguments apply to all functions within rustls-ffi. If we mark one of these functions unsafe, we should really mark them all unsafe, and I'm not convinced that makes sense. But I'm willing to be convinced.