All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext2: support EXT4_FEATURE_INCOMPAT_ENCRYPT
@ 2017-06-15  5:36 Eric Biggers
  2017-06-15 11:12 ` Vladimir 'phcoder' Serbinenko
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2017-06-15  5:36 UTC (permalink / raw)
  To: grub-devel; +Cc: Eric Biggers

From: Eric Biggers <ebiggers@google.com>

Allow GRUB to mount ext2/3/4 filesystems that have the encryption
feature.  On such a filesystem, inodes may have EXT4_ENCRYPT_FLAG set.
For a regular file, this means its contents are encrypted; for a
directory, this means the filenames in its directory entries are
encrypted; and for a symlink, this means its target is encrypted.  Since
GRUB cannot decrypt encrypted contents or filenames, just issue an error
if it would need to do so.  This is sufficient to allow unencrypted boot
files to co-exist with encrypted files elsewhere on the filesystem.

(Note that encrypted regular files and symlinks will not normally be
encountered outside an encrypted directory; however, it's possible via
hard links, so they still need to be handled.)

Tested by booting from an ext4 /boot partition on which I had run
'tune2fs -O encrypt'.  I also verified that the expected error messages
are printed when trying to access encrypted directories, files, and
symlinks from the GRUB command line.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 grub-core/fs/ext2.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
index cdce63bcc..51ccf0066 100644
--- a/grub-core/fs/ext2.c
+++ b/grub-core/fs/ext2.c
@@ -102,6 +102,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
 #define EXT4_FEATURE_INCOMPAT_64BIT		0x0080
 #define EXT4_FEATURE_INCOMPAT_MMP		0x0100
 #define EXT4_FEATURE_INCOMPAT_FLEX_BG		0x0200
+#define EXT4_FEATURE_INCOMPAT_ENCRYPT		0x10000
 
 /* The set of back-incompatible features this driver DOES support. Add (OR)
  * flags here as the related features are implemented into the driver.  */
@@ -109,7 +110,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
                                        | EXT4_FEATURE_INCOMPAT_EXTENTS  \
                                        | EXT4_FEATURE_INCOMPAT_FLEX_BG \
                                        | EXT2_FEATURE_INCOMPAT_META_BG \
-                                       | EXT4_FEATURE_INCOMPAT_64BIT)
+                                       | EXT4_FEATURE_INCOMPAT_64BIT \
+                                       | EXT4_FEATURE_INCOMPAT_ENCRYPT)
 /* List of rationales for the ignored "incompatible" features:
  * needs_recovery: Not really back-incompatible - was added as such to forbid
  *                 ext2 drivers from mounting an ext3 volume with a dirty
@@ -138,6 +140,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
 #define EXT3_JOURNAL_FLAG_DELETED	4
 #define EXT3_JOURNAL_FLAG_LAST_TAG	8
 
+#define EXT4_ENCRYPT_FLAG		0x800
 #define EXT4_EXTENTS_FLAG		0x80000
 
 /* The ext2 superblock.  */
@@ -706,6 +709,12 @@ grub_ext2_read_symlink (grub_fshelp_node_t node)
       grub_ext2_read_inode (diro->data, diro->ino, &diro->inode);
       if (grub_errno)
 	return 0;
+
+      if (diro->inode.flags & grub_cpu_to_le32_compile_time (EXT4_ENCRYPT_FLAG))
+	{
+	  grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("symlink is encrypted"));
+	  return 0;
+	}
     }
 
   symlink = grub_malloc (grub_le_to_cpu32 (diro->inode.size) + 1);
@@ -749,6 +758,12 @@ grub_ext2_iterate_dir (grub_fshelp_node_t dir,
 	return 0;
     }
 
+  if (diro->inode.flags & grub_cpu_to_le32_compile_time (EXT4_ENCRYPT_FLAG))
+    {
+      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("directory is encrypted"));
+      return 0;
+    }
+
   /* Search the file.  */
   while (fpos < grub_le_to_cpu32 (diro->inode.size))
     {
@@ -859,6 +874,12 @@ grub_ext2_open (struct grub_file *file, const char *name)
 	goto fail;
     }
 
+  if (fdiro->inode.flags & grub_cpu_to_le32_compile_time (EXT4_ENCRYPT_FLAG))
+    {
+      err = grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("file is encrypted"));
+      goto fail;
+    }
+
   grub_memcpy (data->inode, &fdiro->inode, sizeof (struct grub_ext2_inode));
   grub_free (fdiro);
 
-- 
2.13.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] ext2: support EXT4_FEATURE_INCOMPAT_ENCRYPT
  2017-06-15  5:36 [PATCH] ext2: support EXT4_FEATURE_INCOMPAT_ENCRYPT Eric Biggers
@ 2017-06-15 11:12 ` Vladimir 'phcoder' Serbinenko
  2017-06-15 19:42   ` Eric Biggers
  0 siblings, 1 reply; 3+ messages in thread
From: Vladimir 'phcoder' Serbinenko @ 2017-06-15 11:12 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Eric Biggers

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

Good. Thank you for doing this. Please don't mark obscure error messages
for translation (N_). In this patch no string needs translation. Would it
also be possible to add some tests?

On Thu, Jun 15, 2017, 07:41 Eric Biggers <ebiggers3@gmail.com> wrote:

> From: Eric Biggers <ebiggers@google.com>
>
> Allow GRUB to mount ext2/3/4 filesystems that have the encryption
> feature.  On such a filesystem, inodes may have EXT4_ENCRYPT_FLAG set.
> For a regular file, this means its contents are encrypted; for a
> directory, this means the filenames in its directory entries are
> encrypted; and for a symlink, this means its target is encrypted.  Since
> GRUB cannot decrypt encrypted contents or filenames, just issue an error
> if it would need to do so.  This is sufficient to allow unencrypted boot
> files to co-exist with encrypted files elsewhere on the filesystem.
>
> (Note that encrypted regular files and symlinks will not normally be
> encountered outside an encrypted directory; however, it's possible via
> hard links, so they still need to be handled.)
>
> Tested by booting from an ext4 /boot partition on which I had run
> 'tune2fs -O encrypt'.  I also verified that the expected error messages
> are printed when trying to access encrypted directories, files, and
> symlinks from the GRUB command line.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  grub-core/fs/ext2.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
> index cdce63bcc..51ccf0066 100644
> --- a/grub-core/fs/ext2.c
> +++ b/grub-core/fs/ext2.c
> @@ -102,6 +102,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define EXT4_FEATURE_INCOMPAT_64BIT            0x0080
>  #define EXT4_FEATURE_INCOMPAT_MMP              0x0100
>  #define EXT4_FEATURE_INCOMPAT_FLEX_BG          0x0200
> +#define EXT4_FEATURE_INCOMPAT_ENCRYPT          0x10000
>
>  /* The set of back-incompatible features this driver DOES support. Add
> (OR)
>   * flags here as the related features are implemented into the driver.  */
> @@ -109,7 +110,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
>                                         | EXT4_FEATURE_INCOMPAT_EXTENTS  \
>                                         | EXT4_FEATURE_INCOMPAT_FLEX_BG \
>                                         | EXT2_FEATURE_INCOMPAT_META_BG \
> -                                       | EXT4_FEATURE_INCOMPAT_64BIT)
> +                                       | EXT4_FEATURE_INCOMPAT_64BIT \
> +                                       | EXT4_FEATURE_INCOMPAT_ENCRYPT)
>  /* List of rationales for the ignored "incompatible" features:
>   * needs_recovery: Not really back-incompatible - was added as such to
> forbid
>   *                 ext2 drivers from mounting an ext3 volume with a dirty
> @@ -138,6 +140,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define EXT3_JOURNAL_FLAG_DELETED      4
>  #define EXT3_JOURNAL_FLAG_LAST_TAG     8
>
> +#define EXT4_ENCRYPT_FLAG              0x800
>  #define EXT4_EXTENTS_FLAG              0x80000
>
>  /* The ext2 superblock.  */
> @@ -706,6 +709,12 @@ grub_ext2_read_symlink (grub_fshelp_node_t node)
>        grub_ext2_read_inode (diro->data, diro->ino, &diro->inode);
>        if (grub_errno)
>         return 0;
> +
> +      if (diro->inode.flags & grub_cpu_to_le32_compile_time
> (EXT4_ENCRYPT_FLAG))
> +       {
> +         grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("symlink is
> encrypted"));
> +         return 0;
> +       }
>      }
>
>    symlink = grub_malloc (grub_le_to_cpu32 (diro->inode.size) + 1);
> @@ -749,6 +758,12 @@ grub_ext2_iterate_dir (grub_fshelp_node_t dir,
>         return 0;
>      }
>
> +  if (diro->inode.flags & grub_cpu_to_le32_compile_time
> (EXT4_ENCRYPT_FLAG))
> +    {
> +      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("directory is
> encrypted"));
> +      return 0;
> +    }
> +
>    /* Search the file.  */
>    while (fpos < grub_le_to_cpu32 (diro->inode.size))
>      {
> @@ -859,6 +874,12 @@ grub_ext2_open (struct grub_file *file, const char
> *name)
>         goto fail;
>      }
>
> +  if (fdiro->inode.flags & grub_cpu_to_le32_compile_time
> (EXT4_ENCRYPT_FLAG))
> +    {
> +      err = grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, N_("file is
> encrypted"));
> +      goto fail;
> +    }
> +
>    grub_memcpy (data->inode, &fdiro->inode, sizeof (struct
> grub_ext2_inode));
>    grub_free (fdiro);
>
> --
> 2.13.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ext2: support EXT4_FEATURE_INCOMPAT_ENCRYPT
  2017-06-15 11:12 ` Vladimir 'phcoder' Serbinenko
@ 2017-06-15 19:42   ` Eric Biggers
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Biggers @ 2017-06-15 19:42 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Eric Biggers

Hi Vladimir,

On Thu, Jun 15, 2017 at 11:12:32AM +0000, Vladimir 'phcoder' Serbinenko wrote:
> Good. Thank you for doing this. Please don't mark obscure error messages
> for translation (N_). In this patch no string needs translation. Would it
> also be possible to add some tests?

Thanks for the review; I've sent v2 which removes the N_() calls and adds an
ext4_encrypt test to grub-fs-tester.

One caveat is that formatting and mounting a filesystem with '-O encrypt' is
only possible with e2fsprogs v1.43+ and Linux v4.1+.  Perhaps those should be
detected as a prerequisite?  I don't think there's a good way to detect the
encryption support other than trying it and seeing if it works, though.

Eric


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-06-15 19:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-15  5:36 [PATCH] ext2: support EXT4_FEATURE_INCOMPAT_ENCRYPT Eric Biggers
2017-06-15 11:12 ` Vladimir 'phcoder' Serbinenko
2017-06-15 19:42   ` Eric Biggers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.