Hi, On Thu, 22 Jul 2021 at 14:54, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Simon, > > On Thu, Jul 22, 2021 at 10:46:40AM -0600, Simon Glass wrote: > > > > > > > >> 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). > > > > Because they are different DTBs, right? Either it can be read-only or > > can't be read-only. At first you said it could not be read-only. Now > > you are saying it needs to be changed. Where is all this coming from? > > > > Not exactly, I said it can be read only, assuming we can switch pages to > RO, as long CONFIG_OF_EMBED is enabled. What happens if you choose > CONFIG_OF_PRIOR_STAGE or CONFIG_OF_SEPARATE? That means the *prior* stage > boot loader needs to know your public key and inject in the dtb it hands > over? > > > > A big portion of the DTBs we build today are horribly outdated > > > compared to the current upstream. Since nowdays there's a spec > > > > That may be true on some boards but it is not my experience, at least > > on ARM. Anyway that is an issue for the board maintainers. I don't > > think this has any bearing on the points we are discussing here. > > > > There's a discussion for DTs and it's evolution. Some are indeed a bit > outdated and I can find details on that. The point I am trying to make > here is that the closer we keep the DTBs to what linux hosts, the easier > it's going to be to keep them up to date. > > > > 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. > > > > There is just so much confusion in all of this and we are going around > > in circles. Let me try to state what I think are points of confusion. > > > > 1. The U-Boot DT needs to be protected against change for lots of > > reasons (drivers misbehaving, etc.). The signature is only one of > > them. > > > > We agree on that > > > 2. The U-Boot DT is separate from the one passed to Linux. So > > discussion about where U-Boot config should go in the Linux DT is not > > germain. > > Not always, there's cases were you can use the same DTB. The reason we don't > is due to the diversion we have. Ideally you should have a single DTB, > which can be embedded into your firmware and you can authenticate it by > just authentication the firmware, but I've abandoned that dream long > ago. > > > > > 3. U-Boot uses DT for its configuration and that is that. It has done > > that for about 7-8 years. U-Boot does not have a user space to provide > > policy and configuration . It cannot do what Linux does and run > > programs and look up filesystems to figure out how to boot. So > > configuration / runtime info go in the DT in U-Boot. I have not seen > > any proposal to do it any other way. I hope you can understand how > > frustrating to have someone come from the Linux world and say, Oh it's > > all wrong... > > Apologies if it sounded like that. I am not trying to point any fingers or > judge any code that's been there for a couple of years. > I never said it's all wrong. I can understand some hardware specific config > going into a DT that's been used for a couple of years. What I don't > understand is how a signature fits that profile. > > > we should put it user space, etc. The alternative to DT is > > a mishmash of random places and ideas with no schema and no > > discoverability, etc, or a forest of CONFIG options, like it used to > > me. > > > > 4. The DT is relocated anyway so is not actually read-only just > > because you put it in the rodata area. > > You mean the signature here right (not the DT). The point is if we fix the > .rodata section and it ends up on RO memory, then you do get what we want > and it's always available not matter who provides the DTB, or what > expectations you have for fixing it up (hence put it in R/W). > > > > > 5. You can make the DT read-only if you want to. You can make any part > > of memory read-only. You need to create an API for that if that's what > > you want. It would be nice to have a command to look at what is > > protected and change it. See for example the 'mtrr' command on x86. > > > > I'll repeat myself but isn't that the case for CONFIG_OF_EMBED only? > > > Also I would add that there has always been a DT spec. I think the > > latest version is here: > > https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3 > > So far as I can tell it does not talk about what can and cannot go in a DT > > > > Perhaps the spec you are referring to is a Linux spec. Do you have a > > link? > > I don't think there's a 'linux spec'. The DTS ended up being maintained in > the kernel. I am not gonna argue if this is a good or bad thing, but that > codebase is trying to follow the spec you pasted > > > So far as I can tell, U-Boot, Zephyr, etc. have had very little > > input into that. I know for a fact that no one has asked what I think. > > For example, even the u-boot,xxx tags are kept in separate files in > > U-Boot because of resistance to putting that in Linux. Zephyr > > completely does its own thing with DT. U-Boot very much follows Binux, > > BUT it has its own things as well, just as Linux does. I would LOVE to > > see that change and if you would like to help with that, or have ideas > > on how, please go ahead. > > I am joining the calls for that exact reason. So we can discuss that there > I guess?
Very unfortunately this patch was silently applied to a tree and is now in mainline, despite this discussion not being resolved. I will send a revert until this can be resolved. Regards, Simon
