Page MenuHomeFreeBSD

crypto: Add a simple API for [X]ChaCha20-Poly1035 on flat buffers.
ClosedPublic

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

Details

Summary

This is a synchronous software API which wraps the existing software
implementation shared with OCF. Note that this will not currently
use optimized backends (such as ossl(4)) but may be appropriate for
working with small buffers.

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:11 AM
sys/crypto/chacha20_poly1305.h
38

Linux's header here uses 'chacha20poly1305_*' as the prefix instead of 'chacha20_poly1305_*. We use underscores in all the other places (cryptodev.h, etc.) in FreeBSD though.

Also, Linux's API apparently doesn't support 12 byte nonce's (as used by TLS/IPsec) as it takes a uint64_t for the nonce instead of an array of bytes. An array of bytes is more consistent with how we treat nonce/IV's in the rest of the crypto code however.

sys/crypto/chacha20_poly1305.c
58

Note that my other series I uploaded that adds multi_block encrypt/decrypt would be applicable here and in other places in this file. I have a followup change in my branch that replaces these loops with a single call to the multi-block variants. If multi-block lands first I will probably squash that into this change.

sys/crypto/chacha20_poly1305.c
39

src_len, aad_len and nonce_len could perhaps be defined const. That'd be another deviation from the Linux interface, I guess, but hopefully not an incompatible one?

68

Shouldn't this be adding resid instead of src_len?

150

I guess it should technically be XCHACHA20_POLY1305_KEY.

jhb marked 2 inline comments as done.Jan 6 2022, 10:09 PM
jhb added inline comments.
sys/crypto/chacha20_poly1305.c
39

I don't know that making size_t arguments passed by value const buys a whole lot, but it should be fine to do (and shouldn't be a meaningful API change relative to Linux as the caller doesn't care, the const only impacts the implementation)

68

Yes, this is a leftover from an earlier version that modified src_len rather than having a local resid.

jhb marked an inline comment as done.
  • Address review feedback from Mark.
sys/crypto/chacha20_poly1305.c
69

I think resid % POLY1305_BLOCK_LEN != 0 is always true here, since we know resid > 0 and resid < CHACHA20_NATIVE_BLOCK_LEN.

sys/crypto/chacha20_poly1305.c
69

Not necessarily. CHACHA20_NATIVE_BLOCK_LEN is 64 whereas POLY1305_BLOCK_LEN, so resid values of 16, 32, and 48 would make this false.

markj added inline comments.
sys/crypto/chacha20_poly1305.c
69

Oops, not sure what I was thinking.

This revision is now accepted and ready to land.Jan 11 2022, 2:33 PM

Now uses multi-block encrypt/decrypt functions.

This revision now requires review to proceed.Jan 13 2022, 10:57 PM

All of the rounddown()s can be rounddown2()s. I would assume that the compiler can figure that out, but maybe it's better to be explicit as in other crypto routines.

This revision is now accepted and ready to land.Jan 14 2022, 5:31 PM