crypto/ciphers: remove unneeded max_key_size in cipher_interface_st#16253
Conversation
|
Overall, it seems to me that the ciphers module has a couple of parameters and implementation details to rethink and cleanup. This is one of them. ciphers aims to provide transparent access to varying block ciphers. Block chaining modes (cbc, ccm, ctr, ...) take advantage of this and can be re-used. Thereby, the blocksize needs to be accessed. The keysize, however, can and should be handled by the specific cipher implementation. Additionally, I cannot even think of the "max" keysize being helpful in the context of the meta module. ACK from my side. @cholin was the original author. Do you have further insights? |
leandrolanzieri
left a comment
There was a problem hiding this comment.
I agree that the check of the key_size in cipher_init is limited and should be done properly in the cipher-specific init function. In fact, the only implementer of the interface has to do a second check of the key_size already.
LGTM, just a small potential improvement.
PeterKietzmann
left a comment
There was a problem hiding this comment.
tests/sys_crypto and tests/unittests/tests-hashes still pass on native and nrf51dk (the latter of which includes CMAC that utilizes ciphers internally)
|
Looks good to me, running a fresh CI build just in case. |
|
Well then, go! |
Contribution description
This PR removes the max_key_size member of the
cipher_interface_st. The only place I found where this struct member is used is this check. To me this check seems unnecessary. In my opinion it should be the job of the specific cipher modules, e.g. aes.c, to have checks related to key size.Testing procedure
This change does not alter the behavior of any cipher. All tests under /tests/sys_crypto should still pass.
Issues/PRs references
#16183