unify two disparate PE executable parsers we currently have into one

[poettering]

Our codebase contains three PE binary parsers: two in userspace, one in EFI. This sucks of course. Unifying disk reading over userspace and EFI is hard, so let's avoid that for now. Let's however unify the userspace parsers already.

Except that I actually had written a 3rd PE parse in #28891 for the Authenticode hash calculation. So, actually unify three parsers into one.

This doesn't bring any features. It just unifies the two parses, moving the generic one to src/shared/pe-binary.[ch], just refactoring hence.

[medhefgo]

Unifying disk reading over userspace and EFI is hard, so let's avoid that for now.

Well, not necessarily. For our uses, only doctored images would have headers that are larger than 4K. So you could make a unified API that only works over a provided memory block to access and handle the headers. Further convenience APIs to operate on the whole (mmapped) image can be provided to userspace, while EFI code would just use the offsets directly as it currently does…

(Just a though)

[medhefgo]

Alternatively, there has got to be a userspace library that we can use for this. This is one of those stupid C-dependencies-suck-so-let's-reinvent-the-wheel-instead cases.

(There is https://lief-project.github.io/doc/stable/index.html, which has a C-API, for example.)

[medhefgo]

PE code looks fine.

[poettering]

Well, not necessarily. For our uses, only doctored images would have headers that are larger than 4K. So you could make a unified API that only works over a provided memory block to access and handle the headers. Further convenience APIs to operate on the whole (mmapped) image can be provided to userspace, while EFI code would just use the offsets directly as it currently does…

(Just a though)

Hmm, the problem is that the embedded DOS stub in a PE binary can be any size (i.e. it's a form of "fat" binary).

I mean, I am not saying it wouldn't be possible to get this abstracted and make it nice even, all I am saying that for now I was too lazy for that ;-)

[poettering]

(There is https://lief-project.github.io/doc/stable/index.html, which has a C-API, for example.)

Hmm, that is not even packaged for Fedora yet. Also, looking at the code it uses unchecked malloc() in the code, which makes me quite suspicous of the code quality. And C++, uh.

I tried to find an existing implementation of the PE authenticode hash stuff that we could link to, but couldn't find anything reasonably nice. I think the implementation in our code is not overly complex. There's certainly a threshold where I'd say "hey, let's better use existing code" (and we do that in many areas, we link against so many libs after all), but I am not sure with the little PE stuff we need that point is already reached here.

[poettering]

Force pushed a new version. Only change is the strncmp() thing and a rebase. Since the only other discussion point left was the UKI check, i.e. whether to include .osrel or not, but I'd vote for doing this separately. Hence taking liberty to set green label.

[bluca]

Force pushed a new version. Only change is the strncmp() thing and a rebase. Since the only other discussion point left was the UKI check, i.e. whether to include .osrel or not, but I'd vote for doing this separately. Hence taking liberty to set green label.

Given direct booting is supported, I think it makes sense that osrel is optional as the spec says. As long as you have a kernel and an initrd, the image will boot it, so those should be the only mandatory sections I'd think?