[PATCH v9] api: crypto: add ZUC 256 algorithm#2335
[PATCH v9] api: crypto: add ZUC 256 algorithm#2335nkaithakadan wants to merge 2 commits intoOpenDataPlane:masterfrom
Conversation
1d8b848 to
99c4cfb
Compare
|
@JannePeltonen Could you review these changes |
99c4cfb to
1cd1130
Compare
1cd1130 to
9606cc7
Compare
9606cc7 to
cf87d65
Compare
JannePeltonen
left a comment
There was a problem hiding this comment.
The new algorithms should be added to the sysinfo example app and to crypto validation tests (I do not mean actual test vectors, but inclusion of the algorithms in the combined test). You can add such commits in this PR, or if you prefer, I can make the changes in a different PR. Just let me know.
| * 256-NCA6 algorithm. | ||
| * | ||
| * Key length is 256 bits. IV size is 16 bytes. | ||
| * Integrity tag size is 16 bytes. |
There was a problem hiding this comment.
I do not think there is a need to fix the tag size to 16 bytes in the API here (you do not have it in NIA6 either) as the 3GPP spec seems to allow other tag sizes too. The tag size is a parameter in session creation and auth capabilities indicate the supported tag size (so an implementation could support only 16 bytes).
| * Key length is 256 bits. IV size is 16 bytes. | ||
| * Integrity tag size is 16 bytes. | ||
| */ | ||
| ODP_CIPHER_ALG_ZUC_NCA6, |
There was a problem hiding this comment.
ODP API handles AEAD algorithms in a bit cumbersome way. For each AEAD algorithm (see e.g. AES-GCM for an example), ODP API defines a cipher algorithm name (e.g. ODP_CIPHER_ALG_AES_GCM) and an auth algorithm name (e.g. ODP_AUTH_ALG_AES_GCM) and requires that they are used together. For instance, if a crypto session is configured with the ODP_CIPHER_ALG_AES_GCM cipher algorithm, then the auth algorithm must be ODP_AUTH_ALG_AES_GCM, and vice versa.
So, to support NCA6, you need to add the corresponding auth algorithm name ODP_AUTH_ALG_ZUC_NCA6 and the API text that requires the "cipher" and "auth" algorithms to be used together.
There was a problem hiding this comment.
It would also be ok to leave NCA6 out of this PR.
There was a problem hiding this comment.
NCA6 is moved out from this PR
| */ | ||
| ODP_CIPHER_ALG_ZUC_EEA3, | ||
|
|
||
| /** NEA6 confidentiality algorithm |
There was a problem hiding this comment.
The commit message could be improved.
I am not sure what the word 'align' in the subject line tires to convey. I think the subject line could be e.g.:
"api: crypto: add ZUC-256 NEA6, NIA6 and NCA6 algorithm"
The I think the commit message body could simply say something like this:
"Add 3GPP defined ZUC based confidentiality, integrity and AEAD algorithms with 256 bit key."
Now the body text is weird since the current API already has a variant of ZUC-256 with 16 byte IV and this commit just introduces another, subtly different variant. Also, I do not think there is anything improper in the current API as the commit message might imply.
Hi Janne, I’m currently on vacation. I’ll add the suggested changes by EOD, if possible. |
There is no hurry, as merging this will anyway have to wait until the next API release. |
7c88f6d to
902e39c
Compare
902e39c to
84ca30b
Compare
Add 3GPP defined ZUC based confidentiality and integrity algorithms with 256 bit key. Signed-off-by: Nithinsen Kaithakadan <nkaithakadan@marvell.com>
Print ZUC 256 based crypto algorithm capabilities. Signed-off-by: Nithinsen Kaithakadan <nkaithakadan@marvell.com>
84ca30b to
20c0719
Compare
|
Hi Janne, |
Add ZUC256 cipher and integrity algorithm.