diff mbox series

[4/6] ovl: Add framework for verity support

Message ID 2b2c5ecaf80f810f46791a94d8638ec4027a3a0e.1681917551.git.alexl@redhat.com (mailing list archive)
State Superseded
Headers show
Series ovl: Add support for fs-verity checking of lowerdata | expand

Commit Message

Alexander Larsson April 20, 2023, 7:44 a.m. UTC
This adds the scaffolding (docs, config, mount options) for supporting
for a new overlay xattr "overlay.verity", which contains a fs-verity
digest. This is used for metacopy files, and the actual fs-verity
digest of the lowerdata file needs to match it. The mount option
"verity" specifies how this xattrs is handled.

Unless you explicitly disable it ("verity=off") all existing xattrs
are validated before use. This is all that happens by default
("verity=validate"), but, if you turn on verity ("verity=on") then
during metacopy we generate verity xattr in the upper metacopy file if
the source file has verity enabled. This means later accesses can
guarantee that the correct data is used.

Additionally you can use "verity=require". In this mode all metacopy
files must have a valid verity xattr. For this to work metadata
copy-up must be able to create a verity xattr (so that later accesses
are validated). Therefore, in this mode, if the lower data file
doesn't have fs-verity enabled we fall back to a full copy rather than
a metacopy.

Actual implementation follows in a separate commit.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
---
 Documentation/filesystems/overlayfs.rst | 33 +++++++++++++++++
 fs/overlayfs/Kconfig                    | 14 +++++++
 fs/overlayfs/ovl_entry.h                |  4 ++
 fs/overlayfs/super.c                    | 49 +++++++++++++++++++++++++
 4 files changed, 100 insertions(+)

Comments

Amir Goldstein April 20, 2023, 8:39 a.m. UTC | #1
On Thu, Apr 20, 2023 at 10:44 AM Alexander Larsson <alexl@redhat.com> wrote:
>
> This adds the scaffolding (docs, config, mount options) for supporting
> for a new overlay xattr "overlay.verity", which contains a fs-verity
> digest. This is used for metacopy files, and the actual fs-verity
> digest of the lowerdata file needs to match it. The mount option
> "verity" specifies how this xattrs is handled.
>
> Unless you explicitly disable it ("verity=off") all existing xattrs
> are validated before use. This is all that happens by default
> ("verity=validate"), but, if you turn on verity ("verity=on") then
> during metacopy we generate verity xattr in the upper metacopy file if
> the source file has verity enabled. This means later accesses can
> guarantee that the correct data is used.
>
> Additionally you can use "verity=require". In this mode all metacopy
> files must have a valid verity xattr. For this to work metadata
> copy-up must be able to create a verity xattr (so that later accesses
> are validated). Therefore, in this mode, if the lower data file
> doesn't have fs-verity enabled we fall back to a full copy rather than
> a metacopy.
>
> Actual implementation follows in a separate commit.
>
> Signed-off-by: Alexander Larsson <alexl@redhat.com>
> ---
>  Documentation/filesystems/overlayfs.rst | 33 +++++++++++++++++
>  fs/overlayfs/Kconfig                    | 14 +++++++
>  fs/overlayfs/ovl_entry.h                |  4 ++
>  fs/overlayfs/super.c                    | 49 +++++++++++++++++++++++++
>  4 files changed, 100 insertions(+)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index c8e04a4f0e21..66895bf71cd1 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -403,6 +403,39 @@ when a "metacopy" file in one of the lower layers above it, has a "redirect"
>  to the absolute path of the "lower data" file in the "data-only" lower layer.
>
>
> +fs-verity support
> +----------------------
> +
> +When metadata copy up is used for a file, then the xattr
> +"trusted.overlay.verity" may be set on the metacopy file. This
> +specifies the expected fs-verity digest of the lowerdata file. This
> +may then be used to verify the content of the source file at the time
> +the file is opened. If enabled, overlayfs can also set this xattr
> +during metadata copy up.
> +
> +This is controlled by the "verity" mount option, which supports
> +these values:
> +
> +- "off":
> +    The verity xattr is never used.
> +- "validate":
> +    Whenever a metacopy files specifies an expected digest, the
> +    corresponding data file must match the specified digest.
> +- "on":
> +    Same as validate, but additionally, when generating a metacopy
> +    file the verity xattr will be set from the source file fs-verity
> +    digest (if it has one).
> +- "require":
> +    Same as "on", but additionally all metacopy files must specify a
> +    verity xattr. Additionally metadata copy up will only be used if
> +    the data file has fs-verity enabled, otherwise a full copy-up is
> +    used.
> +
> +There are two ways to tune the default behaviour. The kernel config
> +option OVERLAY_FS_VERITY, or the module option "verity=BOOL". If
> +either of these are enabled, then verity mode is "on" by default,
> +otherwise it is "validate".
> +
>  Sharing and copying layers
>  --------------------------
>
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index 6708e54b0e30..98d6b1a7baf5 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -124,3 +124,17 @@ config OVERLAY_FS_METACOPY
>           that doesn't support this feature will have unexpected results.
>
>           If unsure, say N.
> +
> +config OVERLAY_FS_VERITY
> +       bool "Overlayfs: turn on verity feature by default"
> +       depends on OVERLAY_FS
> +       depends on OVERLAY_FS_METACOPY
> +       help
> +         If this config option is enabled then overlay filesystems will
> +         try to copy fs-verity digests from the lower file into the
> +         metacopy file at metadata copy-up time. It is still possible
> +         to turn off this feature globally with the "verity=off"
> +         module option or on a filesystem instance basis with the
> +         "verity=off" or "verity=validate" mount option.
> +
> +         If unsure, say N.
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index a7b1006c5321..f759e476dfc7 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -13,6 +13,10 @@ struct ovl_config {
>         bool redirect_dir;
>         bool redirect_follow;
>         const char *redirect_mode;
> +       bool verity_validate;
> +       bool verity_generate;
> +       bool verity_require;
> +       const char *verity_mode;
>         bool index;
>         bool uuid;
>         bool nfs_export;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ef78abc21998..953d76f6a1e3 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -59,6 +59,11 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
>  MODULE_PARM_DESC(metacopy,
>                  "Default to on or off for the metadata only copy up feature");
>
> +static bool ovl_verity_def = IS_ENABLED(CONFIG_OVERLAY_FS_VERITY);
> +module_param_named(verity, ovl_verity_def, bool, 0644);
> +MODULE_PARM_DESC(verity,
> +                "Default to on or validate for the metadata only copy up feature");
> +
>  static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode)
>  {
> @@ -235,6 +240,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         kfree(ofs->config.upperdir);
>         kfree(ofs->config.workdir);
>         kfree(ofs->config.redirect_mode);
> +       kfree(ofs->config.verity_mode);
>         if (ofs->creator_cred)
>                 put_cred(ofs->creator_cred);
>         kfree(ofs);
> @@ -325,6 +331,11 @@ static const char *ovl_redirect_mode_def(void)
>         return ovl_redirect_dir_def ? "on" : "off";
>  }
>
> +static const char *ovl_verity_mode_def(void)
> +{
> +       return ovl_verity_def ? "on" : "validate";
> +}
> +
>  static const char * const ovl_xino_str[] = {
>         "off",
>         "auto",
> @@ -374,6 +385,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>                 seq_puts(m, ",volatile");
>         if (ofs->config.userxattr)
>                 seq_puts(m, ",userxattr");
> +       if (strcmp(ofs->config.verity_mode, ovl_verity_mode_def()) != 0)
> +               seq_printf(m, ",verity=%s", ofs->config.verity_mode);
>         return 0;
>  }
>
> @@ -429,6 +442,7 @@ enum {
>         OPT_METACOPY_ON,
>         OPT_METACOPY_OFF,
>         OPT_VOLATILE,
> +       OPT_VERITY,
>         OPT_ERR,
>  };
>
> @@ -451,6 +465,7 @@ static const match_table_t ovl_tokens = {
>         {OPT_METACOPY_ON,               "metacopy=on"},
>         {OPT_METACOPY_OFF,              "metacopy=off"},
>         {OPT_VOLATILE,                  "volatile"},
> +       {OPT_VERITY,                    "verity=%s"},
>         {OPT_ERR,                       NULL}
>  };
>
> @@ -500,6 +515,25 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
>         return 0;
>  }
>
> +static int ovl_parse_verity_mode(struct ovl_config *config, const char *mode)
> +{
> +       if (strcmp(mode, "validate") == 0) {
> +               config->verity_validate = true;
> +       } else if (strcmp(mode, "on") == 0) {
> +               config->verity_validate = true;
> +               config->verity_generate = true;
> +       } else if (strcmp(mode, "require") == 0) {
> +               config->verity_validate = true;
> +               config->verity_generate = true;
> +               config->verity_require = true;
> +       } else if (strcmp(mode, "off") != 0) {
> +               pr_err("bad mount option \"verity=%s\"\n", mode);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  static int ovl_parse_opt(char *opt, struct ovl_config *config)
>  {
>         char *p;
> @@ -511,6 +545,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (!config->redirect_mode)
>                 return -ENOMEM;
>
> +       config->verity_mode = kstrdup(ovl_verity_mode_def(), GFP_KERNEL);
> +       if (!config->verity_mode)
> +               return -ENOMEM;
> +
>         while ((p = ovl_next_opt(&opt)) != NULL) {
>                 int token;
>                 substring_t args[MAX_OPT_ARGS];
> @@ -611,6 +649,13 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>                         config->userxattr = true;
>                         break;
>
> +               case OPT_VERITY:
> +                       kfree(config->verity_mode);
> +                       config->verity_mode = match_strdup(&args[0]);
> +                       if (!config->verity_mode)
> +                               return -ENOMEM;
> +                       break;
> +
>                 default:
>                         pr_err("unrecognized mount option \"%s\" or missing value\n",
>                                         p);
> @@ -642,6 +687,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (err)
>                 return err;
>
> +       err = ovl_parse_verity_mode(config, config->verity_mode);
> +       if (err)
> +               return err;
> +
>         /*
>          * This is to make the logic below simpler.  It doesn't make any other
>          * difference, since config->redirect_dir is only used for upper.
>

Need to add code to resolve verity -> metacopy dependency
same as for metacopy -> redirect_dir dependency.

However, note that the nfs_export,userxattr -> !metacopy dependencies
also imply nfs_export,userxattr -> !verity.

But unlike metacopy, I am not sure of we can auto disable verity (?)
I guess it is fine to set veritfy=off *along with* metacopy=off because
trying to follow a metacopy would result in EIO anyway when metacopy
is disabled, so the verity mode is meaningless.

Thanks,
Amir.
Miklos Szeredi April 25, 2023, 11:19 a.m. UTC | #2
On Thu, 20 Apr 2023 at 09:44, Alexander Larsson <alexl@redhat.com> wrote:
>
> This adds the scaffolding (docs, config, mount options) for supporting
> for a new overlay xattr "overlay.verity", which contains a fs-verity
> digest. This is used for metacopy files, and the actual fs-verity
> digest of the lowerdata file needs to match it. The mount option
> "verity" specifies how this xattrs is handled.
>
> Unless you explicitly disable it ("verity=off") all existing xattrs
> are validated before use. This is all that happens by default
> ("verity=validate"), but, if you turn on verity ("verity=on") then
> during metacopy we generate verity xattr in the upper metacopy file if
> the source file has verity enabled. This means later accesses can
> guarantee that the correct data is used.
>
> Additionally you can use "verity=require". In this mode all metacopy
> files must have a valid verity xattr. For this to work metadata
> copy-up must be able to create a verity xattr (so that later accesses
> are validated). Therefore, in this mode, if the lower data file
> doesn't have fs-verity enabled we fall back to a full copy rather than
> a metacopy.

Maybe we can reduce the number of modes.  Which mode does your use case need?

>
> Actual implementation follows in a separate commit.
>
> Signed-off-by: Alexander Larsson <alexl@redhat.com>
> ---
>  Documentation/filesystems/overlayfs.rst | 33 +++++++++++++++++
>  fs/overlayfs/Kconfig                    | 14 +++++++
>  fs/overlayfs/ovl_entry.h                |  4 ++
>  fs/overlayfs/super.c                    | 49 +++++++++++++++++++++++++
>  4 files changed, 100 insertions(+)
>
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index c8e04a4f0e21..66895bf71cd1 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -403,6 +403,39 @@ when a "metacopy" file in one of the lower layers above it, has a "redirect"
>  to the absolute path of the "lower data" file in the "data-only" lower layer.
>
>
> +fs-verity support
> +----------------------
> +
> +When metadata copy up is used for a file, then the xattr
> +"trusted.overlay.verity" may be set on the metacopy file. This
> +specifies the expected fs-verity digest of the lowerdata file. This
> +may then be used to verify the content of the source file at the time
> +the file is opened. If enabled, overlayfs can also set this xattr
> +during metadata copy up.
> +
> +This is controlled by the "verity" mount option, which supports
> +these values:
> +
> +- "off":
> +    The verity xattr is never used.
> +- "validate":
> +    Whenever a metacopy files specifies an expected digest, the
> +    corresponding data file must match the specified digest.
> +- "on":
> +    Same as validate, but additionally, when generating a metacopy
> +    file the verity xattr will be set from the source file fs-verity
> +    digest (if it has one).
> +- "require":
> +    Same as "on", but additionally all metacopy files must specify a
> +    verity xattr. Additionally metadata copy up will only be used if
> +    the data file has fs-verity enabled, otherwise a full copy-up is
> +    used.
> +
> +There are two ways to tune the default behaviour. The kernel config
> +option OVERLAY_FS_VERITY, or the module option "verity=BOOL". If
> +either of these are enabled, then verity mode is "on" by default,
> +otherwise it is "validate".

I'm not sure that enabling verity by default is safe.  E.g. a script
mounts overalyfs but doesn't set the verity mount, since it's on by
default.  Then the script is moved to a different system where the
default is off, which will result in verity not being enabled, even
though that was not intended.  Is there an advantage to allowing to
change the default?  I know it's done for most of the overlayfs
options, but I think this is different.

> +
>  Sharing and copying layers
>  --------------------------
>
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index 6708e54b0e30..98d6b1a7baf5 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -124,3 +124,17 @@ config OVERLAY_FS_METACOPY
>           that doesn't support this feature will have unexpected results.
>
>           If unsure, say N.
> +
> +config OVERLAY_FS_VERITY
> +       bool "Overlayfs: turn on verity feature by default"
> +       depends on OVERLAY_FS
> +       depends on OVERLAY_FS_METACOPY
> +       help
> +         If this config option is enabled then overlay filesystems will
> +         try to copy fs-verity digests from the lower file into the
> +         metacopy file at metadata copy-up time. It is still possible
> +         to turn off this feature globally with the "verity=off"
> +         module option or on a filesystem instance basis with the
> +         "verity=off" or "verity=validate" mount option.
> +
> +         If unsure, say N.
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index a7b1006c5321..f759e476dfc7 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -13,6 +13,10 @@ struct ovl_config {
>         bool redirect_dir;
>         bool redirect_follow;
>         const char *redirect_mode;
> +       bool verity_validate;
> +       bool verity_generate;
> +       bool verity_require;
> +       const char *verity_mode;
>         bool index;
>         bool uuid;
>         bool nfs_export;
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index ef78abc21998..953d76f6a1e3 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -59,6 +59,11 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
>  MODULE_PARM_DESC(metacopy,
>                  "Default to on or off for the metadata only copy up feature");
>
> +static bool ovl_verity_def = IS_ENABLED(CONFIG_OVERLAY_FS_VERITY);
> +module_param_named(verity, ovl_verity_def, bool, 0644);
> +MODULE_PARM_DESC(verity,
> +                "Default to on or validate for the metadata only copy up feature");
> +
>  static struct dentry *ovl_d_real(struct dentry *dentry,
>                                  const struct inode *inode)
>  {
> @@ -235,6 +240,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         kfree(ofs->config.upperdir);
>         kfree(ofs->config.workdir);
>         kfree(ofs->config.redirect_mode);
> +       kfree(ofs->config.verity_mode);
>         if (ofs->creator_cred)
>                 put_cred(ofs->creator_cred);
>         kfree(ofs);
> @@ -325,6 +331,11 @@ static const char *ovl_redirect_mode_def(void)
>         return ovl_redirect_dir_def ? "on" : "off";
>  }
>
> +static const char *ovl_verity_mode_def(void)
> +{
> +       return ovl_verity_def ? "on" : "validate";
> +}
> +
>  static const char * const ovl_xino_str[] = {
>         "off",
>         "auto",
> @@ -374,6 +385,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
>                 seq_puts(m, ",volatile");
>         if (ofs->config.userxattr)
>                 seq_puts(m, ",userxattr");
> +       if (strcmp(ofs->config.verity_mode, ovl_verity_mode_def()) != 0)
> +               seq_printf(m, ",verity=%s", ofs->config.verity_mode);
>         return 0;
>  }
>
> @@ -429,6 +442,7 @@ enum {
>         OPT_METACOPY_ON,
>         OPT_METACOPY_OFF,
>         OPT_VOLATILE,
> +       OPT_VERITY,
>         OPT_ERR,
>  };
>
> @@ -451,6 +465,7 @@ static const match_table_t ovl_tokens = {
>         {OPT_METACOPY_ON,               "metacopy=on"},
>         {OPT_METACOPY_OFF,              "metacopy=off"},
>         {OPT_VOLATILE,                  "volatile"},
> +       {OPT_VERITY,                    "verity=%s"},
>         {OPT_ERR,                       NULL}
>  };
>
> @@ -500,6 +515,25 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
>         return 0;
>  }
>
> +static int ovl_parse_verity_mode(struct ovl_config *config, const char *mode)
> +{
> +       if (strcmp(mode, "validate") == 0) {
> +               config->verity_validate = true;
> +       } else if (strcmp(mode, "on") == 0) {
> +               config->verity_validate = true;
> +               config->verity_generate = true;
> +       } else if (strcmp(mode, "require") == 0) {
> +               config->verity_validate = true;
> +               config->verity_generate = true;
> +               config->verity_require = true;
> +       } else if (strcmp(mode, "off") != 0) {
> +               pr_err("bad mount option \"verity=%s\"\n", mode);
> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
>  static int ovl_parse_opt(char *opt, struct ovl_config *config)
>  {
>         char *p;
> @@ -511,6 +545,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (!config->redirect_mode)
>                 return -ENOMEM;
>
> +       config->verity_mode = kstrdup(ovl_verity_mode_def(), GFP_KERNEL);
> +       if (!config->verity_mode)
> +               return -ENOMEM;
> +
>         while ((p = ovl_next_opt(&opt)) != NULL) {
>                 int token;
>                 substring_t args[MAX_OPT_ARGS];
> @@ -611,6 +649,13 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>                         config->userxattr = true;
>                         break;
>
> +               case OPT_VERITY:
> +                       kfree(config->verity_mode);
> +                       config->verity_mode = match_strdup(&args[0]);
> +                       if (!config->verity_mode)
> +                               return -ENOMEM;
> +                       break;
> +
>                 default:
>                         pr_err("unrecognized mount option \"%s\" or missing value\n",
>                                         p);
> @@ -642,6 +687,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
>         if (err)
>                 return err;
>
> +       err = ovl_parse_verity_mode(config, config->verity_mode);
> +       if (err)
> +               return err;
> +
>         /*
>          * This is to make the logic below simpler.  It doesn't make any other
>          * difference, since config->redirect_dir is only used for upper.
> --
> 2.39.2
>
Alexander Larsson April 25, 2023, 1:33 p.m. UTC | #3
On Tue, Apr 25, 2023 at 1:19 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 20 Apr 2023 at 09:44, Alexander Larsson <alexl@redhat.com> wrote:
> >
> > This adds the scaffolding (docs, config, mount options) for supporting
> > for a new overlay xattr "overlay.verity", which contains a fs-verity
> > digest. This is used for metacopy files, and the actual fs-verity
> > digest of the lowerdata file needs to match it. The mount option
> > "verity" specifies how this xattrs is handled.
> >
> > Unless you explicitly disable it ("verity=off") all existing xattrs
> > are validated before use. This is all that happens by default
> > ("verity=validate"), but, if you turn on verity ("verity=on") then
> > during metacopy we generate verity xattr in the upper metacopy file if
> > the source file has verity enabled. This means later accesses can
> > guarantee that the correct data is used.
> >
> > Additionally you can use "verity=require". In this mode all metacopy
> > files must have a valid verity xattr. For this to work metadata
> > copy-up must be able to create a verity xattr (so that later accesses
> > are validated). Therefore, in this mode, if the lower data file
> > doesn't have fs-verity enabled we fall back to a full copy rather than
> > a metacopy.
>
> Maybe we can reduce the number of modes.  Which mode does your use case need?

For composefs I typically always create images with full verity info
so they *can* be used with verity, but I want to allow using these
even on systems that don't support verity. So, at the very minimum
composefs needs "verity=off" (don't even try to verify anything) and
"verity=require" (ensure all redirects have a verity xattr and it is
valid).

These are kind of extremes though, and I think it makes sense to have
something in between where not everything has to be verified.
Currently we have both "validate" and "on". But maybe those could be
consolidated.

> >
> > Actual implementation follows in a separate commit.
> >
> > Signed-off-by: Alexander Larsson <alexl@redhat.com>
> > ---
> >  Documentation/filesystems/overlayfs.rst | 33 +++++++++++++++++
> >  fs/overlayfs/Kconfig                    | 14 +++++++
> >  fs/overlayfs/ovl_entry.h                |  4 ++
> >  fs/overlayfs/super.c                    | 49 +++++++++++++++++++++++++
> >  4 files changed, 100 insertions(+)
> >
> > diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> > index c8e04a4f0e21..66895bf71cd1 100644
> > --- a/Documentation/filesystems/overlayfs.rst
> > +++ b/Documentation/filesystems/overlayfs.rst
> > @@ -403,6 +403,39 @@ when a "metacopy" file in one of the lower layers above it, has a "redirect"
> >  to the absolute path of the "lower data" file in the "data-only" lower layer.
> >
> >
> > +fs-verity support
> > +----------------------
> > +
> > +When metadata copy up is used for a file, then the xattr
> > +"trusted.overlay.verity" may be set on the metacopy file. This
> > +specifies the expected fs-verity digest of the lowerdata file. This
> > +may then be used to verify the content of the source file at the time
> > +the file is opened. If enabled, overlayfs can also set this xattr
> > +during metadata copy up.
> > +
> > +This is controlled by the "verity" mount option, which supports
> > +these values:
> > +
> > +- "off":
> > +    The verity xattr is never used.
> > +- "validate":
> > +    Whenever a metacopy files specifies an expected digest, the
> > +    corresponding data file must match the specified digest.
> > +- "on":
> > +    Same as validate, but additionally, when generating a metacopy
> > +    file the verity xattr will be set from the source file fs-verity
> > +    digest (if it has one).
> > +- "require":
> > +    Same as "on", but additionally all metacopy files must specify a
> > +    verity xattr. Additionally metadata copy up will only be used if
> > +    the data file has fs-verity enabled, otherwise a full copy-up is
> > +    used.
> > +
> > +There are two ways to tune the default behaviour. The kernel config
> > +option OVERLAY_FS_VERITY, or the module option "verity=BOOL". If
> > +either of these are enabled, then verity mode is "on" by default,
> > +otherwise it is "validate".
>
> I'm not sure that enabling verity by default is safe.  E.g. a script
> mounts overalyfs but doesn't set the verity mount, since it's on by
> default.  Then the script is moved to a different system where the
> default is off, which will result in verity not being enabled, even
> though that was not intended.  Is there an advantage to allowing to
> change the default?  I know it's done for most of the overlayfs
> options, but I think this is different.

I sort of agree, in particular because many filesystems still don't
support verity, or need it to be specifically enabled.
So, what about dropping "validate" and go with modes: "off, on,
require", where "off" is the default?

> > +
> >  Sharing and copying layers
> >  --------------------------
> >
> > diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> > index 6708e54b0e30..98d6b1a7baf5 100644
> > --- a/fs/overlayfs/Kconfig
> > +++ b/fs/overlayfs/Kconfig
> > @@ -124,3 +124,17 @@ config OVERLAY_FS_METACOPY
> >           that doesn't support this feature will have unexpected results.
> >
> >           If unsure, say N.
> > +
> > +config OVERLAY_FS_VERITY
> > +       bool "Overlayfs: turn on verity feature by default"
> > +       depends on OVERLAY_FS
> > +       depends on OVERLAY_FS_METACOPY
> > +       help
> > +         If this config option is enabled then overlay filesystems will
> > +         try to copy fs-verity digests from the lower file into the
> > +         metacopy file at metadata copy-up time. It is still possible
> > +         to turn off this feature globally with the "verity=off"
> > +         module option or on a filesystem instance basis with the
> > +         "verity=off" or "verity=validate" mount option.
> > +
> > +         If unsure, say N.
> > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > index a7b1006c5321..f759e476dfc7 100644
> > --- a/fs/overlayfs/ovl_entry.h
> > +++ b/fs/overlayfs/ovl_entry.h
> > @@ -13,6 +13,10 @@ struct ovl_config {
> >         bool redirect_dir;
> >         bool redirect_follow;
> >         const char *redirect_mode;
> > +       bool verity_validate;
> > +       bool verity_generate;
> > +       bool verity_require;
> > +       const char *verity_mode;
> >         bool index;
> >         bool uuid;
> >         bool nfs_export;
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index ef78abc21998..953d76f6a1e3 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -59,6 +59,11 @@ module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
> >  MODULE_PARM_DESC(metacopy,
> >                  "Default to on or off for the metadata only copy up feature");
> >
> > +static bool ovl_verity_def = IS_ENABLED(CONFIG_OVERLAY_FS_VERITY);
> > +module_param_named(verity, ovl_verity_def, bool, 0644);
> > +MODULE_PARM_DESC(verity,
> > +                "Default to on or validate for the metadata only copy up feature");
> > +
> >  static struct dentry *ovl_d_real(struct dentry *dentry,
> >                                  const struct inode *inode)
> >  {
> > @@ -235,6 +240,7 @@ static void ovl_free_fs(struct ovl_fs *ofs)
> >         kfree(ofs->config.upperdir);
> >         kfree(ofs->config.workdir);
> >         kfree(ofs->config.redirect_mode);
> > +       kfree(ofs->config.verity_mode);
> >         if (ofs->creator_cred)
> >                 put_cred(ofs->creator_cred);
> >         kfree(ofs);
> > @@ -325,6 +331,11 @@ static const char *ovl_redirect_mode_def(void)
> >         return ovl_redirect_dir_def ? "on" : "off";
> >  }
> >
> > +static const char *ovl_verity_mode_def(void)
> > +{
> > +       return ovl_verity_def ? "on" : "validate";
> > +}
> > +
> >  static const char * const ovl_xino_str[] = {
> >         "off",
> >         "auto",
> > @@ -374,6 +385,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
> >                 seq_puts(m, ",volatile");
> >         if (ofs->config.userxattr)
> >                 seq_puts(m, ",userxattr");
> > +       if (strcmp(ofs->config.verity_mode, ovl_verity_mode_def()) != 0)
> > +               seq_printf(m, ",verity=%s", ofs->config.verity_mode);
> >         return 0;
> >  }
> >
> > @@ -429,6 +442,7 @@ enum {
> >         OPT_METACOPY_ON,
> >         OPT_METACOPY_OFF,
> >         OPT_VOLATILE,
> > +       OPT_VERITY,
> >         OPT_ERR,
> >  };
> >
> > @@ -451,6 +465,7 @@ static const match_table_t ovl_tokens = {
> >         {OPT_METACOPY_ON,               "metacopy=on"},
> >         {OPT_METACOPY_OFF,              "metacopy=off"},
> >         {OPT_VOLATILE,                  "volatile"},
> > +       {OPT_VERITY,                    "verity=%s"},
> >         {OPT_ERR,                       NULL}
> >  };
> >
> > @@ -500,6 +515,25 @@ static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
> >         return 0;
> >  }
> >
> > +static int ovl_parse_verity_mode(struct ovl_config *config, const char *mode)
> > +{
> > +       if (strcmp(mode, "validate") == 0) {
> > +               config->verity_validate = true;
> > +       } else if (strcmp(mode, "on") == 0) {
> > +               config->verity_validate = true;
> > +               config->verity_generate = true;
> > +       } else if (strcmp(mode, "require") == 0) {
> > +               config->verity_validate = true;
> > +               config->verity_generate = true;
> > +               config->verity_require = true;
> > +       } else if (strcmp(mode, "off") != 0) {
> > +               pr_err("bad mount option \"verity=%s\"\n", mode);
> > +               return -EINVAL;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >  {
> >         char *p;
> > @@ -511,6 +545,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >         if (!config->redirect_mode)
> >                 return -ENOMEM;
> >
> > +       config->verity_mode = kstrdup(ovl_verity_mode_def(), GFP_KERNEL);
> > +       if (!config->verity_mode)
> > +               return -ENOMEM;
> > +
> >         while ((p = ovl_next_opt(&opt)) != NULL) {
> >                 int token;
> >                 substring_t args[MAX_OPT_ARGS];
> > @@ -611,6 +649,13 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >                         config->userxattr = true;
> >                         break;
> >
> > +               case OPT_VERITY:
> > +                       kfree(config->verity_mode);
> > +                       config->verity_mode = match_strdup(&args[0]);
> > +                       if (!config->verity_mode)
> > +                               return -ENOMEM;
> > +                       break;
> > +
> >                 default:
> >                         pr_err("unrecognized mount option \"%s\" or missing value\n",
> >                                         p);
> > @@ -642,6 +687,10 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> >         if (err)
> >                 return err;
> >
> > +       err = ovl_parse_verity_mode(config, config->verity_mode);
> > +       if (err)
> > +               return err;
> > +
> >         /*
> >          * This is to make the logic below simpler.  It doesn't make any other
> >          * difference, since config->redirect_dir is only used for upper.
> > --
> > 2.39.2
> >
>
Miklos Szeredi April 25, 2023, 7:07 p.m. UTC | #4
On Tue, 25 Apr 2023 at 15:33, Alexander Larsson <alexl@redhat.com> wrote:
>
> On Tue, Apr 25, 2023 at 1:19 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Thu, 20 Apr 2023 at 09:44, Alexander Larsson <alexl@redhat.com> wrote:

> > > +There are two ways to tune the default behaviour. The kernel config
> > > +option OVERLAY_FS_VERITY, or the module option "verity=BOOL". If
> > > +either of these are enabled, then verity mode is "on" by default,
> > > +otherwise it is "validate".
> >
> > I'm not sure that enabling verity by default is safe.  E.g. a script
> > mounts overalyfs but doesn't set the verity mount, since it's on by
> > default.  Then the script is moved to a different system where the
> > default is off, which will result in verity not being enabled, even
> > though that was not intended.  Is there an advantage to allowing to
> > change the default?  I know it's done for most of the overlayfs
> > options, but I think this is different.
>
> I sort of agree, in particular because many filesystems still don't
> support verity, or need it to be specifically enabled.
> So, what about dropping "validate" and go with modes: "off, on,
> require", where "off" is the default?

Okay.

Thanks,
Miklos
diff mbox series

Patch

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index c8e04a4f0e21..66895bf71cd1 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -403,6 +403,39 @@  when a "metacopy" file in one of the lower layers above it, has a "redirect"
 to the absolute path of the "lower data" file in the "data-only" lower layer.
 
 
+fs-verity support
+----------------------
+
+When metadata copy up is used for a file, then the xattr
+"trusted.overlay.verity" may be set on the metacopy file. This
+specifies the expected fs-verity digest of the lowerdata file. This
+may then be used to verify the content of the source file at the time
+the file is opened. If enabled, overlayfs can also set this xattr
+during metadata copy up.
+
+This is controlled by the "verity" mount option, which supports
+these values:
+
+- "off":
+    The verity xattr is never used.
+- "validate":
+    Whenever a metacopy files specifies an expected digest, the
+    corresponding data file must match the specified digest.
+- "on":
+    Same as validate, but additionally, when generating a metacopy
+    file the verity xattr will be set from the source file fs-verity
+    digest (if it has one).
+- "require":
+    Same as "on", but additionally all metacopy files must specify a
+    verity xattr. Additionally metadata copy up will only be used if
+    the data file has fs-verity enabled, otherwise a full copy-up is
+    used.
+
+There are two ways to tune the default behaviour. The kernel config
+option OVERLAY_FS_VERITY, or the module option "verity=BOOL". If
+either of these are enabled, then verity mode is "on" by default,
+otherwise it is "validate".
+
 Sharing and copying layers
 --------------------------
 
diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index 6708e54b0e30..98d6b1a7baf5 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -124,3 +124,17 @@  config OVERLAY_FS_METACOPY
 	  that doesn't support this feature will have unexpected results.
 
 	  If unsure, say N.
+
+config OVERLAY_FS_VERITY
+	bool "Overlayfs: turn on verity feature by default"
+	depends on OVERLAY_FS
+	depends on OVERLAY_FS_METACOPY
+	help
+	  If this config option is enabled then overlay filesystems will
+	  try to copy fs-verity digests from the lower file into the
+	  metacopy file at metadata copy-up time. It is still possible
+	  to turn off this feature globally with the "verity=off"
+	  module option or on a filesystem instance basis with the
+	  "verity=off" or "verity=validate" mount option.
+
+	  If unsure, say N.
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index a7b1006c5321..f759e476dfc7 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -13,6 +13,10 @@  struct ovl_config {
 	bool redirect_dir;
 	bool redirect_follow;
 	const char *redirect_mode;
+	bool verity_validate;
+	bool verity_generate;
+	bool verity_require;
+	const char *verity_mode;
 	bool index;
 	bool uuid;
 	bool nfs_export;
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index ef78abc21998..953d76f6a1e3 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -59,6 +59,11 @@  module_param_named(metacopy, ovl_metacopy_def, bool, 0644);
 MODULE_PARM_DESC(metacopy,
 		 "Default to on or off for the metadata only copy up feature");
 
+static bool ovl_verity_def = IS_ENABLED(CONFIG_OVERLAY_FS_VERITY);
+module_param_named(verity, ovl_verity_def, bool, 0644);
+MODULE_PARM_DESC(verity,
+		 "Default to on or validate for the metadata only copy up feature");
+
 static struct dentry *ovl_d_real(struct dentry *dentry,
 				 const struct inode *inode)
 {
@@ -235,6 +240,7 @@  static void ovl_free_fs(struct ovl_fs *ofs)
 	kfree(ofs->config.upperdir);
 	kfree(ofs->config.workdir);
 	kfree(ofs->config.redirect_mode);
+	kfree(ofs->config.verity_mode);
 	if (ofs->creator_cred)
 		put_cred(ofs->creator_cred);
 	kfree(ofs);
@@ -325,6 +331,11 @@  static const char *ovl_redirect_mode_def(void)
 	return ovl_redirect_dir_def ? "on" : "off";
 }
 
+static const char *ovl_verity_mode_def(void)
+{
+	return ovl_verity_def ? "on" : "validate";
+}
+
 static const char * const ovl_xino_str[] = {
 	"off",
 	"auto",
@@ -374,6 +385,8 @@  static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
 		seq_puts(m, ",volatile");
 	if (ofs->config.userxattr)
 		seq_puts(m, ",userxattr");
+	if (strcmp(ofs->config.verity_mode, ovl_verity_mode_def()) != 0)
+		seq_printf(m, ",verity=%s", ofs->config.verity_mode);
 	return 0;
 }
 
@@ -429,6 +442,7 @@  enum {
 	OPT_METACOPY_ON,
 	OPT_METACOPY_OFF,
 	OPT_VOLATILE,
+	OPT_VERITY,
 	OPT_ERR,
 };
 
@@ -451,6 +465,7 @@  static const match_table_t ovl_tokens = {
 	{OPT_METACOPY_ON,		"metacopy=on"},
 	{OPT_METACOPY_OFF,		"metacopy=off"},
 	{OPT_VOLATILE,			"volatile"},
+	{OPT_VERITY,			"verity=%s"},
 	{OPT_ERR,			NULL}
 };
 
@@ -500,6 +515,25 @@  static int ovl_parse_redirect_mode(struct ovl_config *config, const char *mode)
 	return 0;
 }
 
+static int ovl_parse_verity_mode(struct ovl_config *config, const char *mode)
+{
+	if (strcmp(mode, "validate") == 0) {
+		config->verity_validate = true;
+	} else if (strcmp(mode, "on") == 0) {
+		config->verity_validate = true;
+		config->verity_generate = true;
+	} else if (strcmp(mode, "require") == 0) {
+		config->verity_validate = true;
+		config->verity_generate = true;
+		config->verity_require = true;
+	} else if (strcmp(mode, "off") != 0) {
+		pr_err("bad mount option \"verity=%s\"\n", mode);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int ovl_parse_opt(char *opt, struct ovl_config *config)
 {
 	char *p;
@@ -511,6 +545,10 @@  static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	if (!config->redirect_mode)
 		return -ENOMEM;
 
+	config->verity_mode = kstrdup(ovl_verity_mode_def(), GFP_KERNEL);
+	if (!config->verity_mode)
+		return -ENOMEM;
+
 	while ((p = ovl_next_opt(&opt)) != NULL) {
 		int token;
 		substring_t args[MAX_OPT_ARGS];
@@ -611,6 +649,13 @@  static int ovl_parse_opt(char *opt, struct ovl_config *config)
 			config->userxattr = true;
 			break;
 
+		case OPT_VERITY:
+			kfree(config->verity_mode);
+			config->verity_mode = match_strdup(&args[0]);
+			if (!config->verity_mode)
+				return -ENOMEM;
+			break;
+
 		default:
 			pr_err("unrecognized mount option \"%s\" or missing value\n",
 					p);
@@ -642,6 +687,10 @@  static int ovl_parse_opt(char *opt, struct ovl_config *config)
 	if (err)
 		return err;
 
+	err = ovl_parse_verity_mode(config, config->verity_mode);
+	if (err)
+		return err;
+
 	/*
 	 * This is to make the logic below simpler.  It doesn't make any other
 	 * difference, since config->redirect_dir is only used for upper.