by Pascal Van Leeuwen

[permalink] [raw]

Subject: RE: Key endianness?

> -----Original Message-----
> From: Ard Biesheuvel <[email protected]>
> Sent: Monday, October 21, 2019 9:47 PM
> To: Pascal Van Leeuwen <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: Key endianness?
>
> On Mon, 21 Oct 2019 at 21:14, Pascal Van Leeuwen
> <[email protected]> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <[email protected]>
> > > Sent: Monday, October 21, 2019 6:15 PM
> > > To: Pascal Van Leeuwen <[email protected]>
> > > Cc: [email protected]; [email protected]
> > > Subject: Re: Key endianness?
> > >
> > > On Mon, 21 Oct 2019 at 17:55, Pascal Van Leeuwen
> > > <[email protected]> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <[email protected]>
> > > > > Sent: Monday, October 21, 2019 5:32 PM
> > > > > To: Pascal Van Leeuwen <[email protected]>
> > > > > Cc: [email protected]; [email protected]
> > > > > Subject: Re: Key endianness?
> > > > >
> > > > > p[
> > > > >
> > > > > On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > > Sent: Monday, October 21, 2019 2:54 PM
> > > > > > > To: Pascal Van Leeuwen <[email protected]>
> > > > > > > Cc: [email protected]; [email protected]
> > > > > > > Subject: Re: Key endianness?
> > > > > > >
> > > > > > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > > > > > <[email protected]> wrote:
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Ard Biesheuvel <[email protected]>
> > > > > > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > > > > > To: Pascal Van Leeuwen <[email protected]>
> > > > > > > > > Cc: [email protected]; [email protected]
> > > > > > > > > Subject: Re: Key endianness?
> > > > > > > > >
> > > > > > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > > > > > <[email protected]> wrote:
> > > > > > > > > >
> > > > > > > > > > Another endianness question:
> > > > > > > > > >
> > > > > > > > > > I have some data structure that can be either little or big endian,
> > > > > > > > > > depending on the exact use case. Currently, I have it defined as u32.
> > > > > > > > > > This causes sparse errors when accessing it using cpu_to_Xe32() and
> > > > > > > > > > Xe32_to_cpu().
> > > > > > > > > >
> > > > > > > > > > Now, for the big endian case, I could use htonl()/ntohl() instead,
> > > > > > > > > > but this is inconsistent with all other endian conversions in the
> > > > > > > > > > driver ... and there's no little endian alternative I'm aware of.
> > > > > > > > > > So I don't really like that approach.
> > > > > > > > > >
> > > > > > > > > > Alternatively, I could define a union of both a big and little
> > > > > > > > > > endian version of the data but that would require touching a lot
> > > > > > > > > > of legacy code (unless I use a C11 anonymous union ... not sure
> > > > > > > > > > if that would be allowed?) and IMHO is a bit silly.
> > > > > > > > > >
> > > > > > > > > > Is there some way of telling sparse to _not_ check for "correct"
> > > > > > > > > > use of these functions for a certain variable?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > In this case, just use (__force __Xe32*) to cast it to the correct
> > > > > > > > > type. This annotates the cast as being intentionally endian-unclean,
> > > > > > > > > and shuts up Sparse.
> > > > > > > > >
> > > > > > > > Thanks for trying to help out, but that just gives me an
> > > > > > > > "error: not an lvalue" from both sparse and GCC.
> > > > > > > > But I'm probably doing it wrong somehow ...
> > > > > > > >
> > > > > > >
> > > > > > > It depends on what you are casting. But doing something like
> > > > > > >
> > > > > > > u32 l = ...
> > > > > > > __le32 ll = (__force __le32)l
> > > > > > >
> > > > > > > should not trigger a sparse warning.
> > > > > > >
> > > > > > I was actually casting the left side, not the right side,
> > > > > > as that's where my sparse issue was. Must be my poor grasp
> > > > > > of the C language hurting me here as I don't understand why
> > > > > > I'm not allowed to cast an array element to a different type
> > > > > > of the _same size_ ...
> > > > > >
> > > > > > i.e. why can't I do (__be32)some_u32_array[3] = cpu_to_be32(some_value)?
> > > > > >
> > > > >
> > > > > Because you can only change the type of an expression by casting, and
> > > > > an lvalue is not an expression. A variable has a type already, and you
> > > > > cannot cast that away - what would that mean, exactly? Would all
> > > > > occurrences of some_u32_array[] suddenly have a different type? Or
> > > > > only element [3]?
> > > > >
> > > > I think it would be perfectly logical to do such a cast and I'm really
> > > > surprised that it is not legal. Obviously, it would only apply to the
> > > > actual assignment it is used with. It's a cast, not a redefinition.
> > > > After all, a variable or an array item is just some storage area in
> > > > memory. Why shouldn't I be able to write to it _as if_ it is some
> > > > different type (if I know what I'm doing and especially if it is the
> > > > exact same size in memory)?
> > > >
> > > > >
> > > > > > I managed to work around it by doing *(__be32 *)&some_u32_array[3] =
> > > > > > but that's pretty ugly ... a better approach is still welcome.
> > > > > >
> > > > >
> > > > > You need to cast the right hand side, not the left hand side. If
> > > > > some_u32_array is u32[], force cast it to (__force u32)
> > > > >
> > > >
> > > > Sure, you can do the casting on the right hand side, but that may not
> > > > convey what you _really_ want to do, particularly in this case.
> > > > As I _really_ want to write a big endian word there. I don't want to
> > > > pretend I loose the endianness somewhere along the way. That written
> > > > word is still very much big endian.
> > > > (I know practically it makes no difference, but casting the left side
> > > > would just be so much clearer IMHO)
> > > >
> > >
> > >
> > > No, it really isn't, and I am tired of having another endless debate about this.
> > >
> > I was not intending to start a debate - I was just being a newby C programmer
> > being _honestly surprised_ by this limitation. You (or I, anyway) would just expect
> > it to work. The rest is personal taste, which is is not debatable (it just is).
> >
>
> But it is not a limitation. You are arguing that it is natural and
> obvious to cast the expression, but it is not. Really.
>
> (be32)lvalue = <expression of type be32>
>
> would actually mean that the operation applied to the expression is
> the opposite, given the lvalue is *not* a be32 to begin with, and so
> you expect that lvalue is made to be a be32 for the purpose of the
> assignment, after which it is converted back to what it was before? In
> this particular case, this comes down to doing the inverse cast on the
> expression. But what happens is there is no inverse cast? What would
> it mean, for instance, to do
>
> (u16)lvalue = <expression of type s16>
>
> when lvalue is of type s16 itself? Does the sign bit become a data bit
> now? Do we have to keep track of this throughout the code?
>
I thought you were the one not wanting to debate this ...

What I would expect such a cast to do is to simply treat the lefthand
side _as if_ it were defined as being the type of the cast, no more,
no less. So it would do whatever would happen if lvalue was defined
as a u16 in the first place. Which is probably not a whole lot for
this fairly uninteresting example, as, as far as I know, C does not
do anything special if you assign a signed value to an unsigned
variable.

It's difficult to come up with good examples that aren't contrived.
But say I have this variable that is usually a function pointer,
but I have this special case where it stores some integer value.
(because there I don't need the ptr and don't want to waste space)

Then (int)func_ptr = <integer value> is _more_ clear about
my intentions than func_ptr = (function ptr cast)<integer value>

Of course the same could be achieved with a union of a function ptr
and an int, but that may not be convenient if there's a lot of
legacy code using that function pointer already and C11 (and thus
anonymous unions) is not an option.

> The bottom line is that the cast operation is only defined for values,
> not for variables. A variable's type is fixed - you cannot change it
> for the duration of a cast operation and expect the compiler to infer
> what this might mean in the general case, even if you think your
> endianness conversion example is an obvious case.
>
I've understood by now that the C language specification does not
allow it (although ... *(int *)&function_ptr = <integer value> ;-)

Which means this whole discussion is purely theoretical and
significantly off-topic for this mailing list, for which I apologize.
>
> > > C permits casting of expressions, not of lvalues.
> > > ...
> > > > > > >
> > > > > > > > > If the
> > > > > > > > > hardware chooses to reorder those bytes for some reason, it is the
> > > > > > > > > responsibility of the driver to take care of that from the CPU side.
> > > > > > > > >
> > > > > > > > Which still requires you to know the byte order as used by the API.
> > > > > > > >
> > > > > > >
> > > > > > > Only if API means the AES or ChaCha specific helper routines that we
> > > > > > > have in the kernel. If you are using the AES helpers, then yes, you
> > > > > > > need to ensure that you use the same convention. But the algorithms
> > > > > > > themselves are fully defined by their specification, and so what other
> > > > > > > implementations in the kernel do is not really relevant.
> > > > > > >
> > > > > > What is relevant is what the API expects
> > > > >
> > > > > But *which* API? The skcipher API uses u8[] for in/output and keys,
> > > > > and how these byte arrays are interpreted is not (and cannot) be
> > > > > defined at this level of abstraction.
> > > > >
> > > > Yes, skcipher API. Obviously. As that's what we're talking about.
> > > > And _of course_ it has to be defined at that level of abstraction.
> > > > (which doesn't preclude inheriting it from some other specification)
> > > > Otherwise you would not able to e.g. exchange keys between different
> > > > platforms.
> > > >
> > >
> > > So what exactly are you suggesting? That the skcipher API should
> > > specify that the key is u8[], unless the algo in question operates on
> > > 32-bit words, in which case it is le32[], unless the algo in question
> > > operates on 64-bit words, in which case it is le64[] etc etc? Do you
> > > seriously think that at the skcipher API level we should mandate all
> > > of that? That is insane.
> > >
> > You think being _clear_ on the actual byte order is _insane_? Seriously?
>
> That is not what I said.
>
> I said that an abstract API that reasons about unspecified algorithms
> taking inputs and output of an unspecified nature, using keys of an
> unspecified nature should not be expected to define how such
> unspecified quantities are organized if they happen to be interpreted
> as multi-byte words by some of its implementations.
>
If said API wants to achieve some kind of interoperability - i.e. allow
for keys, nonces and IV's to be distributed to other platforms - it must
necessarily define the byte order thereof, either directly or by
implication. I honestly can't believe you're trying to argue the contrary.

> > Like I said, inheriting that from the algorithm spec is fine but not all
> > algorithm specs are clear and unambiguous w.r.t. byte order.
> >
>
> That doesn't make it the job of the abstraction that is layered on top
> to define it.
>
Of course it does. Because if it doesn't, you wouldn't be able to exchange
key material with other implementations/platforms. Or extract it from/
insert it into some protocol data stream.

> > I know this is not crypto perse, but what would be the logical byte order
> > for a CRC32 "key" considering it is a 32 bit word? The CRC32 definition
> > sure isn't going to help you there, it is specified on 32 bit words only.
> > So in such cases it must be clear how the byte stream maps onto the word.
> >
>
> Yes, so for the shash implementations of "crc32" and "crc32c", it is
> defined by the implementation. But that doesn't mean shash should
> specify that this is the case for all algorithms.
>
It should never be "defined by the implementation", for reasons I already
mentioned above. In fact, we have these crypto self-tests to ensure all
implementations behave _exactly_ the same. Which means there must be some
higher level specification they must all comply with. There has to be.

> > > > >
> > > > > > ... and from 20 years of
> > > > > > experience I would say many algorithm specifications are not exactly
> > > > > > very clear on the byte order at all, often assuming this to be
> > > > > > "obvious". (and if it's not little-endian, it's not obvious to me ;-)
> > > > > >
> > > > >
> > > > > I agree that not all specs are crystal clear on this. But it is still
> > > > > the algorithm that needs to define this.
> > > > >
> > > > In an ideal world, probably. In the real world, it is entirely possible
> > > > for an implementation to expect the key bytes in a different order.
> > > > Would not be the first time I run into that.
> > > >
> > >
> > > Of course. But that is not the point. The skcipher API cannot possibly
> > > reason about byte orders of all current and future algorithms that it
> > > may ever encapsulate.
> > >
> > Well, so far I got the impression that at least the intention is
> > to follow the cipher specification. That's a start. You could add
> > some generic rules on top of that, like "if the specification is
> > word based and does not mandate a byte order, than these words
> > shall be stored in little-endian byte order". It's not that hard.
> > You really don't need to know "future algorithm" details for that.
> >
>
> I am not saying it is hard. I am saying it is wrong. The skcipher
> API's job is not to fill holes in the algorithm's specification.
>
It's _at least_ the API's job to ensure there is consistency across
different implementations of the same algorithm, which implies there
_must_ be consensus on the byte order. So "wrong" my ass. QED.

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
http://www.insidesecure.com