Page MenuHomeFreeBSD

crypto: Re-add encrypt/decrypt_multi hooks to enc_xform.
ClosedPublic

Authored by jhb on Dec 17 2021, 12:14 AM.

Details

Summary

These callbacks allow multiple contiguous blocks to be manipulated in
a single call. Note that any trailing partial block for a stream
cipher must still be passed to encrypt/decrypt_last.

While here, document the setkey and reinit hooks and reorder the hooks
in 'struct enc_xform' to better reflect the life cycle.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb requested review of this revision.Dec 17 2021, 12:14 AM
sys/opencrypto/xform_aes_cbc.c
116

Perhaps assert len % AES_BLOCK_LEN == 0 && len >= AES_BLOCK_LEN.

133

Same here.

sys/opencrypto/xform_aes_icm.c
212

I would add the same assertion here. For a stream cipher in particular I might naively assume, ignoring code comments, that len need not be a multiple of the AES block length.

226

This function uses both AES_BLOCK_LEN and AESICM_BLOCKSIZE, but I suspect they should all be the latter if just for consistency.

sys/opencrypto/xform_chacha20_poly1305.c
113

Isn't it supposed to be incremented by the number of blocks?

jhb marked 2 inline comments as done.Dec 17 2021, 10:56 PM
jhb added inline comments.
sys/opencrypto/xform_aes_cbc.c
116

I think len == 0 will generally work on these, but I could assert for both if you think that's better?

sys/opencrypto/xform_chacha20_poly1305.c
113

Ooh, yes.

jhb marked an inline comment as done.
  • Add assertions and fix chacha20 block count increment.
sys/opencrypto/xform_chacha20_poly1305.c
113

Unfortunately I wasn't able to test this via cryptocheck since I've only changed "plain" ciphers to use the multi hooks so far. :-/ I might need to go ahead and convert the AEAD ciphers to use multi just so I can get test coverage.

Seems ok. I wonder why you don't implement the single block implementation in terms of the multi block implementation for all ciphers, as you did for AES-XTS.

sys/opencrypto/xform_aes_cbc.c
116

I think len % AES_BLOCK_LEN == 0 is sufficient then.

This revision is now accepted and ready to land.Dec 29 2021, 5:21 PM

Seems ok. I wonder why you don't implement the single block implementation in terms of the multi block implementation for all ciphers, as you did for AES-XTS.

Hmm, for AES-XTS due to its structure it was just much simpler to do that. For the others the multi block was something that could be added without touching the existing code I guess. It might indeed be simple to redo those. Maybe though I might do that as a separate commit just for bisect ability? Arguably we can retire the single-block encrypt/decrypt routines entirely once all callers are updated which might be a better long-term goal?

In D33529#761489, @jhb wrote:

Seems ok. I wonder why you don't implement the single block implementation in terms of the multi block implementation for all ciphers, as you did for AES-XTS.

Hmm, for AES-XTS due to its structure it was just much simpler to do that. For the others the multi block was something that could be added without touching the existing code I guess. It might indeed be simple to redo those. Maybe though I might do that as a separate commit just for bisect ability? Arguably we can retire the single-block encrypt/decrypt routines entirely once all callers are updated which might be a better long-term goal?

I'm fine with either of those approaches, I was mostly wondering if I'd missed something.