linux-ext4 - Re: [PATCH v1 5/10] ext4: Add DX_HASH

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140923204527.20932.qmail@ns.horizon.com>
Date:	23 Sep 2014 16:45:27 -0400
From:	"George Spelvin" <linux@...izon.com>
To:	adilger@...ger.ca, linux@...izon.com
Cc:	linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH v1 5/10] ext4: Add DX_HASH_SIPHASH24 support

Thanks for the feedback!

> Simple, except that it introduces a serious bug in the code. It duplicates
> the previous on-disk encoding of:
>   DX_HASH_LEGACY | DX_HASH_UNSIGNED_DELTA =3D 3
> 
> with:
>   DX_HASH_SIPHASH24    3
> 
> and that will mean old filesystems would be considered corrupted.

Er... I'll let Ted tell me if I screwed up, but I went through
the code quite carefully figuring out what value to use, and
DX_HASH_LEGACY_UNSIGNED is *not* an on-disk encoding.

The only on-disk encodings are 0-2, with the "Unsigned delta" being an
internal-only way of representing EXT2_FLAGS_UNSIGNED_HASH.

You can see all that in patch 1/10, or even more clearly in the first
hunk of patch 9/10, where e2fsck pass 1 is modified to extend the range
of legal on-disk values:

diff --git a/e2fsck/pass1.c b/e2fsck/pass1.c
index 4fc5311..a89e556 100644
--- a/e2fsck/pass1.c
+++ b/e2fsck/pass1.c
@@ -2228,6 +2228,7 @@ static int handle_htree(e2fsck_t ctx, struct problem_context *pctx,
 	if ((root->hash_version != EXT2_HASH_LEGACY) &&
 	    (root->hash_version != EXT2_HASH_HALF_MD4) &&
 	    (root->hash_version != EXT2_HASH_TEA) &&
+	    (root->hash_version != EXT2_HASH_SIPHASH24) &&
 	    fix_problem(ctx, PR_1_HTREE_HASHV, pctx))
 		return 1;
 
Looks to me like any other value (and in particular, the value 3)
appearing on disk is an error.

I didn't want to introduce a hole in the on-disk numbering, so
I bumped up the internal-only values.  Allowing that is what patches
1 and 6 of the series are for.

I can try clarify the comments if you find it confusing.

Alternatively, with Ted's permission, I can change DX_HASH_UNSIGNED_DELTA
to 256 (and rename it to show it's a bit flag) so it's obviously not
representable on disk.
-- 
	-Colin
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html