linux-kernel - Re: [PATCH 2/2] crypto: ccree: enable support for hardware 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: <20180419033509.dbzetnt4bv7lj7cb@gondor.apana.org.au>
Date:   Thu, 19 Apr 2018 11:35:09 +0800
From:   Herbert Xu <herbert@...dor.apana.org.au>
To:     Gilad Ben-Yossef <gilad@...yossef.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Ofir Drang <ofir.drang@....com>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] crypto: ccree: enable support for hardware keys

On Mon, Apr 09, 2018 at 11:42:31AM +0300, Gilad Ben-Yossef wrote:
>
> Please look again. The stub version of cc_is_hw_key() doing that is being
> replaced in this patch.

The point is that the existing mechanism was unused before and this
is new code.  So you can't really point to the stubbed-out function
as a precedent.
 
> The s390 key and the cryptocell keys are not the same:
> 
> Their is, I believe, is an AES key encrypted by some internal key/algorithm.
> 
> The cryptocell "key" is a token, which is internally comprised of one
> or two indexes, referencing slots in the internal memory in the
> hardware, and a key size, that describe the size of the key.
> 
> I thought it would be confusing to use "paes" to describe both, since
> they are not interchangeable.
> You would not be able to feed an paes key that works with the s390
> version to cryptocell and vice verse and get it work.

Thanks for the info.

> Having said, if you prefer to have "paes" simply designate
> "implementation specific token for an AES key" I'm perfectly fine with
> that.

Well by definition none of these hardware keys will be compatible
with each other so I don't really see the point of using individual
algorithm names such as paes or haes.  This would make sense only if
they were somehow compatible with each other.

So instead of using algorithm names, you really want refer to the
specific driver name, which means that they can all use the same
algorithm name.

> > As to your patch specifically, there is one issue where you're
> > directly dereferencing the key as a struct.  This is a no-no because
> > the key may have come from user-space.  You must treat it as a
> > binary blob.  The s390 code seems to do this correctly.
> 
> As noted above, the haes "key" is really a token encoding 3 different
> pieces of information:

My point is that you should not just cast it but instead do a
copy to properly aligned kernel memory.

Cheers,
-- 
Email: Herbert Xu <herbert@...dor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt