On Tue, Apr 25, 2023 at 1:19 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Thu, 20 Apr 2023 at 09:44, Alexander Larsson <alexl@xxxxxxxxxx> 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@xxxxxxxxxx> > > --- > > 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 > > > -- =-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-= Alexander Larsson Red Hat, Inc alexl@xxxxxxxxxx alexander.larsson@xxxxxxxxx