linux-kernel - Re: [PATCH v2 3/4] kselftests/arm64: add PAuth test for whether exec() changes keys

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: <926691e4-1990-207e-bcb9-40ab6d3b0fa0@arm.com>
Date:   Thu, 3 Sep 2020 11:48:37 +0100
From:   Boyan Karatotev <boyan.karatotev@....com>
To:     Dave Martin <Dave.Martin@....com>
Cc:     linux-arm-kernel@...ts.infradead.org,
        linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
        Will Deacon <will@...nel.org>, boian4o1@...il.com,
        Catalin Marinas <catalin.marinas@....com>,
        amit.kachhap@....com, vincenzo.frascino@....com,
        Shuah Khan <shuah@...nel.org>
Subject: Re: [PATCH v2 3/4] kselftests/arm64: add PAuth test for whether
 exec() changes keys

On 02/09/2020 18:08, Dave Martin wrote:
> On Mon, Aug 31, 2020 at 12:04:49PM +0100, Boyan Karatotev wrote:
>> +/*
>> + * fork() does not change keys. Only exec() does so call a worker program.
>> + * Its only job is to sign a value and report back the resutls
>> + */
>> +TEST(exec_unique_keys)
>> +{
> 
> The kernel doesn't guarantee that keys are unique.
> 
> Can we present all the "unique keys" wording differently, say
> 
> 	exec_key_collision_likely()

I agree that this test's name is a bit out of place. I would rather have
it named "exec_changed_keys" though.

> Otherwise people might infer from this test code that the keys are
> supposed to be truly unique and start reporting bugs on the kernel.
> 
> I can't see an obvious security argument for unique keys (rather, the
> keys just need to be "unique enough".  That's the job of
> get_random_bytes().)

The "exec_unique_keys" test only checks that the keys changed after an
exec() which I think the name change would reflect.

The thing with the "single_thread_unique_keys" test is that the kernel
says the the keys will be random. Yes, there is no uniqueness guarantee
but I'm not sure how to phrase it differently. There is some minuscule
chance that the keys end up the same, but for this test I pretend this
will not happen. Would changing up the comments and the failure message
communicate this? Maybe substitute "unique" for "different" and say how
many keys clashed?

-- 
Regards,
Boyan