Extract SHA-1’s cast with as into a dedicated function.

[SimonSapin]

See #31 (comment)

[briansmith]

It's probably helpful to check out my latest comment in the issue to understand the design goals for this submodule. In particular, it doesn't make sense for a feature of this submodule to take a pointer as a parameter because pointers are unsafe and the goal of this submodule is to define a purely-not-unsafe API implemented on top of unsafe. And, also, the goal is to do so in a way where the feature could reasonably be expected to make it into the Rust language or stdlib.

I actually started trying to do this but I had to switch to working on non-ring stuff before I finished it. Here's my in-progress attempt:

/// Chunks `$slice` into `$len`-length parts, returning (`chunks`, `leftover`),
/// where `chunks` is a subslice of `$slice` with elements of type `[T; $len]`
/// and `leftover` is a subslice of leftover elements.
macro_rules! slice_as_array_ref_chunks {
    ($slice:expr, $chunk_len:expr) => {
        {
            #[allow(unsafe_code)]
            fn slice_as_array_ref_chunks<'a, T>(slice: &'a [T])
                    -> (&'a [[T; $chunk_len]], &'a [T]) {
                let (to_chunk, leftover) = match slice.len() % $chunk_len {
                    n => slice.split_at(slice.len() - ($chunk_len - n))
                };
                let to_chunk = unsafe {
                    to_chunk.as_ptr() as *const [T; $chunk_len]
                };
                let chunks =
                    core::slice::from_raw_parts(to_chunk,
                                                to_chunk.len() / $chunk_len);
                (chunks, leftover)
            }
            slice_as_array_ref_chunks($slice)
        }
    }
}

[SimonSapin]

As long as sha1::block_data_order takes data: *const u8, num: c::size_t, calling this new API would require an unsafe slice::from_raw_parts call:

let data = core::slice::from_raw_parts(data, num * BLOCK_LEN);
let (blocks, rest) = slice_as_array_ref_chunks(data, BLOCK_LEN);
debug_assert!(rest.is_empty());

do so in a way where the feature could reasonably be expected to make it into the Rust language or stdlib.

In my opinion this would be very tough sell for slice_as_array_ref_chunks. It’s a very narrow use case. Who knows how many slightly different functions like this would be needed to cover every use of as ever?

[briansmith]

Sorry to take so long to get back to you.

The purpose of the array_ref stuff is to basically work around the lack of generics over integer values. I don't expect the Rust lang/lib teams to ever implement slice_as_array_ref_chunks but I do expect people can agree that it is useful to have some kind of functionality like that.

Anyway, one goal of the polyfill module to to expose only an interface that is "safe" (doesn't require unsafe), so we should try to find some solution that achieves that goal. I am definitely open to alternatives to what I suggested above.

[briansmith]

let data = core::slice::from_raw_parts(data, num * BLOCK_LEN);

Yes, that has to go into the unsafe function in the sha1 code because of this unusual aspect of ring where we're implementing a normally-performance-critical API in Rust that is almost exclusively implemented with assembly language code. Maybe in the future we can change things, e.g. changing the calling convention used to call the assembly language code, so that it is possible to pass the data in safe (non-pointer) types. I just don't know how to do it yet.

let (blocks, rest) = slice_as_array_ref_chunks(data, BLOCK_LEN);
debug_assert!(rest.is_empty());

These parts should go in the safe function.

As for this being a tough sell: In low-level crypto code, we are almost always breaking slices into fixed-sized chunks. So far in ring we don't implement low-level crypto in Rust, but I guess that will change and there will be lots of uses of it, at least within ring.

Anyway, this ring issue isn't the place to debate changing Rust or its stdlib. The goal of ring::polyfill is to gather data to inform those discussions, but they will happen elsewhere.

[briansmith]

Hey, it seems like nothing's happening with this. Also, I've had to add some as uses in other places too. So, let's just close this and we'll deal with them all in the issue about getting rid of as.