On Thu, 22 Jul 2021 at 16:30, Simon Glass <s...@chromium.org> wrote: > > Hi Ilias, > > On Thu, 22 Jul 2021 at 07:28, Simon Glass <s...@chromium.org> wrote: > > > > Hi Ilias, > > > > On Wed, 21 Jul 2021 at 00:42, Ilias Apalodimas > > <ilias.apalodi...@linaro.org> wrote: > > > > > > Hi Simon, > > > > > > On Tue, 20 Jul 2021 at 20:42, Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Tue, 20 Jul 2021 at 07:32, Sughosh Ganu <sughosh.g...@linaro.org> > > > > wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Tue, 20 Jul 2021 at 18:20, Ilias Apalodimas > > > > > <ilias.apalodi...@linaro.org> wrote: > > > > >> > > > > >> Hi Simon, > > > > >> On Tue, 20 Jul 2021 at 15:33, Simon Glass <s...@chromium.org> wrote: > > > > >> > > > > > >> > Hi Ilias, > > > > >> > > > > > >> > On Sat, 17 Jul 2021 at 08:27, Ilias Apalodimas > > > > >> > <ilias.apalodi...@linaro.org> wrote: > > > > >> > > > > > > >> > > The capsule signature is now part of our DTB. This is > > > > >> > > problematic when a > > > > >> > > user is allowed to change/fixup that DTB from U-Boots command > > > > >> > > line since he > > > > >> > > can overwrite the signature as well. > > > > >> > > > > > >> > Just to repeat my question since it looks like I didn't get a > > > > >> > response > > > > >> > on the last patch: > > > > >> > > > > > >> > Do you mean with the 'fdt' command? > > > > >> > > > > > > >> > If you mean the FDT fixups, they happen to a different DT, the one > > > > >> > being passed to Linux. > > > > >> > > > > >> In some platforms the key is derived from the relocated DTB, which we > > > > >> can overwrite. But I'll let Sughosh who figured it out explain the > > > > >> details. > > > > > > > > > > > > > > > On platforms where the dtb is concatenated with the u-boot image, > > > > > using CONFIG_OF_SEPARATE, the fdt is also getting relocated to the > > > > > main memory. We retrieve the public key from this dtb. By default, > > > > > the fdtcontroladdr env variable is getting set to this relocated dtb > > > > > address -- this address can also be accessed using the bdinfo > > > > > command. Thus the public key can be modified before attempting the > > > > > capsule update. Which is the reason why Ilias is moving the public > > > > > key to the embedded rodata section. > > > > > > > > You should be clearer about what problem you are trying to solve. Are > > > > you worried about a script changing the DT? Or just it being writable > > > > in general? > > > > > > Being writable in general is my main concern. Doing fixup internally > > > from U-Boot might be something we'll always need but the ability to > > > completely change it doesn't play well security. > > > > > > > > > > > U-Boot itself is relocated also, including the rodata. So are you > > > > using the public key from the original location? What if that is not > > > > accessible after relocation? > > > > > > We are accessing he key from the relocated address. > > > > Then in what way are you protecting it? This is so confusing. Are you > > saying that you are protecting the relocated address? If so, protect > > the relocated devicetree too! > >
How? DTBs if fixed up and there's a protocol proposal from Heinrich, which allows fixups from GRUB2. So how exactly are you going to put it in r/o memory (which is what .rodata is supposed to achieve). A big portion of the DTBs we build today are horribly outdated compared to the current upstream. Since nowdays there's a spec describing what can and can't go in a DTB, I'd much rather prefer we stick to that and make a potential update easier. Regards /Ilias > > > > > > > > > > > There is also the 'fdt addr -c' command to find the control DT. It is > > > > not expected to be written to though. So just protect the memory to > > > > which it is relocated, or relocate it to a place that you can protect. > > > > > > Can you define 'protect'? The mmu support in U-Boot is kind of limited > > > from what I can see. In order to protect anything we'd have to switch > > > the pages ro R-- or RX-. Someone please shout if I am wrong, but I > > > couldn't find code doing that in U-Boot. > > > > > > > > > > > If the DT is writable it will affect U-Boot's operation, since that is > > > > where all the config is stored. There is no point in pretending that > > > > pulling one thing out of it and protecting it will result in any sort > > > > of improvement. This needs to be done properly. > > > > > > Tbh I thought the relocation was done properly and the .rodata section > > > was either merged with .text (and was RX-) or R--. > > > If we fix the relocation properly, the .rodata will be read only, > > > while the DTB should still be on RW memory. I understand that you > > > are using the DTB for some configuration on U-Boot ,but arguably the > > > public key of the EFI firmware you need to authenticate hardly > > > classifies as configuration. > > > > What is it then? > > Please just do this properly. Add an API to protect memory, implement > it for your chosen platform and then we will actually fix the problem > you are worried about. Until then, work-arounds and hacks are > pointless and just confuse the issue. Most haste, less speed. > > > > > > Keep in mind that even if we fix the page tables permissions, on a > > > secure platform the command line should be disabled, since anyone > > > could load an arbitrary application and modify them. > > > > Yes we brought in CONFIG_CMDLINE for that reason about five years ago :-) > > Regards, > SImon
