[PATCH v0] Additional security-relevant documentation

[PATCH v0] Additional security-relevant documentation

[Jon McCune]

Tested with 'make html dvi pdf ps info'.  I think the output looks
reasonable in each case, but feedback is welcomed.

One known issue is that while grub-install does not accept --pubkey,
there is some awkward discussion of wrapping grub-mkimage.

Commands associated with check_signatures:
 * High-level discussion in new section "Security and signatures"
 * Environment variable check_signatures
 * New documentation for commands distrust, trust, list_trusted, verify_detached
 * Modifications to documentation for load_env, save_env, hashsum

Signed-off-by: Jon McCune <jonmccune@google.com>
---
 docs/grub.texi | 220 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 218 insertions(+), 2 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 2bf0d8b..6fd9722 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -96,6 +96,7 @@ This edition documents version @value{VERSION}.
 * Commands::                    The list of available builtin commands
 * Internationalisation::        Topics relating to language support
 * Security::                    Authentication and authorisation
+* Security and signatures::     Verifying digital signatures in GRUB
 * Platform limitations::        The list of platform-specific limitations
 * Platform-specific operations:: Platform-specific operations
 * Supported kernels::           The list of supported kernels
@@ -2756,6 +2757,7 @@ These variables have special meaning to GRUB.
 
 @menu
 * biosnum::
+* check_signatures::
 * chosen::
 * color_highlight::
 * color_normal::
@@ -2809,6 +2811,27 @@ For an alternative approach which also changes BIOS drive mappings for the
 chain-loaded system, @pxref{drivemap}.
 
 
+@node check_signatures
+@subsection check_signatures
+
+This variable controls whether GRUB enforces digital signature
+validation (@pxref{Security and signatures}) on all loaded files.  If
+@code{check_signatures=enforce}, then every attempt by the GRUB
+@file{core.img} to load another file @file{foo} (e.g., a loadable
+module, a configuration file, or a Linux kernel) implicitly invokes
+@code{verify_detached foo foo.sig} (@pxref{verify_detached}).
+@code{foo.sig} must contain a valid digital signature over the
+contents of @code{foo}, which can be verified with a public key
+currently trusted by GRUB (@pxref{list_trusted}, @pxref{trust}, and
+@pxref{distrust}).  If validation fails, then file @file{foo} cannot
+be opened.  This failure may halt or otherwise impact the boot
+process.  An initial trusted public key can be embedded within the
+GRUB @file{core.img} using the @code{--pubkey} option to
+@command{grub-mkimage} (@pxref{Invoking grub-install}).  Presently it
+is necessary to write a custom wrapper around @command{grub-mkimage}
+using the @code{--grub-mkimage} flag to @command{grub-install}.
+
+
 @node chosen
 @subsection chosen
 
@@ -3458,6 +3481,7 @@ you forget a command, you can run the command @command{help}
 * cryptomount::                 Mount a crypto device
 * date::                        Display or set current date and time
 * devicetree::                  Load a device tree blob
+* distrust::                    Remove a pubkey from trusted keys
 * drivemap::                    Map a drive to another
 * echo::                        Display a line of text
 * eval::                        Evaluate agruments as GRUB commands
@@ -3475,6 +3499,7 @@ you forget a command, you can run the command @command{help}
 * linux::                       Load a Linux kernel
 * linux16::                     Load a Linux kernel (16-bit mode)
 * list_env::                    List variables in environment block
+* list_trusted::                List trusted public keys
 * loadfont::                    Load font files
 * load_env::                    Load variables from environment block
 * loopback::                    Make a device from a filesystem image
@@ -3506,9 +3531,11 @@ you forget a command, you can run the command @command{help}
 * source::                      Read a configuration file in same context
 * test::                        Check file types and compare values
 * true::                        Do nothing, successfully
+* trust::                       Add public key to list of trusted keys
 * unset::                       Unset an environment variable
 * uppermem::                    Set the upper memory size
 @comment * vbeinfo::                     List available video modes
+* verify_detached::             Verify detached digital signature
 * videoinfo::                   List available video modes
 @end menu
 
@@ -3779,6 +3806,16 @@ but rather replaces it completely.
 @ref{GNU/Linux}.
 @end deffn
 
+@node distrust
+@subsection distrust
+
+@deffn Command distrust pubkey_id
+Remove public key @var{pubkey_id} from GRUB's keyring of trusted keys.
+These keys are used to validate signatures when
+@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
+invocations of @command{verify_detached} (@pxref{verify_detached}).
+@xref{Security and signatures} for more information.
+@end deffn
 
 @node drivemap
 @subsection drivemap
@@ -3937,7 +3974,8 @@ list of @var{hash name} pairs in the same format as used by UNIX
 @command{md5sum} command. Option @option{--prefix}
 may be used to give directory where files are located. Hash verification
 stops after the first mismatch was found unless option @option{--keep-going}
-was given.
+was given.  The exit code @code{$?} is set to 0 if hash verification
+is successful.  If it fails, @code{$?} is set to a nonzero value.
 @end deffn
 
 
@@ -4046,16 +4084,50 @@ The @option{-f} option overrides the default location of the environment
 block.
 @end deffn
 
+@node list_trusted
+@subsection list_trusted
+
+@deffn Command list_trusted
+List all public keys trusted by GRUB for validating signatures. These
+public keys are used implicitly when environment variable
+@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
+invocations of @command{verify_detached}.  @xref{Security and
+signatures} for more information.
+@end deffn
 
 @node load_env
 @subsection load_env
 
-@deffn Command load_env [@option{-f} file]
+@deffn Command load_env [@option{-f} file] [@option{-s}] [whitelisted_variable_name] @dots{}
 Load all variables from the environment block file into the environment.
 @xref{Environment block}.
 
 The @option{-f} option overrides the default location of the environment
 block.
+
+The @option{-s} (long form @option{--skip-sig}) option skips signature
+checking even when the value of @code{check_signatures=enforce}
+(@pxref{check_signatures}).
+
+If one or more variable names are provided as arguments, they are
+interpreted as a whitelist of variables to load from the environment
+block file.  Variables set in the file but not present in the
+whitelist are ignored.
+
+The @option{-s} option should be used with care, and should always be
+used in concert with a whitelist of acceptable variables whose values
+should be set.  Failure to employ a carefully constructed whitelist
+could result in reading a malicious value of critical environment
+variables from the file, such as setting @code{check_signatures=no},
+modifying @code{prefix} to boot from an unexpected location or not at
+all, etc.
+
+When used with care, @option{-s} and the whitelist enable an
+administrator to configure a system to boot only signed
+configurations, but to allow the user to select from among multiple
+configurations, and to enable ``one-shot'' boot attempts and
+``savedefault'' behavior.  @xref{Security and signatures} for more
+information.
 @end deffn
 
 
@@ -4305,6 +4377,16 @@ Save the named variables from the environment to the environment block file.
 
 The @option{-f} option overrides the default location of the environment
 block.
+
+This command will operate successfully even when
+@code{check_signatures=enforce} (@pxref{check_signatures}), since it
+writes to disk and does not alter the behavior of GRUB based on any
+contents of disk that have been read.  It is possible to modify a
+digitally signed environment block file from within GRUB using this
+command, such that its signature will no longer be valid on subsequent
+boots.  Care should be taken in such advanced configurations to avoid
+rendering the system unbootable. @xref{Security and signatures} for
+more information.
 @end deffn
 
 
@@ -4614,6 +4696,20 @@ Do nothing, successfully.  This is mainly useful in control constructs such
 as @code{if} and @code{while} (@pxref{Shell-like scripting}).
 @end deffn
 
+@node trust
+@subsection trust
+
+@deffn Command trust pubkey_file
+Read public key from @var{pubkey_file} and add it to GRUB's internal
+list of trusted public keys.  These keys are used to validate digital
+signatures when @code{check_signatures=enforce}.  Note that if
+@code{check_signatures=enforce} when this command is run, then
+@var{pubkey_file} must itself be signed such that an already-loaded
+public key (@pxref{list_trusted}) can validate that signature.  A
+public key hierarchy can thus be constructed.
+@xref{Security and signatures} for more information.
+@end deffn
+
 
 @node unset
 @subsection unset
@@ -4640,6 +4736,25 @@ only on PC BIOS platforms.
 @end ignore
 
 
+@node verify_detached
+@subsection verify_detached
+
+@deffn Command verify_detached file signature_file [pubkey_file]
+Verifies a GPG-style detached signature, where the signed file is
+@var{file}, and the signature itself is in file @var{signature_file}.
+Optionally, a specific public key to use can be specified using
+@var{pubkey_file}.  Otherwise, public keys from GRUB's trusted keys
+(@pxref{list_trusted}, @pxref{trust}, and @pxref{distrust}) are
+tried.  Note that, when @code{check_signatures=enforce}, an explicitly
+identified @var{pubkey_file} must itself be signed by an
+already-trusted key.
+
+Exit code @code{$?} is set to 0 if the signature validates
+successfully.  If validation fails, it is set to a non-zero value.
+
+@xref{Security and signatures} for more information.
+@end deffn
+
 @node videoinfo
 @subsection videoinfo
 
@@ -5003,6 +5118,101 @@ generating configuration files with authentication.  You can use
 adding @kbd{set superusers=} and @kbd{password} or @kbd{password_pbkdf2}
 commands.
 
+@node Security and signatures
+@chapter Security considerations when using digital signatures
+
+GRUB's @file{core.img} can optionally provide enforcement that all
+files subsequently read from disk are covered by a valid digital
+signature.  This includes GRUB configuration files, the GRUB
+environment block, GRUB loadable modules and their dependency files,
+and loaded operating system files such as a Linux kernel.  This
+document does @strong{not} cover how to ensure that your platform's
+firmware (e.g., GNU Coreboot or UEFI Secure Boot) validates
+@file{core.img}.
+
+GRUB uses GPG-style detached signatures (meaning that a file
+@file{foo.sig} will be produced when file @file{foo} is signed), and
+currently supports the DSA signing algorithm.  Both 2048-bit and
+3072-bit keys are supported. A signing key can be generated as
+follows:
+
+@example
+gpg --gen-key
+@end example
+
+The corresponding public key must be embedded in @file{core.img} when
+executing the @command{grub-mkimage} command (typically as part of
+@command{grub-install}, @pxref{Invoking grub-install}) utility.  This
+can be done using the @code{--pubkey} option to @command{grub-mkimage}
+and manually specifying that the modules required for signature
+verification be embedded in @file{core.img}.  For example:
+
+@example
+# First, wrap grub-mkimage to include your public key(s).
+cat <<EOF > /root/grub-mkimage-pubkey.sh
+#!/bin/sh
+/usr/bin/grub-mkimage --pubkey=/boot/pubkey.gpg $@@
+EOF
+chmod +x /root/grub-mkimage-pubkey.sh
+# Then, invoke grub-install, explicitly including the `verify'
+# module and its dependencies (as verify cannot signature-check
+# itself).
+grub-install \
+  --grub-mkimage=/root/grub-mkimage-pubkey.sh \
+  --modules="verify gcry_rsa gcry_dsa gcry_sha256 hashsum"\
+"gcry_sha1 mpi echo loadenv" \
+  /dev/sda
+@end example
+
+An individual file can be signed as follows:
+
+@example
+gpg --detach-sign /path/to/file
+@end example
+
+For successful validation of all of GRUB's subcomponents and the
+loaded OS kernel, they must all be signed.  One way to accomplish this
+is the following (after having already produced the desired
+@file{grub.cfg} file, e.g., by running @command{grub-mkconfig}
+(@pxref{Invoking grub-mkconfig}):
+
+@example
+@group
+# Edit /dev/shm/passphrase.txt to contain your signing key's passphrase
+for i in `find /boot -name "*.cfg" -or -name "*.lst" -or \
+  -name "*.mod" -or -name "vmlinuz*" -or -name "initrd*" -or \
+  -name "grubenv"`;
+do
+  gpg --batch --detach-sign --passphrase-fd 0 $i < \
+    /dev/shm/passphrase.txt
+done
+shred /dev/shm/passphrase.txt
+@end group
+@end example
+
+See also: @ref{check_signatures}, @ref{verify_detached}, @ref{trust},
+@ref{list_trusted}, @ref{distrust}, @ref{load_env}, @ref{save_env}.
+
+Note that internally signature enforcement is controlled by setting
+the environment variable @code{check_signatures=enforce}.  Passing one
+or more @code{--pubkey} options to @command{grub-mkimage} implicitly
+sets @code{check_signatures=enforce} in @file{core.img} prior to
+processing any configuration files.
+
+Note that signature checking does @strong{not} prevent an attacker
+with (serial, physical, ...) console access from dropping manually to
+the GRUB console and executing:
+
+@example
+set check_signatures=no
+@end example
+
+To prevent this, password-protection (@pxref{Security}) is essential.
+Note that even with GRUB password protection, GRUB itself cannot
+prevent someone with physical access to the machine from altering that
+machine's firmware (e.g., GNU Coreboot or BIOS) configuration to cause
+the machine to boot from a different (attacker-controlled) device.
+GRUB is at best only one link in a secure boot chain.
 
 @node Platform limitations
 @chapter Platform limitations
@@ -5440,6 +5650,12 @@ mounted on
 Recheck the device map, even if @file{/boot/grub/device.map} already
 exists. You should use this option whenever you add/remove a disk
 into/from your computer.
+
+@item --grub-mkimage=@var{program}
+Use @var{program} as @command{grub-mkimage}.  This is primarily useful
+for advanced users who wish to provide custom arguments to
+@command{grub-mkimage}.  @xref{Security and signatures} for an example
+use case.
 @end table
 
 
-- 
1.8.4

Re: [PATCH v0] Additional security-relevant documentation

[Andrey Borzenkov]

Oh, I just really meant document new option to load_env. But thank you
for the efforts!

В Fri, 27 Sep 2013 10:00:37 -0700
Jon McCune <jonmccune@google.com> пишет:

>  * Security::                    Authentication and authorisation
> +* Security and signatures::     Verifying digital signatures in GRUB

I think it should in subsection. Top level Security which includes
subsections Authentication and authorization (current Security) and
Signature verification. There are possibly more topics in the future
and it will unclutter top level menu.

>  @menu
>  * biosnum::
> +* check_signatures::
>  * chosen::
>  * color_highlight::
>  * color_normal::
> @@ -2809,6 +2811,27 @@ For an alternative approach which also changes BIOS drive mappings for the
>  chain-loaded system, @pxref{drivemap}.
>  
>  
> +@node check_signatures
> +@subsection check_signatures
> +
> +This variable controls whether GRUB enforces digital signature
> +validation (@pxref{Security and signatures}) on all loaded files.  If
> +@code{check_signatures=enforce}, 

That looks and sounds strange (to me at least) - how would you pronounce
this sentence? "If set to @samp{enforce}" looks enough, it is in
the description of variable so context in unambiguous.  

> +@file{core.img} to load another file @file{foo} (e.g., a loadable
> +module, a configuration file, or a Linux kernel) implicitly invokes
> +@code{verify_detached foo foo.sig} (@pxref{verify_detached}).
> +@code{foo.sig} must contain a valid digital signature over the
> +contents of @code{foo}, which can be verified with a public key
> +currently trusted by GRUB (@pxref{list_trusted}, @pxref{trust}, and
> +@pxref{distrust}).  If validation fails, then file @file{foo} cannot
> +be opened.  This failure may halt or otherwise impact the boot
> +process.  An initial trusted public key can be embedded within the
> +GRUB @file{core.img} using the @code{--pubkey} option to
> +@command{grub-mkimage} (@pxref{Invoking grub-install}).  Presently it
> +is necessary to write a custom wrapper around @command{grub-mkimage}
> +using the @code{--grub-mkimage} flag to @command{grub-install}.
> +

I have a feeling that all this does not belong here, but in Signature
verification section.

> +
>  @node chosen
>  @subsection chosen
>  
> @@ -3458,6 +3481,7 @@ you forget a command, you can run the command @command{help}
>  * cryptomount::                 Mount a crypto device
>  * date::                        Display or set current date and time
>  * devicetree::                  Load a device tree blob
> +* distrust::                    Remove a pubkey from trusted keys

May be it's better start new section Security related commands.
Commands are already indexed, so there is no need to have one long
list. Grouping them by functions makes it easier to browse.

>  
> +@node distrust
> +@subsection distrust
> +
> +@deffn Command distrust pubkey_id
> +Remove public key @var{pubkey_id} from GRUB's keyring of trusted keys.

I was not able to do it. It is absolutely unclear which part of
list_trusted output should be used as input to this command. If you
know how it works, please document it with examples.

> +These keys are used to validate signatures when
> +@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
> +invocations of @command{verify_detached} (@pxref{verify_detached}).
> +@xref{Security and signatures} for more information.
> +@end deffn
>  

Is it necessary to repeat the same over and over again in every command
description. Reference to section where they are described seems to be
enough.
>  
> +@node list_trusted
> +@subsection list_trusted
> +
> +@deffn Command list_trusted
> +List all public keys trusted by GRUB for validating signatures. 

Please document what is actually output (fingerprint) and how it can be
used in distrust to forget the key.

>                                                                 These
> +public keys are used implicitly when environment variable
> +@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
> +invocations of @command{verify_detached}.  @xref{Security and
> +signatures} for more information.
> +@end deffn
>  

repetition?

>  @node load_env
>  @subsection load_env
>  
> -@deffn Command load_env [@option{-f} file]
> +@deffn Command load_env [@option{-f} file] [@option{-s}] [whitelisted_variable_name] @dots{}

I think it is better to document long options. They make scripts also
more readable.

> +@node trust
> +@subsection trust
> +
> +@deffn Command trust pubkey_file
> +Read public key from @var{pubkey_file} and add it to GRUB's internal
> +list of trusted public keys.  These keys are used to validate digital
> +signatures when @code{check_signatures=enforce}.

When @var{check_signatures} is set to @samp{enforce}.

>                                                   Note that if
> +@code{check_signatures=enforce} when this command is run, then
> +@var{pubkey_file} must itself be signed

Really? Code explicitly disables all filters (including signature
verification) and quick test allows me to trust key without having
valid trusted key. I do not know if this is intentional.

>                                           such that an already-loaded
> +public key (@pxref{list_trusted}) can validate that signature.  A
> +public key hierarchy can thus be constructed.
> +@xref{Security and signatures} for more information.
> +@end deffn
> +
>  
>  @node unset
>  @subsection unset
> @@ -4640,6 +4736,25 @@ only on PC BIOS platforms.
>  @end ignore
>  
>  
> +@node verify_detached
> +@subsection verify_detached
> +
> +@deffn Command verify_detached file signature_file [pubkey_file]
> +Verifies a GPG-style detached signature, where the signed file is
> +@var{file}, and the signature itself is in file @var{signature_file}.
> +Optionally, a specific public key to use can be specified using
> +@var{pubkey_file}.  Otherwise, public keys from GRUB's trusted keys
> +(@pxref{list_trusted}, @pxref{trust}, and @pxref{distrust}) are
> +tried.  Note that, when @code{check_signatures=enforce}, an explicitly
> +identified @var{pubkey_file} must itself be signed by an
> +already-trusted key.

Same. Not tested, but code explicitly disables all filters when reading
all three files.

> +@node Security and signatures
> +@chapter Security considerations when using digital signatures
> +
> +GRUB's @file{core.img} can optionally provide enforcement that all
> +files subsequently read from disk are covered by a valid digital
> +signature.  This includes GRUB configuration files, the GRUB
> +environment block, GRUB loadable modules and their dependency files,
> +and loaded operating system files such as a Linux kernel. 

theme files, font files and whatever else including arguments to hash
checks. Really, "all" means "all", there is no need to explicitly list
what it comprises.

>                                                             This
> +document does @strong{not} cover how to ensure that your platform's
> +firmware (e.g., GNU Coreboot or UEFI Secure Boot) validates
> +@file{core.img}.
> +
> +GRUB uses GPG-style detached signatures (meaning that a file
> +@file{foo.sig} will be produced when file @file{foo} is signed), and
> +currently supports the DSA signing algorithm.  Both 2048-bit and
> +3072-bit keys are supported. A signing key can be generated as
> +follows:
> +
> +@example
> +gpg --gen-key
> +@end example
> +

I think path to keyring is needed here to make it obvious you can have
more than one.

> +The corresponding public key must be embedded in @file{core.img} when
> +executing the @command{grub-mkimage} command (typically as part of
> +@command{grub-install}, @pxref{Invoking grub-install}) utility.

grub-mkimage is internal implementation detail. It should not be
mentioned here.

>                                                                  This
> +can be done using the @code{--pubkey} option to @command{grub-mkimage}
> +and manually specifying that the modules required for signature
> +verification be embedded in @file{core.img}.  For example:
> +
> +@example
> +# First, wrap grub-mkimage to include your public key(s).
> +cat <<EOF > /root/grub-mkimage-pubkey.sh
> +#!/bin/sh
> +/usr/bin/grub-mkimage --pubkey=/boot/pubkey.gpg $@@
> +EOF
> +chmod +x /root/grub-mkimage-pubkey.sh
> +# Then, invoke grub-install, explicitly including the `verify'
> +# module and its dependencies (as verify cannot signature-check
> +# itself).
> +grub-install \
> +  --grub-mkimage=/root/grub-mkimage-pubkey.sh \
> +  --modules="verify gcry_rsa gcry_dsa gcry_sha256 hashsum"\
> +"gcry_sha1 mpi echo loadenv" \
> +  /dev/sda
> +@end example
> +

Nor should this example really be included.

> +An individual file can be signed as follows:
> +
> +@example
> +gpg --detach-sign /path/to/file
> +@end example
> +

Should not you specify path to keyring?

> +For successful validation of all of GRUB's subcomponents and the
> +loaded OS kernel, they must all be signed.  One way to accomplish this
> +is the following (after having already produced the desired
> +@file{grub.cfg} file, e.g., by running @command{grub-mkconfig}
> +(@pxref{Invoking grub-mkconfig}):
> +
> +@example
> +@group
> +# Edit /dev/shm/passphrase.txt to contain your signing key's passphrase
> +for i in `find /boot -name "*.cfg" -or -name "*.lst" -or \
> +  -name "*.mod" -or -name "vmlinuz*" -or -name "initrd*" -or \
> +  -name "grubenv"`;
> +do
> +  gpg --batch --detach-sign --passphrase-fd 0 $i < \
> +    /dev/shm/passphrase.txt
> +done
> +shred /dev/shm/passphrase.txt
> +@end group
> +@end example
> +

Not sure if this belongs here. Either it has to be integrated in
grub-install and documented there or anyone with minimal shell
programming skills can do it. Nor is this example correct, as it does
not mention theme files, font files, locale files and others.

> +
> +@item --grub-mkimage=@var{program}
> +Use @var{program} as @command{grub-mkimage}.  This is primarily useful
> +for advanced users who wish to provide custom arguments to
> +@command{grub-mkimage}.

For me the main usage is to be able to run grub-install from build
directory without installing it. I do not know if this omitted
intentionally, but this is advanced option not required for normal
usage.

Re: [PATCH v0] Additional security-relevant documentation

[Jonathan McCune]

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

Hi,

On Sun, Sep 29, 2013 at 2:29 AM, Andrey Borzenkov <arvidjaar@gmail.com>wrote:

> Oh, I just really meant document new option to load_env. But thank you
> for the efforts!
>

I originally sent a version of this with my first attempt at a patch for
loadenv.c.  I figured since I invested the effort to understand these
mechanisms I could give back.  Plus, the places where I was wrong may be
areas to improve things. :-)

Also, phcoder has since accepted a variant of this documentation patch that
he edited himself to remove the errors regarding public key loading being
signature checked.  However, I like some of your suggestions, so please
find below a response on each of them.  I will send the corresponding (now
smaller) PATCHv1 shortly.


>
> В Fri, 27 Sep 2013 10:00:37 -0700
> Jon McCune <jonmccune@google.com> пишет:
>
> >  * Security::                    Authentication and authorisation
> > +* Security and signatures::     Verifying digital signatures in GRUB
>
> I think it should in subsection. Top level Security which includes
> subsections Authentication and authorization (current Security) and
> Signature verification. There are possibly more topics in the future
> and it will unclutter top level menu.
>

Sounds good. Done.


>
> >  @menu
> >  * biosnum::
> > +* check_signatures::
> >  * chosen::
> >  * color_highlight::
> >  * color_normal::
> > @@ -2809,6 +2811,27 @@ For an alternative approach which also changes
> BIOS drive mappings for the
> >  chain-loaded system, @pxref{drivemap}.
> >
> >
> > +@node check_signatures
> > +@subsection check_signatures
> > +
> > +This variable controls whether GRUB enforces digital signature
> > +validation (@pxref{Security and signatures}) on all loaded files.  If
> > +@code{check_signatures=enforce},
>
> That looks and sounds strange (to me at least) - how would you pronounce
> this sentence? "If set to @samp{enforce}" looks enough, it is in
> the description of variable so context in unambiguous.
>

I read it as "if check_signatures equals enforce", but your suggestion
conveys the same meaning and is also clear to me.  Done.


>
> > +@file{core.img} to load another file @file{foo} (e.g., a loadable
> > +module, a configuration file, or a Linux kernel) implicitly invokes
> > +@code{verify_detached foo foo.sig} (@pxref{verify_detached}).
> > +@code{foo.sig} must contain a valid digital signature over the
> > +contents of @code{foo}, which can be verified with a public key
> > +currently trusted by GRUB (@pxref{list_trusted}, @pxref{trust}, and
> > +@pxref{distrust}).  If validation fails, then file @file{foo} cannot
> > +be opened.  This failure may halt or otherwise impact the boot
> > +process.  An initial trusted public key can be embedded within the
> > +GRUB @file{core.img} using the @code{--pubkey} option to
> > +@command{grub-mkimage} (@pxref{Invoking grub-install}).  Presently it
> > +is necessary to write a custom wrapper around @command{grub-mkimage}
> > +using the @code{--grub-mkimage} flag to @command{grub-install}.
> > +
>
> I have a feeling that all this does not belong here, but in Signature
> verification section.
>

Done, with appropriate cross-references.


>
> > +
> >  @node chosen
> >  @subsection chosen
> >
> > @@ -3458,6 +3481,7 @@ you forget a command, you can run the command
> @command{help}
> >  * cryptomount::                 Mount a crypto device
> >  * date::                        Display or set current date and time
> >  * devicetree::                  Load a device tree blob
> > +* distrust::                    Remove a pubkey from trusted keys
>
> May be it's better start new section Security related commands.
> Commands are already indexed, so there is no need to have one long
> list. Grouping them by functions makes it easier to browse.
>

I did not do this yet.  I'm open to the idea, but don't currently feel
comfortable enough with the disk-encryption commands to ensure I don't
introduce problems.


>
> >
> > +@node distrust
> > +@subsection distrust
> > +
> > +@deffn Command distrust pubkey_id
> > +Remove public key @var{pubkey_id} from GRUB's keyring of trusted keys.
>
> I was not able to do it. It is absolutely unclear which part of
> list_trusted output should be used as input to this command. If you
> know how it works, please document it with examples.
>

Done.  (You use the last 8 hexadecimal digits all concatenated together...
it's the same as the output of gpg --fingerprint.)


>
> > +These keys are used to validate signatures when
> > +@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
> > +invocations of @command{verify_detached} (@pxref{verify_detached}).
> > +@xref{Security and signatures} for more information.
> > +@end deffn
> >
>
> Is it necessary to repeat the same over and over again in every command
> description. Reference to section where they are described seems to be
> enough.
>

Tried to trim it down and use cross-references instead.


> >
> > +@node list_trusted
> > +@subsection list_trusted
> > +
> > +@deffn Command list_trusted
> > +List all public keys trusted by GRUB for validating signatures.
>
> Please document what is actually output (fingerprint) and how it can be
> used in distrust to forget the key.
>

Done.


>
> >                                                                 These
> > +public keys are used implicitly when environment variable
> > +@code{check_signatures=enforce} (@pxref{check_signatures}), and by some
> > +invocations of @command{verify_detached}.  @xref{Security and
> > +signatures} for more information.
> > +@end deffn
> >
>
> repetition?
>

Agree. Dropped.


>
> >  @node load_env
> >  @subsection load_env
> >
> > -@deffn Command load_env [@option{-f} file]
> > +@deffn Command load_env [@option{-f} file] [@option{-s}]
> [whitelisted_variable_name] @dots{}
>
> I think it is better to document long options. They make scripts also
> more readable.
>

Agree. Done.


>
> > +@node trust
> > +@subsection trust
> > +
> > +@deffn Command trust pubkey_file
> > +Read public key from @var{pubkey_file} and add it to GRUB's internal
> > +list of trusted public keys.  These keys are used to validate digital
> > +signatures when @code{check_signatures=enforce}.
>
> When @var{check_signatures} is set to @samp{enforce}.
>

Done.  Note that I did not use @var{}.  The texinfo documentation suggests
that @var is for "metasyntactic" variables.  I.e., it is appropriate for
values that the user replaces, like pubkey_file, but not for explicit
names, like check_signatures.  Whether @samp{} is more appropriate than
@code{} I cannot say.  I did check the generated html, pdf, and info
results, and these values look to be formatted correctly to me.


>
> >                                                   Note that if
> > +@code{check_signatures=enforce} when this command is run, then
> > +@var{pubkey_file} must itself be signed
>
> Really? Code explicitly disables all filters (including signature
> verification) and quick test allows me to trust key without having
> valid trusted key. I do not know if this is intentional.
>

You are right.  I was mistaken about this functionality when I first wrote
this documentation (at the time I did not understand the disable_...()
calls).  As I was not the author, I cannot say whether this behavior is
intentional or not.  If it is not intentional, then that is a bug, and
should probably be resolved independently from this documentation patch.
 (My personal opinion is that it is indeed a bug, and only disabling the
compression filters would be more appropriate. Perhaps I will start another
thread to discuss.)  Corrected the documentation to reflect the current
functionality.


>
> >                                           such that an already-loaded
> > +public key (@pxref{list_trusted}) can validate that signature.  A
> > +public key hierarchy can thus be constructed.
> > +@xref{Security and signatures} for more information.
> > +@end deffn
> > +
> >
> >  @node unset
> >  @subsection unset
> > @@ -4640,6 +4736,25 @@ only on PC BIOS platforms.
> >  @end ignore
> >
> >
> > +@node verify_detached
> > +@subsection verify_detached
> > +
> > +@deffn Command verify_detached file signature_file [pubkey_file]
> > +Verifies a GPG-style detached signature, where the signed file is
> > +@var{file}, and the signature itself is in file @var{signature_file}.
> > +Optionally, a specific public key to use can be specified using
> > +@var{pubkey_file}.  Otherwise, public keys from GRUB's trusted keys
> > +(@pxref{list_trusted}, @pxref{trust}, and @pxref{distrust}) are
> > +tried.  Note that, when @code{check_signatures=enforce}, an explicitly
> > +identified @var{pubkey_file} must itself be signed by an
> > +already-trusted key.
>
> Same. Not tested, but code explicitly disables all filters when reading
> all three files.
>

You are right.  Corrected.


>
> > +@node Security and signatures
> > +@chapter Security considerations when using digital signatures
> > +
> > +GRUB's @file{core.img} can optionally provide enforcement that all
> > +files subsequently read from disk are covered by a valid digital
> > +signature.  This includes GRUB configuration files, the GRUB
> > +environment block, GRUB loadable modules and their dependency files,
> > +and loaded operating system files such as a Linux kernel.
>
> theme files, font files and whatever else including arguments to hash
> checks. Really, "all" means "all", there is no need to explicitly list
> what it comprises.
>

All does not mean arguments to verify_detached if a public key file is
explicitly listed, or arguments to trust.  It is probably better to
explicitly mention these two exceptions.  Done.


>
> >                                                             This
> > +document does @strong{not} cover how to ensure that your platform's
> > +firmware (e.g., GNU Coreboot or UEFI Secure Boot) validates
> > +@file{core.img}.
> > +
> > +GRUB uses GPG-style detached signatures (meaning that a file
> > +@file{foo.sig} will be produced when file @file{foo} is signed), and
> > +currently supports the DSA signing algorithm.  Both 2048-bit and
> > +3072-bit keys are supported. A signing key can be generated as
> > +follows:
> > +
> > +@example
> > +gpg --gen-key
> > +@end example
> > +
>
> I think path to keyring is needed here to make it obvious you can have
> more than one.
>

I don't want to get too tangled up in also documenting gpg.  I'm not
opposed to this, but prefer to consider it out of scope for the current
patch.


>
> > +The corresponding public key must be embedded in @file{core.img} when
> > +executing the @command{grub-mkimage} command (typically as part of
> > +@command{grub-install}, @pxref{Invoking grub-install}) utility.
>
> grub-mkimage is internal implementation detail. It should not be
> mentioned here.
>
>
I tend to agree, but right now it's necessary to understand this.  When
grub-install support for --pubkey matures, this can be removed.


> >                                                                  This
> > +can be done using the @code{--pubkey} option to @command{grub-mkimage}
> > +and manually specifying that the modules required for signature
> > +verification be embedded in @file{core.img}.  For example:
> > +
> > +@example
> > +# First, wrap grub-mkimage to include your public key(s).
> > +cat <<EOF > /root/grub-mkimage-pubkey.sh
> > +#!/bin/sh
> > +/usr/bin/grub-mkimage --pubkey=/boot/pubkey.gpg $@@
> > +EOF
> > +chmod +x /root/grub-mkimage-pubkey.sh
> > +# Then, invoke grub-install, explicitly including the `verify'
> > +# module and its dependencies (as verify cannot signature-check
> > +# itself).
> > +grub-install \
> > +  --grub-mkimage=/root/grub-mkimage-pubkey.sh \
> > +  --modules="verify gcry_rsa gcry_dsa gcry_sha256 hashsum"\
> > +"gcry_sha1 mpi echo loadenv" \
> > +  /dev/sda
> > +@end example
> > +
>
> Nor should this example really be included.
>

Same thoughts as above.  This should get dropped as part of some future
cleanup, but for the moment I think it's necessary.  It's also already
committed so somewhat moot.


>
> > +An individual file can be signed as follows:
> > +
> > +@example
> > +gpg --detach-sign /path/to/file
> > +@end example
> > +
>
> Should not you specify path to keyring?
>

I think this falls out of scope due to it not being GRUB's job to document
GPG.


>
> > +For successful validation of all of GRUB's subcomponents and the
> > +loaded OS kernel, they must all be signed.  One way to accomplish this
> > +is the following (after having already produced the desired
> > +@file{grub.cfg} file, e.g., by running @command{grub-mkconfig}
> > +(@pxref{Invoking grub-mkconfig}):
> > +
> > +@example
> > +@group
> > +# Edit /dev/shm/passphrase.txt to contain your signing key's passphrase
> > +for i in `find /boot -name "*.cfg" -or -name "*.lst" -or \
> > +  -name "*.mod" -or -name "vmlinuz*" -or -name "initrd*" -or \
> > +  -name "grubenv"`;
> > +do
> > +  gpg --batch --detach-sign --passphrase-fd 0 $i < \
> > +    /dev/shm/passphrase.txt
> > +done
> > +shred /dev/shm/passphrase.txt
> > +@end group
> > +@end example
> > +
>
> Not sure if this belongs here. Either it has to be integrated in
> grub-install and documented there or anyone with minimal shell
> programming skills can do it. Nor is this example correct, as it does
> not mention theme files, font files, locale files and others.
>

It's already in as-is.  Cleanup would be great but let that be another
patch.


>
> > +
> > +@item --grub-mkimage=@var{program}
> > +Use @var{program} as @command{grub-mkimage}.  This is primarily useful
> > +for advanced users who wish to provide custom arguments to
> > +@command{grub-mkimage}.
>
> For me the main usage is to be able to run grub-install from build
> directory without installing it. I do not know if this omitted
> intentionally, but this is advanced option not required for normal
> usage.
>

That sounds like developer documentation, but I think we both agree that
this is a minor point.  Leaving as-is for now.

Unless there is a strong reason to keep this thread alive, let's move
discussing to the PATCHv1 thread that I will start shortly.

Thanks!
-Jon

[-- Attachment #2: Type: text/html, Size: 19856 bytes --]

Re: [PATCH v0] Additional security-relevant documentation

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 17.10.2013 20:03, Jonathan McCune wrote:
>     grub-mkimage is internal implementation detail. It should not be
>     mentioned here.
> 
> 
> I tend to agree, but right now it's necessary to understand this.  When
> grub-install support for --pubkey matures, this can be removed.
>  
> 
>     >                                                                  This
>     > +can be done using the @code{--pubkey} option to
>     @command{grub-mkimage}
>     > +and manually specifying that the modules required for signature
>     > +verification be embedded in @file{core.img}.  For example:
>     > +
>     > +@example
>     > +# First, wrap grub-mkimage to include your public key(s).
>     > +cat <<EOF > /root/grub-mkimage-pubkey.sh
>     > +#!/bin/sh
>     > +/usr/bin/grub-mkimage --pubkey=/boot/pubkey.gpg $@@
>     > +EOF
>     > +chmod +x /root/grub-mkimage-pubkey.sh
>     > +# Then, invoke grub-install, explicitly including the `verify'
>     > +# module and its dependencies (as verify cannot signature-check
>     > +# itself).
>     > +grub-install \
>     > +  --grub-mkimage=/root/grub-mkimage-pubkey.sh \
>     > +  --modules="verify gcry_rsa gcry_dsa gcry_sha256 hashsum"\
>     > +"gcry_sha1 mpi echo loadenv" \
>     > +  /dev/sda
>     > +@end example
>     > +
> 
>     Nor should this example really be included.
> 
> 
> Same thoughts as above.  This should get dropped as part of some future
> cleanup, but for the moment I think it's necessary.  It's also already
> committed so somewhat moot.

Not true
a) This part was removed
b) I actually forgot Andrey's message when I committed your patch. Sorry
for this. Most of problems he mentions are valid and should be fixed.
Also, interestingly, I removed most of parts he had problem with even
though I didn't look at his email at that time.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]