RE: [PATCH 0/6] Add Hygon SEV support
> >
> > Uhm ... no, the fact that something is actually *useful* to
> potentially
> > a billion plus people doesn't mean anything ...
>
> Useful does not mean secure, does it? PKZIP encryption was certainly
> useful back in the day, but it was not secure.
>
"Secure" is a relative term anyway. There's always a trade-off between
performance, cost, power consumption and security. Different use cases
require different levels of security. IMHO that decision should be up
to the application / user / market, and not up to some software
engineers that are not experts on the subject matter anyway (but I am
hopeful that some people here are, in fact, experts to some extent).
> "Freedom" didn't apply when Speck was proposed for inclusion in Linux,
> and I would like to make sure I don't make a mistake when adding crypto
> interfaces. If SM2/3/4 were broken, I couldn't care less if someone
> HAS to use them, they can patch their kernel.
>
Is Speck actually used in any real-life protocol or application?
I did not follow the Speck discussion but I have a hunch that was a far
more important reason not to include it than it being a weak cipher or
it's shady NSA origins ...
And yes, they can always fork the kernel and do their own stuff with it,
but that's going to be a support nightmare for people - like us - wanting
to add HW acceleration on top of that. And yes, "we" can do SM3 & SM4.
Full disclosure: it is in my/our interest to keep SM3 & SM4 in the tree.
> But if they're not then I appreciate that you wrote to correct me,
> it's helpful. Please
> understand that 99% of the community has not ever heard of anything but
> SHA-{1,2,3}, ECDSA, Ed25519, AES. If somebody comes up with a patch
> with "strange" crypto, it's up to them to say that they are secure---
> and again, the key word is secure, not useful.
>
I recognise the fact that most people are not experts on the subject
matter. However, there's a lot you can find out in a short Google session
before you start a discussion on incorrect assumptions ...
Anyway, always happy to educate people a bit.
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Inside Secure
RE: [PATCH 2/3] crypto: inside-secure - Reduce stack usage
> -Original Message- > From: Arnd Bergmann > Sent: Monday, September 30, 2019 10:12 PM > To: Pascal Van Leeuwen > Cc: Antoine Tenart ; Herbert Xu > ; > David S. Miller ; Pascal van Leeuwen > ; Ard > Biesheuvel ; Eric Biggers ; > linux- > cry...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/3] crypto: inside-secure - Reduce stack usage > > On Mon, Sep 30, 2019 at 9:04 PM Pascal Van Leeuwen > wrote: > > > > Alternatively, it should be possible to shrink these allocations > > > as the extra buffers appear to be largely unnecessary, but doing > > > this would be a much more invasive change. > > > > > Actually, for HMAC-SHA512 you DO need all that buffer space. > > You could shrink it to 2 * ctx->state_sz but then your simple indexing > > is no longer going to fly. Not sure if that would be worth the effort. > > Stack allocations can no longer be dynamically sized in the kernel, > so that would not work. > I was actually referring to your kzalloc, not to the original stack based implementation ... > What I noticed though is that the largest part of safexcel_ahash_export_state > is used in the 'cache' member, and this is apparently only referenced inside > of > safexcel_hmac_init_iv, which you call twice. If that cache can be allocated > only once, you save SHA512_BLOCK_SIZE bytes in one of the two paths. > Well ... hmmm ... "my"... This is not originally "my" code. And until you brought this up, I did not fully realise it was using this export_state struct to store those digests. Seems to have something to do with directly taking the crypto_ahash_export state output, which is much more than that, in case you need to continue the hash (which you don't here). I guess you need to "catch" that output somewhere, so probably you could save a bit of memory but I doubt it would be a significant improvement. > > I don't like the part where you dynamically allocate the cryto_aes_ctx > > though, I think that was not necessary considering its a lot smaller. > > I count 484 bytes for it, which is really large. > Maybe I should've counted myself ... totally unexpectedly huge. Why?? Anyway, glad I got rid of it already then. > > And it conflicts with another change I have waiting that gets rid of > > aes_expandkey and that struct alltogether (since it was really just > > abused to do a key size check, which was very wasteful since the > > function actually generates all roundkeys we don't need at all ...) > > Right, this is what I noticed there. With 480 of the 484 bytes gone, > you are well below the warning limit even without the other change. > And by "other change" you mean the safexcel_ahash_export_state? Ok, good to known, although I do like to improve that one as well, but preferably by not exporting the cache so I don't need the full struct. Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
RE: [PATCH -next] crypto: inside-secure - Fix randbuild error
> -Original Message-
> From: Ard Biesheuvel
> Sent: Tuesday, October 8, 2019 9:35 AM
> To: Pascal Van Leeuwen
> Cc: YueHaibing ; herb...@gondor.apana.org.au;
> da...@davemloft.net;
> pascalv...@gmail.com; antoine.ten...@bootlin.com;
> linux-cry...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH -next] crypto: inside-secure - Fix randbuild error
>
> On Tue, 8 Oct 2019 at 09:32, Pascal Van Leeuwen
> wrote:
> >
> > > -Original Message-
> > > From: linux-crypto-ow...@vger.kernel.org
> > > On Behalf Of
> > > YueHaibing
> > > Sent: Tuesday, October 8, 2019 9:15 AM
> > > To: herb...@gondor.apana.org.au; da...@davemloft.net;
> > > pascalv...@gmail.com;
> > > antoine.ten...@bootlin.com
> > > Cc: linux-cry...@vger.kernel.org; linux-kernel@vger.kernel.org; YueHaibing
> > >
> > > Subject: [PATCH -next] crypto: inside-secure - Fix randbuild error
> > >
> > > If CRYPTO_DEV_SAFEXCEL is y but CRYPTO_SM3 is m,
> > > building fails:
> > >
> > > drivers/crypto/inside-secure/safexcel_hash.o: In function
> > > `safexcel_ahash_final':
> > > safexcel_hash.c:(.text+0xbc0): undefined reference to
> > > `sm3_zero_message_hash'
> > >
> > > Select CRYPTO_SM3 to fix this.
> > >
> > > Reported-by: Hulk Robot
> > > Fixes: 0f2bc13181ce ("crypto: inside-secure - Added support for basic SM3
> > > ahash")
> > > Signed-off-by: YueHaibing
> > > ---
> > > drivers/crypto/Kconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> > > index 3e51bae..5af17db 100644
> > > --- a/drivers/crypto/Kconfig
> > > +++ b/drivers/crypto/Kconfig
> > > @@ -751,6 +751,7 @@ config CRYPTO_DEV_SAFEXCEL
> > > select CRYPTO_SHA512
> > > select CRYPTO_CHACHA20POLY1305
> > > select CRYPTO_SHA3
> > > + select CRYPTO_SM3
> > > help
> > > This driver interfaces with the SafeXcel EIP-97 and EIP-197
> > > cryptographic
> > > engines designed by Inside Secure. It currently accelerates DES,
> > > 3DES and
> > > --
> > > 2.7.4
> > >
> > But ... I don't really want to build SM3 into the kernel for all Inside
> > Secure drivers, since in the majority of cases, the HW will not actually
> > support SM3 and I don't want to bloat the kernel image in that case.
> >
> > So maybe it's better to #ifdef out the failing part of the driver if
> > CONFIG_SM3 is not set?
> >
>
> Since you are only using the zero length message hash, can we just
> copy that into your driver instead?
If that is really the case - don't have time to look into that right now -
then I would be fine with that too. If no one objects, then I will make a
patch for that when I can find some time to do so (~early next week).
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
RE: [PATCH][next] crypto: inside-secure: fix spelling mistake "algorithmn" -> "algorithm"
> -Original Message- > From: linux-crypto-ow...@vger.kernel.org > On Behalf Of Colin > King > Sent: Tuesday, October 8, 2019 10:14 AM > To: Antoine Tenart ; Herbert Xu > ; David > S . Miller ; linux-cry...@vger.kernel.org > Cc: kernel-janit...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: [PATCH][next] crypto: inside-secure: fix spelling mistake > "algorithmn" -> "algorithm" > > From: Colin Ian King > > There is a spelling mistake in a dev_err message. Fix it. > > Signed-off-by: Colin Ian King > --- > drivers/crypto/inside-secure/safexcel_cipher.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c > b/drivers/crypto/inside- > secure/safexcel_cipher.c > index cecc56073337..8ccc9c59f376 100644 > --- a/drivers/crypto/inside-secure/safexcel_cipher.c > +++ b/drivers/crypto/inside-secure/safexcel_cipher.c > @@ -437,7 +437,7 @@ static int safexcel_aead_setkey(struct crypto_aead *ctfm, > const u8 *key, > goto badkey; > break; > default: > - dev_err(priv->dev, "aead: unsupported hash algorithmn"); > + dev_err(priv->dev, "aead: unsupported hash algorithm"); > goto badkey; > } > > -- > 2.20.1 Actually, the typing error is well spotted, but the fix is not correct. What actually happened here is that a \ got accidentally deleted, there should have been a "\n" at the end of the line ... Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
RE: [PATCH] crypto: inside-secure - select CONFIG_CRYPTO_SM3
> -Original Message- > From: Arnd Bergmann > Sent: Tuesday, October 22, 2019 6:17 PM > To: Pascal Van Leeuwen > Cc: Herbert Xu ; David S. Miller > ; Antoine > Tenart ; Ard Biesheuvel > ; Pascal van > Leeuwen ; linux-cry...@vger.kernel.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH] crypto: inside-secure - select CONFIG_CRYPTO_SM3 > > On Tue, Oct 22, 2019 at 5:42 PM Pascal Van Leeuwen > wrote: > > > > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig > > > index 357e230769c8..1ca8d9a15f2a 100644 > > > --- a/drivers/crypto/Kconfig > > > +++ b/drivers/crypto/Kconfig > > > @@ -753,6 +753,7 @@ config CRYPTO_DEV_SAFEXCEL > > > select CRYPTO_SHA512 > > > select CRYPTO_CHACHA20POLY1305 > > > select CRYPTO_SHA3 > > > + select CRYPTO_SM3 > > > > > Was this problem observed with the latest state of the Cryptodev GIT? > > Because I already attempted to fix this issue with commit 99a59da3723b9725 > > Can you please double check if you still get the compile error with that > > commit included? > > (I can't tell from this mail which version of the sources you are using) > > I was testing on linux-5.4-rc3 plus some of my own patches, so your fix > was not included. > > With your patch applied, mine is no longer needed. > > Arnd Thanks for confirming that :-) Regards, Pascal van Leeuwen Silicon IP Architect, Multi-Protocol Engines @ Verimatrix www.insidesecure.com
