| Message ID | 1477054121-10198-27-git-send-email-richard@nod.at (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 21, 2016 at 02:48:41PM +0200, Richard Weinberger wrote: > Starting with version 5 the following properties change: > - UBIFS_FLG_DOUBLE_HASH is mandatory > - UBIFS_FLG_ENCRYPTION is optional but depdens on UBIFS_FLG_DOUBLE_HASH > - Filesystems with unknown super block flags will be rejected, this > allows us in future to add new features without raising the UBIFS > write version. > > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > fs/ubifs/sb.c | 17 +++++++++++++++++ > fs/ubifs/ubifs-media.h | 4 +++- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c > index 54cef70ea16f..7f1ead29e727 100644 > --- a/fs/ubifs/sb.c > +++ b/fs/ubifs/sb.c > @@ -466,6 +466,16 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup) > goto failed; > } > > + if (!c->double_hash && c->fmt_version >= 5) { > + err = 16; > + goto failed; > + } > + > + if (c->encrypted && c->fmt_version < 5) { > + err = 17; > + goto failed; > + } > + > return 0; > > failed: > @@ -624,6 +634,13 @@ int ubifs_read_superblock(struct ubifs_info *c) > c->double_hash = !!(sup_flags & UBIFS_FLG_DOUBLE_HASH); > c->encrypted = !!(sup_flags & UBIFS_FLG_ENCRYPTION); > > + if ((sup_flags & ~UBIFS_FLG_MASK) != 0) { > + ubifs_err(c, "Unknown feature flags found: %#x", > + sup_flags & ~UBIFS_FLG_MASK); > + err = -EINVAL; > + goto out; > + } > + > #ifndef CONFIG_UBIFS_FS_ENCRYPTION > if (c->encrypted) { > ubifs_err(c, "file system contains encrypted files but UBIFS" > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h > index bdc7935a5e41..e8c23c9d4f4a 100644 > --- a/fs/ubifs/ubifs-media.h > +++ b/fs/ubifs/ubifs-media.h > @@ -46,7 +46,7 @@ > * UBIFS went into mainline kernel with format version 4. The older formats > * were development formats. > */ > -#define UBIFS_FORMAT_VERSION 4 > +#define UBIFS_FORMAT_VERSION 5 Alex Cope is working on a fix for file name encryption in ext4 so that common plaintext prefixes don't result in common ciphertext prefixes. Older kernels will not be able to read the new file names. > > /* > * Read-only compatibility version. If the UBIFS format is changed, older UBIFS > @@ -429,6 +429,8 @@ enum { > UBIFS_FLG_ENCRYPTION = 0x10, > }; > > +#define UBIFS_FLG_MASK (UBIFS_FLG_BIGLPT|UBIFS_FLG_SPACE_FIXUP|UBIFS_FLG_DOUBLE_HASH|UBIFS_FLG_ENCRYPTION) > + > /** > * struct ubifs_ch - common header node. > * @magic: UBIFS node magic number (%UBIFS_NODE_MAGIC) > -- > 2.7.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote: > > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h > > index bdc7935a5e41..e8c23c9d4f4a 100644 > > --- a/fs/ubifs/ubifs-media.h > > +++ b/fs/ubifs/ubifs-media.h > > @@ -46,7 +46,7 @@ > > * UBIFS went into mainline kernel with format version 4. The older formats > > * were development formats. > > */ > > -#define UBIFS_FORMAT_VERSION 4 > > +#define UBIFS_FORMAT_VERSION 5 > > Alex Cope is working on a fix for file name encryption in ext4 so that > common plaintext prefixes don't result in common ciphertext prefixes. > Older kernels will not be able to read the new file names. To be clear, this will be done in the context of a new encryption mode. In terms of how Ubifs will handle things, that's going to depend on whether ubifs uses a single major version number or whether they have a feature bitmask like other filesystems, including ext4. This is better because if the user doesn't use a particular feature, especially a feature like encryption which is optional, we don't want the file system to reject the mount unnecessarily. In the case of encryption, where there may be new encryption algorithms used, or maybe even hardware-specific encryption modes if you are using in-line encryption where the encryption is done in hardware, the question is whether you want to refuse the mount if you know it won't work, or just throw an error when there is an attempt to access a directory or file which is encrypted using an encryption algorithm which a particular kernel version doesn't support. We did leave some room in the ext4 superblock for an encryption format version number, as ewll as an array of which encryption algorithsm are in use in the superblock, but we haven't really decided which strategy we want to use. My current thinking is that there ought to be a warning at mount time if there are some directories which the kernel being used won't be able to access, but we should just fail the mount entirely, since there might be a lot of unencrypted files that the user might want to access. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 21, 2016 at 01:47:59PM -0400, Theodore Ts'o wrote: > On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote: > > > diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h > > > index bdc7935a5e41..e8c23c9d4f4a 100644 > > > --- a/fs/ubifs/ubifs-media.h > > > +++ b/fs/ubifs/ubifs-media.h > > > @@ -46,7 +46,7 @@ > > > * UBIFS went into mainline kernel with format version 4. The older formats > > > * were development formats. > > > */ > > > -#define UBIFS_FORMAT_VERSION 4 > > > +#define UBIFS_FORMAT_VERSION 5 > > > > Alex Cope is working on a fix for file name encryption in ext4 so that > > common plaintext prefixes don't result in common ciphertext prefixes. > > Older kernels will not be able to read the new file names. > > To be clear, this will be done in the context of a new encryption > mode. In terms of how Ubifs will handle things, that's going to > depend on whether ubifs uses a single major version number or whether > they have a feature bitmask like other filesystems, including ext4. > I don't think it's reasonable to require require changes to filesystems whenever someone introduces a new encryption mode --- contents, filenames, or both. Filesystems need to be able to handle unsupported encryption modes in some way that makes sense. Currently, when it sees an unsupported encryption mode fscrypto will behave as if the encryption key is not available and will also print a one-time warning to the kernel log. This happens when a file is accessed, not when the filesystem is mounted. As far as I can tell, ext4, f2fs, and ubifs would all behave this way because this code is shared. I think this is probably the most realistic behavior. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 21, 2016 at 11:19:31AM -0700, Eric Biggers wrote: > > I don't think it's reasonable to require require changes to filesystems whenever > someone introduces a new encryption mode --- contents, filenames, or both. > Filesystems need to be able to handle unsupported encryption modes in some way > that makes sense. Currently, when it sees an unsupported encryption mode > fscrypto will behave as if the encryption key is not available and will also > print a one-time warning to the kernel log. This happens when a file is > accessed, not when the filesystem is mounted. As far as I can tell, ext4, f2fs, > and ubifs would all behave this way because this code is shared. I think this > is probably the most realistic behavior. I tend to agree, but file systems may choose some alternate approach if they want to "fail fast" (e.g., at mount time). I wouldn't want to do that for ext4, but if ubifs (or some other file system) wants do something more draconian, they can be afraid to do that. Failing that, some kind of one-time warning makes sense. What I would like to do though is to is to have a callback so that code in fs/crypto can call a file system specific notification routine. e.g., for ext4, we would probably want to be able to call ext4_warning() and ext4_error() from fs/crypto, and other file systems might want to have a different set of notification routines. This way we can print a message like kernel: EXT4-fs warning (device sdb1): fscrypto_xxx: foo bar baz and if we later on have a way of sending file system specific warnings or errors through some kind of IPC mechanism, such as netlink or some future kdbus scheme, we can send the warning and error messages out the same way we send other filesystem specific error messages. - Ted P.S. BTW, we actually _do_ have something hacked together inside the Google production kernel which pipes ext4_error() messages to a netlink socket, so that monitoring systems don't have scrape dmesg or /var/log/messages. If anyone inside or outside google is interested in that functionality, I can make the code available. There's nothing sensitive or Google specific in it; it's just that unfortunately, getting that code cleaned up and upstreamed has just never made it "above the fold" on the priority list, the engineer who originally implemented it is no longer on the team --- and I never had the time to cleanup work to get the code to upstream quality myself. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Ted, On 21.10.2016 19:47, Theodore Ts'o wrote: > On Fri, Oct 21, 2016 at 10:31:54AM -0700, Michael Halcrow wrote: >>> diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h >>> index bdc7935a5e41..e8c23c9d4f4a 100644 >>> --- a/fs/ubifs/ubifs-media.h >>> +++ b/fs/ubifs/ubifs-media.h >>> @@ -46,7 +46,7 @@ >>> * UBIFS went into mainline kernel with format version 4. The older formats >>> * were development formats. >>> */ >>> -#define UBIFS_FORMAT_VERSION 4 >>> +#define UBIFS_FORMAT_VERSION 5 >> >> Alex Cope is working on a fix for file name encryption in ext4 so that >> common plaintext prefixes don't result in common ciphertext prefixes. >> Older kernels will not be able to read the new file names. > > To be clear, this will be done in the context of a new encryption > mode. In terms of how Ubifs will handle things, that's going to > depend on whether ubifs uses a single major version number or whether > they have a feature bitmask like other filesystems, including ext4. With write version 5, UBIFS has a real feature bitmask. UBIFS has a feature bitmask since ever but never enforced it. i.e. you could set bits which are unknown to UBIFS and it sill mounted. Now UBIFS will refuse to mount when features are set which are not known/enabled by this implementation. Maybe I'll add another bitmap just for crypto do be able to support different cipher modes. Thanks, //richard -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c index 54cef70ea16f..7f1ead29e727 100644 --- a/fs/ubifs/sb.c +++ b/fs/ubifs/sb.c @@ -466,6 +466,16 @@ static int validate_sb(struct ubifs_info *c, struct ubifs_sb_node *sup) goto failed; } + if (!c->double_hash && c->fmt_version >= 5) { + err = 16; + goto failed; + } + + if (c->encrypted && c->fmt_version < 5) { + err = 17; + goto failed; + } + return 0; failed: @@ -624,6 +634,13 @@ int ubifs_read_superblock(struct ubifs_info *c) c->double_hash = !!(sup_flags & UBIFS_FLG_DOUBLE_HASH); c->encrypted = !!(sup_flags & UBIFS_FLG_ENCRYPTION); + if ((sup_flags & ~UBIFS_FLG_MASK) != 0) { + ubifs_err(c, "Unknown feature flags found: %#x", + sup_flags & ~UBIFS_FLG_MASK); + err = -EINVAL; + goto out; + } + #ifndef CONFIG_UBIFS_FS_ENCRYPTION if (c->encrypted) { ubifs_err(c, "file system contains encrypted files but UBIFS" diff --git a/fs/ubifs/ubifs-media.h b/fs/ubifs/ubifs-media.h index bdc7935a5e41..e8c23c9d4f4a 100644 --- a/fs/ubifs/ubifs-media.h +++ b/fs/ubifs/ubifs-media.h @@ -46,7 +46,7 @@ * UBIFS went into mainline kernel with format version 4. The older formats * were development formats. */ -#define UBIFS_FORMAT_VERSION 4 +#define UBIFS_FORMAT_VERSION 5 /* * Read-only compatibility version. If the UBIFS format is changed, older UBIFS @@ -429,6 +429,8 @@ enum { UBIFS_FLG_ENCRYPTION = 0x10, }; +#define UBIFS_FLG_MASK (UBIFS_FLG_BIGLPT|UBIFS_FLG_SPACE_FIXUP|UBIFS_FLG_DOUBLE_HASH|UBIFS_FLG_ENCRYPTION) + /** * struct ubifs_ch - common header node. * @magic: UBIFS node magic number (%UBIFS_NODE_MAGIC)
Starting with version 5 the following properties change: - UBIFS_FLG_DOUBLE_HASH is mandatory - UBIFS_FLG_ENCRYPTION is optional but depdens on UBIFS_FLG_DOUBLE_HASH - Filesystems with unknown super block flags will be rejected, this allows us in future to add new features without raising the UBIFS write version. Signed-off-by: Richard Weinberger <richard@nod.at> --- fs/ubifs/sb.c | 17 +++++++++++++++++ fs/ubifs/ubifs-media.h | 4 +++- 2 files changed, 20 insertions(+), 1 deletion(-)