Add CCM cipher suite variants (2.) #147
Conversation
boaks
force-pushed
the
add_ccm
branch
2 times, most recently
from
304fa0f
to
94dbbb4
Compare
| @@ -105,7 +111,7 @@ typedef struct { | |||
| typedef struct { | |||
| dtls_compression_t compression; /**< compression method */ | |||
|
|
|||
| dtls_cipher_t cipher; /**< cipher type */ | |||
| dtls_cipher_index_t cipher_index; /**< internal index for cipher_suite_params, DTLS_CIPHER_INDEX_NULL for TLS_NULL_WITH_NULL_NULL */ | |||
There was a problem hiding this comment.
Line length is > 80 chars, breaking coding style. This occurs in multiple places
There was a problem hiding this comment.
Should we break / split line for macros as well?
There was a problem hiding this comment.
That would be my preference. @obgm ?
There was a problem hiding this comment.
That requires a lot of concentration to only fix the lines, which are altered with this PR.
Otherwise we get a couple of "format only changes", which I would prefer to have them done "once applying a formatter" (issue #143).
We will see ...
There was a problem hiding this comment.
OK, if I continue to try to fix the format only of the changed lines, that will eat up hour and hour. it requires also to spend a lot of time in the follow-up commits, though these are changing the same lines and cherrypick ends up in also a lot of conflicts.
My proposal: I go "fast over it". And the final "format" approach is then done with #143.
There was a problem hiding this comment.
Fair enough. Tidy up things with clang-format etc. at a later stage.
| @@ -2801,7 +2866,7 @@ dtls_send_client_hello(dtls_context_t *ctx, dtls_peer_t *peer, | |||
| psk = is_psk_supported(ctx); | |||
| ecdsa = is_ecdsa_supported(ctx, 1); | |||
|
|
|||
| cipher_size = 2 + ((ecdsa) ? 2 : 0) + ((psk) ? 2 : 0); | |||
| cipher_size = 2 + ((ecdsa) ? 4 : 0) + ((psk) ? 4 : 0); | |||
There was a problem hiding this comment.
It would be good to have a comment here that the 4 refers to there being 2 algorithms that are going to be defined - to make it more obvious what needs to be done when another algorithm is added.
Perhaps 4 should be 2*2 to emphasize this further.
There was a problem hiding this comment.
I tried to improve the "length documentation" in a commit later 84f834a .
I will add also comment here.
There was a problem hiding this comment.
84f834a certainly helps, but does not specifically bring focus to this line for the supported suites that are going to be announced.
- cipher_size = 2 + ((ecdsa) ? 2 : 0) + ((psk) ? 2 : 0);
+ /* Each announced cipher suite is 2 bytes long */
+ cipher_size = 2 + ((ecdsa) ? 2*2 : 0) + ((psk) ? 2*2 : 0);
There was a problem hiding this comment.
Yes, that commit doesn't address it, because this lines are removed on a commit before.
Therefore I think, it doesn't really pay-off to polish it more and then remove it.
|
Available on "develop" until follow up PR #148 gets merged. |
Add cipher suites with full 16 byte MAC. Signed-off-by: Achim Kraus <[email protected]>
Use cipher_suite_param_t for cipher-suite specific mac_len and key_exchange_algorithm. Introduce dtls_cipher_index_t for simplified cipher-suite parameter lookup. Cleanup old functions. Signed-off-by: Achim Kraus <[email protected]>
|
Now merged in main. |
In case of CCM gets recommended and CCM8 deprecated.
Introduce cipher_suite_param_t for cipher-suite specific mac_len and
key_exchange_algorithm, and introduce dtls_cipher_index_t for simplified
cipher-suite parameter lookup. Cleanup old functions.