Key endianness?

Key endianness?

[Pascal Van Leeuwen]

Herbert,

I'm currently busy fixing some endianness related sparse errors reported
by this kbuild test robot and this triggered my to rethink some endian
conversion being done in the inside-secure driver.

I actually wonder what the endianness is of the input key data, e.g. the
"u8 *key" parameter to the setkey function.

I also wonder what the endianness is of the key data in a structure
like "crypto_aes_ctx", as filled in by the aes_expandkey function.

Since I know my current endianness conversions work on a little endian
CPU, I guess the big question is whether the byte order of this key
data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
algorithm specification).

I know I have some customers using big-endian CPU's, so I do care, but
I unfortunately don't have any platform available to test this with.

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

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 9:47 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> On Mon, 21 Oct 2019 at 21:14, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Sent: Monday, October 21, 2019 6:15 PM
> > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Re: Key endianness?
> > >
> > > On Mon, 21 Oct 2019 at 17:55, Pascal Van Leeuwen
> > > <pvanleeuwen@verimatrix.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > Sent: Monday, October 21, 2019 5:32 PM
> > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > Subject: Re: Key endianness?
> > > > >
> > > > > p[
> > > > >
> > > > > On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> > > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > > Sent: Monday, October 21, 2019 2:54 PM
> > > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > > Subject: Re: Key endianness?
> > > > > > >
> > > > > > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > > > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > > > > Subject: Re: Key endianness?
> > > > > > > > >
> > > > > > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > > > > > <pvanleeuwen@verimatrix.com> 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
www.insidesecure.com

Re: Key endianness?

[Ard Biesheuvel]

On Mon, 21 Oct 2019 at 21:14, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Sent: Monday, October 21, 2019 6:15 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Re: Key endianness?
> >
> > On Mon, 21 Oct 2019 at 17:55, Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Sent: Monday, October 21, 2019 5:32 PM
> > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > Subject: Re: Key endianness?
> > > >
> > > > p[
> > > >
> > > > On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > Sent: Monday, October 21, 2019 2:54 PM
> > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > Subject: Re: Key endianness?
> > > > > >
> > > > > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > > > Subject: Re: Key endianness?
> > > > > > > >
> > > > > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > > > > <pvanleeuwen@verimatrix.com> 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?

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.


> > 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.

> 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.

> 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.

> > > >
> > > > > ... 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.

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 6:15 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> On Mon, 21 Oct 2019 at 17:55, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Sent: Monday, October 21, 2019 5:32 PM
> > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Re: Key endianness?
> > >
> > > p[
> > >
> > > On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> > > <pvanleeuwen@verimatrix.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > Sent: Monday, October 21, 2019 2:54 PM
> > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > Subject: Re: Key endianness?
> > > > >
> > > > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > > Subject: Re: Key endianness?
> > > > > > >
> > > > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > > > <pvanleeuwen@verimatrix.com> 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).

> 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?
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. 

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.

> > >
> > > > ... 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.

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

Re: Key endianness?

[Ard Biesheuvel]

On Mon, 21 Oct 2019 at 17:55, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Sent: Monday, October 21, 2019 5:32 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Re: Key endianness?
> >
> > p[
> >
> > On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Sent: Monday, October 21, 2019 2:54 PM
> > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > Subject: Re: Key endianness?
> > > >
> > > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > > <pvanleeuwen@verimatrix.com> wrote:
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > Subject: Re: Key endianness?
> > > > > >
> > > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > > <pvanleeuwen@verimatrix.com> 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.

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.

> >
> > > ... 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.

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 5:32 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> p[
> 
> On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Sent: Monday, October 21, 2019 2:54 PM
> > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Re: Key endianness?
> > >
> > > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > > <pvanleeuwen@verimatrix.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > Subject: Re: Key endianness?
> > > > >
> > > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > > <pvanleeuwen@verimatrix.com> 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)

> > >
> > > > > > Regards,
> > > > > > Pascal van Leeuwen
> > > > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > > > > www.insidesecure.com
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Pascal Van Leeuwen
> > > > > > > Sent: Monday, October 21, 2019 11:04 AM
> > > > > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > > Subject: Key endianness?
> > > > > > >
> > > > > > > Herbert,
> > > > > > >
> > > > > > > I'm currently busy fixing some endianness related sparse errors reported
> > > > > > > by this kbuild test robot and this triggered my to rethink some endian
> > > > > > > conversion being done in the inside-secure driver.
> > > > > > >
> > > > > > > I actually wonder what the endianness is of the input key data, e.g. the
> > > > > > > "u8 *key" parameter to the setkey function.
> > > > > > >
> > > > > > > I also wonder what the endianness is of the key data in a structure
> > > > > > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > > > > > >
> > > > >
> > > > > crypto_aes_ctx uses CPU endianness for the round keys.
> > > > >
> > > > So these will need to be consistently handled using cpu_to_Xe32.
> > > >
> > >
> > > If you are using the generic aes_expandkey and want to reuse the key
> > > schedule, it is indeed good to be aware that both the round keys
> > > themselves as well as the key length are recorded in CPU endianness.
> > >
> > Actually, I have a big patch standing by getting rid of aes_expandkey()
> > altogether as I don't need _any_ of those round keys generated, ii
> > was just used for AES key validity checks and nothing else.
> >
> > But since that patch is not ready for prime time yet, I have to fix
> > these sparse errors for the time being.
> >
> > > > > In general, though, there is no such thing as endianness for a key
> > > > > that is declared as u8[], it is simply a sequence of bytes.
> > > > >
> > > > Depends a bit on the algorithm. Some keys are indeed defined as byte
> > > > streams, in which case you have a point. Assuming you mean that the
> > > > crypto API follows the byte order as defined by the algorithm spec.
> > > >
> > > > But sometimes the key data is actually a stream of _words_ (example:
> > > > Chacha20) and then endianness _does_ matter. Same thing applies to
> > > > things like nonces and initial counter values BTW.
> > > >
> > >
> > > Endianness always matters, and both AES and ChaCha are rather similar
> > > in that respect in the sense that it is the algorithm that defines how
> > > a byte stream is mapped onto 32-bit words, and in both cases, they use
> > > little endianness.
> > >
> > Thanks, that's actually something I can _use_ ;-)
> >
> > >
> > > > > 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.

> 
> > ... 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.

> > Very often getting the byte order right was just trial and error
> > using known-good reference vectors and just trying every possible
> > byte/word/whatever swap you could think of. (hence "ellendianness")
> >
> > >
> > >
> > > > >
> > > > > > > Since I know my current endianness conversions work on a little endian
> > > > > > > CPU, I guess the big question is whether the byte order of this key
> > > > > > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > > > > > algorithm specification).
> > > > > > >
> > > > > > > I know I have some customers using big-endian CPU's, so I do care, but
> > > > > > > I unfortunately don't have any platform available to test this with.
> > > > > > >
> > > > >
> > > > > You can boot big endian kernels on MacchiatoBin, in case that helps
> > > > > (using u-boot, not EFI)
> > > > >
> > > > I'm sure _someone_ can, I'm not so sure _I_ can ;-)
> > > >
> > > > Regards,
> > > > Pascal van Leeuwen
> > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > > www.insidesecure.com
> >
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com
> >

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

Re: Key endianness?

[Ard Biesheuvel]

p[

On Mon, 21 Oct 2019 at 17:23, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Sent: Monday, October 21, 2019 2:54 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Re: Key endianness?
> >
> > On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> wrote:
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Sent: Monday, October 21, 2019 1:59 PM
> > > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > Subject: Re: Key endianness?
> > > >
> > > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > > <pvanleeuwen@verimatrix.com> 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 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)

> >
> > > > > Regards,
> > > > > Pascal van Leeuwen
> > > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > > > www.insidesecure.com
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Pascal Van Leeuwen
> > > > > > Sent: Monday, October 21, 2019 11:04 AM
> > > > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > > Subject: Key endianness?
> > > > > >
> > > > > > Herbert,
> > > > > >
> > > > > > I'm currently busy fixing some endianness related sparse errors reported
> > > > > > by this kbuild test robot and this triggered my to rethink some endian
> > > > > > conversion being done in the inside-secure driver.
> > > > > >
> > > > > > I actually wonder what the endianness is of the input key data, e.g. the
> > > > > > "u8 *key" parameter to the setkey function.
> > > > > >
> > > > > > I also wonder what the endianness is of the key data in a structure
> > > > > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > > > > >
> > > >
> > > > crypto_aes_ctx uses CPU endianness for the round keys.
> > > >
> > > So these will need to be consistently handled using cpu_to_Xe32.
> > >
> >
> > If you are using the generic aes_expandkey and want to reuse the key
> > schedule, it is indeed good to be aware that both the round keys
> > themselves as well as the key length are recorded in CPU endianness.
> >
> Actually, I have a big patch standing by getting rid of aes_expandkey()
> altogether as I don't need _any_ of those round keys generated, ii
> was just used for AES key validity checks and nothing else.
>
> But since that patch is not ready for prime time yet, I have to fix
> these sparse errors for the time being.
>
> > > > In general, though, there is no such thing as endianness for a key
> > > > that is declared as u8[], it is simply a sequence of bytes.
> > > >
> > > Depends a bit on the algorithm. Some keys are indeed defined as byte
> > > streams, in which case you have a point. Assuming you mean that the
> > > crypto API follows the byte order as defined by the algorithm spec.
> > >
> > > But sometimes the key data is actually a stream of _words_ (example:
> > > Chacha20) and then endianness _does_ matter. Same thing applies to
> > > things like nonces and initial counter values BTW.
> > >
> >
> > Endianness always matters, and both AES and ChaCha are rather similar
> > in that respect in the sense that it is the algorithm that defines how
> > a byte stream is mapped onto 32-bit words, and in both cases, they use
> > little endianness.
> >
> Thanks, that's actually something I can _use_ ;-)
>
> >
> > > > 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.


> ... 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.

> Very often getting the byte order right was just trial and error
> using known-good reference vectors and just trying every possible
> byte/word/whatever swap you could think of. (hence "ellendianness")
>
> >
> >
> > > >
> > > > > > Since I know my current endianness conversions work on a little endian
> > > > > > CPU, I guess the big question is whether the byte order of this key
> > > > > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > > > > algorithm specification).
> > > > > >
> > > > > > I know I have some customers using big-endian CPU's, so I do care, but
> > > > > > I unfortunately don't have any platform available to test this with.
> > > > > >
> > > >
> > > > You can boot big endian kernels on MacchiatoBin, in case that helps
> > > > (using u-boot, not EFI)
> > > >
> > > I'm sure _someone_ can, I'm not so sure _I_ can ;-)
> > >
> > > Regards,
> > > Pascal van Leeuwen
> > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > www.insidesecure.com
>
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com
>

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 2:54 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Sent: Monday, October 21, 2019 1:59 PM
> > > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Re: Key endianness?
> > >
> > > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > > <pvanleeuwen@verimatrix.com> 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)?

I managed to work around it by doing *(__be32 *)&some_u32_array[3] =
but that's pretty ugly ... a better approach is still welcome.

> 
> > > > Regards,
> > > > Pascal van Leeuwen
> > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > > www.insidesecure.com
> > > >
> > > > > -----Original Message-----
> > > > > From: Pascal Van Leeuwen
> > > > > Sent: Monday, October 21, 2019 11:04 AM
> > > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > > Subject: Key endianness?
> > > > >
> > > > > Herbert,
> > > > >
> > > > > I'm currently busy fixing some endianness related sparse errors reported
> > > > > by this kbuild test robot and this triggered my to rethink some endian
> > > > > conversion being done in the inside-secure driver.
> > > > >
> > > > > I actually wonder what the endianness is of the input key data, e.g. the
> > > > > "u8 *key" parameter to the setkey function.
> > > > >
> > > > > I also wonder what the endianness is of the key data in a structure
> > > > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > > > >
> > >
> > > crypto_aes_ctx uses CPU endianness for the round keys.
> > >
> > So these will need to be consistently handled using cpu_to_Xe32.
> >
> 
> If you are using the generic aes_expandkey and want to reuse the key
> schedule, it is indeed good to be aware that both the round keys
> themselves as well as the key length are recorded in CPU endianness.
> 
Actually, I have a big patch standing by getting rid of aes_expandkey()
altogether as I don't need _any_ of those round keys generated, ii
was just used for AES key validity checks and nothing else.

But since that patch is not ready for prime time yet, I have to fix
these sparse errors for the time being.

> > > In general, though, there is no such thing as endianness for a key
> > > that is declared as u8[], it is simply a sequence of bytes.
> > >
> > Depends a bit on the algorithm. Some keys are indeed defined as byte
> > streams, in which case you have a point. Assuming you mean that the
> > crypto API follows the byte order as defined by the algorithm spec.
> >
> > But sometimes the key data is actually a stream of _words_ (example:
> > Chacha20) and then endianness _does_ matter. Same thing applies to
> > things like nonces and initial counter values BTW.
> >
> 
> Endianness always matters, and both AES and ChaCha are rather similar
> in that respect in the sense that it is the algorithm that defines how
> a byte stream is mapped onto 32-bit words, and in both cases, they use
> little endianness.
> 
Thanks, that's actually something I can _use_ ;-)

> 
> > > 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 ... 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 ;-)

Very often getting the byte order right was just trial and error 
using known-good reference vectors and just trying every possible
byte/word/whatever swap you could think of. (hence "ellendianness")

> 
> 
> > >
> > > > > Since I know my current endianness conversions work on a little endian
> > > > > CPU, I guess the big question is whether the byte order of this key
> > > > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > > > algorithm specification).
> > > > >
> > > > > I know I have some customers using big-endian CPU's, so I do care, but
> > > > > I unfortunately don't have any platform available to test this with.
> > > > >
> > >
> > > You can boot big endian kernels on MacchiatoBin, in case that helps
> > > (using u-boot, not EFI)
> > >
> > I'm sure _someone_ can, I'm not so sure _I_ can ;-)
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com


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

Re: Key endianness?

[Ard Biesheuvel]

On Mon, 21 Oct 2019 at 14:40, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Sent: Monday, October 21, 2019 1:59 PM
> > To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> > Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Re: Key endianness?
> >
> > On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> > <pvanleeuwen@verimatrix.com> 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.


> > > Regards,
> > > Pascal van Leeuwen
> > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > www.insidesecure.com
> > >
> > > > -----Original Message-----
> > > > From: Pascal Van Leeuwen
> > > > Sent: Monday, October 21, 2019 11:04 AM
> > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > Subject: Key endianness?
> > > >
> > > > Herbert,
> > > >
> > > > I'm currently busy fixing some endianness related sparse errors reported
> > > > by this kbuild test robot and this triggered my to rethink some endian
> > > > conversion being done in the inside-secure driver.
> > > >
> > > > I actually wonder what the endianness is of the input key data, e.g. the
> > > > "u8 *key" parameter to the setkey function.
> > > >
> > > > I also wonder what the endianness is of the key data in a structure
> > > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > > >
> >
> > crypto_aes_ctx uses CPU endianness for the round keys.
> >
> So these will need to be consistently handled using cpu_to_Xe32.
>

If you are using the generic aes_expandkey and want to reuse the key
schedule, it is indeed good to be aware that both the round keys
themselves as well as the key length are recorded in CPU endianness.

> > In general, though, there is no such thing as endianness for a key
> > that is declared as u8[], it is simply a sequence of bytes.
> >
> Depends a bit on the algorithm. Some keys are indeed defined as byte
> streams, in which case you have a point. Assuming you mean that the
> crypto API follows the byte order as defined by the algorithm spec.
>
> But sometimes the key data is actually a stream of _words_ (example:
> Chacha20) and then endianness _does_ matter. Same thing applies to
> things like nonces and initial counter values BTW.
>

Endianness always matters, and both AES and ChaCha are rather similar
in that respect in the sense that it is the algorithm that defines how
a byte stream is mapped onto 32-bit words, and in both cases, they use
little endianness.


> > 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.



> >
> > > > Since I know my current endianness conversions work on a little endian
> > > > CPU, I guess the big question is whether the byte order of this key
> > > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > > algorithm specification).
> > > >
> > > > I know I have some customers using big-endian CPU's, so I do care, but
> > > > I unfortunately don't have any platform available to test this with.
> > > >
> >
> > You can boot big endian kernels on MacchiatoBin, in case that helps
> > (using u-boot, not EFI)
> >
> I'm sure _someone_ can, I'm not so sure _I_ can ;-)
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 1:59 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> 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 ...

> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com
> >
> > > -----Original Message-----
> > > From: Pascal Van Leeuwen
> > > Sent: Monday, October 21, 2019 11:04 AM
> > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Key endianness?
> > >
> > > Herbert,
> > >
> > > I'm currently busy fixing some endianness related sparse errors reported
> > > by this kbuild test robot and this triggered my to rethink some endian
> > > conversion being done in the inside-secure driver.
> > >
> > > I actually wonder what the endianness is of the input key data, e.g. the
> > > "u8 *key" parameter to the setkey function.
> > >
> > > I also wonder what the endianness is of the key data in a structure
> > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > >
> 
> crypto_aes_ctx uses CPU endianness for the round keys.
> 
So these will need to be consistently handled using cpu_to_Xe32.

> In general, though, there is no such thing as endianness for a key
> that is declared as u8[], it is simply a sequence of bytes.
>
Depends a bit on the algorithm. Some keys are indeed defined as byte
streams, in which case you have a point. Assuming you mean that the 
crypto API follows the byte order as defined by the algorithm spec.

But sometimes the key data is actually a stream of _words_ (example:
Chacha20) and then endianness _does_ matter. Same thing applies to
things like nonces and initial counter values BTW.

> 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.

> 
> > > Since I know my current endianness conversions work on a little endian
> > > CPU, I guess the big question is whether the byte order of this key
> > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > algorithm specification).
> > >
> > > I know I have some customers using big-endian CPU's, so I do care, but
> > > I unfortunately don't have any platform available to test this with.
> > >
> 
> You can boot big endian kernels on MacchiatoBin, in case that helps
> (using u-boot, not EFI)
>
I'm sure _someone_ can, I'm not so sure _I_ can ;-)

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

RE: Key endianness?

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: Monday, October 21, 2019 2:12 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Re: Key endianness?
> 
> On Mon, 21 Oct 2019 at 14:08, Pascal Van Leeuwen
> <pvanleeuwen@verimatrix.com> wrote:
> >
> > And now that we've opened Pandora's box of "ellendianness" (as we
> > say here - a combination of the Dutch word "ellende", for misery,
> > and endianness ;-):
> >
> > The inside-secure driver uses several packed bitfield structures
> > (that are actually used directly by the little-endian hardware)
> > What happens to these on a big-endian machine?
> 
> The C spec does not define how packed bitfields are projected onto
> memory,
>
Yes, I know that. But GCC may still provide guarantees, I don't know.
It's not my code, it's legacy code that was already there, which I
try to touch as little as possible.
(I already decided I didn't like it just based on the results of 
running it through Compiler Explorer - not very efficient ...)

> so relying on that is a mistake. Your code should do the
> bitwise arithmetic explicitly to be portable.
> 
Yeah, probably. Problem is these data structures are used everywhere.
So getting the changes through the whole review process would just
be too painful. It's been like this for years and so far it worked
just fine (I guess).

So I'd prefer a solution that requires minimal changes to existing
code but would still be guaranteed to work on both big and little
endian machines, assuming you use GCC (or Clang?) as the compiler.

> > I've seen examples that hint at having to define the bits in
> > reverse order on big-endian machines, which would require a big
> > "#ifdef LITTLE_ENDIAN / #else" around the whole struct definition.
> >
> > And then on top of that I'll probably still have to swap the bytes
> > within words to get those into the correct order towards the HW.
> > Which is not very convenient for fields crossing byte boundaries.
> > (I'd probably want to use the byte swapping facilities of my HW
> > for that and not the CPU)
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com
> >
> > > -----Original Message-----
> > > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> > > Pascal Van Leeuwen
> > > Sent: Monday, October 21, 2019 12:56 PM
> > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: RE: Key endianness?
> > >
> > > 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?
> > >
> > > Regards,
> > > Pascal van Leeuwen
> > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > www.insidesecure.com
> > >
> > > > -----Original Message-----
> > > > From: Pascal Van Leeuwen
> > > > Sent: Monday, October 21, 2019 11:04 AM
> > > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > > Subject: Key endianness?
> > > >
> > > > Herbert,
> > > >
> > > > I'm currently busy fixing some endianness related sparse errors reported
> > > > by this kbuild test robot and this triggered my to rethink some endian
> > > > conversion being done in the inside-secure driver.
> > > >
> > > > I actually wonder what the endianness is of the input key data, e.g. the
> > > > "u8 *key" parameter to the setkey function.
> > > >
> > > > I also wonder what the endianness is of the key data in a structure
> > > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > > >
> > > > Since I know my current endianness conversions work on a little endian
> > > > CPU, I guess the big question is whether the byte order of this key
> > > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > > algorithm specification).
> > > >
> > > > I know I have some customers using big-endian CPU's, so I do care, but
> > > > I unfortunately don't have any platform available to test this with.
> > > >
> > > > Regards,
> > > > Pascal van Leeuwen
> > > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > > www.insidesecure.com
> >


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

Re: Key endianness?

[Ard Biesheuvel]

On Mon, 21 Oct 2019 at 14:08, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> wrote:
>
> And now that we've opened Pandora's box of "ellendianness" (as we
> say here - a combination of the Dutch word "ellende", for misery,
> and endianness ;-):
>
> The inside-secure driver uses several packed bitfield structures
> (that are actually used directly by the little-endian hardware)
> What happens to these on a big-endian machine?

The C spec does not define how packed bitfields are projected onto
memory, so relying on that is a mistake. Your code should do the
bitwise arithmetic explicitly to be portable.

> I've seen examples that hint at having to define the bits in
> reverse order on big-endian machines, which would require a big
> "#ifdef LITTLE_ENDIAN / #else" around the whole struct definition.
>
> And then on top of that I'll probably still have to swap the bytes
> within words to get those into the correct order towards the HW.
> Which is not very convenient for fields crossing byte boundaries.
> (I'd probably want to use the byte swapping facilities of my HW
> for that and not the CPU)
>
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com
>
> > -----Original Message-----
> > From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> > Pascal Van Leeuwen
> > Sent: Monday, October 21, 2019 12:56 PM
> > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: RE: Key endianness?
> >
> > 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?
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com
> >
> > > -----Original Message-----
> > > From: Pascal Van Leeuwen
> > > Sent: Monday, October 21, 2019 11:04 AM
> > > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > > Subject: Key endianness?
> > >
> > > Herbert,
> > >
> > > I'm currently busy fixing some endianness related sparse errors reported
> > > by this kbuild test robot and this triggered my to rethink some endian
> > > conversion being done in the inside-secure driver.
> > >
> > > I actually wonder what the endianness is of the input key data, e.g. the
> > > "u8 *key" parameter to the setkey function.
> > >
> > > I also wonder what the endianness is of the key data in a structure
> > > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> > >
> > > Since I know my current endianness conversions work on a little endian
> > > CPU, I guess the big question is whether the byte order of this key
> > > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > > algorithm specification).
> > >
> > > I know I have some customers using big-endian CPU's, so I do care, but
> > > I unfortunately don't have any platform available to test this with.
> > >
> > > Regards,
> > > Pascal van Leeuwen
> > > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > > www.insidesecure.com
>

RE: Key endianness?

[Pascal Van Leeuwen]

And now that we've opened Pandora's box of "ellendianness" (as we
say here - a combination of the Dutch word "ellende", for misery,
and endianness ;-):

The inside-secure driver uses several packed bitfield structures
(that are actually used directly by the little-endian hardware)
What happens to these on a big-endian machine?
I've seen examples that hint at having to define the bits in 
reverse order on big-endian machines, which would require a big
"#ifdef LITTLE_ENDIAN / #else" around the whole struct definition.

And then on top of that I'll probably still have to swap the bytes
within words to get those into the correct order towards the HW.
Which is not very convenient for fields crossing byte boundaries.
(I'd probably want to use the byte swapping facilities of my HW
for that and not the CPU)

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

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> Pascal Van Leeuwen
> Sent: Monday, October 21, 2019 12:56 PM
> To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: RE: Key endianness?
> 
> 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?
> 
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com
> 
> > -----Original Message-----
> > From: Pascal Van Leeuwen
> > Sent: Monday, October 21, 2019 11:04 AM
> > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Key endianness?
> >
> > Herbert,
> >
> > I'm currently busy fixing some endianness related sparse errors reported
> > by this kbuild test robot and this triggered my to rethink some endian
> > conversion being done in the inside-secure driver.
> >
> > I actually wonder what the endianness is of the input key data, e.g. the
> > "u8 *key" parameter to the setkey function.
> >
> > I also wonder what the endianness is of the key data in a structure
> > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> >
> > Since I know my current endianness conversions work on a little endian
> > CPU, I guess the big question is whether the byte order of this key
> > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > algorithm specification).
> >
> > I know I have some customers using big-endian CPU's, so I do care, but
> > I unfortunately don't have any platform available to test this with.
> >
> > Regards,
> > Pascal van Leeuwen
> > Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> > www.insidesecure.com

Re: Key endianness?

[Ard Biesheuvel]

On Mon, 21 Oct 2019 at 12:56, Pascal Van Leeuwen
<pvanleeuwen@verimatrix.com> 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.

> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com
>
> > -----Original Message-----
> > From: Pascal Van Leeuwen
> > Sent: Monday, October 21, 2019 11:04 AM
> > To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> > Subject: Key endianness?
> >
> > Herbert,
> >
> > I'm currently busy fixing some endianness related sparse errors reported
> > by this kbuild test robot and this triggered my to rethink some endian
> > conversion being done in the inside-secure driver.
> >
> > I actually wonder what the endianness is of the input key data, e.g. the
> > "u8 *key" parameter to the setkey function.
> >
> > I also wonder what the endianness is of the key data in a structure
> > like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> >

crypto_aes_ctx uses CPU endianness for the round keys.

In general, though, there is no such thing as endianness for a key
that is declared as u8[], it is simply a sequence of bytes. 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.


> > Since I know my current endianness conversions work on a little endian
> > CPU, I guess the big question is whether the byte order of this key
> > data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> > algorithm specification).
> >
> > I know I have some customers using big-endian CPU's, so I do care, but
> > I unfortunately don't have any platform available to test this with.
> >

You can boot big endian kernels on MacchiatoBin, in case that helps
(using u-boot, not EFI)

RE: Key endianness?

[Pascal Van Leeuwen]

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?

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

> -----Original Message-----
> From: Pascal Van Leeuwen
> Sent: Monday, October 21, 2019 11:04 AM
> To: linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au
> Subject: Key endianness?
> 
> Herbert,
> 
> I'm currently busy fixing some endianness related sparse errors reported
> by this kbuild test robot and this triggered my to rethink some endian
> conversion being done in the inside-secure driver.
> 
> I actually wonder what the endianness is of the input key data, e.g. the
> "u8 *key" parameter to the setkey function.
> 
> I also wonder what the endianness is of the key data in a structure
> like "crypto_aes_ctx", as filled in by the aes_expandkey function.
> 
> Since I know my current endianness conversions work on a little endian
> CPU, I guess the big question is whether the byte order of this key
> data is _CPU byte order_ or always some _fixed byte order_ (e.g. as per
> algorithm specification).
> 
> I know I have some customers using big-endian CPU's, so I do care, but
> I unfortunately don't have any platform available to test this with.
> 
> Regards,
> Pascal van Leeuwen
> Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
> www.insidesecure.com