-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for AES-ECB to the PSA Crypto implementation #3480
Changes from 2 commits
ed3c9ec
2a48b53
a6033e9
ffecb7b
177deba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Features | ||
* Added support for AES-ECB to the PSA Crypto cipher API. | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -980,6 +980,15 @@ | |
*/ | ||
#define PSA_ALG_XTS ((psa_algorithm_t)0x044000ff) | ||
|
||
/** The Electronic Code Book (ECB) mode of a block cipher, with no padding. | ||
* | ||
* The underlying block cipher is determined by the key type. | ||
* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please include the warning against use as well. |
||
* This symmetric cipher mode can only be used with messages whose lengths | ||
* are whole number of blocks for the chosen block cipher. | ||
hanno-becker marked this conversation as resolved.
Show resolved
Hide resolved
hanno-becker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
#define PSA_ALG_ECB_NO_PADDING ((psa_algorithm_t)0x04404400) | ||
|
||
/** The CBC block cipher chaining mode, with no padding. | ||
* | ||
* The underlying block cipher is determined by the key type. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -346,7 +346,11 @@ PSA import RSA public key: maximum size exceeded | |
depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_RSA_C | ||
import_rsa_made_up:PSA_VENDOR_RSA_MAX_KEY_BITS+8:0:PSA_ERROR_NOT_SUPPORTED | ||
|
||
PSA key policy: AES | ||
PSA key policy: AES ECB | ||
depends_on:MBEDTLS_AES_C | ||
check_key_policy:PSA_KEY_TYPE_AES:128:PSA_KEY_USAGE_ENCRYPT:PSA_ALG_ECB_NO_PADDING | ||
|
||
PSA key policy: AES CBC | ||
depends_on:MBEDTLS_AES_C | ||
check_key_policy:PSA_KEY_TYPE_AES:128:PSA_KEY_USAGE_ENCRYPT:PSA_ALG_CBC_NO_PADDING | ||
|
||
|
@@ -1122,6 +1126,10 @@ PSA cipher: bad order function calls | |
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_bad_order: | ||
|
||
PSA symmetric encrypt: AES-ECB, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C | ||
cipher_encrypt:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"6bc1bee22e409f96e93d7e117393172a":"3ad77bb40d7a3660a89ecaf32466ef97":PSA_SUCCESS | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a few more test cases (for both encrypt and decrypt): 0 bytes, 32 bytes, a non-multiple of 16. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add at least a 1-block 3DES test case. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3DES isn't in scope for this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR makes Mbed TLS recognize |
||
PSA symmetric encrypt: AES-CBC-nopad, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_encrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee22e409f96e93d7e117393172a":"a076ec9dfbe47d52afc357336f20743b":PSA_SUCCESS | ||
|
@@ -1158,6 +1166,10 @@ PSA symmetric encrypt: 3-key 3DES-CBC-nopad, 8 bytes, good | |
depends_on:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_encrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"2a2a2a2a2a2a2a2a":"eda4011239bc3ac9":"817ca7d69b80d86a":PSA_SUCCESS | ||
|
||
PSA symmetric decrypt: AES-ECB, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C | ||
cipher_decrypt:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"396ee84fb75fdbb5c2b13c7fe5a654aa":"63cecc46a382414d5fa7d2b79387437f":PSA_SUCCESS | ||
|
||
PSA symmetric decrypt: AES-CBC-nopad, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_decrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"396ee84fb75fdbb5c2b13c7fe5a654aa":"49e4e66c89a86b67758df89db9ad6955":PSA_SUCCESS | ||
|
@@ -1194,6 +1206,10 @@ PSA symmetric decrypt: 3-key 3DES-CBC-nopad, 8 bytes, good | |
depends_on:MBEDTLS_DES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_decrypt:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_DES:"01020407080b0d0ec1c2c4c7c8cbcdce31323437383b3d3e":"2a2a2a2a2a2a2a2a":"817ca7d69b80d86a":"eda4011239bc3ac9":PSA_SUCCESS | ||
|
||
PSA symmetric encrypt/decrypt: AES-ECB, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C | ||
cipher_verify_output:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a" | ||
|
||
PSA symmetric encrypt/decrypt: AES-CBC-nopad, 16 bytes, good | ||
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_verify_output:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a" | ||
|
@@ -1210,6 +1226,10 @@ PSA symmetric encrypt/decrypt: AES-CTR | |
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR | ||
cipher_verify_output:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"6bc1bee22e409f96e93d7e117393172a" | ||
|
||
PSA symmetric encryption multipart: AES-ECB, 16+16 bytes | ||
depends_on:MBEDTLS_AES_C | ||
cipher_encrypt_multipart:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"6bc1bee22e409f96e93d7e117393172a5434f378a597bcef1389318c7fc865ef":16:16:16:"3ad77bb40d7a3660a89ecaf32466ef9755ed5e9e066820fa52c729886d18854c" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a test case where the input isn't split on a block boundary (e.g. 1+15). And please add a bad test case where the total input isn't a whole number of blocks. All for both encrypt and decrypt. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There weren't any negative test cases being done with _multipart tests. Is it really up to me to introduce that here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A prior test gap is not a justification for expanding the test gap in general. But ok, let's defer negative testing for multipart operations since it's a preexisting lack of test code. Do please add test with input that isn't split on a block boundary, which was already done for CBC. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure (push incoming for the non-block-multiple operations). |
||
PSA symmetric encryption multipart: AES-CBC-nopad, 7+9 bytes | ||
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_encrypt_multipart:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee22e409f96e93d7e117393172a":7:0:16:"a076ec9dfbe47d52afc357336f20743b" | ||
|
@@ -1274,6 +1294,10 @@ PSA symmetric encryption multipart: AES-CTR, 16+0 bytes | |
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CTR | ||
cipher_encrypt_multipart:PSA_ALG_CTR:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"6bc1bee22e409f96e93d7e117393172a":16:16:0:"8f9408fe80a81d3e813da3c7b0b2bd32" | ||
|
||
PSA symmetric decryption multipart: AES-ECB, 16+16 bytes | ||
depends_on:MBEDTLS_AES_C | ||
cipher_decrypt_multipart:PSA_ALG_ECB_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"":"3ad77bb40d7a3660a89ecaf32466ef9755ed5e9e066820fa52c729886d18854c":16:16:16:"6bc1bee22e409f96e93d7e117393172a5434f378a597bcef1389318c7fc865ef" | ||
|
||
PSA symmetric decryption multipart: AES-CBC-nopad, 7+9 bytes | ||
depends_on:MBEDTLS_AES_C:MBEDTLS_CIPHER_MODE_CBC | ||
cipher_decrypt_multipart:PSA_ALG_CBC_NO_PADDING:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":"2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a":"a076ec9dfbe47d52afc357336f20743b":7:0:16:"6bc1bee22e409f96e93d7e117393172a" | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3347,7 +3347,11 @@ void cipher_encrypt( int alg_arg, int key_type_arg, | |||||||||||
PSA_ASSERT( psa_cipher_encrypt_setup( &operation, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
if( iv->len > 0 ) | ||||||||||||
{ | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
hanno-becker marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
} | ||||||||||||
|
||||||||||||
output_buffer_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
ASSERT_ALLOC( output, output_buffer_size ); | ||||||||||||
|
@@ -3410,7 +3414,11 @@ void cipher_encrypt_multipart( int alg_arg, int key_type_arg, | |||||||||||
PSA_ASSERT( psa_cipher_encrypt_setup( &operation, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
if( iv->len > 0 ) | ||||||||||||
{ | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
} | ||||||||||||
|
||||||||||||
output_buffer_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
ASSERT_ALLOC( output, output_buffer_size ); | ||||||||||||
|
@@ -3479,7 +3487,9 @@ void cipher_decrypt_multipart( int alg_arg, int key_type_arg, | |||||||||||
PSA_ASSERT( psa_cipher_decrypt_setup( &operation, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
if( iv->len > 0 ) { | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: The brace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
} | ||||||||||||
|
||||||||||||
output_buffer_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
|
@@ -3546,7 +3556,9 @@ void cipher_decrypt( int alg_arg, int key_type_arg, | |||||||||||
PSA_ASSERT( psa_cipher_decrypt_setup( &operation, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
if( iv->len > 0 ) { | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation, iv->x, iv->len ) ); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
} | ||||||||||||
|
||||||||||||
output_buffer_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
|
@@ -3613,9 +3625,11 @@ void cipher_verify_output( int alg_arg, int key_type_arg, | |||||||||||
PSA_ASSERT( psa_cipher_decrypt_setup( &operation2, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_generate_iv( &operation1, | ||||||||||||
iv, iv_size, | ||||||||||||
&iv_length ) ); | ||||||||||||
if( alg != PSA_ALG_ECB_NO_PADDING ) { | ||||||||||||
PSA_ASSERT( psa_cipher_generate_iv( &operation1, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
iv, iv_size, | ||||||||||||
&iv_length ) ); | ||||||||||||
} | ||||||||||||
output1_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
ASSERT_ALLOC( output1, output1_size ); | ||||||||||||
|
@@ -3635,8 +3649,11 @@ void cipher_verify_output( int alg_arg, int key_type_arg, | |||||||||||
output2_size = output1_length; | ||||||||||||
ASSERT_ALLOC( output2, output2_size ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation2, | ||||||||||||
iv, iv_length ) ); | ||||||||||||
if( iv_length > 0 ) { | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation2, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
iv, iv_length ) ); | ||||||||||||
} | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_update( &operation2, output1, output1_length, | ||||||||||||
output2, output2_size, | ||||||||||||
&output2_length ) ); | ||||||||||||
|
@@ -3698,9 +3715,12 @@ void cipher_verify_output_multipart( int alg_arg, | |||||||||||
PSA_ASSERT( psa_cipher_decrypt_setup( &operation2, | ||||||||||||
handle, alg ) ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_generate_iv( &operation1, | ||||||||||||
iv, iv_size, | ||||||||||||
&iv_length ) ); | ||||||||||||
if( alg != PSA_ALG_ECB_NO_PADDING ) { | ||||||||||||
PSA_ASSERT( psa_cipher_generate_iv( &operation1, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
iv, iv_size, | ||||||||||||
&iv_length ) ); | ||||||||||||
} | ||||||||||||
|
||||||||||||
output1_buffer_size = ( (size_t) input->len + | ||||||||||||
PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ) ); | ||||||||||||
ASSERT_ALLOC( output1, output1_buffer_size ); | ||||||||||||
|
@@ -3730,8 +3750,10 @@ void cipher_verify_output_multipart( int alg_arg, | |||||||||||
output2_buffer_size = output1_length; | ||||||||||||
ASSERT_ALLOC( output2, output2_buffer_size ); | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation2, | ||||||||||||
iv, iv_length ) ); | ||||||||||||
if( iv_length > 0 ) { | ||||||||||||
PSA_ASSERT( psa_cipher_set_iv( &operation2, | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now, sorry for the oversight. |
||||||||||||
iv, iv_length ) ); | ||||||||||||
} | ||||||||||||
|
||||||||||||
PSA_ASSERT( psa_cipher_update( &operation2, output1, first_part_size, | ||||||||||||
output2, output2_buffer_size, | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it should be support for ECB. Not specifically AES-ECB, which isn't a specific thing in the API.