Patchwork Stop putting useless or sensitive information in coreboot images

login
register
about
Submitter Uwe Hermann
Date 2010-10-07 17:35:18
Message ID <20101007173518.GN3256@greenwood>
Download mbox | patch
Permalink /patch/2069/
State New
Headers show

Comments

Uwe Hermann - 2010-10-07 17:35:18
See patch.

This is only build-tested, please carefully review if it might have
impact on software that depends on the coreboot tables.


Uwe.
Stefan Reinauer - 2010-10-07 20:25:24
On 10/7/10 10:35 AM, Uwe Hermann wrote:
> See patch.
>
> This is only build-tested, please carefully review if it might have
> impact on software that depends on the coreboot tables.
>
>
> Uwe.
I don't think this information is useless.

Knowing the user name and domain tells you who built the image (in an
environment where nobody tries to obfuscate it, obviously). And, I
certainly would want to know if someone from nsa.gov built my image ;-)

As for it being sensitive, I could understand that people want some sane
information in there. We could add a Kconfig option to overwrite these
strings. Or we could make their inclusion an option.

Please don't just drop it.

Stefan
Aurélien - 2010-10-07 21:18:53
On Thu, Oct 7, 2010 at 7:35 PM, Uwe Hermann <uwe@hermann-uwe.de> wrote:
> See patch.
>
> This is only build-tested, please carefully review if it might have
> impact on software that depends on the coreboot tables.
>

While I do agree that it would be nice to have an option to keep this
information from appearing in the final rom, I think that:

 - Usernames are not that sensitive, and can be used to track who
compiled a particular image in a small development team. This can be
useful. For a bigger company, i would expect a build bot username at
this place :)

 - You should never build anything on a non-development machine,
particularly on exposed hosts - so I hope to never cross a coreboot
rom built on "firewall1" :) I do get your point, however. For
origin-tracing purposes, an unqualified host name should be
sufficient, no need for the domain part. Development servers should be
on private LANs, so these names should not directly resolve from
outside. Yes, I know, should.

Best regards,

Patch

Stop putting useless or sensitive information in coreboot images.

While recording the compiler/linker/assembler version, or the coreboot
svn revision, and maybe even the time when the coreboot.rom was built may
all be useful in some situations (e.g. for debugging), it's pretty useless
to save some other information.

Not only that, I think it's even potentially privacy-sensitive stuff we
record, so stop doing it.

Drop the following information:

 - Which local user account was used to build the image. Nobody should care
   about this, and it reveals potentially private information (real name
   of employees, or whatever; usernames such as "accouting23" or "itsecurity5",
   "johncdvorak" or "supersexyguy12" is not what we want to make public).

 - The hostname of where the image was built. Equally useless information
   IMHO, and could reveal company-internal and potentially security-sensitive
   information (think "firewall1", "vnp3", or "dmz" hostnames).

 - The domain of the host. Same as above, we don't care and it could reveal
   sensitive information. User name "pauljohnson" on host "firewall2" and
   domain "whitehouse.gov" anyone? I don't think we want that.

Note that all this stuff can be viewed with "strings coreboot.rom" on
publically posted coreboot images that users send around or post on websites
for debugging purposes, and partially also in (publically posted) coreboot
log files etc. etc.

Signed-off-by: Uwe Hermann <uwe@hermann-uwe.de>

Index: src/include/version.h
===================================================================
--- src/include/version.h	(Revision 5918)
+++ src/include/version.h	(Arbeitskopie)
@@ -12,9 +12,6 @@ 
 
 /* When coreboot was compiled */
 extern const char coreboot_compile_time[];
-extern const char coreboot_compile_by[];
-extern const char coreboot_compile_host[];
-extern const char coreboot_compile_domain[];
 extern const char coreboot_compiler[];
 extern const char coreboot_linker[];
 extern const char coreboot_assembler[];
Index: src/lib/version.c
===================================================================
--- src/lib/version.c	(Revision 5918)
+++ src/lib/version.c	(Arbeitskopie)
@@ -18,12 +18,6 @@ 
 #ifndef COREBOOT_COMPILE_TIME
 #error  COREBOOT_COMPILE_TIME not defined
 #endif
-#ifndef COREBOOT_COMPILE_BY
-#error  COREBOOT_COMPILE_BY not defined
-#endif
-#ifndef COREBOOT_COMPILE_HOST
-#error  COREBOOT_COMPILE_HOST not defined
-#endif
 
 #ifndef COREBOOT_COMPILER
 #error  COREBOOT_COMPILER not defined
@@ -47,9 +41,6 @@ 
 const char coreboot_build[] = COREBOOT_BUILD;
 
 const char coreboot_compile_time[]   = COREBOOT_COMPILE_TIME;
-const char coreboot_compile_by[]     = COREBOOT_COMPILE_BY;
-const char coreboot_compile_host[]   = COREBOOT_COMPILE_HOST;
-const char coreboot_compile_domain[] = COREBOOT_COMPILE_DOMAIN;
 const char coreboot_compiler[]       = COREBOOT_COMPILER;
 const char coreboot_linker[]         = COREBOOT_LINKER;
 const char coreboot_assembler[]      = COREBOOT_ASSEMBLER;
Index: src/arch/i386/boot/coreboot_table.c
===================================================================
--- src/arch/i386/boot/coreboot_table.c	(Revision 5918)
+++ src/arch/i386/boot/coreboot_table.c	(Arbeitskopie)
@@ -218,9 +218,6 @@ 
 		{ LB_TAG_EXTRA_VERSION,  coreboot_extra_version,  },
 		{ LB_TAG_BUILD,          coreboot_build,          },
 		{ LB_TAG_COMPILE_TIME,   coreboot_compile_time,   },
-		{ LB_TAG_COMPILE_BY,     coreboot_compile_by,     },
-		{ LB_TAG_COMPILE_HOST,   coreboot_compile_host,   },
-		{ LB_TAG_COMPILE_DOMAIN, coreboot_compile_domain, },
 		{ LB_TAG_COMPILER,       coreboot_compiler,       },
 		{ LB_TAG_LINKER,         coreboot_linker,         },
 		{ LB_TAG_ASSEMBLER,      coreboot_assembler,      },
Index: Makefile
===================================================================
--- Makefile	(Revision 5918)
+++ Makefile	(Arbeitskopie)
@@ -320,9 +320,6 @@ 
 	printf "#define COREBOOT_ASSEMBLER \"$(shell LANG= $(AS) --version | head -n1)\"\n" >> $(obj)/build.ht
 	printf "#define COREBOOT_LINKER \"$(shell LANG= $(LD) --version | head -n1)\"\n" >> $(obj)/build.ht
 	printf "#define COREBOOT_COMPILE_TIME \"`LANG= date +%T`\"\n" >> $(obj)/build.ht
-	printf "#define COREBOOT_COMPILE_BY \"$(subst \,@,$(shell PATH=$$PATH:/usr/ucb whoami))\"\n" >> $(obj)/build.ht
-	printf "#define COREBOOT_COMPILE_HOST \"$(shell hostname -s 2>/dev/null)\"\n" >> $(obj)/build.ht
-	printf "#define COREBOOT_COMPILE_DOMAIN \"$(shell test `uname -s` = "Linux" && dnsdomainname || domainname 2>/dev/null)\"\n" >> $(obj)/build.ht
 	printf "#endif\n" >> $(obj)/build.ht
 	mv $(obj)/build.ht $(obj)/build.h