6242 sha512 is broken in grub

Review Request #100 — Created Oct. 7, 2015 and submitted Oct. 9, 2015, 8:16 p.m.

Information
Owner:	tsoome
Repository:	illumos-gate
Branch:
Bugs:	6242
Depends On:
Commit:	9267603...
Reviewers
Groups:	general
People:

Description

the root and immediate causes for checksum mismatch are missing type casts and wrong byte order in resulting checksum returned to caller.

However, this update also is addressing the case where last block is not full block - perhaps its not actually issue in zfs; also this code is using explicit 128bit message length setup.

Testing Done

build and boot from previously unaccessible pool with grub; accessing files from loader.

Bugs:

+	6242

Change Summary:

updated copyright to make git-pbchk happy

Commit:

-	acb2ec5ebfe438346bdc00cb506684c997c41043
+	f00dbcf32c75fedb63d8d80249da4f78c79f2b01

Diff:

Revision 2 (+36 -10)

Show changes

usr/src/grub/grub-0.97/stage2/zfs_sha256.c

Ship it!

```
Ship It!
```

Ship it!

It isn't clear to me why we need to byteswap the checksum here, but if it works then I'm happy. It would be great if you can also get Saso to review this since he wrote it originally.

tsoome Oct. 7, 2015, 12:55 p.m.

I havent checked the actual standard doc for exact explanation - I just assume our kernel module [sha2.c] (and libgcrypt for that matter) have implemented it according to standards;) and yes, without this byteswap indeed we get resulting bytes in resulting octlets in reverse order and therefore the checksum match will obviously fail - I actually did double-check this while I was developing the initial fix in loader project and while implementing this bugfix for grub.

skiselkov Oct. 9, 2015, 10:19 a.m.

Matt: this is because the GRUB implementation of SHA2 is somewhat simplified and didn't use to do stuff 100% by the book (before this fix). So while calculating, SHA-512 keeps state in eight 64-bit registers labeled H[0] through H[7]. SHA-2 allows little-endian platforms to perform the actual transform arithmetic in native byte order to preserve speed, but for the final product to be consistent with the standard, little-endian hardware then needs to byte-swap output. This wasn't done in GRUB and instead I incorrectly used the little-endian state. If you have a look at Encode64 in usr/src/common/crypto/sha2/sha2.c (which is used during the final output phase in the regular in-kernel SHA2Final()), it byteswaps on little endian hardware.

Ship it!

Just one minor documentation issue, other than that looks great!

usr/src/grub/grub-0.97/stage2/zfs_sha256.c (Diff revision 2)

You might want to add a note to this function to make it clear that it's always encoding in big-endian.