Use cpu_endian to check local endianness

[sarroutbi]

Resolves: #283

[codecov]

Codecov Report

Merging #637 (225c3f9) into master (5965a15) will increase coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 80.64%.

Additional details and impacted files

Flag	Coverage Δ
e2e-testsuite	59.49% <70.96%> (+0.01%)	⬆️
upstream-unit-tests	59.80% <78.57%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
keylime/src/endian.rs	66.66% <66.66%> (ø)
keylime/src/ima/entry.rs	75.83% <100.00%> (+0.10%)	⬆️
keylime/src/tpm.rs	86.39% <100.00%> (ø)

[sarroutbi]

I have tried making endianness handling in a templatized way. Something like:

+pub fn local_endianness<T, const SIZE: usize>(t: T) -> [u8; SIZE] {
+    match cpu_endian::working() {
+        cpu_endian::Endian::Little => t.to_le_bytes(),
+        cpu_endian::Endian::Big => t.to_be_bytes(),
+        _ => t.to_le_bytes(),
+    }
+}
+

However, this seems to be not allowed:

6 | pub fn local_endianness<T, const SIZE: usize>(t: T) -> [u8; SIZE] {
  |                         - method `to_le_bytes` not found for this type parameter
7 |     match cpu_endian::working() {
8 |         cpu_endian::Endian::Little => t.to_le_bytes(),
  |                                         ^^^^^^^^^^^ method not found in `T`

error[E0599]: no method named `to_be_bytes` found for type parameter `T` in the current scope
 --> keylime/src/endian.rs:9:38
  |
6 | pub fn local_endianness<T, const SIZE: usize>(t: T) -> [u8; SIZE] {
  |                         - method `to_be_bytes` not found for this type parameter
...
9 |         cpu_endian::Endian::Big => t.to_be_bytes(),
  |                                      ^^^^^^^^^^^ method not found in `T`

error[E0599]: no method named `to_le_bytes` found for type parameter `T` in the current scope
  --> keylime/src/endian.rs:10:16
   |
6  | pub fn local_endianness<T, const SIZE: usize>(t: T) -> [u8; SIZE] {
   |                         - method `to_le_bytes` not found for this type parameter
...
10 |         _ => t.to_le_bytes(),
   |                ^^^^^^^^^^^ method not found in `T`

If anybody can help with this, I am grateful. Otherwise, I will keep the PR as it is right now

[aplanas]

If anybody can help with this, I am grateful. Otherwise, I will keep the PR as it is right now

You need to add a bound to "T". IIUC "T" will always be "u32", "u16" or "u8". I that case maybe you can use something like this: https://users.rust-lang.org/t/generic-function-for-from-be-bytes/59629/5

[ansasaki]

@sarroutbi Have you tested the interoperability of this between a big endian and a little endian machine?

I'm afraid this can break interoperability. The reason of making the endianess fixed is for all machines to encode data in the same way.

Another downside is the addition of a new dependency. It is not available in Fedora for example, which will cause problems on packaging.

[sarroutbi]

@sarroutbi Have you tested the interoperability of this between a big endian and a little endian machine?

I'm afraid this can break interoperability. The reason of making the endianess fixed is for all machines to encode data in the same way.

Another downside is the addition of a new dependency. It is not available in Fedora for example, which will cause problems on packaging.

You are right. I did not figure out that scenario. If codification is hard coded intentionally, I guess #283 should be closed with no resolution. I will close this PR. Sorry for confusion

[ansasaki]

@sarroutbi Sorry, I didn't mean you should close this, but to test if this would actually work in an interoperability scenario.

We can test it and reopen this if it works. But then we would have to work to introduce the new dependency in Fedora.

If it doesn't work, then we should do what you proposed and close #283 with the reasoning.

[sarroutbi]

@sarroutbi Sorry, I didn't mean you should close this, but to test if this would actually work in an interoperability scenario.

We can test it and reopen this if it works. But then we would have to work to introduce the new dependency in Fedora.

If it doesn't work, then we should do what you proposed and close #283 with the reasoning.

No problem. I will reopen it and once tested, will let you know.

[aplanas]

I think @ansasaki is right and my review was wrong. Now the data sent from a x86-64 and aarch64 will have different de-serialization order

[sarroutbi]

I will close this PR, as it has been confirmed that current development status is intentionally done. My suggestion is closing #283, as it can be confusing