| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 29 Mar 2021 13:05:55 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Leah Rumancik <leah.rumancik@...il.com>
Cc: "Darrick J. Wong" <djwong@...nel.org>,
harshad shirwadkar <harshadshirwadkar@...il.com>,
Ext4 Developers List <linux-ext4@...r.kernel.org>,
"Theodore Y. Ts'o" <tytso@....edu>
Subject: Re: [PATCH 1/2] ext4: wipe filename upon file deletion
> On Mar 29, 2021, at 10:06 AM, Leah Rumancik <leah.rumancik@...il.com> wrote:
>
> On Fri, Mar 26, 2021 at 07:08:23PM -0700, Darrick J. Wong wrote:
>> On Fri, Mar 26, 2021 at 06:43:52PM -0700, harshad shirwadkar wrote:
>>> On Fri, Mar 26, 2021 at 4:08 PM Andreas Dilger <adilger@...ger.ca> wrote:
>>>>
>>>> On Mar 25, 2021, at 12:12 PM, Leah Rumancik <leah.rumancik@...il.com> wrote:
>>>>>
>>>>> Zero out filename field when file is deleted. Also, add mount option
>>>>> nowipe_filename to disable this filename wipeout if desired.
>>>>
>>>> I would personally be against "wipe out entries on delete" as the default
>>>> behavior. I think most users would prefer that their data is maximally
>>>> recoverable, rather than the minimal security benefit of erasing deleted
>>>> content by default.
>>> I understand that persistence of filenames provides recoverability
>>> that users might like but I feel like that may break sooner or later.
>>> For example, if we get directory shrinking patches[1] merged in or if
>>> we redesign the directory htree using generic btrees (which will
>>> hopefully support shrinking), this kind of recovery will become very
>>> hard.
>>>
>>> Also, I was wondering if persistence of file names was by design? or
>>> it was there due to the way we implemented directories?
>>
>> I bet it wasn't done by design -- afaict all the recovery tools are
>> totally opportunistic in that /if/ they find something that looks like a
>> directory entry, /then/ it will pick that up. The names will eventually
>> get overwritten, so that's the best they can do.
>>
>> (I would also wager that people don't like opt-out for new behaviors
>> unless you're adding it as part of a new feature...)
>>
>> --D
>
> What about a mount option to enable the filename wipe (with the wiping
> disabled by default)?
I would be OK with a mount option to enable it globally, or EXT2_SECRM_FL
to trigger this on a per-file or per-directory basis (entry is zeroed
if this is set on either the file or the directory it is located in, and
possibly the file's data blocks are trimmed from flash).
Cheers, Andreas
>>> [1] https://patchwork.ozlabs.org/project/linux-ext4/list/?series=166560
>>>
>>> Thanks,
>>> Harshad
>>>>
>>>> I think that Darrick made a good point that using the EXT4_SECRM_FL on
>>>> the inode gives users a good option to enable/disable this on a per
>>>> file or directory basis.
>>>>
>>>>> Signed-off-by: Leah Rumancik <leah.rumancik@...il.com>
>>>>> ---
>>>>> fs/ext4/ext4.h | 1 +
>>>>> fs/ext4/namei.c | 4 ++++
>>>>> fs/ext4/super.c | 11 ++++++++++-
>>>>> 3 files changed, 15 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>>>>> index 826a56e3bbd2..8011418176bc 100644
>>>>> --- a/fs/ext4/ext4.h
>>>>> +++ b/fs/ext4/ext4.h
>>>>> @@ -1247,6 +1247,7 @@ struct ext4_inode_info {
>>>>> #define EXT4_MOUNT2_JOURNAL_FAST_COMMIT 0x00000010 /* Journal fast commit */
>>>>> #define EXT4_MOUNT2_DAX_NEVER 0x00000020 /* Do not allow Direct Access */
>>>>> #define EXT4_MOUNT2_DAX_INODE 0x00000040 /* For printing options only */
>>>>> +#define EXT4_MOUNT2_WIPE_FILENAME 0x00000080 /* Wipe filename on del entry */
>>>>>
>>>>>
>>>>> #define clear_opt(sb, opt) EXT4_SB(sb)->s_mount_opt &= \
>>>>> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
>>>>> index 883e2a7cd4ab..ae6ecabd4d97 100644
>>>>> --- a/fs/ext4/namei.c
>>>>> +++ b/fs/ext4/namei.c
>>>>> @@ -2492,6 +2492,10 @@ int ext4_generic_delete_entry(struct inode *dir,
>>>>> else
>>>>> de->inode = 0;
>>>>> inode_inc_iversion(dir);
>>>>> +
>>>>> + if (test_opt2(dir->i_sb, WIPE_FILENAME))
>>>>> + memset(de_del->name, 0, de_del->name_len);
>>>>> +
>>>>> return 0;
>>>>> }
>>>>> i += ext4_rec_len_from_disk(de->rec_len, blocksize);
>>>>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>>>> index b9693680463a..5e8737b3f171 100644
>>>>> --- a/fs/ext4/super.c
>>>>> +++ b/fs/ext4/super.c
>>>>> @@ -1688,7 +1688,7 @@ enum {
>>>>> Opt_dioread_nolock, Opt_dioread_lock,
>>>>> Opt_discard, Opt_nodiscard, Opt_init_itable, Opt_noinit_itable,
>>>>> Opt_max_dir_size_kb, Opt_nojournal_checksum, Opt_nombcache,
>>>>> - Opt_prefetch_block_bitmaps,
>>>>> + Opt_prefetch_block_bitmaps, Opt_nowipe_filename,
>>>>> #ifdef CONFIG_EXT4_DEBUG
>>>>> Opt_fc_debug_max_replay, Opt_fc_debug_force
>>>>> #endif
>>>>> @@ -1787,6 +1787,7 @@ static const match_table_t tokens = {
>>>>> {Opt_test_dummy_encryption, "test_dummy_encryption"},
>>>>> {Opt_inlinecrypt, "inlinecrypt"},
>>>>> {Opt_nombcache, "nombcache"},
>>>>> + {Opt_nowipe_filename, "nowipe_filename"},
>>>>> {Opt_nombcache, "no_mbcache"}, /* for backward compatibility */
>>>>> {Opt_prefetch_block_bitmaps, "prefetch_block_bitmaps"},
>>>>> {Opt_removed, "check=none"}, /* mount option from ext2/3 */
>>>>> @@ -2007,6 +2008,8 @@ static const struct mount_opts {
>>>>> {Opt_max_dir_size_kb, 0, MOPT_GTE0},
>>>>> {Opt_test_dummy_encryption, 0, MOPT_STRING},
>>>>> {Opt_nombcache, EXT4_MOUNT_NO_MBCACHE, MOPT_SET},
>>>>> + {Opt_nowipe_filename, EXT4_MOUNT2_WIPE_FILENAME, MOPT_CLEAR | MOPT_2 |
>>>>> + MOPT_EXT4_ONLY},
>>>>> {Opt_prefetch_block_bitmaps, EXT4_MOUNT_PREFETCH_BLOCK_BITMAPS,
>>>>> MOPT_SET},
>>>>> #ifdef CONFIG_EXT4_DEBUG
>>>>> @@ -2621,6 +2624,10 @@ static int _ext4_show_options(struct seq_file *seq, struct super_block *sb,
>>>>> } else if (test_opt2(sb, DAX_INODE)) {
>>>>> SEQ_OPTS_PUTS("dax=inode");
>>>>> }
>>>>> +
>>>>> + if (!test_opt2(sb, WIPE_FILENAME))
>>>>> + SEQ_OPTS_PUTS("nowipe_filename");
>>>>> +
>>>>> ext4_show_quota_options(seq, sb);
>>>>> return 0;
>>>>> }
>>>>> @@ -4161,6 +4168,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>>>>> if (def_mount_opts & EXT4_DEFM_DISCARD)
>>>>> set_opt(sb, DISCARD);
>>>>>
>>>>> + set_opt2(sb, WIPE_FILENAME);
>>>>> +
>>>>> sbi->s_resuid = make_kuid(&init_user_ns, le16_to_cpu(es->s_def_resuid));
>>>>> sbi->s_resgid = make_kgid(&init_user_ns, le16_to_cpu(es->s_def_resgid));
>>>>> sbi->s_commit_interval = JBD2_DEFAULT_MAX_COMMIT_AGE * HZ;
>>>>> --
>>>>> 2.31.0.291.g576ba9dcdaf-goog
>>>>>
>>>>
>>>>
>>>> Cheers, Andreas
Cheers, Andreas
Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)
Powered by blists - more mailing lists