Hi,
the attached patches change the OLPC build system to use relative payload paths instead of using a hardcoded file in /tmp which will sooner or later cause security problems. Additionally, a check for config file correctness has been added so you can't accidentially generate the wrong payload for a given LinuxBIOS config. I forgot how to invert command return codes in makefiles, so it would be cool if somebody could replace the FIXME in the olpc patch with the inverted grep command of some lines later.
Please note: The buildrom patch can only be applied upstream after teh LinuxBIOS patch has been applied upstream (and the needed svn revision obviously also has to be updated).
Regards, Carl-Daniel
* Carl-Daniel U. Hailfinger c-d.hailfinger.devel.2006@gmx.net [060821 19:16]:
- payload /tmp/olpcpayload.elf
- payload ../../../../../../../../deploy/olpc-payload
end
Hm. I dont know. The above path is pretty unreadable. Or at least really ugly.
Can we have it parse environment variables instead? $(TOPDIR) / $(PAYLOADDIR) ?
On 21/08/06 19:16 +0200, Carl-Daniel U. Hailfinger wrote:
Hi,
the attached patches change the OLPC build system to use relative payload paths instead of using a hardcoded file in /tmp which will sooner or later cause security problems. Additionally, a check for config file correctness has been added so you can't accidentially generate the wrong payload for a given LinuxBIOS config.
- payload /tmp/olpcpayload.elf
- payload ../../../../../../../../deploy/olpc-payload
I agree that using /tmp is a problem, but this is definitely not the way to fix it. First of all, it breaks LinuxBIOS only builds, and secondly, it is dependent on how buildrom works, which may or may not change on a moment's notice. it would be smarter to use sed to modify the configuration on the fly, as the attached patch indicates.
Jordan
LINUXBIOS: Update the payload string to be more secure
From: Jordan Crouse jordan.crouse@amd.com
---
packages/linuxbios/linuxbios.mk | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/packages/linuxbios/linuxbios.mk b/packages/linuxbios/linuxbios.mk index 8b2ffc2..2e35c30 100644 --- a/packages/linuxbios/linuxbios.mk +++ b/packages/linuxbios/linuxbios.mk @@ -47,16 +47,14 @@ endif $(LINUXBIOS_VER) > $(LINUXBIOS_FETCH_LOG) 2>&1 @ touch $@
-# fix me sooner or later! -/tmp/olpcpayload.elf: $(PAYLOAD_TARGET) - @ cp $< $@ - $(LINUXBIOS_STAMP_DIR)/.configured: $(LINUXBIOS_STAMP_DIR)/.unpacked_$(LINUXBIOS_VER) + @ cp -f $(LINUXBIOS_TARGET_DIR)/$(LINUXBIOS_CONFIG_NAME) $(LINUXBIOS_TARGET_DIR)/$(LINUXBIOS_CONFIG_NAME).orig + cat $(LINUXBIOS_TARGET_DIR)/$(LINUXBIOS_CONFIG_NAME).orig | sed -e 's:payload .*$$:payload $(PAYLOAD_TARGET):' > $(LINUXBIOS_TARGET_DIR)/$(LINUXBIOS_CONFIG_NAME) @( cd $(LINUXBIOS_TARGET_DIR); \ ./buildtarget $(LINUXBIOS_CONFIG_NAME) > $(LINUXBIOS_CONFIG_LOG) 2>&1) @ touch $@
-$(LINUXBIOS_BUILD_DIR)/linuxbios.rom: $(LINUXBIOS_STAMP_DIR)/.configured /tmp/olpcpayload.elf +$(LINUXBIOS_BUILD_DIR)/linuxbios.rom: $(LINUXBIOS_STAMP_DIR)/.configured $(PAYLOAD_TARGET) @ echo "Building linuxbios..." @ make -C $(LINUXBIOS_BUILD_DIR) > $(LINUXBIOS_BUILD_LOG) 2>&1
I agree that using /tmp is a problem, but this is definitely not the way to fix it. First of all, it breaks LinuxBIOS only builds, and secondly, it is dependent on how buildrom works, which may or may not change on a moment's notice. it would be smarter to use sed to modify the configuration on the fly, as the attached patch indicates.
While very elegant, I think changing the config file on the fly makes it harder to see whats going on. Rather than copy the payload into temp can we not just copy the payload into $(LINUXBIOS_TARGET_NAME)
Then the config file just has PAYLOAD ../olpcpayload.elf which should work for both buildrom and non-buildrom builds.
Perhaps we can build and export a $(LINUXBIOS_PAYLOAD_PATH)? So the higher level make code knows where to stick it.
That to me would seem to be a much more discoverable behavior than changing the config file automagically.
On 21/08/06 14:41 -0500, Richard Smith wrote:
I agree that using /tmp is a problem, but this is definitely not the way to fix it. First of all, it breaks LinuxBIOS only builds, and secondly, it is dependent on how buildrom works, which may or may not change on a moment's notice. it would be smarter to use sed to modify the configuration on the fly, as the attached patch indicates.
While very elegant, I think changing the config file on the fly makes it harder to see whats going on. Rather than copy the payload into temp can we not just copy the payload into $(LINUXBIOS_TARGET_NAME)
Then the config file just has PAYLOAD ../olpcpayload.elf which should work for both buildrom and non-buildrom builds.
Perhaps we can build and export a $(LINUXBIOS_PAYLOAD_PATH)? So the higher level make code knows where to stick it.
That to me would seem to be a much more discoverable behavior than changing the config file automagically.
True - I have less of a problem with this since I percieve payload to be an variable that the LinuxBIOS build system expects the user to set. Regardless, changing the config file on the fly is very much a Bad Thing. I wouldn't have any problem redirecting the payload somewhere within the LinuxBIOS build, assuming the LinuxBIOS owners are cool with that paradigm.
Jordan
I wouldn't have any problem redirecting the payload somewhere within the LinuxBIOS build, assuming the LinuxBIOS owners are cool with that paradigm.
Speaking as one I don't have an issue.
In fact thats normally how I setup my config files anyway. Most LinuxBIOS payloads are build outside of LB. I usually copy them into my target directory with my config files so that if I need to blow away my build dir and start over clean I can without nuking my payload(s).
Stefan Reinauer wrote:
- Carl-Daniel U. Hailfinger c-d.hailfinger.devel.2006@gmx.net [060821 19:16]:
- payload /tmp/olpcpayload.elf
- payload ../../../../../../../../deploy/olpc-payload
end
Hm. I dont know. The above path is pretty unreadable. Or at least really ugly.
Can we have it parse environment variables instead? $(TOPDIR) / $(PAYLOADDIR) ?
using an environment variable instead of a hardcoded path has been on my list for a long time. Rather than all this path finagling, we might want to consider doing that.
thanks
ron
-
Carl-Daniel U. Hailfinger -
Jordan Crouse -
Richard Smith -
Ronald G Minnich -
Stefan Reinauer