Conversation
|
I guess you need to make some of the TLS tests run conditionally? |
|
I think at least some of the failures are due to some ciphers not being built into openssl. I added a check for cipher availability, let's see how the tests look. |
simo5
left a comment
simo5
left a comment
There was a problem hiding this comment.
Mostly just code organization issues, I will take another pass once the two ciphers are better separated
ilie-halip-nxp
force-pushed
the
feature/chacha20_poly1305
branch
from
e801e38 to
5b70dfe
Compare
simo5
left a comment
simo5
left a comment
There was a problem hiding this comment.
Thanks for the cosmetic changes, I have just one more, if you can squash and rebase as you apply it I can then review and merge, I do not see anything else out of place.
ilie-halip-nxp
force-pushed
the
feature/chacha20_poly1305
branch
from
5b70dfe to
00568d3
Compare
ilie-halip-nxp
force-pushed
the
feature/chacha20_poly1305
branch
2 times, most recently
from
e45af82 to
13d3d32
Compare
simo5
left a comment
simo5
left a comment
There was a problem hiding this comment.
Just a couple of minor nits, but this looks good now and is basically ready to merge
src/cipher.c
Outdated
| return CKR_ARGUMENTS_BAD; | ||
| } | ||
|
|
||
| if (!ivlen && ivlen > EVP_MAX_IV_LENGTH) { |
There was a problem hiding this comment.
This is hard to parse, is this trying to ensure (ivlen != 0) ? if that is the case please explicitly write it that way, we use !var basically only for pointers.
There was a problem hiding this comment.
Note that Covscan also mark this as dead code because I this translates to:
if (ivlen == 0 && ivlen > EVP_MAX_IV_LENGHT)
which is an impossible condition.
src/cipher.c
Outdated
| CK_SALSA20_CHACHA20_POLY1305_MSG_PARAMS_PTR chacha = | ||
| (CK_SALSA20_CHACHA20_POLY1305_MSG_PARAMS_PTR)mech->pParameter; | ||
|
|
||
| if (iv && ivlen) { |
Similarly to AES-GCM, add CHACHA20_POLY1305 AEAD support. It requrires a 96-bit nonce, as that selects the CHACHA20 algorithm variant instead of the original SALSA20. The tag length is hardcoded to 16, which is the minimum required by the PKCS#11 spec. This includes a bit of refactoring to make the code for GCM & POLY1305 simpler. Signed-off-by: Ilie Halip <ilie.halip@nxp.com>
Add CHACHA20_POLY1305 to the list for AEAD algorithms that can be tested. Since this algorithms only supports a 96-bit nonce, hardcode the IV instead of trying multiple IV lengths, also for GCM. Signed-off-by: Ilie Halip <ilie.halip@nxp.com>
To make sure multiple ciphersuites are exercised in the tests (not just the ones that are negociated by the server+client), add more cases for explicit ciphersuites. Signed-off-by: Ilie Halip <ilie.halip@nxp.com>
SKEY_SUPPORT is not defined for the test sources, use an OpenSSL macro instead. Moreover, tokens may support only a subset of AEAD algorithms, so skip algorithms that are not available. Signed-off-by: Ilie Halip <ilie.halip@nxp.com>
ilie-halip-nxp
force-pushed
the
feature/chacha20_poly1305
branch
from
13d3d32 to
3c6fc6e
Compare
2 participants
Description
Add support for
CHACHA20_POLY1305AEAD mechanism. This piggybacks on the current AES-GCM code because in most cases they behave the same.The IV length is hardcoded to 12, and the tag length to 16. Both these values are specified in PKCS#11.
Checklist
Reviewer's checklist: