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

On Thu, Sep 03, 2020 at 11:48:37AM +0100, Boyan Karatotev wrote:
> 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?

Yes, something like that seems reasonable.

Cheers
---Dave