Extract more info from toc0 files by jameshilliard · Pull Request #7 · apritzel/sunxi-fw

jameshilliard · 2025-03-31T06:26:52Z

It's handy to be able to extract and compute everything needed to calculate the rotpk from toc0 for validating that images are being generated correctly. Probably not everything is being computed correctly but it does look like the rotpk hash at least is correct.

Example output:

@   0: toc0: signed boot image
TOC0 Name: 'TOC0.GLH'
Magic: 0x89119800 (valid)
End Marker: 0x3b45494d (valid)
Serial Number: 0x00000000
Status: 0x00000000
Number of TOC0 items: 2
Total size: 98304 bytes
Platform: 00 00 00 00
Checksum: 0xb6f9d982 (valid)

Item:
  Name: 0x010101 (SBROMSW_CERTIF)
  Offset: 0x00000c80
  Length: 764 bytes
  Status: 0x00000000
  Type: 1
  Run Address: 0x00000000
  End Marker: 0x3b454949 (valid)
  -> Certificate File
  ROTPK (modulus + exponent):
  b3 70 a4 d4 f5 ee 5e 32 d0 0c cb e9 22 9f 08 1f 
  5c 74 c5 fc a5 8b 8b 21 db 86 a8 de 92 82 7c ef 
  70 bc 40 d1 20 87 2f 10 dd e2 97 49 dd 9d a3 18 
  b2 37 61 49 7b 71 a2 81 69 cb 8a 79 e3 16 d6 4e 
  05 fa 98 77 4b 83 8d d8 a1 f0 c3 9e 68 49 ed fc 
  c7 58 4f e9 d7 41 fb 5c ea e0 49 d2 35 46 82 2f 
  68 fd 3b fe c5 06 72 30 d4 b1 29 7b b8 8f 94 cd 
  4c 45 31 d2 47 5b 43 b1 03 27 04 ed 0e 47 50 1d 
  e9 bb 30 9f 64 35 f2 2a 13 4d 47 5c 2a c7 1d 49 
  05 0d a4 39 c5 e6 2b 79 6f 19 6e fc f9 9c b5 2a 
  e7 d6 9a 42 b4 a7 2c 69 8d f3 22 28 0e 20 ff 93 
  be 9f 4a 22 8e ad 36 54 f7 24 14 1b b1 55 0c 3d 
  3d e4 68 c2 34 7b dc 58 72 c5 66 cf a9 4c 48 6f 
  32 07 ca 6c 80 2d c4 99 e5 6b c0 38 34 a5 d5 9c 
  c7 ad d5 16 c2 83 e6 59 d1 37 e1 cc c1 59 8f cd 
  e0 1f 5f 4b 8e 71 36 f9 e3 a5 b6 ea 48 b6 f1 d3 
  01 00 01 
  ROTPK SHA256 (from CERTIF):
  3c 40 45 7a be f9 1c 13 79 8b e0 d7 e2 64 2d e2 
  a7 3f ef ee 13 2c 3b 66 11 7f 15 2c 2a 7a 65 6f 

Item:
  Name: 0x010202 (SBROMSW_FW)
  Offset: 0x00000f80
  Length: 94208 bytes
  Status: 0x00000000
  Type: 2
  Run Address: 0x00020480
  End Marker: 0x3b454949 (valid)
  -> Signed Boot Firmware

apritzel

Hi James, thanks for the contribution, TOC0 was indeed some kind of blind spot for the tool.
However this patch is quite massive, can you possibly split this up? If you have any fixes (PREFIX default) or refactorings, they come first. Then I'd see one patch that just extents the reporting of the TOC0 header and items, without the validation parts. That's a nice extra, but ideally I'd make this optional, to avoid the OpenSSL dependency.
Some first comments inside, in general please adhere to kernel coding style, and allow streaming data, so no backward seeks.

apritzel · 2025-04-01T23:31:55Z

Makefile

-CFLAGS=-Wall -g -O
-PREFIX ?=/usr/local
+CFLAGS=$(shell pkg-config --cflags openssl) -Wall -g -O
+LDFLAGS=$(shell pkg-config --libs openssl) -lfdt


Can we please make this OpenSSL dependency optional somehow? I like to keep a statically compiled version around, and verifying TOC0 images is quite a niche use case, so I don't think this justifies this dependency.

Can we please make this OpenSSL dependency optional somehow?

Yeah, let me see how best to do that.

I like to keep a statically compiled version around, and verifying TOC0 images is quite a niche use case, so I don't think this justifies this dependency.

I don't think this prevents statically linking openssl right? I think in general this is a bit of a niche tool so I'm not sure how important it would be to make a common dependency like OpenSSL optional.

Openssl is now optional.

apritzel · 2025-04-01T23:32:37Z

Makefile

-PREFIX ?=/usr/local
+CFLAGS=$(shell pkg-config --cflags openssl) -Wall -g -O
+LDFLAGS=$(shell pkg-config --libs openssl) -lfdt
+PREFIX ?= /usr/local


That's indeed fixing an omission, but can you put this in a separate patch, please?

The prefix? I'm not sure what you're referring to.

sunxi-fw.h

apritzel · 2025-04-01T23:39:10Z

sunxi-toc0.c

+#include <unistd.h>
+#include <errno.h>
+#include <openssl/sha.h>
+#include <openssl/core_names.h>


This is an OpenSSL3 header file, thus breaks compilation on older installations. Officially 1.1.1 is obsolete, but there are maintained packages of that, with backports. And I'd appreciate if you don't break the build on my Slackware 15.0 installation (shipping 1.1.1zb) ;-)
U-Boot's mkimage manages to get away with an 1.1.1 subset of functions as well, so can you please refrain to use only functions and features that both versions support?

If we make openssl optional then this isn't a major concern right?

apritzel · 2025-04-01T23:42:48Z

sunxi-toc0.c

+	} totalSequence;
+};
+
+static inline bool is_rsa_pubkey_tag(const uint8_t *buf) {


Kernel coding style please, so opening brackets for functions on a new line. And inline is not useful here, please remove.

apritzel · 2025-04-01T23:47:33Z

sunxi-toc0.c

+	uint32_t *buf = (uint32_t *)buff;
+	uint32_t sum = BROM_STAMP_VALUE;
+	for (uint32_t i = 0; i < length / 4; ++i)
+		sum += buf[i];


and an empty line here, please

apritzel · 2025-04-01T23:54:24Z

sunxi-toc0.c

+}
+
+static int verify_signature(const uint8_t *sig, size_t sig_len,
+							const uint8_t *tbs, size_t tbs_len,


Please observe kernel indentation rules and 80 columns, if possible.

Ran this through clang-format with kernel style configs.

apritzel · 2025-04-01T23:56:02Z

sunxi-toc0.c

+
+	bld = OSSL_PARAM_BLD_new();
+	if (!bld ||
+		!OSSL_PARAM_BLD_push_BN(bld, "n", BN_bin2bn(mod, mod_len, NULL)) ||


kernel indentation style please: so line continuation vertically aligned with the opening bracket, so that the body of the function sticks out because of the different alignment.

apritzel · 2025-04-01T23:59:40Z

sunxi-toc0.c

+}
+
+int output_toc0_info(void *sector, FILE *inf, FILE *outf, bool verbose) {
+	fseek(inf, 0, SEEK_END);


Sorry, but this is breaking an assumption this tool makes: the input data could be a stream, for instance from a piped decompressor (xz -cd foo.img.xz | sunxi-fw info), so no absolute seeks. There is pseek() if you need to skip some bytes.

Yeah, I guess I can probably just use the size pulled from the header instead.

This should be fixed now.

apritzel · 2025-04-02T00:00:49Z

sunxi-toc0.c

+	if (file_size <= 0)
+		return -EINVAL;
+
+	uint8_t *toc0_data = malloc(file_size);


Please gather all variable declarations at the beginning of the function. If you find this too far away, consider splitting up the function.

Should be fixed now.

It's handy to be able to extract and compute everything needed to calculate the rotpk from toc0 for validating that images are being generated correctly.

jameshilliard · 2025-04-02T07:27:02Z

If you have any fixes (PREFIX default) or refactorings, they come first.

I think the PREFIX default was already set on a different line wasn't it? There wasn't really much code here so there's not really much refactoring.

However this patch is quite massive, can you possibly split this up?

It's a bit tricky to split up further IMO, I mean already this is just an initial attempt at parsing some of the values out, there's still a lot needed to fully parse these toc0 files AFAIU.

jameshilliard force-pushed the calc-rotpk branch 5 times, most recently from e1f509f to 04f118e Compare April 1, 2025 23:01

apritzel requested changes Apr 2, 2025

View reviewed changes

jameshilliard force-pushed the calc-rotpk branch 6 times, most recently from d081142 to 80c19d7 Compare April 2, 2025 06:27

Extract more info from toc0 files

b2ac8ee

It's handy to be able to extract and compute everything needed to calculate the rotpk from toc0 for validating that images are being generated correctly.

jameshilliard force-pushed the calc-rotpk branch from 80c19d7 to b2ac8ee Compare April 2, 2025 07:16

jameshilliard requested a review from apritzel April 2, 2025 07:31

Conversation

jameshilliard commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apritzel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jameshilliard commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jameshilliard commented Mar 31, 2025 •

edited

Loading