[PATCH] crypto: qat - remove redundant arbiter configuration
The default arbiter configuration for ring weights and response ordering is exactly what we want so we don't need to configure anything more. This will also fix the problem where number of bundles is different between different devices. Reported-by: Ahsan Atta Signed-off-by: Tadeusz Struk --- drivers/crypto/qat/qat_common/adf_hw_arbiter.c | 19 --- 1 file changed, 19 deletions(-) diff --git a/drivers/crypto/qat/qat_common/adf_hw_arbiter.c b/drivers/crypto/qat/qat_common/adf_hw_arbiter.c index f267d9e..d7dd18d 100644 --- a/drivers/crypto/qat/qat_common/adf_hw_arbiter.c +++ b/drivers/crypto/qat/qat_common/adf_hw_arbiter.c @@ -49,7 +49,6 @@ #include "adf_transport_internal.h" #define ADF_ARB_NUM 4 -#define ADF_ARB_REQ_RING_NUM 8 #define ADF_ARB_REG_SIZE 0x4 #define ADF_ARB_WTR_SIZE 0x20 #define ADF_ARB_OFFSET 0x3 @@ -64,15 +63,6 @@ ADF_CSR_WR(csr_addr, ADF_ARB_RINGSRVARBEN_OFFSET + \ (ADF_ARB_REG_SLOT * index), value) -#define WRITE_CSR_ARB_RESPORDERING(csr_addr, index, value) \ - ADF_CSR_WR(csr_addr, (ADF_ARB_OFFSET + \ - ADF_ARB_RO_EN_OFFSET) + (ADF_ARB_REG_SIZE * index), value) - -#define WRITE_CSR_ARB_WEIGHT(csr_addr, arb, index, value) \ - ADF_CSR_WR(csr_addr, (ADF_ARB_OFFSET + \ - ADF_ARB_WTR_OFFSET) + (ADF_ARB_WTR_SIZE * arb) + \ - (ADF_ARB_REG_SIZE * index), value) - #define WRITE_CSR_ARB_SARCONFIG(csr_addr, index, value) \ ADF_CSR_WR(csr_addr, ADF_ARB_OFFSET + \ (ADF_ARB_REG_SIZE * index), value) @@ -99,15 +89,6 @@ int adf_init_arb(struct adf_accel_dev *accel_dev) for (arb = 0; arb < ADF_ARB_NUM; arb++) WRITE_CSR_ARB_SARCONFIG(csr, arb, arb_cfg); - /* Setup service weighting */ - for (arb = 0; arb < ADF_ARB_NUM; arb++) - for (i = 0; i < ADF_ARB_REQ_RING_NUM; i++) - WRITE_CSR_ARB_WEIGHT(csr, arb, i, 0x); - - /* Setup ring response ordering */ - for (i = 0; i < ADF_ARB_REQ_RING_NUM; i++) - WRITE_CSR_ARB_RESPORDERING(csr, i, 0x); - /* Setup worker queue registers */ for (i = 0; i < hw_data->num_engines; i++) WRITE_CSR_ARB_WQCFG(csr, i, i); -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: implement DH primitives under akcipher API
Hi Tadeusz, >>> In this way we can define a generic user side of the key exchange interface, and on the the driver side of the akcipher, the implementations would overload the existing akcipher encrypt(), decrypt(), set_pub_key(), set_priv_key() methods and do what should be done for a given algorithm. We just need to agree on the format of the input parameters to these operations. >> I think trying to heavily map this to encrypt and decrypt is a bad idea. >> These callbacks are not really designed to return what you need them to >> return. Key exchange is different. >> >> So why are we trying to push this into akcipher in the first place? I mean >> lets just create a keyexch (or similar name) and design it explicitly for >> key exchange handling. We are also not trying to map hashes into skcipher. I >> think the clear distinction of semantics calls for a different API. >> > > I just think that before introducing new API we should try to reuse what's > already > there. Looking at RSA and DH, they basically perform x = a^b mod p, so we > should > have what's needed for both and we just could overload the methods already > defined. > I'm fine either way. Herbert, what's your preference? I do not think that it is a valid argument because the operations are the same to use akcipher. While operations are similar, the inputs are totally different. And it is really not performing encrypt or decrypt operations here. You use different inputs and get different outputs. You are not encrypting or decrypting anything. Especially key generation via decrypt is something I can not wrap my mind around. >>> Agree, we need to support all the cases, but dealing with different type of >>> keys needs to happen above this API. This API should solely be an interface to math primitives of certain operations, which can be implemented in SW or can be HW accelerated. Modules like public_key or afalg_akcipher need to be smart enough and know what key types they deal with and do the right thing depending on that knowledge. >> I am not sure this can be that easily generalized. Just pushing the problem >> of key types and formats off to userspace just forces complexity and extra >> knowledge there. We need to be really carefully here. And I really dislike >> introducing custom ASN.1 structures that are not backed by an actual RFC >> document. > > I'm not saying that we should push this to user space. I'm just saying this > should > be handled above the crypto API. > >>> Agree, and to make this work with afalg_akcipher, I have proposed new ALG_SET_KEY_ID and ALG_SET_KEY_ID operations. See here: https://lkml.org/lkml/2015/12/26/65 The plan is that in this case the afalg_akcipher will not use akcipher api at all. In this case the algif_akcipher will use the request_key() interface to retrieve the key type info and will use the operations that the new TPM interface will define. >> The ALG_SET_KEY_ID is great for skcipher where you key is a fixed size blob. >> There is works really well. And you will need if we let keyctl derive the >> session key from a set of asymmetric keys. >> >> I get that we want to enable OpenSSL and others to gain access to kernel >> crypto operations. We want the same actually for ELL as well. However what >> is dangerous here is that AF_ALG can never support hardware based keys. So >> that means support in OpenSSL is broken by design. See earlier emails from >> David Woodhouse. If you have your certificates / keys in hardware then you >> need to be able to make them work with OpenSSL. We can not ignore this. It >> is a design we need to consider from the beginning. > > If we will have the TPM interface that allows to invoke functions for HW keys > and > the *_KEY_ID, then I don't see why AF_ALG would not work. As I said, in case > when > the keys will be in HW the algif_akcipher will not use akcipher api, but use > the > TPM defined functions in the same way as the keyctl would use it. > So I think both AF_ALG and keyctl should work just fine. When David and I talked to Herbert at the kernel summit in Seoul, he said that the crypto subsystem (and with that AF_ALG) always needs to support software fallback crypto. In case of hardware based keys, there is no software fallback possible. If you have a hardware key, then it always needs to use the hardware crypto that comes with the key. And that is precisely my point in going forward with keyctl for all of these. We need _one_ interface that can handle software and hardware keys / key pairs. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] crypto: authenc - add TLS type encryption
Hi Cristian, On 03/08/2016 12:20 AM, Cristian Stoica wrote: > There is also a follow-up in the next paragraph: > > "That pretty much sums up the new attack: the side-channel defenses that were > hoped to be sufficient were found not to be (again). So the answer, this time > I believe, is to make the processing rigorously constant-time." > > The author makes new changes and continues instrumenting the code and still > finds 20 CPU cycles (out of 18000) difference between medians for different > paddings. This small difference was detected also on a timing side-channel - > which is the point I'm making. > > SSL/TLS is prone to this implementation issue and many user-space libraries > got this wrong. It would be good to see some numbers to back-up the claim of > timing differences as not being an issue for this one. It is hard to get the implementation right when the protocol design is error prone. Later we should run some tests on it and see how relevant will this be for a remote timing attack. Thanks, -- TS -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] crypto: rsa-pkcs1pad: indent a couple statements
These if statements aren't indented far enough and it makes static checkers complain. Signed-off-by: Dan Carpenter --- Hopefully GCC6 will start complaining about these as well? diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c index 1cea67d..1eb5cbe 100644 --- a/crypto/rsa-pkcs1pad.c +++ b/crypto/rsa-pkcs1pad.c @@ -705,7 +705,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb) CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s)", rsa_alg->base.cra_driver_name) >= CRYPTO_MAX_ALG_NAME) - goto out_drop_alg; + goto out_drop_alg; } else { if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)", @@ -715,7 +715,7 @@ static int pkcs1pad_create(struct crypto_template *tmpl, struct rtattr **tb) CRYPTO_MAX_ALG_NAME, "pkcs1pad(%s,%s)", rsa_alg->base.cra_driver_name, hash_name) >= CRYPTO_MAX_ALG_NAME) - goto out_free_hash; + goto out_free_hash; } inst->alg.base.cra_flags = rsa_alg->base.cra_flags & CRYPTO_ALG_ASYNC; -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] crypto: authenc - add TLS type encryption
Hi Tadeusz, There is also a follow-up in the next paragraph: "That pretty much sums up the new attack: the side-channel defenses that were hoped to be sufficient were found not to be (again). So the answer, this time I believe, is to make the processing rigorously constant-time." The author makes new changes and continues instrumenting the code and still finds 20 CPU cycles (out of 18000) difference between medians for different paddings. This small difference was detected also on a timing side-channel - which is the point I'm making. SSL/TLS is prone to this implementation issue and many user-space libraries got this wrong. It would be good to see some numbers to back-up the claim of timing differences as not being an issue for this one. Cristian S. From: Tadeusz Struk Sent: Monday, March 7, 2016 4:31 PM To: Cristian Stoica; herb...@gondor.apana.org.au Cc: linux-crypto@vger.kernel.org; linux-ker...@vger.kernel.org; da...@davemloft.net Subject: Re: [PATCH 1/3] crypto: authenc - add TLS type encryption Hi Cristian, On 03/07/2016 01:05 AM, Cristian Stoica wrote: > Hi Tadeusz, > > > +static int crypto_encauth_dgst_verify(struct aead_request *req, > + unsigned int flags) > +{ > + struct crypto_aead *tfm = crypto_aead_reqtfm(req); > + unsigned int authsize = crypto_aead_authsize(tfm); > + struct aead_instance *inst = aead_alg_instance(tfm); > + struct crypto_encauth_ctx *ctx = crypto_aead_ctx(tfm); > + struct encauth_instance_ctx *ictx = aead_instance_ctx(inst); > + struct crypto_ahash *auth = ctx->auth; > + struct encauth_request_ctx *areq_ctx = aead_request_ctx(req); > + struct ahash_request *ahreq = (void *)(areq_ctx->tail + ictx->reqoff); > + u8 *hash = areq_ctx->tail; > + int i, err = 0, padd_err = 0; > + u8 paddlen, *ihash; > + u8 padd[255]; > + > + scatterwalk_map_and_copy(&paddlen, req->dst, req->assoclen + > +req->cryptlen - 1, 1, 0); > + > + if (paddlen > 255 || paddlen > req->cryptlen) { > + paddlen = 1; > + padd_err = -EBADMSG; > + } > + > + scatterwalk_map_and_copy(padd, req->dst, req->assoclen + > +req->cryptlen - paddlen, paddlen, 0); > + > + for (i = 0; i < paddlen; i++) { > + if (padd[i] != paddlen) > + padd_err = -EBADMSG; > + } > > > This part seems to have the same issue my TLS patch has. > See for reference what Andy Lutomirski had to say about it: > > http://www.mail-archive.com/linux-crypto%40vger.kernel.org/msg11719.html Thanks for reviewing and for pointing this out. I was aware of the timing-side issues and done everything I could to avoid it. The main issue that allowed the Lucky Thirteen attack was that the digest wasn't performed at all if the padding verification failed. This is not an issue here. The other issue, which is caused by the length of data to digest being dependent on the padding length is inevitable and there is nothing we can do about it. As the note in the paper says: "However, our behavior matches OpenSSL, so we leak only as much as they do." Thanks, -- TS-- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html