[PATCH API-NEXT v5] Major cryptography code update

[lumag]

• incorporate [PATCH API-NEXT v4] Update Linux crypto implementation to support recent API additions #24
• fix AES-GCM thread safety
• unify cipher handling code
• make crypto code work across segmented packets

[muvarov]

From: Dmitry Eremin-Solenikov 
 On 31.05.2017 14:33, Savolainen, Petri (Nokia - FI/Espoo) wrote:
> 
> 
>> -----Original Message-----
>> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of
>> Github ODP bot
>> Sent: Tuesday, May 23, 2017 1:00 AM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [PATCH API-NEXT v2 2/20] test: crypto: explicitly pass
>> AAD to crypto subsystem
>>
>> From: Dmitry Eremin-Solenikov 
>>
>> Signed-off-by: Dmitry Eremin-Solenikov 
>> ---
>> /** Email created from pull request 34 (lumag:crypto-update-main-new)
>>  ** https://github.com/Linaro/odp/pull/34
>>  ** Patch: https://github.com/Linaro/odp/pull/34.patch
>>  ** Base sha: 826ee894aa0ebd09d42a17e1de077c46bc5b366a
>>  ** Merge commit sha: 7c49c61063e2d57f049a5436cf12a3c36710bb34
>>  **/
>>  .../validation/api/crypto/odp_crypto_test_inp.c    | 37
>> ++++++++++++++++++++++
>>  1 file changed, 37 insertions(+)
>>
>> diff --git a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
>> b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
>> index 3d09e374..3f57a907 100644
>> --- a/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
>> +++ b/test/common_plat/validation/api/crypto/odp_crypto_test_inp.c
>> @@ -74,6 +74,8 @@ static void alg_test(odp_crypto_op_t op,
>>  		     odp_crypto_key_t auth_key,
>>  		     odp_packet_data_range_t *cipher_range,
>>  		     odp_packet_data_range_t *auth_range,
>> +		     uint8_t *aad,
>> +		     uint32_t aad_len,
>>  		     const uint8_t *plaintext,
>>  		     unsigned int plaintext_len,
>>  		     const uint8_t *ciphertext,
>> @@ -240,6 +242,9 @@ static void alg_test(odp_crypto_op_t op,
>>  	if (op_iv_ptr)
>>  		op_params.override_iv_ptr = op_iv_ptr;
>>
>> +	op_params.aad.ptr = aad;
>> +	op_params.aad.length = aad_len;
>> +
>>  	op_params.hash_result_offset = plaintext_len;
>>  	if (0 != digest_len) {
>>  		memcpy(data_addr + op_params.hash_result_offset,
>> @@ -472,6 +477,7 @@ void crypto_test_enc_alg_3des_cbc(void)
>>  			 ODP_AUTH_ALG_NULL,
>>  			 auth_key,
>>  			 NULL, NULL,
>> +			 NULL, 0,
>>  			 tdes_cbc_reference_plaintext[i],
>>  			 tdes_cbc_reference_length[i],
>>  			 tdes_cbc_reference_ciphertext[i],
>> @@ -508,6 +514,7 @@ void crypto_test_enc_alg_3des_cbc_ovr_iv(void)
>>  			 ODP_AUTH_ALG_NULL,
>>  			 auth_key,
>>  			 NULL, NULL,
>> +			 NULL, 0,
>>  			 tdes_cbc_reference_plaintext[i],
>>  			 tdes_cbc_reference_length[i],
>>  			 tdes_cbc_reference_ciphertext[i],
>> @@ -548,6 +555,7 @@ void crypto_test_dec_alg_3des_cbc(void)
>>  			 ODP_AUTH_ALG_NULL,
>>  			 auth_key,
>>  			 NULL, NULL,
>> +			 NULL, 0,
>>  			 tdes_cbc_reference_ciphertext[i],
>>  			 tdes_cbc_reference_length[i],
>>  			 tdes_cbc_reference_plaintext[i],
>> @@ -586,6 +594,7 @@ void crypto_test_dec_alg_3des_cbc_ovr_iv(void)
>>  			 ODP_AUTH_ALG_NULL,
>>  			 auth_key,
>>  			 NULL, NULL,
>> +			 NULL, 0,
>>  			 tdes_cbc_reference_ciphertext[i],
>>  			 tdes_cbc_reference_length[i],
>>  			 tdes_cbc_reference_plaintext[i],
>> @@ -636,6 +645,9 @@ void crypto_test_enc_alg_aes128_gcm(void)
>>  			 &aes128_gcm_cipher_range[i],
>>  			 &aes128_gcm_auth_range[i],
> 
> Shouldn't you insert the new parameters in her? Between auth_range and plaintext.
> 
>>  			 aes128_gcm_reference_plaintext[i],
>> +			 aes128_gcm_cipher_range[i].offset -
>> +			 aes128_gcm_auth_range[i].offset,
> 
> Shouldn't this first added param be aad pointer ?

In fact I have reused original test vectors, which come in form of
packet ::= AAD DATA

with auth_range pointing to AAD start and cipher_range pointing to data.
If you think this is counterintuitive, I can split AAD away.

Probably I should do that, as the question arose.

> 
>> +			 aes128_gcm_reference_plaintext[i],
> 
> Shouldn't this second added param be aad length ?
> 
> 
> The same issues are copy-pasted to all aes128 gcm cases.

-- 
With best wishes
Dmitry
 

[codecov-io]

Codecov Report

Merging #34 into api-next will decrease coverage by 0.104%.
The diff coverage is 86.046%.

@@              Coverage Diff               @@
##           api-next       #34       +/-   ##
==============================================
- Coverage    75.979%   75.875%   -0.105%     
==============================================
  Files           171       170        -1     
  Lines         29192     29165       -27     
==============================================
- Hits          22180     22129       -51     
- Misses         7012      7036       +24

Impacted Files	Coverage Δ
test/common_plat/performance/odp_crypto.c	78.853% <63.636%> (+0.01%)	⬆️
...n_plat/validation/api/crypto/odp_crypto_test_inp.c	85.21% <90.654%> (+0.504%)	⬆️
platform/linux-generic/pktio/ipc.c	61.341% <0%> (-15.655%)	⬇️
test/linux-generic/pktio_ipc/ipc_common.c	67.213% <0%> (-8.197%)	⬇️
test/common_plat/validation/api/timer/timer.c	79.124% <0%> (-2.357%)	⬇️
test/linux-generic/pktio_ipc/pktio_ipc1.c	69.273% <0%> (-1.676%)	⬇️
platform/linux-generic/_ishm.c	73.211% <0%> (-0.305%)	⬇️
test/linux-generic/pktio_ipc/pktio_ipc2.c
... and 3 more

[lumag]

Merged