[PATCH] erase variable data on user unset

[Robert Millan <rmh@aybabtu.com>]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Sun, Feb 10, 2008 at 03:00:31PM -0500, Isaac Dupree wrote:
> Robert Millan wrote:
> >On Sun, Feb 10, 2008 at 01:00:50PM -0500, Isaac Dupree wrote:
> >>anyway if a hash is used that takes (by design) around one second on the 
> >>machine (e.g. sha256 repeated thousands? millions? of times), then I 
> >>suppose the time taken to erase the memory used by GRUB would be trivial 
> >>in comparison, assuming(rightly or wrongly) a good implementation...
> >
> >The problem is not time, it's just to find the right way to do it.
> 
> yeah. probably involves thinking about GRUB's allocation and 
> deallocation mechanisms, which I don't know anything about and don't 
> have time to investigate :-/

This should address your concern.  As to why I propose to put this in unset
command rather than kernel, since GRUB itself doesn't have any mechanisms
where a variable would contain sensible information, I think it's better to
protect user variables only.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[-- Attachment #2: unset.diff --]
[-- Type: text/x-diff, Size: 979 bytes --]

2008-02-10  Robert Millan  <rmh@aybabtu.com>

	* normal/command.c (unset_command): Erase the contents of the variable
	we're about to unset, before actually unsetting it.

diff -x configure -x config.h.in -x CVS -x '*~' -x '*.mk' -urp ../grub2/normal/command.c ./normal/command.c
--- ../grub2/normal/command.c	2007-07-22 01:32:29.000000000 +0200
+++ ./normal/command.c	2008-02-10 21:42:44.000000000 +0100
@@ -274,10 +274,19 @@ static grub_err_t
 unset_command (struct grub_arg_list *state __attribute__ ((unused)),
 	       int argc, char **args)
 {
+  char *value;
+
   if (argc < 1)
     return grub_error (GRUB_ERR_BAD_ARGUMENT,
 		       "no environment variable specified");
 
+  value = grub_env_get (args[0]);
+
+  /* Users may store sensitive information in their variables (e.g. passwords),
+     so erase its content here when they choose to unset them.  */
+  if (value)
+    grub_memset (value, 0, grub_strlen (value));
+
   grub_env_unset (args[0]);
   return 0;
 }