Skip to content
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

Consider mechanisms to convert &[u32] to &[u8] #37

Closed
erickt opened this Issue Oct 1, 2015 · 15 comments

Comments

Projects
None yet
5 participants
@erickt
Copy link

erickt commented Oct 1, 2015

It makes me a little sad to see unsafe being used to convert a &[u32] into a &[u8] in octavo:

    fn crypt(&mut self, input: &[u8], output: &mut [u8]) {
        assert_eq!(input.len(), output.len());

        if self.index == STATE_BYTES { self.update() }

        let buffer = unsafe {
            slice::from_raw_parts(self.buffer.as_ptr() as *const u8, STATE_BYTES)
        };

        for i in self.index..input.len() {
            output[i] = input[i] ^ buffer[i];
        }

        self.index = input.len();
    }

We really ought to have a place to centralize this functionality so that it's well tested and safe across our ecosystem. Would it make sense to have this functionality be in byteorder?

It'd also be interesting to also support the inverse operation, where a &[u8] is converted into a (&[u8], &[u32], &[u8]), where the first and last slice are there to read a byte-at-a-time until the the slice is aligned. This style operation could be useful to safely massage a slice into something that can use simd (or at least simd-ish operations over a usize value).

cc @huonw, @bluss, @hauleth

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Oct 1, 2015

@erickt Can you help me understand the code you posted? I see &[u8] and &mut [u8], but I don't see any &[u32]. In particular, where exactly is a byte order conversion happening?

@erickt

This comment has been minimized.

Copy link
Author

erickt commented Oct 1, 2015

The &[u32] is here. If I understand this code that most of the time the ChaCha20 is working with u32 but the actual encryption happens on a byte stream. This is more indirectly a byteorder issue, where there is is useful to use NativeEndian to turn types to and from bytes. I'm not sure how often this would come up for little and big endian streams though.

@hauleth

This comment has been minimized.

Copy link
Contributor

hauleth commented Oct 1, 2015

@erickt I really can use bytorder like that:

let mut buffer = [0u8; STATE_BYTES];
{
    let mut p = &mut buffer[..];
    for word in self.buffer {
        p.write_u32::<NativeEndian>(word);
    }
}

It looks kind of ugly, but could be moved to additional function. The main reason why I don't want that is that byteorder is dependent on std. It isn't issue, but I want Octavo to use only core so I need find a way to achieve correct conversions between arrays of numbers without byteorder which is wrapper around std::io::{Write, Read}.

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Oct 1, 2015

@hauleth Yup, that looks right to me. No extra APIs needed. :-)

With respect to core, it would be nice if byteorder could provide something without std (at the very least, the ByteOrder trait). I'm not quite sure what our conditional compilation options are in this space though.

@hauleth

This comment has been minimized.

Copy link
Contributor

hauleth commented Oct 9, 2015

@BurntSushi I have idea how to read from &[u8] - it could be iterator that will allow us to iterate through &[u8] and Iterator::Item == u32. It would be perfect solution IMHO as I could use that iterator then as an input instead of &[u8] (API from the user side will still be the same).

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Oct 9, 2015

@hauleth That might be OK. I guess the ReadBytesExt trait would grow methods that return iterators? As of now, I think that would result in a new method for each integer type, which would be unfortunate. However, when I get around to merging #34, we should be able to rely on return type polymorphism.

For now, I think I'd recommend writing an iterator over u32 in your own code. I think it is straight-forward to do?

@hauleth

This comment has been minimized.

Copy link
Contributor

hauleth commented Oct 9, 2015

I try to implement that on my own. Do you mind if I borrow some code from you? LittleEndian and BigEndian to be exact.

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Oct 9, 2015

@hauleth You should be able to implement it on top of the existing methods exposed by byteorder. e.g., Your iterator next method should just repeatedly call read_u32.

@hauleth

This comment has been minimized.

Copy link
Contributor

hauleth commented Oct 9, 2015

Yeah, but I want to make it libstd independent as libcore is marked to be stabilized in next beta.

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Oct 9, 2015

Right right... Forgot about that. I really do hope to figure something out for byteorder because it does seem like something that should be usable off of libstd. But yeah, I suppose in the mean time, copying code from here might be the thing to do. :-) (It is permissively licensed.)

@hauleth

This comment has been minimized.

Copy link
Contributor

hauleth commented Oct 9, 2015

Yeah, I know about license but it is always polite to ask. To be honest I think that Write and Read traits should land in libcore as both are OS independent and it would simplify some work, even without libstd (you sometimes need to write things to memory or similar, this crate is great example).

Łukasz Jan Niemier

Dnia 9 paź 2015 o godz. 23:06 Andrew Gallant notifications@github.com napisał(a):

Right right... Forgot about that. I really do hope to figure something out for byteorder because it does seem like something that should be usable off of libstd. But yeah, I suppose in the mean time, copying code from here might be the thing to do. :-) (It is permissively licensed.)


Reply to this email directly or view it on GitHub.

@bluss

This comment has been minimized.

Copy link

bluss commented Oct 10, 2015

Moving Read & Write would be nice. Unfortunately Read cannot move to libcore, it has stable methods that use libcollections (String and Vec). Would need some sweet magic to manage to split it up into a core-compatible part and the rest.

@bluss

This comment has been minimized.

Copy link

bluss commented Oct 10, 2015

I'm not sure how much can be done, but let's put the idea out there so we can give it a shot, filed an issue over at rust-lang/rfcs/issues/1314

@BurntSushi

This comment has been minimized.

Copy link
Owner

BurntSushi commented Aug 1, 2016

Today, byteorder can work without no_std, but at the cost of losing Read/Write convenience impls. I don't think there's much byteorder itself can do about that, but I think we've deviated far from the original problem being solved in this issue.

I'm going to close this, but if there is indeed a problem that is feasible for byteorder to solve, then I welcome a more directed issue.

@BurntSushi BurntSushi closed this Aug 1, 2016

@burdges

This comment has been minimized.

Copy link

burdges commented Oct 5, 2016

As an aside, I'd think crypto like octavo could just do the << operations to make the conversion safe and unambiguous. I'd hope LLVM would optimize these into a noop or byte swap, but if not the cost is negligible compared with the cryptographic operation the conversion prepares for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.